[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section
mstorsjo abandoned this revision. mstorsjo added a comment. Herald added a subscriber: chrib. 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/. https://reviews.llvm.org/D44494 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section
joerg added a comment. 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. https://reviews.llvm.org/D44494 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section
mstorsjo updated this revision to Diff 140410.
mstorsjo edited the summary of this revision.
mstorsjo added a comment.
Herald added a subscriber: JDevlieghere.
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.
https://reviews.llvm.org/D44494
Files:
src/DwarfParser.hpp
src/UnwindCursor.hpp
src/libunwind.cpp
Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -326,20 +326,27 @@
/// IPI: for __register_frame()
void _unw_add_dynamic_fde(unw_word_t fde) {
- CFI_Parser::FDE_Info fdeInfo;
- CFI_Parser::CIE_Info cieInfo;
- const char *message = CFI_Parser::decodeFDE(
- LocalAddressSpace::sThisAddressSpace,
- (LocalAddressSpace::pint_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::add((LocalAddressSpace::pint_t)mh_group,
- fdeInfo.pcStart, fdeInfo.pcEnd,
- fdeInfo.fdeStart);
+ if (CFI_Parser::isCIE(LocalAddressSpace::sThisAddressSpace,
+ (LocalAddressSpace::pint_t) fde)) {
+// This wasn't a plain FDE, but it started with a CIE - register a full
+// .eh_frame section.
+DwarfFDECache::addSection((LocalAddressSpace::pint_t)fde);
} else {
-_LIBUNWIND_DEBUG_LOG("_unw_add_dynamic_fde: bad fde: %s", message);
+CFI_Parser::FDE_Info fdeInfo;
+CFI_Parser::CIE_Info cieInfo;
+const char *message = CFI_Parser::decodeFDE(
+ LocalAddressSpace::sThisAddressSpace,
+(LocalAddressSpace::pint_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::add((LocalAddressSpace::pint_t)mh_group,
+fdeInfo.pcStart, fdeInfo.pcEnd,
+fdeInfo.fdeStart);
+} else {
+ _LIBUNWIND_DEBUG_LOG("_unw_add_dynamic_fde: bad fde: %s", message);
+}
}
}
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -44,10 +44,12 @@
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 static void iterateSections(Func func);
private:
@@ -70,6 +72,11 @@
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
@@ -88,6 +95,21 @@
typename DwarfFDECache::entry DwarfFDECache::_initialBuffer[64];
template
+typename DwarfFDECache::pint_t *
+DwarfFDECache::_sections = _initialSectionsBuffer;
+
+template
+typename DwarfFDECache::pint_t *
+DwarfFDECache::_sectionsUsed = _initialSectionsBuffer;
+
+template
+typename DwarfFDECache::pint_t *
+DwarfFDECache::_sectionsEnd = &_initialSectionsBuffer[64];
+
+template
+typename DwarfFDECache::pint_t DwarfFDECache::_initialSectionsBuffer[64];
+
+template
RWMutex DwarfFDECache::_lock;
#ifdef __APPLE__
@@ -144,6 +166,28 @@
}
template
+void DwarfFDECache::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 = [oldSize];
+_sectionsEnd = [newSize];
+ }
+ *_sectionsUsed = section_start;
+ ++_sectionsUsed;
+ _LIBUNWIND_LOG_IF_FALSE(_lock.unlock());
+#endif
+}
+
+template
void DwarfFDECache::removeAllIn(pint_t mh) {
_LIBUNWIND_LOG_IF_FALSE(_lock.lock());
entry *d = _buffer;
@@ -155,6 +199,15
[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section
mstorsjo added a subscriber: joerg. mstorsjo added a comment. 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"). https://reviews.llvm.org/D44494 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section
mstorsjo added a comment. In https://reviews.llvm.org/D44494#1050803, @mstorsjo wrote: > In https://reviews.llvm.org/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. https://reviews.llvm.org/D44494 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section
mstorsjo added a comment. In https://reviews.llvm.org/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. https://reviews.llvm.org/D44494 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section
mstorsjo added a comment. In https://reviews.llvm.org/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). https://reviews.llvm.org/D44494 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section
compnerd requested changes to this revision. compnerd added a comment. This revision now requires changes to proceed. 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. https://reviews.llvm.org/D44494 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section
mstorsjo updated this revision to Diff 138451.
mstorsjo added a comment.
Avoid using the C++ header.
https://reviews.llvm.org/D44494
Files:
src/UnwindCursor.hpp
src/libunwind.cpp
Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -339,7 +339,9 @@
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::addSection((LocalAddressSpace::pint_t)fde);
}
}
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -44,10 +44,12 @@
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 static void iterateSections(Func func);
private:
@@ -70,6 +72,11 @@
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
@@ -88,6 +95,21 @@
typename DwarfFDECache::entry DwarfFDECache::_initialBuffer[64];
template
+typename DwarfFDECache::pint_t *
+DwarfFDECache::_sections = _initialSectionsBuffer;
+
+template
+typename DwarfFDECache::pint_t *
+DwarfFDECache::_sectionsUsed = _initialSectionsBuffer;
+
+template
+typename DwarfFDECache::pint_t *
+DwarfFDECache::_sectionsEnd = &_initialSectionsBuffer[64];
+
+template
+typename DwarfFDECache::pint_t DwarfFDECache::_initialSectionsBuffer[64];
+
+template
RWMutex DwarfFDECache::_lock;
#ifdef __APPLE__
@@ -144,6 +166,28 @@
}
template
+void DwarfFDECache::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 = [oldSize];
+_sectionsEnd = [newSize];
+ }
+ *_sectionsUsed = section_start;
+ ++_sectionsUsed;
+ _LIBUNWIND_LOG_IF_FALSE(_lock.unlock());
+#endif
+}
+
+template
void DwarfFDECache::removeAllIn(pint_t mh) {
_LIBUNWIND_LOG_IF_FALSE(_lock.lock());
entry *d = _buffer;
@@ -155,6 +199,15 @@
}
}
_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());
}
@@ -174,6 +227,16 @@
}
_LIBUNWIND_LOG_IF_FALSE(_lock.unlock());
}
+
+template
+template
+void DwarfFDECache::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)
@@ -1352,6 +1415,28 @@
}
}
+ bool foundSection = false;
+ DwarfFDECache::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::FDE_Info fdeInfo;
+CFI_Parser::CIE_Info cieInfo;
+// Only parse and see if it is the right frame here,
+// to avoid recursive self-locking.
+foundSection = CFI_Parser::findFDE(_addressSpace, pc, sects.dwarf_section,
+ (uint32_t)sects.dwarf_section_length, 0,
+ , );
+ });
+ // 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;
___
cfe-commits mailing list
[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section
mstorsjo added inline comments. Comment at: src/UnwindCursor.hpp:53 unw_word_t fde, unw_word_t mh)); + static void iterateSections(std::function func); rnk wrote: > I'm concerned that std::function might introduce runtime dependencies from > libc++abi back to libc++. I think we get away with the 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. Ok, will change. Repository: rUNW libunwind https://reviews.llvm.org/D44494 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section
rnk added inline comments. Comment at: src/UnwindCursor.hpp:53 unw_word_t fde, unw_word_t mh)); + static void iterateSections(std::function func); I'm concerned that std::function might introduce runtime dependencies from libc++abi back to libc++. I think we get away with the 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. Repository: rUNW libunwind https://reviews.llvm.org/D44494 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section
mstorsjo created this revision.
mstorsjo added reviewers: rnk, compnerd.
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).
Repository:
rUNW libunwind
https://reviews.llvm.org/D44494
Files:
src/UnwindCursor.hpp
src/libunwind.cpp
Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -347,7 +347,9 @@
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::addSection((LocalAddressSpace::pint_t)fde);
}
}
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -13,6 +13,7 @@
#define __UNWINDCURSOR_HPP__
#include
+#include
#include
#include
#include
@@ -44,10 +45,12 @@
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));
+ static void iterateSections(std::function func);
private:
@@ -70,6 +73,11 @@
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
@@ -88,6 +96,21 @@
typename DwarfFDECache::entry DwarfFDECache::_initialBuffer[64];
template
+typename DwarfFDECache::pint_t *
+DwarfFDECache::_sections = _initialSectionsBuffer;
+
+template
+typename DwarfFDECache::pint_t *
+DwarfFDECache::_sectionsUsed = _initialSectionsBuffer;
+
+template
+typename DwarfFDECache::pint_t *
+DwarfFDECache::_sectionsEnd = &_initialSectionsBuffer[64];
+
+template
+typename DwarfFDECache::pint_t DwarfFDECache::_initialSectionsBuffer[64];
+
+template
RWMutex DwarfFDECache::_lock;
#ifdef __APPLE__
@@ -144,6 +167,28 @@
}
template
+void DwarfFDECache::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 = [oldSize];
+_sectionsEnd = [newSize];
+ }
+ *_sectionsUsed = section_start;
+ ++_sectionsUsed;
+ _LIBUNWIND_LOG_IF_FALSE(_lock.unlock());
+#endif
+}
+
+template
void DwarfFDECache::removeAllIn(pint_t mh) {
_LIBUNWIND_LOG_IF_FALSE(_lock.lock());
entry *d = _buffer;
@@ -155,6 +200,15 @@
}
}
_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());
}
@@ -174,6 +228,15 @@
}
_LIBUNWIND_LOG_IF_FALSE(_lock.unlock());
}
+
+template
+void DwarfFDECache::iterateSections(std::function 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)
@@ -1352,6 +1415,28 @@
}
}
+ bool foundSection = false;
+ DwarfFDECache::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::FDE_Info fdeInfo;
+CFI_Parser::CIE_Info cieInfo;
+// Only parse and see if it is the right frame here,
+// to avoid recursive self-locking.
+foundSection = CFI_Parser::findFDE(_addressSpace, pc, sects.dwarf_section,
+ (uint32_t)sects.dwarf_section_length, 0,
+ , );
+ });
+ // This call will lock the cache when adding a new match; only do
