Trying to modify a varying from fragment shader crashes the editor #83742

ze2j · 2023-10-21T17:54:49Z

Godot version

4.2.beta (6543495)

System information

Godot v4.2.beta (6543495) - Ubuntu 22.04.3 LTS 22.04 - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1070 (nvidia; 525.125.06) - Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz (8 Threads)

Issue description

In a spatial shader, I inadvertently tried to modify the value of a varying from the fragment method. It crashed the editor.

I am not sure if this should be allowed or not in the first place, but I didn't see any mention of such restriction in the documentation.

Steps to reproduce

Create a spatial shader like below and uncomment the last line:

shader_type spatial;

varying vec3 vertex_interpolated;

void vertex() {
	vertex_interpolated = VERTEX;
}

void fragment() {
	// vertex_interpolated.x = 42.0; // This crash the editor
}

Result:

Godot Engine v4.2.beta.custom_build.6543495b4 - https://godotengine.org
Vulkan API 1.3.224 - Forward+ - Using Vulkan Device #0: NVIDIA - NVIDIA GeForce GTX 1070
 
WARNING: Blend file import is enabled in the project settings, but no Blender path is configured in the editor settings. Blend files will not be imported.
     at: _editor_init (modules/gltf/register_types.cpp:63)
ERROR: Error compiling Fragment shader, variant #0 (
#define MODE_RENDER_DEPTH
).
   at: _compile_variant (servers/rendering/renderer_rd/shader_rd.cpp:293)
ERROR: Failed parse:
WARNING: 0:76: '' : all default precisions are highp; use precision statements to quiet warning, e.g.:
         "precision mediump int; precision highp float;" 
ERROR: 0:2178: 'frag_to_light' : undeclared identifier 
ERROR: 0:2178: 'm_vertex_interpolated' : vector swizzle too long 
ERROR: 0:2178: 'm_vertex_interpolated' : unknown swizzle selection 
ERROR: 0:2178: 'm_vertex_interpolated' : unknown swizzle selection 
ERROR: 0:2178: 'm_vertex_interpolated' : unknown swizzle selection 
ERROR: 0:2178: 'm_vertex_interpolated' : unknown swizzle selection 
ERROR: 0:2178: '' : compilation terminated 
ERROR: 7 compilation errors.  No code generated.



   at: _compile_variant (servers/rendering/renderer_rd/shader_rd.cpp:294)
ERROR: code:
   1 | 
   2 | #version 450
   3 | 

