[libunwind] Support __register_frame with a full .eh_frame section

[mstorsjo]

Summary

Previously, the __register_frame function supported registering an FDE for one individual function at a time. The function could also be used registering a full .eh_frame section of a module (which is how libgcc sets up unwinding in some configurations).

[rnk]

src/UnwindCursor.hpp

52

I'm concerned that std::function might introduce runtime dependencies from libc++abi back to libc++. I think we get away with the <algorithm> include above because it's templated for the most part. Since this stuff is all in-header anyway, we could just take a non-type template parameter callable and call it directly.

[mstorsjo]

src/UnwindCursor.hpp

52

Ok, will change.

[mstorsjo]

Avoid using the C++ <functional> header.

[compnerd]

I really don't like this approach. I think that we should introduce a different entry point for this behavior rather than saying that we go through the existing interface. Having a reference to the .eh_frame_hdr seems better, as it would be better to make use of that optimization and we already have handling for that in libunwind.

[mstorsjo]

In D44494#1050783, @compnerd wrote:

I really don't like this approach. I think that we should introduce a different entry point for this behavior rather than saying that we go through the existing interface. Having a reference to the .eh_frame_hdr seems better, as it would be better to make use of that optimization and we already have handling for that in libunwind.

Well, this is just a plain .eh_frame, not an .eh_frame_hdr, since nothing for MinGW actually produces such a section for COFF (yet), GNU binutils doesn't either. Adding support for an indexed .eh_frame_hdr, while good from a performance PoV, feels like an orthogonal issue from this.

This else clause isn't about .eh_frame vs .eh_frame_hdr, but about registering a single FDE (which libunwind's __register_frame currently does) vs registering a full .eh_frame section (which libgcc's __register_frame does).

[mstorsjo]

In D44494#1050797, @mstorsjo wrote:

This else clause isn't about .eh_frame vs .eh_frame_hdr, but about registering a single FDE (which libunwind's __register_frame currently does) vs registering a full .eh_frame section (which libgcc's __register_frame does).

However, this function actually just is _unw_add_dynamic_fde, while __register_frame in UnwindLevel1-gcc-ext.c just calls this function. So we should probably check there instead, whether it's an FDE (which libgcc doesn't support via that entry point) or a full .eh_frame section.

[mstorsjo]

In D44494#1050803, @mstorsjo wrote:

In D44494#1050797, @mstorsjo wrote:

This else clause isn't about .eh_frame vs .eh_frame_hdr, but about registering a single FDE (which libunwind's __register_frame currently does) vs registering a full .eh_frame section (which libgcc's __register_frame does).

However, this function actually just is _unw_add_dynamic_fde, while __register_frame in UnwindLevel1-gcc-ext.c just calls this function. So we should probably check there instead, whether it's an FDE (which libgcc doesn't support via that entry point) or a full .eh_frame section.

In this case, it triggers the "FDE is really a CIE" case in decodeFDE, so we could do a quick check in __register_frame and see if it is a CIE or an FDE, and then call the right underlying functions based on that, instead of adding it in the error handling else statement like this.

[mstorsjo]

Also, relating to this, @joerg mentioned on irc that one should rather use __register_frame_info_bases instead of __register_frame. That function is a bit tricky to call though, since it uses a libgcc internal struct struct object, only available in libgcc headers and in Unwind_AppleExtras.cpp (as struct libgcc_object, with the comment "undocumented libgcc struct object").

[mstorsjo]

Added and using a helper function isCIE to avoid using the error return path from decodeFDE.

Technically, the decision between a full section and a plain FDE probably should be outside of _unw_add_dynamic_fde, in __register_frame in UnwindLevel1-gcc-ext.c. That'd require adding a C wrapper for isCIE in libunwind.cpp though.

[joerg]

The "struct object" is an implementation detail of the unwind implementation. You are guaranteed historically to get at least 8 longs / 8 pointers for internal use statically allocated in each object. What is stored inside is up to the unwind implementation.

[mstorsjo]

FWIW, I'm not going to put any more effort into this one at the moment, I managed to fix my use case in a much simpler way by providing a Windows Store compatible version of EnumProcessModules, see https://sourceforge.net/p/mingw-w64/mingw-w64/ci/0e1f41358845706a1adafcf4f9d27db0319dd3d9/.

src/UnwindCursor.hpp

 #if defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
 /// Cache of recently found FDEs.
 template <typename A>
 class _LIBUNWIND_HIDDEN DwarfFDECache {
   typedef typename A::pint_t pint_t;
 public:
   static pint_t findFDE(pint_t mh, pint_t pc);
   static void add(pint_t mh, pint_t ip_start, pint_t ip_end, pint_t fde);
+  static void addSection(pint_t section_start);
   static void removeAllIn(pint_t mh);
   static void iterateCacheEntries(void (*func)(unw_word_t ip_start,
                                                unw_word_t ip_end,
                                                unw_word_t fde, unw_word_t mh));
+  template<typename Func> static void iterateSections(Func func);
 
 private:
 
   struct entry {
     pint_t mh;
     pint_t ip_start;
     pint_t ip_end;
     pint_t fde;
   };
 
   // These fields are all static to avoid needing an initializer.
   // There is only one instance of this class per process.
   static RWMutex _lock;
 #ifdef __APPLE__
   static void dyldUnloadHook(const struct mach_header *mh, intptr_t slide);
   static bool _registeredForDyldUnloads;
 #endif
   // Can't use std::vector<> here because this code is below libc++.
   static entry *_buffer;
   static entry *_bufferUsed;
   static entry *_bufferEnd;
   static entry _initialBuffer[64];
+
+  static pint_t *_sections;
+  static pint_t *_sectionsUsed;
+  static pint_t *_sectionsEnd;
+  static pint_t _initialSectionsBuffer[64];
 };
 
 template <typename A>
 typename DwarfFDECache<A>::entry *
 DwarfFDECache<A>::_buffer = _initialBuffer;
 
 template <typename A>
 typename DwarfFDECache<A>::entry *
 DwarfFDECache<A>::_bufferUsed = _initialBuffer;
 
 template <typename A>
 typename DwarfFDECache<A>::entry *
 DwarfFDECache<A>::_bufferEnd = &_initialBuffer[64];
 
 template <typename A>
 typename DwarfFDECache<A>::entry DwarfFDECache<A>::_initialBuffer[64];
 
 template <typename A>
+typename DwarfFDECache<A>::pint_t *
+DwarfFDECache<A>::_sections = _initialSectionsBuffer;
+
+template <typename A>
+typename DwarfFDECache<A>::pint_t *
+DwarfFDECache<A>::_sectionsUsed = _initialSectionsBuffer;
+
+template <typename A>
+typename DwarfFDECache<A>::pint_t *
+DwarfFDECache<A>::_sectionsEnd = &_initialSectionsBuffer[64];
+
+template <typename A>
+typename DwarfFDECache<A>::pint_t DwarfFDECache<A>::_initialSectionsBuffer[64];
+
+template <typename A>
 RWMutex DwarfFDECache<A>::_lock;
 
 #ifdef __APPLE__
 template <typename A>
 bool DwarfFDECache<A>::_registeredForDyldUnloads = false;
 #endif
 
 template <typename A>
if (!_registeredForDyldUnloads) {
     _registeredForDyldUnloads = true;
   }
 #endif
   _LIBUNWIND_LOG_IF_FALSE(_lock.unlock());
 #endif
 }
 
 template <typename A>
+void DwarfFDECache<A>::addSection(pint_t section_start) {
+#if !defined(_LIBUNWIND_NO_HEAP)
+  _LIBUNWIND_LOG_IF_FALSE(_lock.lock());
+  if (_sectionsUsed >= _sectionsEnd) {
+    size_t oldSize = (size_t)(_sectionsEnd - _sections);
+    size_t newSize = oldSize * 4;
+    // Can't use operator new (we are below it).
+    pint_t *newSections = (pint_t *)malloc(newSize * sizeof(pint_t));
+    memcpy(newSections, _sections, oldSize * sizeof(pint_t));
+    if (_sections != _initialSectionsBuffer)
+      free(_sections);
+    _sections = newSections;
+    _sectionsUsed = &newSections[oldSize];
+    _sectionsEnd = &newSections[newSize];
+  }
+  *_sectionsUsed = section_start;
+  ++_sectionsUsed;
+  _LIBUNWIND_LOG_IF_FALSE(_lock.unlock());
+#endif
+}
+
+template <typename A>
 void DwarfFDECache<A>::removeAllIn(pint_t mh) {
   _LIBUNWIND_LOG_IF_FALSE(_lock.lock());
   entry *d = _buffer;
   for (const entry *s = _buffer; s < _bufferUsed; ++s) {
     if (s->mh != mh) {
       if (d != s)
         *d = *s;
       ++d;
     }
   }
   _bufferUsed = d;
+  pint_t *sectionsDest = _sections;
+  for (const pint_t *s = _sections; s < _sectionsUsed; ++s) {
+    if (*s != mh) {
+      if (sectionsDest != s)
+        *sectionsDest = *s;
+      ++sectionsDest;
+    }
+  }
+  _sectionsUsed = sectionsDest;
   _LIBUNWIND_LOG_IF_FALSE(_lock.unlock());
 }
 
 #ifdef __APPLE__
 template <typename A>
 void DwarfFDECache<A>::dyldUnloadHook(const struct mach_header *mh, intptr_t ) {
   removeAllIn((pint_t) mh);
 }
 #endif
 
 template <typename A>
 void DwarfFDECache<A>::iterateCacheEntries(void (*func)(
     unw_word_t ip_start, unw_word_t ip_end, unw_word_t fde, unw_word_t mh)) {
   _LIBUNWIND_LOG_IF_FALSE(_lock.lock());
   for (entry *p = _buffer; p < _bufferUsed; ++p) {
     (*func)(p->ip_start, p->ip_end, p->fde, p->mh);
   }
   _LIBUNWIND_LOG_IF_FALSE(_lock.unlock());
 }
+
+template <typename A>
+template <typename Func>
+void DwarfFDECache<A>::iterateSections(Func func) {
+  _LIBUNWIND_LOG_IF_FALSE(_lock.lock());
+  for (pint_t *p = _sections; p < _sectionsUsed; ++p) {
+    func(*p);
+  }
+  _LIBUNWIND_LOG_IF_FALSE(_lock.unlock());
+}
 #endif // defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
 
 
 #define arrayoffsetof(type, index, field) ((size_t)(&((type *)0)[index].field))
 
 #if defined(_LIBUNWIND_SUPPORT_COMPACT_UNWIND)
 template <typename A> class UnwindSectionHeader {
 public:
if (msg == NULL) {
         _info.unwind_info      = fdeInfo.fdeStart;
         _info.unwind_info_size = (uint32_t)fdeInfo.fdeLength;
         _info.extra            = 0;
         return;
       }
     }
   }
 
+  bool foundSection = false;
+  DwarfFDECache<A>::iterateSections([&](pint_t section) {
+    if (foundSection)
+      return;
+    sects.dso_base = section;
+    sects.dwarf_section = section;
+    sects.dwarf_section_length = ~(pint_t)0 - section;
+    if (sects.dwarf_section_length > ~(uint32_t)0)
+        sects.dwarf_section_length = ~(uint32_t)0;
+    CFI_Parser<LocalAddressSpace>::FDE_Info fdeInfo;
+    CFI_Parser<LocalAddressSpace>::CIE_Info cieInfo;
+    // Only parse and see if it is the right frame here,
+    // to avoid recursive self-locking.
+    foundSection = CFI_Parser<A>::findFDE(_addressSpace, pc, sects.dwarf_section,
+                                          (uint32_t)sects.dwarf_section_length, 0,
+                                          &fdeInfo, &cieInfo);
+  });
+  // This call will lock the cache when adding a new match; only do this
+  // outside of the iteration (which also keeps a lock).
+  if (foundSection && this->getInfoFromDwarfSection(pc, sects))
+    return;
+
   // Lastly, ask AddressSpace object about platform specific ways to locate
   // other FDEs.
   pint_t fde;
   if (_addressSpace.findOtherFDE(pc, fde)) {
     CFI_Parser<LocalAddressSpace>::FDE_Info fdeInfo;
     CFI_Parser<LocalAddressSpace>::CIE_Info cieInfo;
     if (!CFI_Parser<A>::decodeFDE(_addressSpace, fde, &fdeInfo, &cieInfo)) {
       // Double check this FDE is for a function that includes the pc.

src/libunwind.cpp

void _unw_add_dynamic_fde(unw_word_t fde) {
   if (message == NULL) {
     // dynamically registered FDEs don't have a mach_header group they are in.
     // Use fde as mh_group
     unw_word_t mh_group = fdeInfo.fdeStart;
     DwarfFDECache<LocalAddressSpace>::add((LocalAddressSpace::pint_t)mh_group,
                                           fdeInfo.pcStart, fdeInfo.pcEnd,
                                           fdeInfo.fdeStart);
   } else {
+    // This wasn't a plain FDE; then it's probably a full .eh_frame section.
     _LIBUNWIND_DEBUG_LOG("_unw_add_dynamic_fde: bad fde: %s", message);
+    DwarfFDECache<LocalAddressSpace>::addSection((LocalAddressSpace::pint_t)fde);
   }
 }
 
 /// IPI: for __deregister_frame()
 void _unw_remove_dynamic_fde(unw_word_t fde) {
   // fde is own mh_group
   DwarfFDECache<LocalAddressSpace>::removeAllIn((LocalAddressSpace::pint_t)fde);
 }