r296213 - [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)
Vedant Kumar via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 24 16:43:36 PST 2017
Author: vedantk
Date: Fri Feb 24 18:43:36 2017
New Revision: 296213
URL: http://llvm.org/viewvc/llvm-project?rev=296213&view=rev
Log:
[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
Added:
cfe/trunk/test/CodeGen/ubsan-promoted-arith.cpp
Modified:
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/test/CodeGen/compound-assign-overflow.c
cfe/trunk/test/CodeGen/unsigned-promotion.c
Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=296213&r1=296212&r2=296213&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Fri Feb 24 18:43:36 2017
@@ -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 Exp
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::EmitIncD
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::EmitUndefinedBeh
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
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
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()) {
Modified: cfe/trunk/test/CodeGen/compound-assign-overflow.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/compound-assign-overflow.c?rev=296213&r1=296212&r2=296213&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/compound-assign-overflow.c (original)
+++ cfe/trunk/test/CodeGen/compound-assign-overflow.c Fri Feb 24 18:43:36 2017
@@ -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*), {{.*}})
}
Added: cfe/trunk/test/CodeGen/ubsan-promoted-arith.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-promoted-arith.cpp?rev=296213&view=auto
==============================================================================
--- cfe/trunk/test/CodeGen/ubsan-promoted-arith.cpp (added)
+++ cfe/trunk/test/CodeGen/ubsan-promoted-arith.cpp Fri Feb 24 18:43:36 2017
@@ -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++; }
Modified: cfe/trunk/test/CodeGen/unsigned-promotion.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/unsigned-promotion.c?rev=296213&r1=296212&r2=296213&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/unsigned-promotion.c (original)
+++ cfe/trunk/test/CodeGen/unsigned-promotion.c Fri Feb 24 18:43:36 2017
@@ -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;
-}
More information about the cfe-commits
mailing list