This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] do not take mutex in _Unwind_Find_registered_FDE if there is no registered objects

On Wed, Jul 27, 2016 at 05:12:18PM -0600, Jeff Law wrote:
> On 07/25/2016 07:44 AM, Gleb Natapov wrote:
> > _Unwind_Find_FDE calls _Unwind_Find_registered_FDE and it takes lock even
> > when there is no registered objects. As far as I see only statically
> > linked applications call __register_frame_info* functions, so for
> > dynamically linked executables taking the lock to check unseen_objects
> > and seen_objects is a pessimization. Since the function is called on
> > each thrown exception this is a lot of unneeded locking.  This patch
> > checks unseen_objects and seen_objects outside of the lock and returns
> > earlier if both are NULL.
> > 
> > diff --git a/libgcc/unwind-dw2-fde.c b/libgcc/unwind-dw2-fde.c
> > index 5b16a1f..41de746 100644
> > --- a/libgcc/unwind-dw2-fde.c
> > +++ b/libgcc/unwind-dw2-fde.c
> > @@ -1001,6 +1001,13 @@ _Unwind_Find_FDE (void *pc, struct dwarf_eh_bases *bases)
> >    struct object *ob;
> >    const fde *f = NULL;
> > 
> > +  /* __atomic_write is not used to modify unseen_objects and seen_objects
> > +     since they are modified in locked sections only and unlock provides
> > +     release semantics. */
> > +  if (!__atomic_load_n(&unseen_objects, __ATOMIC_ACQUIRE)
> > +      && !__atomic_load_n(&seen_objects, __ATOMIC_ACQUIRE))
> > +      return NULL;
> As as Andrew noted, this might be bad on targets that don't have atomics.
> For those we could easily end up inside a libfunc which would be
> unfortunate.  Certain mips/arm variants come to mind here.
> For targets that don't have atomics or any kind of synchronization libfunc,
> we'll emit nop-asm-barriers to at least stop the optimizers from munging
> things too badly.
> It's been a couple years since I've really thought about these kinds of
> synchronization issues -- is it really safe in a weakly ordered processor to
> rely on the mutex lock/unlock of the "object_mutex" to order the
> loads/stores of "unseen_objects" and "seen_objects"?
I am pretty sure it is. After mutex unlock another cpu will not see
old value (provided it uses acquire semantics to read value).

But when I wrote the patch I did not notice that Jakub already wrote one
that address the same issue and avoids both of your concerns. It can be
found here: Can we
apply his version?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]