Index: lib/CodeGen/CGExprScalar.cpp =================================================================== --- lib/CodeGen/CGExprScalar.cpp +++ lib/CodeGen/CGExprScalar.cpp @@ -24,6 +24,7 @@ #include "clang/AST/StmtVisitor.h" #include "clang/Basic/TargetInfo.h" #include "clang/Frontend/CodeGenOptions.h" +#include "llvm/ADT/Optional.h" #include "llvm/IR/CFG.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DataLayout.h" @@ -58,6 +59,54 @@ return E->getType()->isNullPtrType(); } +/// If \p E is a widened promoted integer, get its base (unpromoted) type. +static llvm::Optional getUnwidenedIntegerType(const ASTContext &Ctx, + const Expr *E) { + const Expr *Base = E->IgnoreImpCasts(); + if (E == Base) + return llvm::None; + + QualType BaseTy = Base->getType(); + if (!BaseTy->isPromotableIntegerType() || + Ctx.getTypeSize(BaseTy) >= Ctx.getTypeSize(E->getType())) + return llvm::None; + + return BaseTy; +} + +/// Check if we can skip the overflow check for \p Op. +static bool CanElideOverflowCheck(const ASTContext &Ctx, const BinOpInfo &Op) { + assert(isa(Op.E) || + isa(Op.E) && "Expected a unary or binary operator"); + + if (const auto *UO = dyn_cast(Op.E)) + return getUnwidenedIntegerType(Ctx, UO->getSubExpr()).hasValue(); + + const auto *BO = cast(Op.E); + auto OptionalLHSTy = getUnwidenedIntegerType(Ctx, BO->getLHS()); + if (!OptionalLHSTy) + return false; + + auto OptionalRHSTy = getUnwidenedIntegerType(Ctx, BO->getRHS()); + if (!OptionalRHSTy) + return false; + + 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. + 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. + unsigned PromotedSize = Ctx.getTypeSize(Op.E->getType()); + return (2 * Ctx.getTypeSize(LHSTy)) < PromotedSize || + (2 * Ctx.getTypeSize(RHSTy)) < PromotedSize; +} + class ScalarExprEmitter : public StmtVisitor { CodeGenFunction &CGF; @@ -456,20 +505,21 @@ // Binary Operators. Value *EmitMul(const BinOpInfo &Ops) { if (Ops.Ty->isSignedIntegerOrEnumerationType()) { - switch (CGF.getLangOpts().getSignedOverflowBehavior()) { - case LangOptions::SOB_Defined: + auto SOB = CGF.getLangOpts().getSignedOverflowBehavior(); + if (SOB == LangOptions::SOB_Defined) { return Builder.CreateMul(Ops.LHS, Ops.RHS, "mul"); - case LangOptions::SOB_Undefined: - if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) - return Builder.CreateNSWMul(Ops.LHS, Ops.RHS, "mul"); - // Fall through. - case LangOptions::SOB_Trapping: + } else if ((SOB == LangOptions::SOB_Undefined && + !CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) || + CanElideOverflowCheck(CGF.getContext(), Ops)) { + return Builder.CreateNSWMul(Ops.LHS, Ops.RHS, "mul"); + } else { return EmitOverflowCheckedBinOp(Ops); } } if (Ops.Ty->isUnsignedIntegerType() && - CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) + CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) && + !CanElideOverflowCheck(CGF.getContext(), Ops)) return EmitOverflowCheckedBinOp(Ops); if (Ops.LHS->getType()->isFPOrFPVectorTy()) @@ -1637,17 +1687,16 @@ llvm::Value *Amount = llvm::ConstantInt::get(InVal->getType(), IsInc ? 1 : -1, true); StringRef Name = IsInc ? "inc" : "dec"; - switch (CGF.getLangOpts().getSignedOverflowBehavior()) { - case LangOptions::SOB_Defined: + auto SOB = CGF.getLangOpts().getSignedOverflowBehavior(); + if (SOB == LangOptions::SOB_Defined) { return Builder.CreateAdd(InVal, Amount, Name); - case LangOptions::SOB_Undefined: - if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) - return Builder.CreateNSWAdd(InVal, Amount, Name); - // Fall through. - case LangOptions::SOB_Trapping: + } else if ((SOB == LangOptions::SOB_Undefined && + !CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) || + getUnwidenedIntegerType(CGF.getContext(), E->getSubExpr())) { + return Builder.CreateNSWAdd(InVal, Amount, Name); + } else { return EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(E, InVal, IsInc)); } - llvm_unreachable("Unknown SignedOverflowBehaviorTy"); } llvm::Value * @@ -2263,8 +2312,10 @@ SanitizerKind::IntegerDivideByZero)); } + const auto *BO = cast(Ops.E); if (CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow) && - Ops.Ty->hasSignedIntegerRepresentation()) { + Ops.Ty->hasSignedIntegerRepresentation() && + !getUnwidenedIntegerType(CGF.getContext(), BO->getLHS())) { llvm::IntegerType *Ty = cast(Zero->getType()); llvm::Value *IntMin = @@ -2608,20 +2659,21 @@ return emitPointerArithmetic(CGF, op, /*subtraction*/ false); if (op.Ty->isSignedIntegerOrEnumerationType()) { - switch (CGF.getLangOpts().getSignedOverflowBehavior()) { - case LangOptions::SOB_Defined: + auto SOB = CGF.getLangOpts().getSignedOverflowBehavior(); + if (SOB == LangOptions::SOB_Defined) { return Builder.CreateAdd(op.LHS, op.RHS, "add"); - case LangOptions::SOB_Undefined: - if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) - return Builder.CreateNSWAdd(op.LHS, op.RHS, "add"); - // Fall through. - case LangOptions::SOB_Trapping: + } else if ((SOB == LangOptions::SOB_Undefined && + !CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) || + CanElideOverflowCheck(CGF.getContext(), op)) { + return Builder.CreateNSWAdd(op.LHS, op.RHS, "add"); + } else { return EmitOverflowCheckedBinOp(op); } } if (op.Ty->isUnsignedIntegerType() && - CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) + CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) && + !CanElideOverflowCheck(CGF.getContext(), op)) return EmitOverflowCheckedBinOp(op); if (op.LHS->getType()->isFPOrFPVectorTy()) { @@ -2639,20 +2691,21 @@ // The LHS is always a pointer if either side is. if (!op.LHS->getType()->isPointerTy()) { if (op.Ty->isSignedIntegerOrEnumerationType()) { - switch (CGF.getLangOpts().getSignedOverflowBehavior()) { - case LangOptions::SOB_Defined: + auto SOB = CGF.getLangOpts().getSignedOverflowBehavior(); + if (SOB == LangOptions::SOB_Defined) { return Builder.CreateSub(op.LHS, op.RHS, "sub"); - case LangOptions::SOB_Undefined: - if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) - return Builder.CreateNSWSub(op.LHS, op.RHS, "sub"); - // Fall through. - case LangOptions::SOB_Trapping: + } else if ((SOB == LangOptions::SOB_Undefined && + !CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) || + CanElideOverflowCheck(CGF.getContext(), op)) { + return Builder.CreateNSWSub(op.LHS, op.RHS, "sub"); + } else { return EmitOverflowCheckedBinOp(op); } } if (op.Ty->isUnsignedIntegerType() && - CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) + CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow) && + !CanElideOverflowCheck(CGF.getContext(), op)) return EmitOverflowCheckedBinOp(op); if (op.LHS->getType()->isFPOrFPVectorTy()) { Index: test/CodeGen/compound-assign-overflow.c =================================================================== --- test/CodeGen/compound-assign-overflow.c +++ test/CodeGen/compound-assign-overflow.c @@ -25,11 +25,9 @@ // CHECK: @__ubsan_handle_add_overflow(i8* bitcast ({{.*}} @[[LINE_200]] to i8*), {{.*}}) } -int8_t a, b; - // CHECK: @compdiv void compdiv() { #line 300 - a /= b; + x /= x; // CHECK: @__ubsan_handle_divrem_overflow(i8* bitcast ({{.*}} @[[LINE_300]] to i8*), {{.*}}) } Index: test/CodeGen/ubsan-promoted-arith.cpp =================================================================== --- /dev/null +++ test/CodeGen/ubsan-promoted-arith.cpp @@ -0,0 +1,116 @@ +// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=signed-integer-overflow,unsigned-integer-overflow | FileCheck %s + +typedef unsigned char uchar; +typedef unsigned short ushort; + +enum E1 : int { + a +}; + +enum E2 : char { + b +}; + +// CHECK-LABEL: define signext i8 @_Z4add1 +// CHECK-NOT: sadd.with.overflow +char add1(char c) { return c + c; } + +// CHECK-LABEL: define zeroext i8 @_Z4add2 +// CHECK-NOT: uadd.with.overflow +uchar add2(uchar uc) { return uc + uc; } + +// CHECK-LABEL: define i32 @_Z4add3 +// CHECK: sadd.with.overflow +int add3(E1 e) { return e + a; } + +// CHECK-LABEL: define signext i8 @_Z4add4 +// CHECK-NOT: sadd.with.overflow +char add4(E2 e) { return e + b; } + +// CHECK-LABEL: define signext i8 @_Z4sub1 +// CHECK-NOT: ssub.with.overflow +char sub1(char c) { return c - c; } + +// CHECK-LABEL: define zeroext i8 @_Z4sub2 +// CHECK-NOT: usub.with.overflow +uchar sub2(uchar uc) { return uc - uc; } + +// CHECK-LABEL: define signext i8 @_Z4sub3 +// CHECK-NOT: ssub.with.overflow +char sub3(char c) { return -c; } + +// Note: -INT_MIN can overflow. +// +// CHECK-LABEL: define i32 @_Z4sub4 +// CHECK: ssub.with.overflow +int sub4(int i) { return -i; } + +// CHECK-LABEL: define signext i8 @_Z4mul1 +// CHECK-NOT: smul.with.overflow +char mul1(char c) { return c * c; } + +// CHECK-LABEL: define zeroext i8 @_Z4mul2 +// CHECK-NOT: smul.with.overflow +uchar mul2(uchar uc) { return uc * uc; } + +// Note: -SHRT_MIN * -SHRT_MIN can overflow. +// +// CHECK-LABEL: define zeroext i16 @_Z4mul3 +// CHECK: smul.with.overflow +ushort mul3(ushort us) { return us * us; } + +// CHECK-LABEL: define i32 @_Z4mul4 +// CHECK: smul.with.overflow +int mul4(int i, char c) { return i * c; } + +// CHECK-LABEL: define i32 @_Z4mul5 +// CHECK: smul.with.overflow +int mul5(int i, char c) { return c * i; } + +// CHECK-LABEL: define signext i16 @_Z4mul6 +// CHECK-NOT: smul.with.overflow +short mul6(short s) { return s * s; } + +// CHECK-LABEL: define signext i8 @_Z4div1 +// CHECK-NOT: ubsan_handle_divrem_overflow +char div1(char c) { return c / c; } + +// CHECK-LABEL: define zeroext i8 @_Z4div2 +// CHECK-NOT: ubsan_handle_divrem_overflow +uchar div2(uchar uc) { return uc / uc; } + +// Note: -INT_MIN / -1 can overflow. +// +// CHECK-LABEL: define signext i8 @_Z4div3 +// CHECK: ubsan_handle_divrem_overflow +char div3(int i, char c) { return i / c; } + +// CHECK-LABEL: define signext i8 @_Z4rem1 +// CHECK-NOT: ubsan_handle_divrem_overflow +char rem1(char c) { return c % c; } + +// CHECK-LABEL: define zeroext i8 @_Z4rem2 +// CHECK-NOT: ubsan_handle_divrem_overflow +uchar rem2(uchar uc) { return uc % uc; } + +// FIXME: This is a long-standing false negative. +// +// CHECK-LABEL: define signext i8 @_Z4rem3 +// rdar30301609: ubsan_handle_divrem_overflow +char rem3(int i, char c) { return i % c; } + +// CHECK-LABEL: define signext i8 @_Z4inc1 +// CHECK-NOT: sadd.with.overflow +char inc1(char c) { return c++ + (char)0; } + +// CHECK-LABEL: define zeroext i8 @_Z4inc2 +// CHECK-NOT: uadd.with.overflow +uchar inc2(uchar uc) { return uc++ + (uchar)0; } + +// CHECK-LABEL: define void @_Z4inc3 +// CHECK-NOT: sadd.with.overflow +void inc3(char c) { c++; } + +// CHECK-LABEL: define void @_Z4inc4 +// CHECK-NOT: uadd.with.overflow +void inc4(uchar uc) { uc++; } Index: test/CodeGen/unsigned-promotion.c =================================================================== --- test/CodeGen/unsigned-promotion.c +++ test/CodeGen/unsigned-promotion.c @@ -7,53 +7,6 @@ // RUN: -fsanitize=unsigned-integer-overflow | FileCheck %s --check-prefix=CHECKU unsigned short si, sj, sk; -unsigned char ci, cj, ck; - -extern void opaqueshort(unsigned short); -extern void opaquechar(unsigned char); - -// CHECKS-LABEL: define void @testshortadd() -// CHECKU-LABEL: define void @testshortadd() -void testshortadd() { - // CHECKS: load i16, i16* @sj - // CHECKS: load i16, i16* @sk - // CHECKS: [[T1:%.*]] = call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]]) - // CHECKS-NEXT: [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0 - // CHECKS-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1 - // CHECKS: call void @__ubsan_handle_add_overflow - // - // CHECKU: [[T1:%.*]] = load i16, i16* @sj - // CHECKU: [[T2:%.*]] = zext i16 [[T1]] - // CHECKU: [[T3:%.*]] = load i16, i16* @sk - // CHECKU: [[T4:%.*]] = zext i16 [[T3]] - // CHECKU-NOT: llvm.sadd - // CHECKU-NOT: llvm.uadd - // CHECKU: [[T5:%.*]] = add nsw i32 [[T2]], [[T4]] - - si = sj + sk; -} - -// CHECKS-LABEL: define void @testshortsub() -// CHECKU-LABEL: define void @testshortsub() -void testshortsub() { - - // CHECKS: load i16, i16* @sj - // CHECKS: load i16, i16* @sk - // CHECKS: [[T1:%.*]] = call { i32, i1 } @llvm.ssub.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]]) - // CHECKS-NEXT: [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0 - // CHECKS-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1 - // CHECKS: call void @__ubsan_handle_sub_overflow - // - // CHECKU: [[T1:%.*]] = load i16, i16* @sj - // CHECKU: [[T2:%.*]] = zext i16 [[T1]] - // CHECKU: [[T3:%.*]] = load i16, i16* @sk - // CHECKU: [[T4:%.*]] = zext i16 [[T3]] - // CHECKU-NOT: llvm.ssub - // CHECKU-NOT: llvm.usub - // CHECKU: [[T5:%.*]] = sub nsw i32 [[T2]], [[T4]] - - si = sj - sk; -} // CHECKS-LABEL: define void @testshortmul() // CHECKU-LABEL: define void @testshortmul() @@ -75,69 +28,3 @@ // CHECKU: [[T5:%.*]] = mul nsw i32 [[T2]], [[T4]] si = sj * sk; } - -// CHECKS-LABEL: define void @testcharadd() -// CHECKU-LABEL: define void @testcharadd() -void testcharadd() { - - // CHECKS: load i8, i8* @cj - // CHECKS: load i8, i8* @ck - // CHECKS: [[T1:%.*]] = call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]]) - // CHECKS-NEXT: [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0 - // CHECKS-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1 - // CHECKS: call void @__ubsan_handle_add_overflow - // - // CHECKU: [[T1:%.*]] = load i8, i8* @cj - // CHECKU: [[T2:%.*]] = zext i8 [[T1]] - // CHECKU: [[T3:%.*]] = load i8, i8* @ck - // CHECKU: [[T4:%.*]] = zext i8 [[T3]] - // CHECKU-NOT: llvm.sadd - // CHECKU-NOT: llvm.uadd - // CHECKU: [[T5:%.*]] = add nsw i32 [[T2]], [[T4]] - - ci = cj + ck; -} - -// CHECKS-LABEL: define void @testcharsub() -// CHECKU-LABEL: define void @testcharsub() -void testcharsub() { - - // CHECKS: load i8, i8* @cj - // CHECKS: load i8, i8* @ck - // CHECKS: [[T1:%.*]] = call { i32, i1 } @llvm.ssub.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]]) - // CHECKS-NEXT: [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0 - // CHECKS-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1 - // CHECKS: call void @__ubsan_handle_sub_overflow - // - // CHECKU: [[T1:%.*]] = load i8, i8* @cj - // CHECKU: [[T2:%.*]] = zext i8 [[T1]] - // CHECKU: [[T3:%.*]] = load i8, i8* @ck - // CHECKU: [[T4:%.*]] = zext i8 [[T3]] - // CHECKU-NOT: llvm.ssub - // CHECKU-NOT: llvm.usub - // CHECKU: [[T5:%.*]] = sub nsw i32 [[T2]], [[T4]] - - ci = cj - ck; -} - -// CHECKS-LABEL: define void @testcharmul() -// CHECKU-LABEL: define void @testcharmul() -void testcharmul() { - - // CHECKS: load i8, i8* @cj - // CHECKS: load i8, i8* @ck - // CHECKS: [[T1:%.*]] = call { i32, i1 } @llvm.smul.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]]) - // CHECKS-NEXT: [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0 - // CHECKS-NEXT: [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1 - // CHECKS: call void @__ubsan_handle_mul_overflow - // - // CHECKU: [[T1:%.*]] = load i8, i8* @cj - // CHECKU: [[T2:%.*]] = zext i8 [[T1]] - // CHECKU: [[T3:%.*]] = load i8, i8* @ck - // CHECKU: [[T4:%.*]] = zext i8 [[T3]] - // CHECKU-NOT: llvm.smul - // CHECKU-NOT: llvm.umul - // CHECKU: [[T5:%.*]] = mul nsw i32 [[T2]], [[T4]] - - ci = cj * ck; -}