gfx-rs/wgpu

[naga] `PendingArraySize::Expression` handle not validated

Opened this issue · 1 comments

Naga validation does not ensure that the expr handle in PendingArraySize::Expression(expr) is valid. That variant is not mentioned in naga/src/valid/handles.rs at all.

Fixing this is not as simple as just adding a bit of code to validate_module_handles. Handle validation needs to show that the structure of the module is acyclic, but with the introduction of PendingArraySize::Expression, Module::types and Module::global_expressions can now refer to each other, making it a bit trickier to show an absence of cycles.

cross-posting #6788 (comment):

I wonder if we can get away with saying that no expression can refer to an override-sized array and check this in the validator.

This would require not just checking the Handle<Type>s that appear in Expression variants, but the types those types refer to. But since override-sized arrays can't be used in many contexts (@kentslaney added the CREATION_RESOLVED flag to check this), we could just assume that they don't appear deeper in the type.

But one of the principles of the validator is that handle validation runs first, so that the rest of the validator can just assume that whatever handles it runs into are okay. We used to sort of check handles in the midst of everything else, and of course we forgot to do it all the time.

Being cycle-free, well, I'm not positive where this guarantee is taken advantage of. But assuming the absence of cycles where they might actually be present is an absolutely classic kind of bug, in every variety of programming, and I think this shows that people sort of generally don't anticipate cycles, whether they should know better or not. So I think it's good for handle validation to just settle the question once and for all, to allow rest of the validator to be naive and common-sensey.

And I think that means that we shouldn't be depending on type validation to establish the absence of cycles.