-
Notifications
You must be signed in to change notification settings - Fork 555
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
spirv-val: Not catching OOB access into an UniformConstant array #5468
Comments
SPIR-V phrases that as undefined behaviour and not a validation rule. So technically the module is valid, but the behaviour is unreliable. This is another example where a best effort linter (as in #5455) is preferable to validation in my opinion. |
Where is the line in the spec about this? Also do you mean |
It comes from OpAccessChain
SPIR-V distinguishes between static and runtime differently than Vulkan. See https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_validity_and_defined_behavior. |
Ok, this was educational would you agree that this should be then caught at runtime from the Vulkan Validation Layer if used in Vulkan, or do we expect all implementations to be able to handle this? If first one, will move issue to VVL, if second, want to get CTS written |
I'm not certain what that means. I think crashing is within bounds of undefined behavior. You could try to validate this in VVL, but unless it's added to GPUAV it won't catch everything. Maybe that's ok though. |
We already have GPU-AV for OOB access like this for descriptor indexing, I guess not fully understanding why we aren't able to statically detect |
You can in this case for sure, but not non-constant cases. |
ok yes, agree, if it is not a as for the static, |
Based on your earlier comment about how Vulkan treats UB, I think so. Things to watch out for:
|
If trying to index into an sampler array were we know it is OOB,
spirv-val
doesn't detect itThe text was updated successfully, but these errors were encountered: