Message ID | adeb300eebaa792d64fca85f1e72fed03c1b32d1.1635955148.git.fweimer@redhat.com |
---|---|
State | New |
Headers | show |
Series | Use _dl_find_eh_frame to locate DWARF EH data in the unwinder | expand |
On Wed, Nov 03, 2021 at 05:28:55PM +0100, Florian Weimer wrote: > --- a/libgcc/unwind-dw2-fde-dip.c > +++ b/libgcc/unwind-dw2-fde-dip.c > @@ -129,6 +129,30 @@ unw_eh_callback_data_dbase (const struct unw_eh_callback_data *data > #endif > } > > +#ifdef DL_FIND_EH_FRAME_DBASE > +#if DL_FIND_EH_FRAME_DBASE != NEED_DBASE_MEMBER > +#error "DL_FIND_EH_FRAME_DBASE != NEED_DBASE_MEMBER" Instead of #error just don't define USE_DL_FIND_EH_FRAME? I.e. #ifdef DL_FIND_EH_FRAME_DBASE #if DL_FIND_EH_FRAME_DBASE == NEED_DBASE_MEMBER > +#define USE_DL_FIND_EH_FRAME 1 > +#define DL_FIND_EH_FRAME_CONDITION 1 > +#endif ? Is it a good idea to have different arguments of the function for i386/nios2/frv/bfind vs. the rest, wouldn't just always passing void **__dbase argument be cleaner? Internally it is fine to differentiate based on NEED_DBASE_MEMBER, but doing that on a public interface? > +/* Fallback declaration for old glibc headers. DL_FIND_EH_FRAME_DBASE is used > + as a proxy to determine if <dlfcn.h> declares _dl_find_eh_frame. */ > +#if defined __GLIBC__ && !defined DL_FIND_EH_FRAME_DBASE > +#if NEED_DBASE_MEMBER > +void *_dl_find_eh_frame (void *__pc, void **__dbase) __attribute__ ((weak)); > +#else > +void *_dl_find_eh_frame (void *__pc) __attribute__ ((weak)); > +#endif > +#define USE_DL_FIND_EH_FRAME 1 > +#define DL_FIND_EH_FRAME_CONDITION (_dl_find_eh_frame != NULL) > +#endif I'd prefer not to do this. If we find glibc with the support in the headers, let's use it, otherwise let's keep using what we were doing before. > +#if NEED_DBASE_MEMBER > + eh_frame = _dl_find_eh_frame (pc, &dbase); > +#else > + dbase = NULL; > + eh_frame = _dl_find_eh_frame (pc); > +#endif > + if (eh_frame == NULL) > + return NULL; > + > + struct find_fde_tail_result result > + = find_fde_tail ((_Unwind_Ptr) pc, eh_frame, (_Unwind_Ptr) dbase); > + if (result.entry != NULL) > + { > + bases->tbase = NULL; > + bases->dbase = (void *) dbase; > + bases->func = result.func; > + } > + return result.entry; > + } > +#endif > + > data.pc = (_Unwind_Ptr) pc; > #if NEED_DBASE_MEMBER > data.dbase = NULL; > -- > 2.31.1 Otherwise LGTM. Jakub
* Jakub Jelinek: >> +/* Fallback declaration for old glibc headers. DL_FIND_EH_FRAME_DBASE is used >> + as a proxy to determine if <dlfcn.h> declares _dl_find_eh_frame. */ >> +#if defined __GLIBC__ && !defined DL_FIND_EH_FRAME_DBASE >> +#if NEED_DBASE_MEMBER >> +void *_dl_find_eh_frame (void *__pc, void **__dbase) __attribute__ ((weak)); >> +#else >> +void *_dl_find_eh_frame (void *__pc) __attribute__ ((weak)); >> +#endif >> +#define USE_DL_FIND_EH_FRAME 1 >> +#define DL_FIND_EH_FRAME_CONDITION (_dl_find_eh_frame != NULL) >> +#endif > > I'd prefer not to do this. If we find glibc with the support in the > headers, let's use it, otherwise let's keep using what we were doing before. I've included a simplified version below, based on the _dl_find_object patch for glibc. This is a bit difficult to test, but I ran a full toolchain bootstrap with GCC + glibc on all glibc-supported architectures (except Hurd and one m68k variant; they do not presnetly build, see Joseph's testers). I also tested this by copying the respective GCC-built libgcc_s into a glibc build tree for run-time testing on i686-linux-gnu and x86_64-linux-gnu. There weren't any issues. There are a buch of unwinder tests in glibc, giving at least some coverage. Thanks, Florian Subject: libgcc: Use _dl_find_object in _Unwind_Find_FDE libgcc/ChangeLog: * unwind-dw2-fde-dip.c (_Unwind_Find_FDE): Call _dl_find_object if available. --- libgcc/unwind-dw2-fde-dip.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/libgcc/unwind-dw2-fde-dip.c b/libgcc/unwind-dw2-fde-dip.c index fbb0fbdebb9..b837d8e4904 100644 --- a/libgcc/unwind-dw2-fde-dip.c +++ b/libgcc/unwind-dw2-fde-dip.c @@ -504,6 +504,24 @@ _Unwind_Find_FDE (void *pc, struct dwarf_eh_bases *bases) if (ret != NULL) return ret; + /* Use DLFO_STRUCT_HAS_EH_DBASE as a proxy for the existence of a glibc-style + _dl_find_object function. */ +#ifdef DLFO_STRUCT_HAS_EH_DBASE + { + struct dl_find_object dlfo; + if (_dl_find_object (pc, &dlfo) == 0) + return find_fde_tail ((_Unwind_Ptr) pc, dlfo.dlfo_eh_frame, +# if DLFO_STRUCT_HAS_EH_DBASE + (_Unwind_Ptr) dlfo.dlfo_eh_dbase, +# else + NULL, +# endif + bases); + else + return NULL; + } +#endif /* DLFO_STRUCT_HAS_EH_DBASE */ + data.pc = (_Unwind_Ptr) pc; #if NEED_DBASE_MEMBER data.dbase = NULL;
On Thu, Nov 25, 2021 at 09:42:53PM +0100, Florian Weimer wrote: > I've included a simplified version below, based on the _dl_find_object > patch for glibc. > > This is a bit difficult to test, but I ran a full toolchain bootstrap > with GCC + glibc on all glibc-supported architectures (except Hurd and > one m68k variant; they do not presnetly build, see Joseph's testers). > > I also tested this by copying the respective GCC-built libgcc_s into a > glibc build tree for run-time testing on i686-linux-gnu and > x86_64-linux-gnu. There weren't any issues. There are a buch of > unwinder tests in glibc, giving at least some coverage. See the comment I've just sent on this patch. If the DLFO_STRUCT_HAS_EH_DBASE and DLFO_STRUCT_HAS_EH_COUNT macros are gone, we'd need to use __GLIBC_PREREQ or configure test or combination of the two. Otherwise LGTM. > Subject: libgcc: Use _dl_find_object in _Unwind_Find_FDE > > libgcc/ChangeLog: > > * unwind-dw2-fde-dip.c (_Unwind_Find_FDE): Call _dl_find_object > if available. Jakub
diff --git a/libgcc/unwind-dw2-fde-dip.c b/libgcc/unwind-dw2-fde-dip.c index 272c0ec46c0..b5b4a23dc56 100644 --- a/libgcc/unwind-dw2-fde-dip.c +++ b/libgcc/unwind-dw2-fde-dip.c @@ -129,6 +129,30 @@ unw_eh_callback_data_dbase (const struct unw_eh_callback_data *data #endif } +#ifdef DL_FIND_EH_FRAME_DBASE +#if DL_FIND_EH_FRAME_DBASE != NEED_DBASE_MEMBER +#error "DL_FIND_EH_FRAME_DBASE != NEED_DBASE_MEMBER" +#endif +#define USE_DL_FIND_EH_FRAME 1 +#define DL_FIND_EH_FRAME_CONDITION 1 +#endif + +/* Fallback declaration for old glibc headers. DL_FIND_EH_FRAME_DBASE is used + as a proxy to determine if <dlfcn.h> declares _dl_find_eh_frame. */ +#if defined __GLIBC__ && !defined DL_FIND_EH_FRAME_DBASE +#if NEED_DBASE_MEMBER +void *_dl_find_eh_frame (void *__pc, void **__dbase) __attribute__ ((weak)); +#else +void *_dl_find_eh_frame (void *__pc) __attribute__ ((weak)); +#endif +#define USE_DL_FIND_EH_FRAME 1 +#define DL_FIND_EH_FRAME_CONDITION (_dl_find_eh_frame != NULL) +#endif + +#ifndef USE_DL_FIND_EH_FRAME +#define USE_DL_FIND_EH_FRAME 0 +#endif + struct unw_eh_frame_hdr { unsigned char version; @@ -501,6 +525,32 @@ _Unwind_Find_FDE (void *pc, struct dwarf_eh_bases *bases) if (ret != NULL) return ret; +#if USE_DL_FIND_EH_FRAME + if (DL_FIND_EH_FRAME_CONDITION) + { + void *dbase; + void *eh_frame; +#if NEED_DBASE_MEMBER + eh_frame = _dl_find_eh_frame (pc, &dbase); +#else + dbase = NULL; + eh_frame = _dl_find_eh_frame (pc); +#endif + if (eh_frame == NULL) + return NULL; + + struct find_fde_tail_result result + = find_fde_tail ((_Unwind_Ptr) pc, eh_frame, (_Unwind_Ptr) dbase); + if (result.entry != NULL) + { + bases->tbase = NULL; + bases->dbase = (void *) dbase; + bases->func = result.func; + } + return result.entry; + } +#endif + data.pc = (_Unwind_Ptr) pc; #if NEED_DBASE_MEMBER data.dbase = NULL;