[clang] 20667db - [clang][CodeGen] Fix shift-exponent ubsan check for signed _BitInt (#88004)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 18 23:11:44 PDT 2024
Author: Björn Pettersson
Date: 2024-04-19T08:11:40+02:00
New Revision: 20667dbec30354723d6467b833a081db998eb70a
URL: https://github.com/llvm/llvm-project/commit/20667dbec30354723d6467b833a081db998eb70a
DIFF: https://github.com/llvm/llvm-project/commit/20667dbec30354723d6467b833a081db998eb70a.diff
LOG: [clang][CodeGen] Fix shift-exponent ubsan check for signed _BitInt (#88004)
Commit 5f87957fefb21d454f2f (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.
Added:
Modified:
clang/lib/CodeGen/CGExprScalar.cpp
clang/test/CodeGen/ubsan-shift-bitint.c
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 1f18e0d5ba409a..c76052ff6280f9 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/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(
diff InChars, 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);
}
diff --git a/clang/test/CodeGen/ubsan-shift-bitint.c b/clang/test/CodeGen/ubsan-shift-bitint.c
index 844d5c4ad84618..af65ed60918b08 100644
--- a/clang/test/CodeGen/ubsan-shift-bitint.c
+++ b/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;
+}
+
More information about the cfe-commits
mailing list