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

[Jakub Jelinek]

On Thu, Sep 01, 2016 at 09:39:37AM +0200, Richard Biener 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

Yes.

> 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

Oh right, using single_imm_use of the VDEF should work.

> > 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?

They do:

// Lazy-initialized and never deleted.
static VectorOfGlobals *dynamic_init_globals;
...
void __asan_before_dynamic_init(const char *module_name) {
  if (!flags()->check_initialization_order ||
      !CanPoisonMemory())
    return;
  bool strict_init_order = flags()->strict_init_order;
  CHECK(dynamic_init_globals);
...
  for (uptr i = 0, n = dynamic_init_globals->size(); i < n; ++i) {

and thus if no globals have been registered, dynamic_init_globals is NULL,
and without the assertion it would crash on dynamic_init_globals->size().
The fix would be just to do:
   if (!flags()->check_initialization_order ||
+      !dynamic_init_globals ||
       !CanPoisonMemory())
     return;
   bool strict_init_order = flags()->strict_init_order;
-  CHECK(dynamic_init_globals);
or so (and similarly in the other function).

	Jakub