Excessive calls to iterate_phdr during exception handling

[Ryan Johnson]

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.
Per-thread cache, either allocated and populated at the start of every 
unwind, or maintained in TLS with version checks against the dl 
adds/subs counts. The former reduces the lock grabbing to one per 
unwind; the latter would virtually eliminate locking during unwind, but 
would require changes to libc to expose the adds/subs counts in some 
lock-free way.

And yes, if we can't afford to malloc(), then a fixed-size cache with a 
few dozen entries should suffice for the vast majority of apps, falling 
back to dl_iterate_phdr if the chosen cache size is too small. Emacs is 
the worst kitchen sink I can think of; it links against ~70 shared 
libraries, which would fit comfortably in ~2kB of tmp space [1]. Or we 
could use mmap... it's not officially on the async-safe list from posix, 
but I'd be shocked if it's actually unsafe to call from signal context 
in practice (and presumably gcc/glibc would be in a position to know 
that sort of thing).

[1] if your target or app can't spare 2kB of RAM for unwind, you 
probably have plenty of other reasons not to use exceptions or unwinding 
already...

Thoughts?
Ryan