From d0e7ed37fc4ee17948a8a6597ce95a4fdab2b769 Mon Sep 17 00:00:00 2001 From: craig stout Date: Sat, 26 Sep 2020 20:43:30 -0400 Subject: [PATCH] [spirv-remap] Fix undefined behavior in hashing (#2403) There's a statement that intends to generate a 32-bit hashcode, but due to integer promotion, the intermediate values can trigger signed integer overflow, which is undefined behavior. To avoid this, cast at least one operand to unsigned int before multiplying, which will cause the result to be promoted to unsigned int instead of signed int. With this patch, I'm able to build core for qemu-x64 with host_asan-ubsan. Fixed: 60128 Change-Id: Idd644e534116bf29dca8013936ac39901bbe68fc Reviewed-on: https://fuchsia-review.googlesource.com/c/third_party/glslang/+/428254 Reviewed-by: John Bauman Co-authored-by: Drew Fisher --- SPIRV/SPVRemapper.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/SPIRV/SPVRemapper.cpp b/SPIRV/SPVRemapper.cpp index 1adc6a30..56d7f431 100644 --- a/SPIRV/SPVRemapper.cpp +++ b/SPIRV/SPVRemapper.cpp @@ -830,7 +830,15 @@ namespace spv { [&](spv::Id& id) { if (thisOpCode != spv::OpNop) { ++idCounter; - const std::uint32_t hashval = opCounter[thisOpCode] * thisOpCode * 50047 + idCounter + fnId * 117; + const std::uint32_t hashval = + // Explicitly cast operands to unsigned int to avoid integer + // promotion to signed int followed by integer overflow, + // which would result in undefined behavior. + static_cast(opCounter[thisOpCode]) + * thisOpCode + * 50047 + + idCounter + + static_cast(fnId) * 117; if (isOldIdUnmapped(id)) localId(id, nextUnusedId(hashval % softTypeIdLimit + firstMappedID));