Re: [PATCH] Fix up -fsanitize=address ctor order (PR sanitizer/77396)

[Richard Biener <rguenther at suse dot de>]

On Thu, 1 Sep 2016, Jakub Jelinek wrote:

> On Thu, Sep 01, 2016 at 08:59:57AM +0200, Richard Biener wrote:
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > Hmm, maybe simply do
> > 
> >   if (gimple_call_builtin_p (g, BUILT_IN_ASAN_AFTER_DYNAMIC_INIT))
> >     {
> >       gimple *def = SSA_NAME_DEF_STMT (gimple_vuse (g));
> >       if (gimple_call_builtin_p (def, BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT))
> >         ... remove both ...
> > 
> > intervening non-memory-def stmts do not really matter, do they?  If
> > loads matter then checking for the single-vdef-use of
> > BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT to be BUILT_IN_ASAN_AFTER_DYNAMIC_INIT
> > would also work.
> 
> Loads matter too.  What I've been worried about is that some optimizations
> would turn the original
>   __asan_before_dynamic_init ("foobar");
>   ...
>   __asan_after_dynamic_init ();
> into:
>   __asan_before_dynamic_init ("foobar");
>   ...
>   if (...)
>     {
>       ...
>       __asan_after_dynamic_init ();
>       return;
>     }
>   ...
>   __asan_after_dynamic_init ();
> or something similar and then if removing both on seeing after and its vuse
> before, we'd then keep around the other after call.

The dynamic init pairs form a SESE region initially, right?  Then I
don't see how we'd ever create sth that would mimic the above
but still pass the single-imm-use of the VDEF of the before-dynamic-init
stmt being the after one (in the above example you'd not have a single
use).  So we'd need to have instead

   if (...)
    __asan_before_dynamic_init ();
  else
    __asan_before_dynamic_init ();
  __asan_after_dynamic_init ();

but even then the single-uses would be the merging PHI.

> Maybe instead of removing both I could just remember both, if it ever finds
> more than one of each kind of calls, punt, and otherwise remove at the end
> of the stmt walk?

> > Otherwise looks ok but I wonder why libasan needs to assert this ..
> 
> It a tiny bit simplifies the code, and of course, a perfect compiler would
> never emit those if there isn't anything to protect, it is just that it is
> hard on the compiler side to guarantee it always.

I believe it's quite impossible even.  What do the expect to happen
in between the two?  I suppose some other asan runtime call?

> // PR sanitizer/77396
> // { dg-do run }
> // { dg-additional-options "-O2" }
> // { dg-set-target-env-var ASAN_OPTIONS "check_initialization_order=true" }
> 
> struct S { S () { asm volatile ("" : : : "memory"); } };
> static S c;
> 
> int
> main ()
> {
>   return 0;
> }
> 
> is what still fails even with the patch.
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)