On 29/05/2013 3:36 AM, Jakub Jelinek wrote:
Hi!
On Wed, May 29, 2013 at 12:02:27AM -0400, Ryan Johnson wrote:
Note, swapping the order of dl_iterate_phdr and _Unwind_Find_registered_FDE
IMHO is fine.
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?
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.
But if you don't take a lock and look at the 8 entry cache without holding a
lock, then some other thread might be in a middle of updating that
structure and so it is definitely unsafe to access it.
The cache would be built from scratch as part of firing up the unwind
machinery (and discarded at the end of unwind), or stored in TLS and
thus thread-private. A global cache would require something like RCU,
which I don't see happening any time soon.
So the cache to be usable across all _Unwind_Find_FDE calls from the same
_Unwind_ForcedUnwind, _Unwind_Resume, _Unwind_Resume_or_Rethrow,
_Unwind_Backtrace call would need to be an automatic (set of) variable(s) from
those
functions or __thread with some way to flush that cache upon
uw_install_context resp. return from _Unwind_Backtrace.
Note that some of those functions may run some user code for each frame,
in theory that code could e.g. dlclose libraries already left, but can't
validly free the upper frames, thus perhaps it would be possible to just use
such a cache.
The big problem is that the unwinder is an ABI minefield, e.g. on some
architectures _Unwind_Find_FDE lives in glibc as well as libgcc, and which
one is called is unknown. So, to be able to handle this we'd perhaps need
to introduce a wrapper around _Unwind_Find_FDE that would be passed an extra
argument, ideally hidden in libgcc unwind-dw2-fde.c, verify that
_Unwind_Find_FDE binds to the current TU _Unwind_Find_FDE symbol,
look at the cache provided by an extra argument (without lock) and just
proceed with the second half of _Unwind_IterarePhdrCallback (perhaps moved
into a helper function?) in that case. If not found in the cache, do
what _Unwind_Find_FDE does, but additionally record the details in the
local cache in addition to the global one. If _Unwind_Find_FDE symbol binds
to different _Unwind_Find_FDE, it would need to run that _Unwind_Find_FDE as
a fallback (which often would mean that dl_iterate_phdr is called again by
glibc), but generally that should happen only if .eh_frame is not present.
The reason for having more than one _Unwind_Find_FDE is due to the pre
.eh_frame_hdr world, there has to be a single registry, otherwise each FDE
registry might be told about some subset of libraries, but you might not
know all of them. And at some point glibc was also exporting such registry
(it linked libgcc.a into it and that at some point also contained the
unwinder).
Gotcha. Thanks for the historical context, that helps clear up why
things are the way they are right now.
And yes, the question of how to make the cache available is a tricky
question that I'm not equipped to give an answer to, though the "extra
struct member" trick has been used profitably before...
Ryan