<<< shader dumps skipped >>>

 3610 | 
   at: _compile_variant (servers/rendering/renderer_rd/shader_rd.cpp:297)

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.2.beta.custom_build (6543495b49613d20f7e32f2b9d38e4a2f1d06db1)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib/x86_64-linux-gnu/libc.so.6(+0x42520) [0x7f0f88097520] (??:0)
[2] ShaderRD::_allocate_placeholders(ShaderRD::Version*, int) (/media/mla/Tera/Dev/godot/servers/rendering/renderer_rd/shader_rd.cpp:488 (discriminator 3))
[3] ShaderRD::version_is_valid(RID) (/media/mla/Tera/Dev/godot/servers/rendering/renderer_rd/shader_rd.cpp:640 (discriminator 1))
[4] RendererSceneRenderImplementation::SceneShaderForwardClustered::ShaderData::set_code(String const&) (/media/mla/Tera/Dev/godot/servers/rendering/renderer_rd/forward_clustered/scene_shader_forward_clustered.cpp:172)
[5] RendererRD::MaterialStorage::shader_set_code(RID, String const&) (/media/mla/Tera/Dev/godot/servers/rendering/renderer_rd/storage_rd/material_storage.cpp:1921)
[6] RenderingServerDefault::shader_set_code(RID, String const&) (/media/mla/Tera/Dev/godot/servers/rendering/rendering_server_default.h:235)
[7] Shader::set_code(String const&) (/media/mla/Tera/Dev/godot/scene/resources/shader.cpp:110)
[8] TextShaderEditor::apply_shaders() (/media/mla/Tera/Dev/godot/editor/plugins/text_shader_editor.cpp:957)
[9] void call_with_variant_args_helper<TextShaderEditor>(TextShaderEditor*, void (TextShaderEditor::*)(), Variant const**, Callable::CallError&, IndexSequence<>) (/media/mla/Tera/Dev/godot/./core/variant/binder_common.h:308 (discriminator 4))
[10] void call_with_variant_args<TextShaderEditor>(TextShaderEditor*, void (TextShaderEditor::*)(), Variant const**, int, Callable::CallError&) (/media/mla/Tera/Dev/godot/./core/variant/binder_common.h:418)
[11] CallableCustomMethodPointer<TextShaderEditor>::call(Variant const**, int, Variant&, Callable::CallError&) const (/media/mla/Tera/Dev/godot/./core/object/callable_method_pointer.h:105)
[12] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/media/mla/Tera/Dev/godot/core/variant/callable.cpp:57)
[13] Object::emit_signalp(StringName const&, Variant const**, int) (/media/mla/Tera/Dev/godot/core/object/object.cpp:1127)
[14] Node::emit_signalp(StringName const&, Variant const**, int) (/media/mla/Tera/Dev/godot/scene/main/node.cpp:3607)
[15] Error Object::emit_signal<>(StringName const&) (/media/mla/Tera/Dev/godot/./core/object/object.h:920)
[16] ShaderTextEditor::_validate_script() (/media/mla/Tera/Dev/godot/editor/plugins/text_shader_editor.cpp:435)
[17] CodeTextEditor::_text_changed_idle_timeout() (/media/mla/Tera/Dev/godot/editor/code_editor.cpp:1781)
[18] void call_with_variant_args_helper<CodeTextEditor>(CodeTextEditor*, void (CodeTextEditor::*)(), Variant const**, Callable::CallError&, IndexSequence<>) (/media/mla/Tera/Dev/godot/./core/variant/binder_common.h:308 (discriminator 4))
[19] void call_with_variant_args<CodeTextEditor>(CodeTextEditor*, void (CodeTextEditor::*)(), Variant const**, int, Callable::CallError&) (/media/mla/Tera/Dev/godot/./core/variant/binder_common.h:418)
[20] CallableCustomMethodPointer<CodeTextEditor>::call(Variant const**, int, Variant&, Callable::CallError&) const (/media/mla/Tera/Dev/godot/./core/object/callable_method_pointer.h:105)
[21] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/media/mla/Tera/Dev/godot/core/variant/callable.cpp:57)
[22] Object::emit_signalp(StringName const&, Variant const**, int) (/media/mla/Tera/Dev/godot/core/object/object.cpp:1127)
[23] Node::emit_signalp(StringName const&, Variant const**, int) (/media/mla/Tera/Dev/godot/scene/main/node.cpp:3607)
[24] Error Object::emit_signal<>(StringName const&) (/media/mla/Tera/Dev/godot/./core/object/object.h:920)
[25] Timer::_notification(int) (/media/mla/Tera/Dev/godot/scene/main/timer.cpp:62)
[26] Timer::_notificationv(int, bool) (/media/mla/Tera/Dev/godot/scene/main/timer.h:37 (discriminator 14))
[27] Object::notification(int, bool) (/media/mla/Tera/Dev/godot/core/object/object.cpp:839)
[28] SceneTree::_process_group(SceneTree::ProcessGroup*, bool) (/media/mla/Tera/Dev/godot/scene/main/scene_tree.cpp:950)
[29] SceneTree::_process(bool) (/media/mla/Tera/Dev/godot/scene/main/scene_tree.cpp:1023 (discriminator 2))
[30] SceneTree::process(double) (/media/mla/Tera/Dev/godot/scene/main/scene_tree.cpp:510)
[31] Main::iteration() (/media/mla/Tera/Dev/godot/main/main.cpp:3603)
[32] OS_LinuxBSD::run() (/media/mla/Tera/Dev/godot/platform/linuxbsd/os_linuxbsd.cpp:933)
[33] /media/mla/Tera/Dev/godot/bin/godot.linuxbsd.editor.dev.x86_64(main+0x19f) [0x55a7e8b633a8] (/media/mla/Tera/Dev/godot/platform/linuxbsd/godot_linuxbsd.cpp:76)
[34] /lib/x86_64-linux-gnu/libc.so.6(+0x29d90) [0x7f0f8807ed90] (??:0)
[35] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80) [0x7f0f8807ee40] (??:0)
[36] /media/mla/Tera/Dev/godot/bin/godot.linuxbsd.editor.dev.x86_64(_start+0x25) [0x55a7e8b63145] (??:?)
-- END OF BACKTRACE --
================================================================
Aborted (core dumped)

