tint: Fix C++ UB when shifting abstract 0 left by >= 64 In WGSL, we can shift left abstracts by >= 64, as long as the result is representable in the data type we choose for it. When shifting 0, we can shift by any positive u32 value (result is always 0), but in C++, it's UB to shift by more than the bit width of the data type, so we need to handle this. This bug was caught by ClusterFuzz. Bug: chromium:1372963 Change-Id: I638ca190b93538908ca6472f3735627ea8531c5a Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/106266 Reviewed-by: David Neto <dneto@google.com> Commit-Queue: Antonio Maiorano <amaiorano@google.com> Reviewed-by: Dan Sinclair <dsinclair@chromium.org> Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/tint/resolver/const_eval.cc b/src/tint/resolver/const_eval.cc index d9fde25..c5cce2d 100644 --- a/src/tint/resolver/const_eval.cc +++ b/src/tint/resolver/const_eval.cc
@@ -1474,6 +1474,10 @@ AddError(OverflowErrorMessage(e1, "<<", e2), source); return nullptr; } + + // It's UB in C++ to shift by greater or equal to the bit width (even if the lhs + // is 0), so we make sure to avoid this by setting the shift value to 0. + e2 = 0; } } else { if (static_cast<size_t>(e2) >= bit_width) {
diff --git a/src/tint/resolver/const_eval_binary_op_test.cc b/src/tint/resolver/const_eval_binary_op_test.cc index 5f43499..b5a046c 100644 --- a/src/tint/resolver/const_eval_binary_op_test.cc +++ b/src/tint/resolver/const_eval_binary_op_test.cc
@@ -598,7 +598,7 @@ // Shift type is u32 for non-abstract using ST = std::conditional_t<IsAbstract<T>, T, u32>; using B = BitValues<T>; - return { + auto r = std::vector<Case>{ C(T{0b1010}, ST{0}, T{0b0000'0000'1010}), // C(T{0b1010}, ST{1}, T{0b0000'0001'0100}), // C(T{0b1010}, ST{2}, T{0b0000'0010'1000}), // @@ -626,6 +626,25 @@ Vec(ST{6}, ST{7}, ST{8}), // Vec(T{0b0010'1000'0000}, T{0b0101'0000'0000}, T{0b1010'0000'0000})), // }; + + // Only abstract 0 can be shifted left as much as we like. For concrete 0 (and any number), it + // cannot be shifted equal or more than the number of bits of the lhs (see + // ResolverConstEvalShiftLeftConcreteGeqBitWidthError) + ConcatIntoIf<IsAbstract<T>>( // + r, std::vector<Case>{ + C(T{0}, ST{64}, T{0}), + C(T{0}, ST{65}, T{0}), + C(T{0}, ST{65}, T{0}), + C(T{0}, ST{10000}, T{0}), + C(T{0}, T::Highest(), T{0}), + C(Negate(T{0}), ST{64}, Negate(T{0})), + C(Negate(T{0}), ST{65}, Negate(T{0})), + C(Negate(T{0}), ST{65}, Negate(T{0})), + C(Negate(T{0}), ST{10000}, Negate(T{0})), + C(Negate(T{0}), T::Highest(), Negate(T{0})), + }); + + return r; } INSTANTIATE_TEST_SUITE_P(ShiftLeft, ResolverConstEvalBinaryOpTest, @@ -850,15 +869,37 @@ } INSTANTIATE_TEST_SUITE_P(Test, ResolverConstEvalShiftLeftConcreteGeqBitWidthError, - testing::Values( // - std::make_tuple(Val(1_i), Val(32_u)), // - std::make_tuple(Val(1_i), Val(33_u)), // - std::make_tuple(Val(1_i), Val(34_u)), // - std::make_tuple(Val(1_i), Val(99999999_u)), // - std::make_tuple(Val(1_u), Val(32_u)), // - std::make_tuple(Val(1_u), Val(33_u)), // - std::make_tuple(Val(1_u), Val(34_u)), // - std::make_tuple(Val(1_u), Val(99999999_u)) // + testing::Values( // + std::make_tuple(Val(0_u), Val(32_u)), // + std::make_tuple(Val(0_u), Val(33_u)), // + std::make_tuple(Val(0_u), Val(34_u)), // + std::make_tuple(Val(0_u), Val(10000_u)), // + std::make_tuple(Val(0_u), Val(u32::Highest())), // + std::make_tuple(Val(0_i), Val(32_u)), // + std::make_tuple(Val(0_i), Val(33_u)), // + std::make_tuple(Val(0_i), Val(34_u)), // + std::make_tuple(Val(0_i), Val(10000_u)), // + std::make_tuple(Val(0_i), Val(u32::Highest())), // + std::make_tuple(Val(Negate(0_u)), Val(32_u)), // + std::make_tuple(Val(Negate(0_u)), Val(33_u)), // + std::make_tuple(Val(Negate(0_u)), Val(34_u)), // + std::make_tuple(Val(Negate(0_u)), Val(10000_u)), // + std::make_tuple(Val(Negate(0_u)), Val(u32::Highest())), // + std::make_tuple(Val(Negate(0_i)), Val(32_u)), // + std::make_tuple(Val(Negate(0_i)), Val(33_u)), // + std::make_tuple(Val(Negate(0_i)), Val(34_u)), // + std::make_tuple(Val(Negate(0_i)), Val(10000_u)), // + std::make_tuple(Val(Negate(0_i)), Val(u32::Highest())), // + std::make_tuple(Val(1_i), Val(32_u)), // + std::make_tuple(Val(1_i), Val(33_u)), // + std::make_tuple(Val(1_i), Val(34_u)), // + std::make_tuple(Val(1_i), Val(10000_u)), // + std::make_tuple(Val(1_i), Val(u32::Highest())), // + std::make_tuple(Val(1_u), Val(32_u)), // + std::make_tuple(Val(1_u), Val(33_u)), // + std::make_tuple(Val(1_u), Val(34_u)), // + std::make_tuple(Val(1_u), Val(10000_u)), // + std::make_tuple(Val(1_u), Val(u32::Highest())) // )); // AInt left shift results in sign change error
diff --git a/test/tint/bug/chromium/1372963.wgsl b/test/tint/bug/chromium/1372963.wgsl new file mode 100644 index 0000000..d281bc3 --- /dev/null +++ b/test/tint/bug/chromium/1372963.wgsl
@@ -0,0 +1,8 @@ +fn g() -> vec4<i32> { + return vec4(-0) << vec4(2147483649); +} + +@fragment +fn main() { + g(); +}
diff --git a/test/tint/bug/chromium/1372963.wgsl.expected.dxc.hlsl b/test/tint/bug/chromium/1372963.wgsl.expected.dxc.hlsl new file mode 100644 index 0000000..16a37a3a --- /dev/null +++ b/test/tint/bug/chromium/1372963.wgsl.expected.dxc.hlsl
@@ -0,0 +1,8 @@ +int4 g() { + return (0).xxxx; +} + +void main() { + g(); + return; +}
diff --git a/test/tint/bug/chromium/1372963.wgsl.expected.fxc.hlsl b/test/tint/bug/chromium/1372963.wgsl.expected.fxc.hlsl new file mode 100644 index 0000000..16a37a3a --- /dev/null +++ b/test/tint/bug/chromium/1372963.wgsl.expected.fxc.hlsl
@@ -0,0 +1,8 @@ +int4 g() { + return (0).xxxx; +} + +void main() { + g(); + return; +}
diff --git a/test/tint/bug/chromium/1372963.wgsl.expected.glsl b/test/tint/bug/chromium/1372963.wgsl.expected.glsl new file mode 100644 index 0000000..1ff5381 --- /dev/null +++ b/test/tint/bug/chromium/1372963.wgsl.expected.glsl
@@ -0,0 +1,15 @@ +#version 310 es +precision mediump float; + +ivec4 g() { + return ivec4(0); +} + +void tint_symbol() { + g(); +} + +void main() { + tint_symbol(); + return; +}
diff --git a/test/tint/bug/chromium/1372963.wgsl.expected.msl b/test/tint/bug/chromium/1372963.wgsl.expected.msl new file mode 100644 index 0000000..c3aaae2 --- /dev/null +++ b/test/tint/bug/chromium/1372963.wgsl.expected.msl
@@ -0,0 +1,12 @@ +#include <metal_stdlib> + +using namespace metal; +int4 g() { + return int4(0); +} + +fragment void tint_symbol() { + g(); + return; +} +
diff --git a/test/tint/bug/chromium/1372963.wgsl.expected.spvasm b/test/tint/bug/chromium/1372963.wgsl.expected.spvasm new file mode 100644 index 0000000..c567407 --- /dev/null +++ b/test/tint/bug/chromium/1372963.wgsl.expected.spvasm
@@ -0,0 +1,26 @@ +; SPIR-V +; Version: 1.3 +; Generator: Google Tint Compiler; 0 +; Bound: 12 +; Schema: 0 + OpCapability Shader + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %main "main" + OpExecutionMode %main OriginUpperLeft + OpName %g "g" + OpName %main "main" + %int = OpTypeInt 32 1 + %v4int = OpTypeVector %int 4 + %1 = OpTypeFunction %v4int + %6 = OpConstantNull %v4int + %void = OpTypeVoid + %7 = OpTypeFunction %void + %g = OpFunction %v4int None %1 + %5 = OpLabel + OpReturnValue %6 + OpFunctionEnd + %main = OpFunction %void None %7 + %10 = OpLabel + %11 = OpFunctionCall %v4int %g + OpReturn + OpFunctionEnd
diff --git a/test/tint/bug/chromium/1372963.wgsl.expected.wgsl b/test/tint/bug/chromium/1372963.wgsl.expected.wgsl new file mode 100644 index 0000000..955d6c6 --- /dev/null +++ b/test/tint/bug/chromium/1372963.wgsl.expected.wgsl
@@ -0,0 +1,8 @@ +fn g() -> vec4<i32> { + return (vec4(0) << vec4(2147483649)); +} + +@fragment +fn main() { + g(); +}