r293572 - Re-apply "[ubsan] Sanity-check shift amounts before truncation"

[Vedant Kumar via cfe-commits]

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;
+}