This is the mail archive of the gcc@gcc.gnu.org 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: Excessive calls to iterate_phdr during exception handling


On 28/05/2013 11:05 PM, Alan Modra wrote:
On Tue, May 28, 2013 at 09:19:48PM -0400, Ryan Johnson wrote:
On 28/05/2013 8:47 PM, Ian Lance Taylor wrote:
On Mon, May 27, 2013 at 3:20 PM, Ryan Johnson
<ryan.johnson@cs.utoronto.ca> wrote:
I'm bringing the issue up here, rather than filing a bug, because I'm not
sure whether this is an oversight, a known problem that's hard to fix, or a
feature (e.g. somehow required for reliable unwinding). I suspect the
former, because _Unwind_Find_FDE tries a call to _Unwind_Find_registered_FDE
before falling back to dl_iterate_phdr, but the former never succeeds in my
trace (iterate_phdr is always called).
The issue is dlclose followed by dlopen.  If we had a cache ahead of
dl_iterate_phdr, we would need some way to clear out any information
cached from a dlclose'd library.  Otherwise we might pick up the old
information when looking up an address from a new dlopen.  So 1)
locking will always be required; 2) any caching system to reduce the
number of locks will require support for dlclose, somehow.  It's worth
working on but there isn't going to be a simple solution.
I have mixed feelings on this... on the one had it would be bad to
risk sending unwind off to la-la land because somebody did a quick
dlclose/dlopen pair on code we're about to unwind through... but on
the other hand anybody who does a dlclose/dlopen pair on code we're
about to unwind through (a) is asking for trouble and (b) is
perfectly free to do so in spite of the mutex [1].
Yes of course you can shoot yourself in the foot.  The mutex is there
to stop the glibc dl_iterate_phdr list traversal running awry when
dlopen/dlclose happens in another thread.  To be clear, I'm talking
about a dlclose on an object that your thread doesn't access.  Such a
dlclose shouldn't affect your thread in any way.  But if glibc's
list of loaded objects was allowed to change while your thread was
running dl_iterate_phdr, then dl_iterate_phdr could potentially read
freed list entries.
Understood. It's only safe to populate a cache with header info if you hold the loader mutex.

... but once you have that cache, the rest of unwind is up for grabs...

For example, it should be reasonably safe to let __cxa_allocate_exception call dl_iterate_phdr in order to build a list of object headers valid at the time unwind begins. It already calls malloc, so allocating space for a cache (holding pointers to at most a few dozen object headers) wouldn't be so terrible; __cxa_free_exception could release the space after the dust settles. In order to optimize non-throw uses of unwinding, it might make sense to build the cache somewhere besides __cxa_allocate_exception, but the basic concept doesn't change. During unwind, _Unwind_find_FDE could work exclusively from the cache instead of calling dl_iterate_phdr, reducing call count for the latter from 4-6 per stack frame unwound to one per throw, which seems like a pretty big win [1].

A more aggressive optimization would stash the the header cache in TLS between uses, rebuilding only if dladd/dlsub count changes. That would require a lock-free method to access the two counts, however, which I don't think currently exists. There would be a race between reading the two variables, and another between read and use of either variable, but the whole race window arises during a time when any change that mattered would have dire consequences whether the cache is stale or not.

[1] I'd also move the call to _Unwind_Find_registered_FDE after the cache check, so it becomes a fallback (thus avoiding the libgcc mutex lock in the common case of "newer" executables and shared libraries). Unless by some cruel twist of history, there exist out there objects that register their FDEs manually *and* provide .eh_frame whose invalid entries must be ignored?

Ryan


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