Commit d3a601b0 authored by Vedant Kumar's avatar Vedant Kumar
Browse files

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

llvm-svn: 293572
parent 578cf7aa

clang/lib/CodeGen/CGExprScalar.cpp

+9 −7
Original line number Diff line number Diff line
@@ -2751,8 +2751,8 @@ 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, 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,10 +2767,12 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) { 
      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",

      llvm::Value *BitsShiftedOff = Builder.CreateLShr(

          Ops.LHS, Builder.CreateSub(PromotedWidthMinusOne, RHS, "shl.zeros",

                                     /*NUW*/ true, /*NSW*/ true),

          "shl.check");

      if (CGF.getLangOpts().CPlusPlus) {

clang/test/CodeGen/ubsan-shift.c

0 → 100644
+47 −0
Original line number Diff line number Diff line 
// 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;

}