[ubsan] Skip overflow checks on safe arithmetic (fixes PR32874)
Currently, ubsan emits overflow checks for arithmetic that is known to
be safe at compile-time, e.g:
1 + 1 => CheckedAdd(1, 1)
This leads to breakage when using the __builtin_prefetch intrinsic. LLVM
expects the arguments to @llvm.prefetch to be constant integers, and
when ubsan inserts unnecessary checks on the operands to the intrinsic,
this contract is broken, leading to verifier failures (see PR32874).
Instead of special-casing __builtin_prefetch for ubsan, this patch fixes
the underlying problem, i.e that clang currently emits unnecessary
overflow checks.
Testing: I ran the check-clang and check-ubsan targets with a stage2,
ubsan-enabled build of clang. I added a regression test for PR32874, and
some extra checking to make sure we don't regress runtime checking for
unsafe arithmetic. The existing ubsan-promoted-arithmetic.cpp test also
provides coverage for this change.
llvm-svn: 301988
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index a643038..70b7416 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -51,6 +51,64 @@
BinaryOperator::Opcode Opcode; // Opcode of BinOp to perform
FPOptions FPFeatures;
const Expr *E; // Entire expr, for error unsupported. May not be binop.
+
+ /// Check if the binop can result in integer overflow.
+ bool mayHaveIntegerOverflow() const {
+ // Without constant input, we can't rule out overflow.
+ const auto *LHSCI = dyn_cast<llvm::ConstantInt>(LHS);
+ const auto *RHSCI = dyn_cast<llvm::ConstantInt>(RHS);
+ if (!LHSCI || !RHSCI)
+ return true;
+
+ // Assume overflow is possible, unless we can prove otherwise.
+ bool Overflow = true;
+ const auto &LHSAP = LHSCI->getValue();
+ const auto &RHSAP = RHSCI->getValue();
+ if (Opcode == BO_Add) {
+ if (Ty->hasSignedIntegerRepresentation())
+ (void)LHSAP.sadd_ov(RHSAP, Overflow);
+ else
+ (void)LHSAP.uadd_ov(RHSAP, Overflow);
+ } else if (Opcode == BO_Sub) {
+ if (Ty->hasSignedIntegerRepresentation())
+ (void)LHSAP.ssub_ov(RHSAP, Overflow);
+ else
+ (void)LHSAP.usub_ov(RHSAP, Overflow);
+ } else if (Opcode == BO_Mul) {
+ if (Ty->hasSignedIntegerRepresentation())
+ (void)LHSAP.smul_ov(RHSAP, Overflow);
+ else
+ (void)LHSAP.umul_ov(RHSAP, Overflow);
+ } else if (Opcode == BO_Div || Opcode == BO_Rem) {
+ if (Ty->hasSignedIntegerRepresentation() && !RHSCI->isZero())
+ (void)LHSAP.sdiv_ov(RHSAP, Overflow);
+ else
+ return false;
+ }
+ return Overflow;
+ }
+
+ /// Check if the binop computes a division or a remainder.
+ bool isDivisionLikeOperation() const {
+ return Opcode == BO_Div || Opcode == BO_Rem || Opcode == BO_DivAssign ||
+ Opcode == BO_RemAssign;
+ }
+
+ /// Check if the binop can result in an integer division by zero.
+ bool mayHaveIntegerDivisionByZero() const {
+ if (isDivisionLikeOperation())
+ if (auto *CI = dyn_cast<llvm::ConstantInt>(RHS))
+ return CI->isZero();
+ return true;
+ }
+
+ /// Check if the binop can result in a float division by zero.
+ bool mayHaveFloatDivisionByZero() const {
+ if (isDivisionLikeOperation())
+ if (auto *CFP = dyn_cast<llvm::ConstantFP>(RHS))
+ return CFP->isZero();
+ return true;
+ }
};
static bool MustVisitNullValue(const Expr *E) {
@@ -85,9 +143,17 @@
assert((isa<UnaryOperator>(Op.E) || isa<BinaryOperator>(Op.E)) &&
"Expected a unary or binary operator");
+ // If the binop has constant inputs and we can prove there is no overflow,
+ // we can elide the overflow check.
+ if (!Op.mayHaveIntegerOverflow())
+ return true;
+
+ // If a unary op has a widened operand, the op cannot overflow.
if (const auto *UO = dyn_cast<UnaryOperator>(Op.E))
return IsWidenedIntegerOp(Ctx, UO->getSubExpr());
+ // We usually don't need overflow checks for binops with widened operands.
+ // Multiplication with promoted unsigned operands is a special case.
const auto *BO = cast<BinaryOperator>(Op.E);
auto OptionalLHSTy = getUnwidenedIntegerType(Ctx, BO->getLHS());
if (!OptionalLHSTy)
@@ -100,14 +166,14 @@
QualType LHSTy = *OptionalLHSTy;
QualType RHSTy = *OptionalRHSTy;
- // We usually don't need overflow checks for binary operations with widened
- // operands. Multiplication with promoted unsigned operands is a special case.
+ // This is the simple case: binops without unsigned multiplication, and with
+ // widened operands. No overflow check is needed here.
if ((Op.Opcode != BO_Mul && Op.Opcode != BO_MulAssign) ||
!LHSTy->isUnsignedIntegerType() || !RHSTy->isUnsignedIntegerType())
return true;
- // The overflow check can be skipped if either one of the unpromoted types
- // are less than half the size of the promoted type.
+ // For unsigned multiplication the overflow check can be elided if either one
+ // of the unpromoted types are less than half the size of the promoted type.
unsigned PromotedSize = Ctx.getTypeSize(Op.E->getType());
return (2 * Ctx.getTypeSize(LHSTy)) < PromotedSize ||
(2 * Ctx.getTypeSize(RHSTy)) < PromotedSize;
@@ -2377,7 +2443,8 @@
const auto *BO = cast<BinaryOperator>(Ops.E);
if (CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow) &&
Ops.Ty->hasSignedIntegerRepresentation() &&
- !IsWidenedIntegerOp(CGF.getContext(), BO->getLHS())) {
+ !IsWidenedIntegerOp(CGF.getContext(), BO->getLHS()) &&
+ Ops.mayHaveIntegerOverflow()) {
llvm::IntegerType *Ty = cast<llvm::IntegerType>(Zero->getType());
llvm::Value *IntMin =
@@ -2400,11 +2467,13 @@
CodeGenFunction::SanitizerScope SanScope(&CGF);
if ((CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero) ||
CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) &&
- Ops.Ty->isIntegerType()) {
+ Ops.Ty->isIntegerType() &&
+ (Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow())) {
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, true);
} else if (CGF.SanOpts.has(SanitizerKind::FloatDivideByZero) &&
- Ops.Ty->isRealFloatingType()) {
+ Ops.Ty->isRealFloatingType() &&
+ Ops.mayHaveFloatDivisionByZero()) {
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
llvm::Value *NonZero = Builder.CreateFCmpUNE(Ops.RHS, Zero);
EmitBinOpCheck(std::make_pair(NonZero, SanitizerKind::FloatDivideByZero),
@@ -2439,7 +2508,8 @@
// Rem in C can't be a floating point type: C99 6.5.5p2.
if ((CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero) ||
CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) &&
- Ops.Ty->isIntegerType()) {
+ Ops.Ty->isIntegerType() &&
+ (Ops.mayHaveIntegerDivisionByZero() || Ops.mayHaveIntegerOverflow())) {
CodeGenFunction::SanitizerScope SanScope(&CGF);
llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
EmitUndefinedBehaviorIntegerDivAndRemCheck(Ops, Zero, false);