Minimal reproduction project

mrp.zip

The text was updated successfully, but these errors were encountered:

jsjtxietian · 2023-10-22T13:56:53Z

Looks like because the shader is failed to compile, it's p_version->variants is null, but in _allocate_placeholders it's still trying to access it.

This kind of usage of varying is wrong and needs to be handled well in shader processing, the above code will generate the following code, for vertex:

layout(location=12) out highp vec3 m_vertex_interpolated;
// in vertex_shader
m_vertex_interpolated=vertex;

For fragment the generated code is wrong:

layout(location=12) in highp vec3 m_vertex_interpolated;
// in fragment shader:
frag_to_light.m_vertex_interpolated.x=42.0;

AThousandShips · 2023-10-22T15:10:18Z

Note that assigning to a varying in more than one stage is not allowed, unsure why that error does not show up, might be something with the swizzling, what happens if you just assigns the whole variable?

Also what happens if you assign with swizzle in the vertex shader?

jsjtxietian · 2023-10-23T04:37:51Z

might be something with the swizzling

Yes, if I change to vertex_interpolated.x = 42.0 to something like vertex_interpolated = COLOR, it won't crash.

Also what happens if you assign with swizzle in the vertex shader?

It's fine, no error, no warning.

Looks like using swizzle will make godot's varying assignment check fail.

When use swizzle, it get parsed as a member node with it's owner set as the varying variable.

clayjohn · 2023-10-23T09:18:40Z

#83780 Will stop this MRP from crashing, but this is also an error that needs to be detected in our shader compiler so we can provide a helpful error in the editor and highlight the right line for the user.

jsjtxietian · 2023-10-23T09:55:20Z

I think I found where the culprit is, I will make a seperate PR for this.

AThousandShips · 2023-10-23T14:06:25Z

What's the progress of, or limitations to, adding tests for the shader compiler? Ensuring some fundamental functionality

AThousandShips added bug topic:shaders labels Oct 22, 2023

jsjtxietian mentioned this issue Oct 22, 2023

Prevent _allocate_placeholders crash if p_version->variants is null #83780

Merged

akien-mga modified the milestone: 4.2 Oct 23, 2023

jsjtxietian mentioned this issue Oct 23, 2023

Fix assign with swizzle in shader not doing varying validation check #83830

Merged

akien-mga added this to the 4.2 milestone Oct 23, 2023

akien-mga closed this as completed in #83830 Oct 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trying to modify a varying from fragment shader crashes the editor #83742

Trying to modify a varying from fragment shader crashes the editor #83742

ze2j commented Oct 21, 2023

jsjtxietian commented Oct 22, 2023 •

edited

Loading

AThousandShips commented Oct 22, 2023 •

edited

Loading

jsjtxietian commented Oct 23, 2023 •

edited

Loading

clayjohn commented Oct 23, 2023

jsjtxietian commented Oct 23, 2023

AThousandShips commented Oct 23, 2023

Trying to modify a varying from fragment shader crashes the editor #83742

Trying to modify a varying from fragment shader crashes the editor #83742

Comments

ze2j commented Oct 21, 2023

Godot version

System information

Issue description

Steps to reproduce

Minimal reproduction project

jsjtxietian commented Oct 22, 2023 • edited Loading

AThousandShips commented Oct 22, 2023 • edited Loading

jsjtxietian commented Oct 23, 2023 • edited Loading

clayjohn commented Oct 23, 2023

jsjtxietian commented Oct 23, 2023

AThousandShips commented Oct 23, 2023

jsjtxietian commented Oct 22, 2023 •

edited

Loading

AThousandShips commented Oct 22, 2023 •

edited

Loading

jsjtxietian commented Oct 23, 2023 •

edited

Loading