On Thu, 1 Sep 2016, Jakub Jelinek wrote:
> 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).
Ah, so it expects sth _before_ before-dynamic-init? At least it seems
that globals are not only registered inbetween before/after-dynamic-init.
Maybe simply always init dynamic_init_globals?
Richard.
