Remove undefined behaviour when folding shifts. (#2157)

We currently simulate all shift operations when the two operand are
constants.  The problem is that if the shift amount is larger than
32, the result is undefined.

I'm changing the folder to return 0 if the shift value is too high.
That way, we will have defined behaviour.

https://crbug.com/910937.
This commit is contained in:
Steven Perron 2018-12-04 10:04:02 -05:00 committed by GitHub
parent d81a0c0ec4
commit 17cba4695c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 44 additions and 2 deletions

View File

@ -114,12 +114,23 @@ uint32_t InstructionFolder::BinaryOperate(SpvOp opcode, uint32_t a,
}
// Shifting
case SpvOp::SpvOpShiftRightLogical: {
return a >> b;
case SpvOp::SpvOpShiftRightLogical:
if (b > 32) {
// This is undefined behaviour. Choose 0 for consistency.
return 0;
}
return a >> b;
case SpvOp::SpvOpShiftRightArithmetic:
if (b > 32) {
// This is undefined behaviour. Choose 0 for consistency.
return 0;
}
return (static_cast<int32_t>(a)) >> b;
case SpvOp::SpvOpShiftLeftLogical:
if (b > 32) {
// This is undefined behaviour. Choose 0 for consistency.
return 0;
}
return a << b;
// Bitwise operations

View File

@ -179,6 +179,7 @@ OpName %main "main"
%uint_3 = OpConstant %uint 3
%uint_4 = OpConstant %uint 4
%uint_32 = OpConstant %uint 32
%uint_42 = OpConstant %uint 42
%uint_max = OpConstant %uint 4294967295
%v2int_undef = OpUndef %v2int
%v2int_0_0 = OpConstantComposite %v2int %int_0 %int_0
@ -474,6 +475,36 @@ INSTANTIATE_TEST_CASE_P(TestCase, IntegerInstructionFoldingTest,
"%2 = OpUMod %uint %uint_1 %uint_0\n" +
"OpReturn\n" +
"OpFunctionEnd",
2, 0),
// Test case 22: fold unsigned n >> 42 (undefined, so set to zero).
InstructionFoldingCase<uint32_t>(
Header() + "%main = OpFunction %void None %void_func\n" +
"%main_lab = OpLabel\n" +
"%n = OpVariable %_ptr_uint Function\n" +
"%load = OpLoad %uint %n\n" +
"%2 = OpShiftRightLogical %uint %load %uint_42\n" +
"OpReturn\n" +
"OpFunctionEnd",
2, 0),
// Test case 21: fold signed n >> 42 (undefined, so set to zero).
InstructionFoldingCase<uint32_t>(
Header() + "%main = OpFunction %void None %void_func\n" +
"%main_lab = OpLabel\n" +
"%n = OpVariable %_ptr_int Function\n" +
"%load = OpLoad %int %n\n" +
"%2 = OpShiftRightLogical %int %load %uint_42\n" +
"OpReturn\n" +
"OpFunctionEnd",
2, 0),
// Test case 22: fold n << 42 (undefined, so set to zero).
InstructionFoldingCase<uint32_t>(
Header() + "%main = OpFunction %void None %void_func\n" +
"%main_lab = OpLabel\n" +
"%n = OpVariable %_ptr_int Function\n" +
"%load = OpLoad %int %n\n" +
"%2 = OpShiftLeftLogical %int %load %uint_42\n" +
"OpReturn\n" +
"OpFunctionEnd",
2, 0)
));
// clang-format on