You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
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.
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.
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.
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
, thenaga/tests/in/spv/
directory contains various.spv
binary files. There are a few approaches:.spvasm
files as input instead. We could add a dev dependency on a SPIR-V assembler, or require thespirv-as
command to be present on the path. We would need to assess the impact on test run time..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.The text was updated successfully, but these errors were encountered: