[clang][CodeGen] Fix shift-exponent ubsan check for signed _BitInt (#88004)

Commit 5f87957 (pull-requst #80515) corrected some codegen
problems related to _BitInt types being used as shift exponents. But it
did not fix it properly for the special case when the shift count
operand is a signed _BitInt.

The basic problem is the same as the one solved for unsigned _BitInt. As
we use an unsigned comparison to see if the shift exponent is
out-of-bounds, then we need to find an unsigned maximum allowed shift
amount to use in the check. Normally the shift amount is limited by
bitwidth of the LHS of the shift. However, when the RHS type is small in
relation to the LHS then we need to use a value that fits inside the
bitwidth of the RHS instead.

The earlier fix simply used the unsigned maximum when deterining the max
shift amount based on the RHS type. It did however not take into
consideration that the RHS type could have a signed representation. In
such situations we need to use the signed maximum instead. Otherwise we
do not recognize a negative shift exponent as UB.

Author: bjope

clang/lib/CodeGen/CGExprScalar.cpp

@@ -147,6 +147,15 @@ struct BinOpInfo {
       return UnOp->getSubExpr()->getType()->isFixedPointType();
     return false;
   }
+
+  /// Check if the RHS has a signed integer representation.
+  bool rhsHasSignedIntegerRepresentation() const {
+    if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {
+      QualType RHSType = BinOp->getRHS()->getType();
+      return RHSType->hasSignedIntegerRepresentation();
+    }
+    return false;
+  }
 };
 
 static bool MustVisitNullValue(const Expr *E) {
@@ -782,7 +791,7 @@ class ScalarExprEmitter
   void EmitUndefinedBehaviorIntegerDivAndRemCheck(const BinOpInfo &Ops,
                                                   llvm::Value *Zero,bool isDiv);
   // Common helper for getting how wide LHS of shift is.
-  static Value *GetMaximumShiftAmount(Value *LHS, Value *RHS);
+  static Value *GetMaximumShiftAmount(Value *LHS, Value *RHS, bool RHSIsSigned);
 
   // Used for shifting constraints for OpenCL, do mask for powers of 2, URem for
   // non powers of two.
@@ -4344,7 +4353,8 @@ Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) {
   return Builder.CreateExactSDiv(diffInChars, divisor, "sub.ptr.div");
 }
 
-Value *ScalarExprEmitter::GetMaximumShiftAmount(Value *LHS, Value *RHS) {
+Value *ScalarExprEmitter::GetMaximumShiftAmount(Value *LHS, Value *RHS,
+                                                bool RHSIsSigned) {
   llvm::IntegerType *Ty;
   if (llvm::VectorType *VT = dyn_cast<llvm::VectorType>(LHS->getType()))
     Ty = cast<llvm::IntegerType>(VT->getElementType());
@@ -4355,7 +4365,9 @@ Value *ScalarExprEmitter::GetMaximumShiftAmount(Value *LHS, Value *RHS) {
   // this in ConstantInt::get, this results in the value getting truncated.
   // Constrain the return value to be max(RHS) in this case.
   llvm::Type *RHSTy = RHS->getType();
-  llvm::APInt RHSMax = llvm::APInt::getMaxValue(RHSTy->getScalarSizeInBits());
+  llvm::APInt RHSMax =
+      RHSIsSigned ? llvm::APInt::getSignedMaxValue(RHSTy->getScalarSizeInBits())
+                  : llvm::APInt::getMaxValue(RHSTy->getScalarSizeInBits());
   if (RHSMax.ult(Ty->getBitWidth()))
     return llvm::ConstantInt::get(RHSTy, RHSMax);
   return llvm::ConstantInt::get(RHSTy, Ty->getBitWidth() - 1);
@@ -4370,7 +4382,7 @@ Value *ScalarExprEmitter::ConstrainShiftValue(Value *LHS, Value *RHS,
     Ty = cast<llvm::IntegerType>(LHS->getType());
 
   if (llvm::isPowerOf2_64(Ty->getBitWidth()))
-    return Builder.CreateAnd(RHS, GetMaximumShiftAmount(LHS, RHS), Name);
+    return Builder.CreateAnd(RHS, GetMaximumShiftAmount(LHS, RHS, false), Name);
 
   return Builder.CreateURem(
       RHS, llvm::ConstantInt::get(RHS->getType(), Ty->getBitWidth()), Name);
@@ -4403,7 +4415,9 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
            isa<llvm::IntegerType>(Ops.LHS->getType())) {
     CodeGenFunction::SanitizerScope SanScope(&CGF);
     SmallVector<std::pair<Value *, SanitizerMask>, 2> Checks;
-    llvm::Value *WidthMinusOne = GetMaximumShiftAmount(Ops.LHS, Ops.RHS);
+    bool RHSIsSigned = Ops.rhsHasSignedIntegerRepresentation();
+    llvm::Value *WidthMinusOne =
+        GetMaximumShiftAmount(Ops.LHS, Ops.RHS, RHSIsSigned);
     llvm::Value *ValidExponent = Builder.CreateICmpULE(Ops.RHS, WidthMinusOne);
 
     if (SanitizeExponent) {
@@ -4421,7 +4435,7 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
       Builder.CreateCondBr(ValidExponent, CheckShiftBase, Cont);
       llvm::Value *PromotedWidthMinusOne =
           (RHS == Ops.RHS) ? WidthMinusOne
-                           : GetMaximumShiftAmount(Ops.LHS, RHS);
+                           : GetMaximumShiftAmount(Ops.LHS, RHS, RHSIsSigned);
       CGF.EmitBlock(CheckShiftBase);
       llvm::Value *BitsShiftedOff = Builder.CreateLShr(
           Ops.LHS, Builder.CreateSub(PromotedWidthMinusOne, RHS, "shl.zeros",
@@ -4471,8 +4485,9 @@ Value *ScalarExprEmitter::EmitShr(const BinOpInfo &Ops) {
   else if (CGF.SanOpts.has(SanitizerKind::ShiftExponent) &&
            isa<llvm::IntegerType>(Ops.LHS->getType())) {
     CodeGenFunction::SanitizerScope SanScope(&CGF);
-    llvm::Value *Valid =
-        Builder.CreateICmpULE(Ops.RHS, GetMaximumShiftAmount(Ops.LHS, Ops.RHS));
+    bool RHSIsSigned = Ops.rhsHasSignedIntegerRepresentation();
+    llvm::Value *Valid = Builder.CreateICmpULE(
+        Ops.RHS, GetMaximumShiftAmount(Ops.LHS, Ops.RHS, RHSIsSigned));
     EmitBinOpCheck(std::make_pair(Valid, SanitizerKind::ShiftExponent), Ops);
   }
 

clang/test/CodeGen/ubsan-shift-bitint.c

@@ -6,14 +6,14 @@
 // CHECK-LABEL: define{{.*}} i32 @test_left_variable
 int test_left_variable(unsigned _BitInt(5) b, unsigned _BitInt(2) e) {
   // CHECK: [[E_REG:%.+]] = load [[E_SIZE:i2]]
-  // CHECK: icmp ule [[E_SIZE]] [[E_REG]], -1
+  // CHECK: icmp ule [[E_SIZE]] [[E_REG]], -1,
   return b << e;
 }
 
 // CHECK-LABEL: define{{.*}} i32 @test_right_variable
 int test_right_variable(unsigned _BitInt(2) b, unsigned _BitInt(3) e) {
   // CHECK: [[E_REG:%.+]] = load [[E_SIZE:i3]]
-  // CHECK: icmp ule [[E_SIZE]] [[E_REG]], 1
+  // CHECK: icmp ule [[E_SIZE]] [[E_REG]], 1,
   return b >> e;
 }
 
@@ -34,3 +34,32 @@ int test_right_literal(unsigned _BitInt(2) b) {
   // CHECK: br i1 false, label %cont, label %handler.shift_out_of_bounds
   return b >> 4uwb;
 }
+
+// CHECK-LABEL: define{{.*}} i32 @test_signed_left_variable
+int test_signed_left_variable(unsigned _BitInt(15) b, _BitInt(2) e) {
+  // CHECK: [[E_REG:%.+]] = load [[E_SIZE:i2]]
+  // CHECK: icmp ule [[E_SIZE]] [[E_REG]], 1,
+  return b << e;
+}
+
+// CHECK-LABEL: define{{.*}} i32 @test_signed_right_variable
+int test_signed_right_variable(unsigned _BitInt(32) b, _BitInt(4) e) {
+  // CHECK: [[E_REG:%.+]] = load [[E_SIZE:i4]]
+  // CHECK: icmp ule [[E_SIZE]] [[E_REG]], 7,
+  return b >> e;
+}
+
+// CHECK-LABEL: define{{.*}} i32 @test_signed_left_literal
+int test_signed_left_literal(unsigned _BitInt(16) b) {
+  // CHECK-NOT: br i1 true, label %cont, label %handler.shift_out_of_bounds
+  // CHECK: br i1 false, label %cont, label %handler.shift_out_of_bounds
+  return b << (_BitInt(4))-2wb;
+}
+
+// CHECK-LABEL: define{{.*}} i32 @test_signed_right_literal
+int test_signed_right_literal(unsigned _BitInt(16) b) {
+  // CHECK-NOT: br i1 true, label %cont, label %handler.shift_out_of_bounds
+  // CHECK: br i1 false, label %cont, label %handler.shift_out_of_bounds
+  return b >> (_BitInt(4))-8wb;
+}
+