Another example + possible fix (two WidthMinusOne, one per (possibly different) bitwidth): (not fully tested)
int f() { return 0 << 0L; } diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index 40d949dece..5c9055d49a 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -2751,8 +2751,9 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) { isa<llvm::IntegerType>(Ops.LHS->getType())) { CodeGenFunction::SanitizerScope SanScope(&CGF); SmallVector<std::pair<Value *, SanitizerMask>, 2> Checks; - llvm::Value *WidthMinusOne = GetWidthMinusOneValue(Ops.LHS, Ops.RHS); - llvm::Value *ValidExponent = Builder.CreateICmpULE(Ops.RHS, WidthMinusOne); + llvm::Value *WidthMinusOneRHS = GetWidthMinusOneValue(Ops.LHS, Ops.RHS); + llvm::Value *WidthMinusOneLHS = GetWidthMinusOneValue(Ops.LHS, Ops.LHS); + llvm::Value *ValidExponent = Builder.CreateICmpULE(Ops.RHS, WidthMinusOneRHS); if (SanitizeExponent) { Checks.push_back( @@ -2768,11 +2769,11 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) { llvm::BasicBlock *CheckShiftBase = CGF.createBasicBlock("check"); Builder.CreateCondBr(ValidExponent, CheckShiftBase, Cont); 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(WidthMinusOneLHS, 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 F On Mon, Jan 30, 2017 at 11:51 AM, Alex L via cfe-commits <cfe-commits@lists.llvm.org> wrote: > Hi Vedant, > > This commit has caused a compiler crash in our stage 2 green dragon > ASAN+Ubsan bot > (http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_build/). I have > reverted it in r293475. The following example reproduces the crash with > -fsanitize=undefined : > > typedef unsigned long long uint64_t; > typedef signed long long int64_t; > > uint64_t foo(int64_t x, unsigned i) { > return x << i; > } > > Alex > > > On 27 January 2017 at 23:02, Vedant Kumar via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> >> Author: vedantk >> Date: Fri Jan 27 17:02:44 2017 >> New Revision: 293343 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=293343&view=rev >> Log: >> [ubsan] Sanity-check shift amounts before truncation (fixes PR27271) >> >> 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=293343&r1=293342&r2=293343&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Fri Jan 27 17:02:44 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( >> >> Added: cfe/trunk/test/CodeGen/ubsan-shift.c >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-shift.c?rev=293343&view=auto >> >> ============================================================================== >> --- cfe/trunk/test/CodeGen/ubsan-shift.c (added) >> +++ cfe/trunk/test/CodeGen/ubsan-shift.c Fri Jan 27 17:02:44 2017 >> @@ -0,0 +1,29 @@ >> +// RUN: %clang_cc1 -triple=x86_64-apple-darwin -fsanitize=shift-exponent >> -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); >> +} >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits