[ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

C requires the operands of arithmetic expressions to be promoted if
their types are smaller than an int. Ubsan emits overflow checks when
this sort of type promotion occurs, even if there is no way to actually
get an overflow with the promoted type.

This patch teaches clang how to omit the superflous overflow checks
(addressing PR20193).

Testing: check-clang and check-ubsan.

Differential Revision: https://reviews.llvm.org/D29369

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@296213 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp
index 63031d44644c08b02106ad399b01e89abc4b748c..c14e6d12da8219f741979bd5f52632ef5c88c8c2 100644 (file)
--- a/lib/CodeGen/CGExprScalar.cpp
+++ b/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 @@ static bool MustVisitNullValue(const Expr *E) {
   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 @@ public:
           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 @@ llvm::Value *ScalarExprEmitter::EmitIncDecConsiderOverflowBehavior(
       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 @@ void ScalarExprEmitter::EmitUndefinedBehaviorIntegerDivAndRemCheck(
                                     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 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) {
         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 @@ Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) {
           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()) {

diff --git a/test/CodeGen/compound-assign-overflow.c b/test/CodeGen/compound-assign-overflow.c
index f126bb05d53c1e5f76239d033d4c842b7d8aa580..92ae249eb9ff4d45972e42d1a87f9ab0a10a5741 100644 (file)
--- a/test/CodeGen/compound-assign-overflow.c
+++ b/test/CodeGen/compound-assign-overflow.c
@@ -25,11 +25,9 @@ void compaddunsigned() {
   // 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*), {{.*}})
 }

diff --git a/test/CodeGen/ubsan-promoted-arith.cpp b/test/CodeGen/ubsan-promoted-arith.cpp
new file mode 100644 (file)
index 0000000..19cbc0e
--- /dev/null
+++ b/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++; }

diff --git a/test/CodeGen/unsigned-promotion.c b/test/CodeGen/unsigned-promotion.c
index 4e7a4426a03e2f43573c882aa6f032d2b01797dc..4b13f68781b9036322cba621e07f1fb2fe546c0f 100644 (file)
--- a/test/CodeGen/unsigned-promotion.c
+++ b/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 @@ void testshortmul() {
   // 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;
-}