[llvm] r262337 - calculate builtin_object_size if argument is a removable pointer

[Petar Jovanovic via llvm-commits]

Hi Evgenii,

I already reverted the change. Check r262349.

Regards,
Petar
________________________________________
From: Evgenii Stepanov [eugeni.stepanov at gmail.com]
Sent: Tuesday, March 01, 2016 8:19 PM
To: Petar Jovanovic
Cc: LLVM Commits
Subject: Re: [llvm] r262337 - calculate builtin_object_size if argument is a removable pointer

Hi,

this broke a UBSan bot. You are binding a reference to a null pointer.
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/11092/steps/check-llvm%20ubsan/logs/stdio

On Tue, Mar 1, 2016 at 6:39 AM, Petar Jovanovic via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: petarj
> Date: Tue Mar  1 08:39:55 2016
> New Revision: 262337
>
> URL: http://llvm.org/viewvc/llvm-project?rev=262337&view=rev
> Log:
> calculate builtin_object_size if argument is a removable pointer
>
> This patch fixes calculating correct value for builtin_object_size function
> when pointer is used only in builtin_object_size function call and never
> after that.
>
> Patch by Strahinja Petrovic.
>
> Differential Revision: http://reviews.llvm.org/D17337
>
>
> Added:
>     llvm/trunk/test/Transforms/InstCombine/builtin-object-size-ptr.ll
> Modified:
>     llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=262337&r1=262336&r2=262337&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Tue Mar  1 08:39:55 2016
> @@ -1942,6 +1942,25 @@ Instruction *InstCombiner::visitAllocSit
>    SmallVector<WeakVH, 64> Users;
>    if (isAllocSiteRemovable(&MI, Users, TLI)) {
>      for (unsigned i = 0, e = Users.size(); i != e; ++i) {
> +      // Lowering all @llvm.objectsize calls first because they may
> +      // use a bitcast/GEP of the alloca we are removing.
> +      Instruction *I = cast_or_null<Instruction>(&*Users[i]);
> +      if (!I) continue;
> +
> +      if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
> +        if (II->getIntrinsicID() == Intrinsic::objectsize) {
> +          uint64_t Size;
> +          if (!getObjectSize(II->getArgOperand(0), Size, DL, TLI)) {
> +            ConstantInt *CI = cast<ConstantInt>(II->getArgOperand(1));
> +            Size = CI->isZero() ? -1ULL : 0;
> +          }
> +          replaceInstUsesWith(*I, ConstantInt::get(I->getType(), Size));
> +          eraseInstFromFunction(*I);
> +          Users[i] = nullptr; // Skip examining in the next loop.
> +        }
> +      }
> +    }
> +    for (unsigned i = 0, e = Users.size(); i != e; ++i) {
>        Instruction *I = cast_or_null<Instruction>(&*Users[i]);
>        if (!I) continue;
>
> @@ -1951,12 +1970,6 @@ Instruction *InstCombiner::visitAllocSit
>                                               C->isFalseWhenEqual()));
>        } else if (isa<BitCastInst>(I) || isa<GetElementPtrInst>(I)) {
>          replaceInstUsesWith(*I, UndefValue::get(I->getType()));
> -      } else if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
> -        if (II->getIntrinsicID() == Intrinsic::objectsize) {
> -          ConstantInt *CI = cast<ConstantInt>(II->getArgOperand(1));
> -          uint64_t DontKnow = CI->isZero() ? -1ULL : 0;
> -          replaceInstUsesWith(*I, ConstantInt::get(I->getType(), DontKnow));
> -        }
>        }
>        eraseInstFromFunction(*I);
>      }
>
> Added: llvm/trunk/test/Transforms/InstCombine/builtin-object-size-ptr.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/builtin-object-size-ptr.ll?rev=262337&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/builtin-object-size-ptr.ll (added)
> +++ llvm/trunk/test/Transforms/InstCombine/builtin-object-size-ptr.ll Tue Mar  1 08:39:55 2016
> @@ -0,0 +1,34 @@
> +; RUN: opt -instcombine -S < %s | FileCheck %s
> +
> +; int foo() {
> +; struct V { char buf1[10];
> +;            int b;
> +;            char buf2[10];
> +;           } var;
> +;
> +;           char *p = &var.buf1[1];
> +;           return __builtin_object_size (p, 0);
> +; }
> +
> +%struct.V = type { [10 x i8], i32, [10 x i8] }
> +
> +define i32 @foo() #0 {
> +entry:
> +  %var = alloca %struct.V, align 4
> +  %0 = bitcast %struct.V* %var to i8*
> +  call void @llvm.lifetime.start(i64 28, i8* %0) #3
> +  %buf1 = getelementptr inbounds %struct.V, %struct.V* %var, i32 0, i32 0
> +  %arrayidx = getelementptr inbounds [10 x i8], [10 x i8]* %buf1, i64 0, i64 1
> +  %1 = call i64 @llvm.objectsize.i64.p0i8(i8* %arrayidx, i1 false)
> +  %conv = trunc i64 %1 to i32
> +  call void @llvm.lifetime.end(i64 28, i8* %0) #3
> +  ret i32 %conv
> +; CHECK: ret i32 27
> +; CHECK-NOT: ret i32 -1
> +}
> +
> +declare void @llvm.lifetime.start(i64, i8* nocapture) #1
> +
> +declare i64 @llvm.objectsize.i64.p0i8(i8*, i1) #2
> +
> +declare void @llvm.lifetime.end(i64, i8* nocapture) #1
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits