Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default member initialization crashes compiler #6328

Closed
exdal opened this issue Feb 10, 2025 · 7 comments · Fixed by #6338
Closed

Default member initialization crashes compiler #6328

exdal opened this issue Feb 10, 2025 · 7 comments · Fixed by #6338
Assignees
Labels
goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang

Comments

@exdal
Copy link
Contributor

exdal commented Feb 10, 2025

Default initialized RW/Textures/Resources seem to be crashing compiler despite a condition is set to false, this is started happening after I bumped slang version from v2024.17.4 to v2025.5

struct Foo {
    bool sample_bar = false;
    Texture2D<float4> bar = {};
};

float4 sample_foo(in Foo foo) {
    if (foo.sample_bar) {
        // Compiler crashes here despite `sample_bar` is set to false
        return bar.Sample(...);
    }

    return float4(0.0);
}

float4 main() {
    Foo foo;
    return sample_foo(foo);
}

Repro: https://gist.github.com/exdal/2d40533d7a8df813a6996771b1f9f95d
Command line:

./slangc -capability spirv_1_4 -target spirv test.slang -o ./test.bin
test.slang(59): warning 41016: use of uninitialized variable 'foo'
    const let result = process(foo);
                              ^
[1]    28517 segmentation fault (core dumped)  ./slangc -capability spirv_1_4 -target spirv test.slang -o ./test.bin

Version: v2025.5 (57b09a8986668626c37055e431fa0ac6449d7214)
Target profile: spirv_1_4

Link to original Discord post: https://discord.com/channels/1303735196696445038/1337851943942619298/1337851943942619298

@cheneym2
Copy link
Collaborator

I didn't get a repro using Slang @ 2/5/2025

commit d3e5f39cafbf1d65cf93cdd42c20c472c68197a2 (HEAD -> master, origin/master, origin/HEAD)
Author: Jay Kwak <[email protected]>
Date:   Wed Feb 5 00:43:02 2025 -0800
    Use 0.0.7 version of VK-GL-CTS-for-Slang on nightly CTS CI (#6278)

I compiled the shader in https://gist.github.com/exdal/2d40533d7a8df813a6996771b1f9f95d (named to repro.slang locally)

With this command:

$ build/Debug/bin/slangc -capability spirv_1_4 -target spirv-asm ~/repro.slang -o ./test.bin

@cheneym2
Copy link
Collaborator

I am able to reproduce with v2025.5 though

@cheneym2
Copy link
Collaborator

No repro on v2025.4, so regression is recent

@cheneym2
Copy link
Collaborator

#6058 Is the regression point. @kaizhangNV

@kaizhangNV
Copy link
Contributor

test.slang(59): warning 41016: use of uninitialized variable 'foo'

this is as expected, because in the intializer list proposal SP004, we remove the implicit constructor call for a struct that dones't define an explicit constructor.

So for your current example since struct Foo doesn't define __init, statement like
Foo foo; won't be converted to Foo foo = Foo();

you have to explicit write it like
Foo foo = Foo();
or
Foo foo = {};

So your statement Foo foo; will be treated as uninitialized, check:
https://github.com/shader-slang/slang/blob/master/docs/proposals/004-initialization.md#variable-initialization
for more detail.

@csyonghe
Copy link
Collaborator

@kaizhangNV the warning is good, but the compiler shouldn't crash. Where are we crashing?

@kaizhangNV
Copy link
Contributor

kaizhangNV commented Feb 11, 2025

I'm currently looking into the crash.

It crashes at IR link stage, we eliminate the uninitialized variable at call site:

 Foo foo;
 const let result = process(foo);

becomes:

 const let result = process();

specifically, it's wiped out in this call legalizeResourceTypes.

kaizhangNV added a commit to kaizhangNV/slang that referenced this issue Feb 11, 2025
close shader-slang#6328.

When declare a var with struct type, if the struct has a resource
type field and it doesn't provide explicit constructor, because
slang won't implicit construct such variable at declare site if user
doesn't explicitly call the initializer list, we should report the
resource type field uninitialized as an error to prevent future possible
crash when legalize any use of such variable.
kaizhangNV added a commit to kaizhangNV/slang that referenced this issue Feb 12, 2025
close shader-slang#6328.

When declare a var with struct type, if the struct has a resource
type field and it doesn't provide explicit constructor, because
slang won't implicit construct such variable at declare site if user
doesn't explicitly call the initializer list, we should report the
resource type field uninitialized as an error to prevent future possible
crash when legalize any use of such variable.
kaizhangNV added a commit to kaizhangNV/slang that referenced this issue Feb 12, 2025
close shader-slang#6328.

When declare a var with struct type, if the struct has a resource
type field and it doesn't provide explicit constructor, because
slang won't implicit construct such variable at declare site if user
doesn't explicitly call the initializer list, we should report the
resource type field uninitialized as an error to prevent future possible
crash when legalize any use of such variable.
@bmillsNV bmillsNV added this to the Q1 2025 (Winter) milestone Feb 12, 2025
@bmillsNV bmillsNV added the goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang label Feb 12, 2025
kaizhangNV added a commit to kaizhangNV/slang that referenced this issue Feb 12, 2025
close shader-slang#6328.

When declare a var with struct type, if the struct has a resource
type field and it doesn't provide explicit constructor, because
slang won't implicit construct such variable at declare site if user
doesn't explicitly call the initializer list, we should report the
resource type field uninitialized as an error to prevent future possible
crash when legalize any use of such variable.
kaizhangNV added a commit to kaizhangNV/slang that referenced this issue Feb 12, 2025
close shader-slang#6328.

When declare a var with struct type, if the struct has a resource
type field and it doesn't provide explicit constructor, because
slang won't implicit construct such variable at declare site if user
doesn't explicitly call the initializer list, we should report the
resource type field uninitialized as an error to prevent future possible
crash when legalize any use of such variable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants