KhronosGroup / glslang

Khronos-reference front end for GLSL/ESSL, partial front end for HLSL, and a SPIR-V generator.
Other
2.9k stars 813 forks source link

Undefined behavior in "bitwise operators for mask bit combining" #3485

Open cdotstout opened 5 months ago

cdotstout commented 5 months ago

Branch based on glslang commit 57d86ab763da7b2cd1e00ecec8aa697403a8fd20

Some of the bitwise operators for mask combining in SPIRV.hpp create mask values that are outside the valid range of the enum, which apparently is undefined behavior as flagged by the Fuchsia platform build:


FAILED: host_x64-asan-ubsan/gen/src/graphics/lib/compute/radix_sort/platforms/vk/targets/vendors/nvidia/sm35/u64/radix_sort_vk_nvidia_sm35_u64_outputs/spv/scatter_1_odd.spv
../../prebuilt/third_party/python3/linux-x64/bin/python3 -S ../../build/rbe/output_leak_scanner.py --label //src/graphics/lib/compute/radix_sort/platforms/vk/targets/vendors/nvidia/sm35/u64:gen_spv_radix_sort_vk_nvidia_sm35_u64_outputs_spirv_modules\(//build/toolchain:host_x64-asan-ubsan\) host_x64-asan-ubsan/gen/src/graphics/lib/compute/radix_sort/platforms/vk/targets/vendors/nvidia/sm35/u64/radix_sort_vk_nvidia_sm35_u64_outputs/spv/scatter_1_odd.spv -- ../../build/gn_run_binary.sh ../../prebuilt/third_party/clang/linux-x64/bin host_x64-asan-ubsan/glslang_validator --quiet -o host_x64-asan-ubsan/gen/src/graphics/lib/compute/radix_sort/platforms/vk/targets/vendors/nvidia/sm35/u64/radix_sort_vk_nvidia_sm35_u64_outputs/spv/scatter_1_odd.spv ../../src/graphics/lib/compute/radix_sort/platforms/vk/shaders/scatter_1_odd.comp -I../../src/graphics/lib/compute/radix_sort/platforms/vk/targets/vendors/nvidia/sm35/u64 -I../../src/graphics/lib/compute/radix_sort/platforms/vk/shaders --target-env vulkan1.2
../../third_party/glslang/SPIRV/spirv.hpp:2773:124: runtime error: load of value 4294967287, which is not a valid value for type 'MemoryAccessMask'
    #0 0x55ab45f9a393 in spv::operator&(spv::MemoryAccessMask, spv::MemoryAccessMask) ../../third_party/glslang/SPIRV/spirv.hpp:2773:124
    #1 0x55ab45f97cb2 in (anonymous namespace)::TGlslangToSpvTraverser::accessChainLoad(glslang::TType const&) ../../third_party/glslang/SPIRV/GlslangToSpv.cpp:5033:99
    #2 0x55ab45e7a4d0 in (anonymous namespace)::TGlslangToSpvTraverser::visitBinary(glslang::TVisit, glslang::TIntermBinary*) ../../third_party/glslang/SPIRV/GlslangToSpv.cpp:2170:30
    #3 0x55ab46312b39 in glslang::TIntermBinary::traverse(glslang::TIntermTraverser*) ../../third_party/glslang/glslang/MachineIndependent/IntermTraverse.cpp:92:21
    #4 0x55ab463150f4 in glslang::TIntermAggregate::traverse(glslang::TIntermTraverser*) ../../third_party/glslang/glslang/MachineIndependent/IntermTraverse.cpp:175:25
    #5 0x55ab463150f4 in glslang::TIntermAggregate::traverse(glslang::TIntermTraverser*) ../../third_party/glslang/glslang/MachineIndependent/IntermTraverse.cpp:175:25
    #6 0x55ab463150f4 in glslang::TIntermAggregate::traverse(glslang::TIntermTraverser*) ../../third_party/glslang/glslang/MachineIndependent/IntermTraverse.cpp:175:25
    #7 0x55ab463150f4 in glslang::TIntermAggregate::traverse(glslang::TIntermTraverser*) ../../third_party/glslang/glslang/MachineIndependent/IntermTraverse.cpp:175:25
    #8 0x55ab46026e7b in (anonymous namespace)::TGlslangToSpvTraverser::visitFunctions(glslang::TVector<TIntermNode*> const&) ../../third_party/glslang/SPIRV/GlslangToSpv.cpp:5539:19
    #9 0x55ab45e8b26b in (anonymous namespace)::TGlslangToSpvTraverser::visitAggregate(glslang::TVisit, glslang::TIntermAggregate*) ../../third_party/glslang/SPIRV/GlslangToSpv.cpp:2889:17
    #10 0x55ab463145eb in glslang::TIntermAggregate::traverse(glslang::TIntermTraverser*) ../../third_party/glslang/glslang/MachineIndependent/IntermTraverse.cpp:159:21
    #11 0x55ab45e56b2b in glslang::GlslangToSpv(glslang::TIntermediate const&, std::__2::vector<unsigned int, std::__2::allocator<unsigned int>>&, spv::SpvBuildLogger*, glslang::SpvOptions*) ../../third_party/glslang/SPIRV/GlslangToSpv.cpp:10178:11
    #12 0x55ab45dd48ae in CompileAndLinkShaderUnits(std::__2::vector<ShaderCompUnit, std::__2::allocator<ShaderCompUnit>>) ../../third_party/glslang/StandAlone/StandAlone.cpp:1540:17
    #13 0x55ab45ddb640 in CompileAndLinkShaderFiles(glslang::TWorklist&) ../../third_party/glslang/StandAlone/StandAlone.cpp:1633:12
    #14 0x55ab45dddf61 in singleMain() ../../third_party/glslang/StandAlone/StandAlone.cpp:1706:9
    #15 0x55ab45de0829 in main ../../third_party/glslang/StandAlone/StandAlone.cpp:1760:15
    #16 0x7f2069d6d6c9  (/lib/x86_64-linux-gnu/libc.so.6+0x276c9) (BuildId: 2ac5fa07c22f99cfd5dc47c70cd5f0e78b974269)
    #17 0x7f2069d6d784 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x27784) (BuildId: 2ac5fa07c22f99cfd5dc47c70cd5f0e78b974269)
    #18 0x55ab45ce3f98  (/usr/local/google/home/cstout/remote-fuchsia/out/x64-debug/host_x64-asan-ubsan/glslang_validator+0x1773f98) (BuildId: 9e5707f79d6dadbc)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../third_party/glslang/SPIRV/spirv.hpp:2773:124 in```
arcady-lunarg commented 5 months ago

The line in question does spv::MemoryAccessMask accessMask = spv::MemoryAccessMask(TranslateMemoryAccess(coherentFlags) & ~spv::MemoryAccessMakePointerAvailableKHRMask which is a pattern that is kind of unavoidable when your enum is a bitmask. Wouldn't the combined values after a bitwise OR also not technically be members of the enum type, resulting in the same problem? In which case this is really a problem with spirv.hpp.

dneto0 commented 4 months ago

Yes, the problem is inherent in doing bitwise operations on an enum type that is really a bunch of masks.

I wonder if switching to spirv.hpp11 would help because then those enums are enum classes based on unsigned which we can presume are 32bit.

(Note: Glslang's spirv.hpp is a copy of the original spirv.hpp from the SPIRV-Headers project. It gets copied in on an as needed basis.)

Oh, nope. I think that wouldn't help. See https://cplusplus.github.io/CWG/issues/1766.html which says casting a numeric value to an enum type that doesn't have that value is undefined behaviour.

So that's interesting...