-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(naga): don't crash on zero value of dynamically-sized array #8741
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
Conversation
andyleiserson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a relevant CTS test webgpu:shader,validation,expression,call,builtin,value_constructor:array_zero_value:case="invalid_rta", but it isn't failing, because it expects any validation error, and is getting one related to constructibility. (I'm looking at fixing this in the CTS.)
There are also some related CTS tests that expose other broken cases (atomics, override-sized arrays). Also see #4720.
Submitting as comments right now, but this is all nits, there's nothing here that is essential to fix before merging this.
gpuweb/cts#4550 has my proposed change to the CTS. |
andyleiserson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
Connections
Fixes #8442
Description
Usage of dynamically sized arrays is restricted to a few special cases.
let a = array<f32>()is trying to construct theZeroValueof a dynamically sized array, which is not allowed. With this PR,ZeroValueconstruction of dynamically sized arrays gets validated out.Compiling the wgsl from #8442 doesn't crash naga anymore but instead throws an error:
Testing
Adds test
zero_value_dyn_array_error.Squash or Rebase?
Squash
Checklist
cargo fmt.taplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.