Naga snapshot tests should not include binary files

[jimblandy]

The Naga snapshot tests should be changed to take SPIR-V input as assembly language, not binary files. Alternatively, CI should disassemble the binary SPIR-V files and assert that they match accompanying source files.

The xz utils backdoor concealed the portion of its code that should have aroused suspicion as test input for the decompression library. Binary data discourages review in a way that textual data does not. (As evidence for this, recent PRs from perfectly trustworthy contributors to wgpu have included .spv input files that didn't match the disassembly present alongside them in the PR. These problems were caught in PR review, but the author themselves hadn't noticed.)

In wgpu, the naga/tests/in/spv/ directory contains various .spv binary files. There are a few approaches:

• We could have snapshot tests take .spvasm files as input instead. We could add a dev dependency on a SPIR-V assembler, or require the spirv-as command to be present on the path. We would need to assess the impact on test run time.
• We could have CI disassembly each .spv file and verify that the result matches the accompanying .spvasm. file. Thus, any PR that affected a .spv file would also inlude a diff to its .spvasm file.

[cwfitzgerald]

I don't see easy bindings to spirv-as, but shelling out likely isn't too costly, and it's not unreasonable to expect the vulkan sdk installed and in-path.

[evilpie]

Rust does have the advantage compared to C code (including xz) that the build system is a lot more sane and it's harder to hide some nefarious instructions in there. It would be nice if wgpu didn't use build.rs at all, but the current usage is at least very easy to audit.

[Imberflur]

Some thoughts on this:

• Not having to coordinate these files would make it easier for contributors adding new tests.
• A potential downside with shelling out to spirv-as is if there are inconsistencies in the generated spirv binary from contributors having different versions installed.

[cwfitzgerald]

Not having to coordinate these files would make it easier for contributors adding new tests.

Agreed

A potential downside with shelling out to spirv-as is if there are inconsistencies in the generated spirv binary from contributors having different versions installed.

I am worried about this a bit when it comes to compilers, but assemblers shouldn't be that different between versions.