[PATCH] Fix up -fsanitize=address ctor order (PR sanitizer/77396)
Jakub Jelinek
jakub@redhat.com
Thu Sep 1 07:55:00 GMT 2016
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
More information about the Gcc-patches
mailing list