Faulty -fsanitize=shift-exponent checks (seen with _BitInt)

[bjope]

Consider these examples:

int test_left(unsigned _BitInt(17) x) {
  return x << 3uwb;  // Shift exponent is OK (3 < 17)  
}

int test_right(unsigned _BitInt(4) x) {
  return x >> 16uwb; // Shift exponent is NOK (16 >= 4)
}

When compiled with -O1 -fsanitize=shift-exponent -emit-llvm -g0 the result is:

define dso_local noundef i32 @test_left(i17 noundef zeroext %0) local_unnamed_addr #0 {
  %2 = zext i17 %0 to i64
  tail call void @__ubsan_handle_shift_out_of_bounds(ptr nonnull @2, i64 %2, i64 3) #3
  %3 = shl i17 %0, 3
  %4 = zext i17 %3 to i32
  ret i32 %4
}

define dso_local noundef i32 @test_right(i4 noundef zeroext %0) local_unnamed_addr #2 {
  %2 = zext i4 %0 to i32
  ret i32 %2
}

So we actually see the opposite from the expected result (test_left will report UB and test_right will not report UB).

Quick analysis show:
Faults seem to be in ScalarExprEmitter::EmitShl / ScalarExprEmitter::EmitShr / ScalarExprEmitter::GetWidthMinusOneValue.

For the left shift EmitShl uses the type of the right hand side when doing the ubsan checks. But when using the GetWidthMinusOneValue helper it is not considered that the upper shift count limit given by the size of the left hand side operand might not fit in the type used by the right hand side. So the value is simply truncated and that results in an incorrect upper bound when doing the check.

For the right shift EmitShr is truncating the shift exponent operand to the size of the left hand side operand before doing the out-of-bounds check. This seems incorrect, as the check really should be performed using the type of the shift exponent operand.

[llvmbot]

@llvm/issue-subscribers-bug

Author: Björn Pettersson (bjope)

Consider these examples: ``` int test_left(unsigned _BitInt(17) x) { return x << 3uwb; // Shift exponent is OK (3 < 17) }

int test_right(unsigned _BitInt(4) x) {
return x >> 16uwb; // Shift exponent is NOK (16 >= 4)
}

When compiled with `-O1 -fsanitize=shift-exponent -emit-llvm -g0` the result is:

define dso_local noundef i32 @test_left(i17 noundef zeroext %0) local_unnamed_addr #0 {
%2 = zext i17 %0 to i64
tail call void @__ubsan_handle_shift_out_of_bounds(ptr nonnull @2, i64 %2, i64 3) #3
%3 = shl i17 %0, 3
%4 = zext i17 %3 to i32
ret i32 %4
}

define dso_local noundef i32 @test_right(i4 noundef zeroext %0) local_unnamed_addr #2 {
%2 = zext i4 %0 to i32
ret i32 %2
}

So we actually see the opposite from the expected result (test_left will report UB and test_right will not report UB).

Quick analysis show:
Faults seem to be in ScalarExprEmitter::EmitShl / ScalarExprEmitter::EmitShr / ScalarExprEmitter::GetWidthMinusOneValue.

For the left shift  EmitShl uses the type of the right hand side when doing the ubsan checks. But when using the GetWidthMinusOneValue helper  it is not considered that the upper shift count limit given by the size of the left hand side operand might not fit in the type used by the right hand side. So the value is simply truncated and that results in an incorrect upper bound when doing the check.

For the right shift EmitShr is truncating the shift exponent operand to the size of the left hand side operand before doing the out-of-bounds check. This seems incorrect, as the check really should be performed using the type of the shift exponent operand.
</details>

[llvmbot]

@llvm/issue-subscribers-clang-codegen

Author: Björn Pettersson (bjope)

Consider these examples: ``` int test_left(unsigned _BitInt(17) x) { return x << 3uwb; // Shift exponent is OK (3 < 17) }

int test_right(unsigned _BitInt(4) x) {
return x >> 16uwb; // Shift exponent is NOK (16 >= 4)
}

When compiled with `-O1 -fsanitize=shift-exponent -emit-llvm -g0` the result is:

define dso_local noundef i32 @test_left(i17 noundef zeroext %0) local_unnamed_addr #0 {
%2 = zext i17 %0 to i64
tail call void @__ubsan_handle_shift_out_of_bounds(ptr nonnull @2, i64 %2, i64 3) #3
%3 = shl i17 %0, 3
%4 = zext i17 %3 to i32
ret i32 %4
}

define dso_local noundef i32 @test_right(i4 noundef zeroext %0) local_unnamed_addr #2 {
%2 = zext i4 %0 to i32
ret i32 %2
}

So we actually see the opposite from the expected result (test_left will report UB and test_right will not report UB).

Quick analysis show:
Faults seem to be in ScalarExprEmitter::EmitShl / ScalarExprEmitter::EmitShr / ScalarExprEmitter::GetWidthMinusOneValue.

For the left shift  EmitShl uses the type of the right hand side when doing the ubsan checks. But when using the GetWidthMinusOneValue helper  it is not considered that the upper shift count limit given by the size of the left hand side operand might not fit in the type used by the right hand side. So the value is simply truncated and that results in an incorrect upper bound when doing the check.

For the right shift EmitShr is truncating the shift exponent operand to the size of the left hand side operand before doing the out-of-bounds check. This seems incorrect, as the check really should be performed using the type of the shift exponent operand.
</details>