r293572 - Re-apply "[ubsan] Sanity-check shift amounts before truncation"
Vedant Kumar via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 30 15:38:54 PST 2017
Author: vedantk
Date: Mon Jan 30 17:38:54 2017
New Revision: 293572
URL: http://llvm.org/viewvc/llvm-project?rev=293572&view=rev
Log:
Re-apply "[ubsan] Sanity-check shift amounts before truncation"
This re-applies r293343 (reverts commit r293475) with a fix for an
assertion failure caused by a missing integer cast. I tested this patch
by using the built compiler to compile X86FastISel.cpp.o with ubsan.
Original commit message:
Ubsan does not report UB shifts in some cases where the shift exponent
needs to be truncated to match the type of the shift base. We perform a
range check on the truncated shift amount, leading to false negatives.
Fix the issue (PR27271) by performing the range check on the original
shift amount.
Differential Revision: https://reviews.llvm.org/D29234
Added:
cfe/trunk/test/CodeGen/ubsan-shift.c
Modified:
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=293572&r1=293571&r2=293572&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Mon Jan 30 17:38:54 2017
@@ -2751,8 +2751,8 @@ Value *ScalarExprEmitter::EmitShl(const
isa<llvm::IntegerType>(Ops.LHS->getType())) {
CodeGenFunction::SanitizerScope SanScope(&CGF);
SmallVector<std::pair<Value *, SanitizerMask>, 2> Checks;
- llvm::Value *WidthMinusOne = GetWidthMinusOneValue(Ops.LHS, RHS);
- llvm::Value *ValidExponent = Builder.CreateICmpULE(RHS, WidthMinusOne);
+ llvm::Value *WidthMinusOne = GetWidthMinusOneValue(Ops.LHS, Ops.RHS);
+ llvm::Value *ValidExponent = Builder.CreateICmpULE(Ops.RHS, WidthMinusOne);
if (SanitizeExponent) {
Checks.push_back(
@@ -2767,12 +2767,14 @@ Value *ScalarExprEmitter::EmitShl(const
llvm::BasicBlock *Cont = CGF.createBasicBlock("cont");
llvm::BasicBlock *CheckShiftBase = CGF.createBasicBlock("check");
Builder.CreateCondBr(ValidExponent, CheckShiftBase, Cont);
+ llvm::Value *PromotedWidthMinusOne =
+ (RHS == Ops.RHS) ? WidthMinusOne
+ : GetWidthMinusOneValue(Ops.LHS, RHS);
CGF.EmitBlock(CheckShiftBase);
- llvm::Value *BitsShiftedOff =
- Builder.CreateLShr(Ops.LHS,
- Builder.CreateSub(WidthMinusOne, RHS, "shl.zeros",
- /*NUW*/true, /*NSW*/true),
- "shl.check");
+ llvm::Value *BitsShiftedOff = Builder.CreateLShr(
+ Ops.LHS, Builder.CreateSub(PromotedWidthMinusOne, RHS, "shl.zeros",
+ /*NUW*/ true, /*NSW*/ true),
+ "shl.check");
if (CGF.getLangOpts().CPlusPlus) {
// In C99, we are not permitted to shift a 1 bit into the sign bit.
// Under C++11's rules, shifting a 1 bit into the sign bit is
Added: cfe/trunk/test/CodeGen/ubsan-shift.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-shift.c?rev=293572&view=auto
==============================================================================
--- cfe/trunk/test/CodeGen/ubsan-shift.c (added)
+++ cfe/trunk/test/CodeGen/ubsan-shift.c Mon Jan 30 17:38:54 2017
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin -fsanitize=shift-exponent,shift-base -emit-llvm %s -o - | FileCheck %s
+
+// CHECK-LABEL: define i32 @f1
+int f1(int c, int shamt) {
+// CHECK: icmp ule i32 %{{.*}}, 31, !nosanitize
+// CHECK: icmp ule i32 %{{.*}}, 31, !nosanitize
+ return 1 << (c << shamt);
+}
+
+// CHECK-LABEL: define i32 @f2
+int f2(long c, int shamt) {
+// CHECK: icmp ule i32 %{{.*}}, 63, !nosanitize
+// CHECK: icmp ule i64 %{{.*}}, 31, !nosanitize
+ return 1 << (c << shamt);
+}
+
+// CHECK-LABEL: define i32 @f3
+unsigned f3(unsigned c, int shamt) {
+// CHECK: icmp ule i32 %{{.*}}, 31, !nosanitize
+// CHECK: icmp ule i32 %{{.*}}, 31, !nosanitize
+ return 1U << (c << shamt);
+}
+
+// CHECK-LABEL: define i32 @f4
+unsigned f4(unsigned long c, int shamt) {
+// CHECK: icmp ule i32 %{{.*}}, 63, !nosanitize
+// CHECK: icmp ule i64 %{{.*}}, 31, !nosanitize
+ return 1U << (c << shamt);
+}
+
+// CHECK-LABEL: define i32 @f5
+int f5(int c, long long shamt) {
+// CHECK: icmp ule i64 %{{[0-9]+}}, 31, !nosanitize
+//
+// CHECK: sub nuw nsw i32 31, %sh_prom, !nosanitize
+// CHECK: lshr i32 %{{.*}}, %shl.zeros, !nosanitize
+ return c << shamt;
+}
+
+// CHECK-LABEL: define i32 @f6
+int f6(int c, int shamt) {
+// CHECK: icmp ule i32 %[[WIDTH:.*]], 31, !nosanitize
+//
+// CHECK: sub nuw nsw i32 31, %[[WIDTH]], !nosanitize
+// CHECK: lshr i32 %{{.*}}, %shl.zeros, !nosanitize
+ return c << shamt;
+}
More information about the cfe-commits
mailing list