Re: Excessive calls to iterate_phdr during exception handling

[Ryan Johnson <ryan dot johnson at cs dot utoronto dot ca>]

On 29/05/2013 9:41 AM, Ian Lance Taylor wrote:
> On Tue, May 28, 2013 at 9:02 PM, Ryan Johnson
> <ryan.johnson@cs.utoronto.ca> wrote:
> > Maybe I misunderstood... there's currently a (very small) cache
> > (unwind-dw2-fde-dip.c) that lives behind the loader mutex. It contains 8
> > entries and each entry holds the start and end addresses for one loaded
> > object, along with a pointer to the eh_frame_header. The cache resides in
> > static storage, and so accessing it is always safe.
> >
> > I think what you're saying is that the p_eh_frame_hdr field could end up
> > with a dangling pointer due to a dlclose call?
> Yes, that can happen.
>
> > If so, my argument is that, as long as the cache is up to date as of the
> > start of unwind, any attempt to access a dangling p_eh_frame_hdr means that
> > in-use code was dlclosed, in which case unwind is guaranteed to fail anyway.
> > The failure would just have different symptoms with such a cache in place.
> >
> > Am I missing something?
> I think you're right about that.  But what happens if the entry is not
> in the cache?  Or, do you mean you want to look in the cache before
> calling dl_iterate_phdr?  That should be safe but of course you still
> need a lock as multiple threads can be manipulating the cache at the
> same time.
OK, here's a proof of concept I threw together as a preload library of 
sorts. You can compile it as a .o and link into the app directly, or 
compile it as .so and LD_PRELOAD it into an existing app. Linking 
directly against the .so doesn't seem to work for some reason. However 
the app incorporates it, though, I confirmed that it removes the 
bottleneck.

The short version is that it overrides _Unwind_Find_FDE and allocates a 
4kB global FDE table cache (~85 entries), which threads search 
lock-free. Per-entry checksums ensure that threads don't try to use 
inconsistent entries in the event they race with an updater. Calls to 
dlclose blow away the cache. Whenever a thread misses in cache, it calls 
dl_iterate_phdr to serve the miss and, if a suitable FDE table is found 
also inserts a new cache entry for it. In the event that this also fails 
(if the FDE table is not sorted, or if it's an obsolete object that uses 
registered unwind info rather than eh_frame), it falls back to the 
original implementation.

The .h just cobbles together various definitions from the gcc sources 
(with citations) so the code can compile stand-alone; the .cpp started 
from the original _Unwind_Find_FDE and tweaked it to add the caching.

I'm not sure the best way to incorporate something like this into gcc, 
but it turned out surprisingly self-contained, which should at least 
help uptake. Some of the uglier parts of the code (the dlsym hacks and 
the extra mutex) would be unnecessary in a properly integrated solution.

FYI, for a bit I had a bug where readers grabbed the mutex as well, and 
performance was almost as good as lock-free because the critical section 
was so much shorter than before (FDE table search having been moved 
outside). Still, I was only using a few threads in my tests, so the 
lock-free method is probably better.

Thoughts?
Ryan

Attachment: debug-find-fde.cpp
Description: Text document

Attachment: debug-find-fde.h
Description: Text document