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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits