This revision was automatically updated to reflect the committed changes.
Closed by commit rL296213: [ubsan] Omit superflous overflow checks for promoted
arithmetic (PR20193) (authored by vedantk).
Changed prior to commit:
https://reviews.llvm.org/D29369?vs=89733&id=89752#toc
Repository:
rL LLVM
https://reviews.llvm.org/D29369
Files:
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/test/CodeGen/compound-assign-overflow.c
cfe/trunk/test/CodeGen/ubsan-promoted-arith.cpp
cfe/trunk/test/CodeGen/unsigned-promotion.c
Index: cfe/trunk/test/CodeGen/ubsan-promoted-arith.cpp
===================================================================
--- cfe/trunk/test/CodeGen/ubsan-promoted-arith.cpp
+++ cfe/trunk/test/CodeGen/ubsan-promoted-arith.cpp
@@ -0,0 +1,124 @@
+// 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: USHRT_MAX * USHRT_MAX 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; }
+
+// CHECK-LABEL: define signext i8 @_Z4div3
+// CHECK-NOT: ubsan_handle_divrem_overflow
+char div3(char c, int i) { return c / i; }
+
+// CHECK-LABEL: define signext i8 @_Z4div4
+// CHECK: ubsan_handle_divrem_overflow
+char div4(int i, char c) { return i / c; }
+
+// Note: INT_MIN / -1 can overflow.
+//
+// CHECK-LABEL: define signext i8 @_Z4div5
+// CHECK: ubsan_handle_divrem_overflow
+char div5(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: cfe/trunk/test/CodeGen/unsigned-promotion.c
===================================================================
--- cfe/trunk/test/CodeGen/unsigned-promotion.c
+++ cfe/trunk/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;
-}
Index: cfe/trunk/test/CodeGen/compound-assign-overflow.c
===================================================================
--- cfe/trunk/test/CodeGen/compound-assign-overflow.c
+++ cfe/trunk/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: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===================================================================
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/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,59 @@
return E->getType()->isNullPtrType();
}
+/// If \p E is a widened promoted integer, get its base (unpromoted) type.
+static llvm::Optional<QualType> 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 \p E is a widened promoted integer.
+static bool IsWidenedIntegerOp(const ASTContext &Ctx, const Expr *E) {
+ return getUnwidenedIntegerType(Ctx, E).hasValue();
+}
+
+/// Check if we can skip the overflow check for \p Op.
+static bool CanElideOverflowCheck(const ASTContext &Ctx, const BinOpInfo &Op) {
+ assert(isa<UnaryOperator>(Op.E) ||
+ isa<BinaryOperator>(Op.E) && "Expected a unary or binary operator");
+
+ if (const auto *UO = dyn_cast<UnaryOperator>(Op.E))
+ return IsWidenedIntegerOp(Ctx, UO->getSubExpr());
+
+ const auto *BO = cast<BinaryOperator>(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<ScalarExprEmitter, Value*> {
CodeGenFunction &CGF;
@@ -482,12 +536,15 @@
return Builder.CreateNSWMul(Ops.LHS, Ops.RHS, "mul");
// Fall through.
case LangOptions::SOB_Trapping:
+ if (CanElideOverflowCheck(CGF.getContext(), Ops))
+ return Builder.CreateNSWMul(Ops.LHS, Ops.RHS, "mul");
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())
@@ -1663,6 +1720,8 @@
return Builder.CreateNSWAdd(InVal, Amount, Name);
// Fall through.
case LangOptions::SOB_Trapping:
+ if (IsWidenedIntegerOp(CGF.getContext(), E->getSubExpr()))
+ return Builder.CreateNSWAdd(InVal, Amount, Name);
return EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(E, InVal, IsInc));
}
llvm_unreachable("Unknown SignedOverflowBehaviorTy");
@@ -2281,8 +2340,10 @@
SanitizerKind::IntegerDivideByZero));
}
+ const auto *BO = cast<BinaryOperator>(Ops.E);
if (CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow) &&
- Ops.Ty->hasSignedIntegerRepresentation()) {
+ Ops.Ty->hasSignedIntegerRepresentation() &&
+ !IsWidenedIntegerOp(CGF.getContext(), BO->getLHS())) {
llvm::IntegerType *Ty = cast<llvm::IntegerType>(Zero->getType());
llvm::Value *IntMin =
@@ -2634,12 +2695,15 @@
return Builder.CreateNSWAdd(op.LHS, op.RHS, "add");
// Fall through.
case LangOptions::SOB_Trapping:
+ if (CanElideOverflowCheck(CGF.getContext(), op))
+ return Builder.CreateNSWAdd(op.LHS, op.RHS, "add");
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()) {
@@ -2665,12 +2729,15 @@
return Builder.CreateNSWSub(op.LHS, op.RHS, "sub");
// Fall through.
case LangOptions::SOB_Trapping:
+ if (CanElideOverflowCheck(CGF.getContext(), op))
+ return Builder.CreateNSWSub(op.LHS, op.RHS, "sub");
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()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
