llvm dev - Sep 2019 - An issue with "lifetime.start" and storing "undef"

Hi All,

In order to confine the scope of a local variable allocated using
"alloca",
we had written a pass named 'undeflocal'. This helps avoid pseudo phi
nodes
inserted because of the path (carrying an undefined value) starting from
the "entry" block. This unwanted phi instruction was preventing some
of the
optimizations from getting triggered later on.

For example, consider the following case:

// void fn() {
//    Block 1.... block 3
//    for (i=0 ; i<n ; i++) {
//      int k;
//      if (<>)
//       define k
//      use of k
// }

Though the value of k gets defined in the "if" block inside the
"for" loop,
due to the presence of "alloca" in entry block, the initial garbage
value
of k flows from the entry block of the function.  This expands the scope of
the variable k. The undeflocal pass adds a store instruction (say SI) to
store value "undef", to the local variable pointer. SI is added just
"before" the "lifetime.start" intrinsic for the variable k;
the intention
was to kill the liveness of the variable k at this point and help avoid
insertion of the phi instruction.

However, now we are facing an issue triggered by the AddressSanitizer, when
"clang" is built using our bootstrap build. It throws the
"stack-use-after-scope" error. We identified this is due to the
"store"
introduced by "undeflocal" pass before the lifetime.start intrinsic.
The
value of the additional store is changed to “0” in place of “undef” by the
SROA pass later invoked. As “undeflocal”  has inserted it before the
lifetime.start intrinsic and now the store has a legal value “0” stored
into it, which in turn causes the address sanitizer to complain about
use-after-scope.

Here is the part of the IR that does this



*After undeflocal*


if.end9:                                          ; preds
%_ZN4llvm10IndexedMapIN12_GLOBAL__N_18RAGreedy7RegInfoENS_20VirtReg2IndexFunctorEEixEj.exit,
%if.then8

  %Cascade.0 = phi i32 [ %7,
%_ZN4llvm10IndexedMapIN12_GLOBAL__N_18RAGreedy7RegInfoENS_20VirtReg2IndexFunctorEEixEj.exit
], [ %8, %if.then8 ], !dbg !22012

  store i64 undef, i64* %Cost, align 8, !dbg !22013

  %9 = bitcast i64* %Cost to i8*, !dbg !22013

  call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %9) #24, !dbg
!22013



*After SROA*



if.end9:                                          ; preds
%_ZN4llvm10IndexedMapIN12_GLOBAL__N_18RAGreedy7RegInfoENS_20VirtReg2IndexFunctorEEixEj.exit,
%if.then8

  %Cascade.0 = phi i32 [ %7,
%_ZN4llvm10IndexedMapIN12_GLOBAL__N_18RAGreedy7RegInfoENS_20VirtReg2IndexFunctorEEixEj.exit
], [ %8, %if.then8 ], !dbg !22012

  store i32 0, i32* %Cost.sroa.10, !dbg !22013

  %Cost.sroa.10.0..sroa_cast349 = bitcast i32* %Cost.sroa.10 to i8*, !dbg
!22013

  call void @llvm.lifetime.start.p0i8(i64 4, i8*
%Cost.sroa.10.0..sroa_cast349), !dbg !22013


It could be great, if we get suggestions for the proper solution for this.
We have thought about the following solutions, however, not certain about
its implications on other passes.


   1. To make “undeflocal” pass add the store instruction after
   lifetime.start.
   2. To stop SROA from converting undef to 0.
   3. To add 'metadata', to SI so that the AddressSanitizer ignores SI.


We are skeptical about the solution 1 because the semantics of
lifetime.start defined in "lib/CodeGen/StackColoring.cpp" is a bit
confusing. It says:

//   L2) on a `lifetime.start`, a stack slot is marked as *in-scope* and

//   the stack slot is overwritten with `undef`.

As it mentioned, "on" a 'lifteime.start', we are unsure about
if it is
meant to be before or after it. Will there be any issue in later passes, if
we have an "undef" value "store" after lifetime.start.

Regards,

------------------------------
Raghesh Aloor
AMD India Pvt. Ltd.
Bengaluru.
------------------------------
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190924/afdf1db4/attachment.html>

Hi Raghesh,

On Tue, 24 Sep 2019 at 09:26, raghesh via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> In order to confine the scope of a local variable allocated using
"alloca", we had written a pass named 'undeflocal'. This helps
avoid pseudo phi nodes inserted because of the path (carrying an undefined
value) starting from the "entry" block. This unwanted phi instruction
was preventing some of the optimizations from getting triggered later on.
It sounds like this is the real problem. Whatever's seeing that store
you're inserting and using it to improve the phi should also be taught
about @llvm.lifetime.start.

Cheers.

Tim.

Hi Tim,

Are you suggesting solution 3? i.e,. To add 'metadata', to SI so that
the
AddressSanitizer ignores SI.

Regards,
------------------------------
Raghesh Aloor
AMD India Pvt. Ltd.
Bengaluru.
------------------------------


On Tue, Sep 24, 2019 at 2:27 PM Tim Northover <t.p.northover at gmail.com>
wrote:
> Hi Raghesh,
>
> On Tue, 24 Sep 2019 at 09:26, raghesh via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> > In order to confine the scope of a local variable allocated using
> "alloca", we had written a pass named 'undeflocal'. This
helps avoid pseudo
> phi nodes inserted because of the path (carrying an undefined value)
> starting from the "entry" block. This unwanted phi instruction
was
> preventing some of the optimizations from getting triggered later on.
>
> It sounds like this is the real problem. Whatever's seeing that store
> you're inserting and using it to improve the phi should also be taught
> about @llvm.lifetime.start.
>
> Cheers.
>
> Tim.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190924/a260119d/attachment.html>