r293343 - [ubsan] Sanity-check shift amounts before truncation (fixes PR27271)

[Alex L via cfe-commits]

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 at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170130/5847dfa4/attachment.html>