Skip to content
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

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

Open
cdotstout opened this issue Jan 22, 2024 · 2 comments
Open

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

cdotstout opened this issue Jan 22, 2024 · 2 comments

Comments

@cdotstout
Copy link
Contributor

cdotstout commented Jan 22, 2024

Branch based on glslang commit 57d86ab

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
Copy link
Contributor

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
Copy link
Contributor

dneto0 commented Feb 16, 2024

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants