[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

[Peter S. Housel via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbab39816085d: [libunwind] Add an interface for dynamic 
.eh_frame registration (authored by housel).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111863/new/

https://reviews.llvm.org/D111863

Files:
  libunwind/src/DwarfParser.hpp
  libunwind/src/libunwind.cpp
  libunwind/src/libunwind_ext.h

Index: libunwind/src/libunwind_ext.h
===
--- libunwind/src/libunwind_ext.h
+++ libunwind/src/libunwind_ext.h
@@ -51,6 +51,9 @@
 extern void __unw_add_dynamic_fde(unw_word_t fde);
 extern void __unw_remove_dynamic_fde(unw_word_t fde);
 
+extern void __unw_add_dynamic_eh_frame_section(unw_word_t eh_frame_start);
+extern void __unw_remove_dynamic_eh_frame_section(unw_word_t eh_frame_start);
+
 #if defined(_LIBUNWIND_ARM_EHABI)
 extern const uint32_t* decode_eht_entry(const uint32_t*, size_t*, size_t*);
 extern _Unwind_Reason_Code _Unwind_VRS_Interpret(_Unwind_Context *context,
Index: libunwind/src/libunwind.cpp
===
--- libunwind/src/libunwind.cpp
+++ libunwind/src/libunwind.cpp
@@ -292,6 +292,35 @@
   // fde is own mh_group
   DwarfFDECache::removeAllIn((LocalAddressSpace::pint_t)fde);
 }
+
+void __unw_add_dynamic_eh_frame_section(unw_word_t eh_frame_start) {
+  // The eh_frame section start serves as the mh_group
+  unw_word_t mh_group = eh_frame_start;
+  CFI_Parser::CIE_Info cieInfo;
+  CFI_Parser::FDE_Info fdeInfo;
+  auto p = (LocalAddressSpace::pint_t)eh_frame_start;
+  while (true) {
+if (CFI_Parser::decodeFDE(
+LocalAddressSpace::sThisAddressSpace, p, , ,
+true) == NULL) {
+  DwarfFDECache::add((LocalAddressSpace::pint_t)mh_group,
+fdeInfo.pcStart, fdeInfo.pcEnd,
+fdeInfo.fdeStart);
+  p += fdeInfo.fdeLength;
+} else if (CFI_Parser::parseCIE(
+   LocalAddressSpace::sThisAddressSpace, p, ) == NULL) {
+  p += cieInfo.cieLength;
+} else
+  return;
+  }
+}
+
+void __unw_remove_dynamic_eh_frame_section(unw_word_t eh_frame_start) {
+  // The eh_frame section start serves as the mh_group
+  DwarfFDECache::removeAllIn(
+  (LocalAddressSpace::pint_t)eh_frame_start);
+}
+
 #endif // defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
 #endif // !defined(__USING_SJLJ_EXCEPTIONS__)
 
Index: libunwind/src/DwarfParser.hpp
===
--- libunwind/src/DwarfParser.hpp
+++ libunwind/src/DwarfParser.hpp
@@ -154,7 +154,8 @@
   uintptr_t sectionLength, pint_t fdeHint, FDE_Info *fdeInfo,
   CIE_Info *cieInfo);
   static const char *decodeFDE(A , pint_t fdeStart,
-   FDE_Info *fdeInfo, CIE_Info *cieInfo);
+   FDE_Info *fdeInfo, CIE_Info *cieInfo,
+   bool useCIEInfo = false);
   static bool parseFDEInstructions(A , const FDE_Info ,
const CIE_Info , pint_t upToPC,
int arch, PrologInfo *results);
@@ -162,10 +163,14 @@
   static const char *parseCIE(A , pint_t cie, CIE_Info *cieInfo);
 };
 
-/// Parse a FDE into a CIE_Info and an FDE_Info
+/// Parse a FDE into a CIE_Info and an FDE_Info. If useCIEInfo is
+/// true, treat cieInfo as already-parsed CIE_Info (whose start offset
+/// must match the one specified by the FDE) rather than parsing the
+/// one indicated within the FDE.
 template 
 const char *CFI_Parser::decodeFDE(A , pint_t fdeStart,
- FDE_Info *fdeInfo, CIE_Info *cieInfo) {
+ FDE_Info *fdeInfo, CIE_Info *cieInfo,
+ bool useCIEInfo) {
   pint_t p = fdeStart;
   pint_t cfiLength = (pint_t)addressSpace.get32(p);
   p += 4;
@@ -181,9 +186,14 @@
 return "FDE is really a CIE"; // this is a CIE not an FDE
   pint_t nextCFI = p + cfiLength;
   pint_t cieStart = p - ciePointer;
-  const char *err = parseCIE(addressSpace, cieStart, cieInfo);
-  if (err != NULL)
-return err;
+  if (useCIEInfo) {
+if (cieInfo->cieStart != cieStart)
+  return "CIE start does not match";
+  } else {
+const char *err = parseCIE(addressSpace, cieStart, cieInfo);
+if (err != NULL)
+  return err;
+  }
   p += 4;
   // Parse pc begin and range.
   pint_t pcStart =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

[Peter S. Housel via Phabricator via cfe-commits]

housel updated this revision to Diff 382884.
housel added a comment.

Added additional comments to `CFI_Parser::decodeFDE`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111863/new/

https://reviews.llvm.org/D111863

Files:
  libunwind/src/DwarfParser.hpp
  libunwind/src/libunwind.cpp
  libunwind/src/libunwind_ext.h

Index: libunwind/src/libunwind_ext.h
===
--- libunwind/src/libunwind_ext.h
+++ libunwind/src/libunwind_ext.h
@@ -51,6 +51,9 @@
 extern void __unw_add_dynamic_fde(unw_word_t fde);
 extern void __unw_remove_dynamic_fde(unw_word_t fde);
 
+extern void __unw_add_dynamic_eh_frame_section(unw_word_t eh_frame_start);
+extern void __unw_remove_dynamic_eh_frame_section(unw_word_t eh_frame_start);
+
 #if defined(_LIBUNWIND_ARM_EHABI)
 extern const uint32_t* decode_eht_entry(const uint32_t*, size_t*, size_t*);
 extern _Unwind_Reason_Code _Unwind_VRS_Interpret(_Unwind_Context *context,
Index: libunwind/src/libunwind.cpp
===
--- libunwind/src/libunwind.cpp
+++ libunwind/src/libunwind.cpp
@@ -292,6 +292,35 @@
   // fde is own mh_group
   DwarfFDECache::removeAllIn((LocalAddressSpace::pint_t)fde);
 }
+
+void __unw_add_dynamic_eh_frame_section(unw_word_t eh_frame_start) {
+  // The eh_frame section start serves as the mh_group
+  unw_word_t mh_group = eh_frame_start;
+  CFI_Parser::CIE_Info cieInfo;
+  CFI_Parser::FDE_Info fdeInfo;
+  auto p = (LocalAddressSpace::pint_t)eh_frame_start;
+  while (true) {
+if (CFI_Parser::decodeFDE(
+LocalAddressSpace::sThisAddressSpace, p, , ,
+true) == NULL) {
+  DwarfFDECache::add((LocalAddressSpace::pint_t)mh_group,
+fdeInfo.pcStart, fdeInfo.pcEnd,
+fdeInfo.fdeStart);
+  p += fdeInfo.fdeLength;
+} else if (CFI_Parser::parseCIE(
+   LocalAddressSpace::sThisAddressSpace, p, ) == NULL) {
+  p += cieInfo.cieLength;
+} else
+  return;
+  }
+}
+
+void __unw_remove_dynamic_eh_frame_section(unw_word_t eh_frame_start) {
+  // The eh_frame section start serves as the mh_group
+  DwarfFDECache::removeAllIn(
+  (LocalAddressSpace::pint_t)eh_frame_start);
+}
+
 #endif // defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
 #endif // !defined(__USING_SJLJ_EXCEPTIONS__)
 
Index: libunwind/src/DwarfParser.hpp
===
--- libunwind/src/DwarfParser.hpp
+++ libunwind/src/DwarfParser.hpp
@@ -154,7 +154,8 @@
   uintptr_t sectionLength, pint_t fdeHint, FDE_Info *fdeInfo,
   CIE_Info *cieInfo);
   static const char *decodeFDE(A , pint_t fdeStart,
-   FDE_Info *fdeInfo, CIE_Info *cieInfo);
+   FDE_Info *fdeInfo, CIE_Info *cieInfo,
+   bool useCIEInfo = false);
   static bool parseFDEInstructions(A , const FDE_Info ,
const CIE_Info , pint_t upToPC,
int arch, PrologInfo *results);
@@ -162,10 +163,14 @@
   static const char *parseCIE(A , pint_t cie, CIE_Info *cieInfo);
 };
 
-/// Parse a FDE into a CIE_Info and an FDE_Info
+/// Parse a FDE into a CIE_Info and an FDE_Info. If useCIEInfo is
+/// true, treat cieInfo as already-parsed CIE_Info (whose start offset
+/// must match the one specified by the FDE) rather than parsing the
+/// one indicated within the FDE.
 template 
 const char *CFI_Parser::decodeFDE(A , pint_t fdeStart,
- FDE_Info *fdeInfo, CIE_Info *cieInfo) {
+ FDE_Info *fdeInfo, CIE_Info *cieInfo,
+ bool useCIEInfo) {
   pint_t p = fdeStart;
   pint_t cfiLength = (pint_t)addressSpace.get32(p);
   p += 4;
@@ -181,9 +186,14 @@
 return "FDE is really a CIE"; // this is a CIE not an FDE
   pint_t nextCFI = p + cfiLength;
   pint_t cieStart = p - ciePointer;
-  const char *err = parseCIE(addressSpace, cieStart, cieInfo);
-  if (err != NULL)
-return err;
+  if (useCIEInfo) {
+if (cieInfo->cieStart != cieStart)
+  return "CIE start does not match";
+  } else {
+const char *err = parseCIE(addressSpace, cieStart, cieInfo);
+if (err != NULL)
+  return err;
+  }
   p += 4;
   // Parse pc begin and range.
   pint_t pcStart =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

[Peter S. Housel via Phabricator via cfe-commits]

housel added a comment.

To be clear, this new code parses exactly as much of each FDE as the existing 
`__register_frame`/`__unw_add_dynamic_fde` does, including doing the same work 
to compute the record length. Neither needs to parse the instructions at 
registration time.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111863/new/

https://reviews.llvm.org/D111863

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

[Peter S. Housel via Phabricator via cfe-commits]

housel added a comment.

In D111863#3072829 , @joerg wrote:

> Are you mixing up of `__register_frame` and `__register_frame_info`?

No? FreeBSD doesn't patch `__register_frame_info`, and so (like base libunwind) 
it does nothing in the code I linked.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111863/new/

https://reviews.llvm.org/D111863

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

[Peter S. Housel via Phabricator via cfe-commits]

housel added a comment.

It's also worth noting that FreeBSD's version of libgcc exception handling is 
actually based on the libunwind code, with a local patch 

 that implements compatibility with libgcc `__register_frame` by changing it to 
parse an entire `.eh_frame` section (in a slightly more ad hoc fashion than 
this code). Having this new entry point in-tree would simplify the FreeBSD 
local changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111863/new/

https://reviews.llvm.org/D111863

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

[Peter S. Housel via Phabricator via cfe-commits]

housel added a comment.

In D111863#3069279 , @lhames wrote:

> I think the ORC runtime provides a much more natural way to test this. Did 
> you manage to come up with some ORC-runtime based tests in the end?

My current plan is to automate what I've been doing manually, namely running a 
test of C++ exception handling using `llvm-jitlink`, both with the default 
configuration (using libgcc-provided unwinding), and with libunwind 
`LD_PRELOAD`ed to force it as the unwinding provider.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111863/new/

https://reviews.llvm.org/D111863

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

[Peter S. Housel via Phabricator via cfe-commits]

housel added a comment.

In D111863#3065992 , @MaskRay wrote:

> I looked at the libgcc mechanism at one time. I remember that in most cases 
> it just uses `PT_GNU_EH_FRAME` and these eh_frame boundary registry functions 
> are not needed.
> Can ORC just use `PT_GNU_EH_FRAME`?

ORC doesn't go through the system dynamic loader, so there's no program header 
(JITLink doesn't generate one). None of the dynamic loaders I've looked at have 
any way to dynamically register program headers so that they'll be returned by 
`dl_iterate_phdr`. With ORC, code and data segments, including `.eh_frame` 
segments, get allocated in the target execution space and initialized by the 
JITLink memory manager, and the ORC runtime calling these registration 
functions is the only way libunwind will know about these tables.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111863/new/

https://reviews.llvm.org/D111863

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

[Peter S. Housel via Phabricator via cfe-commits]

housel created this revision.
housel added a reviewer: cfe-commits.
Herald added a project: libunwind.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libunwind.
housel requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The libgcc runtime library provides __register_frame and __deregister_frame 
functions, which can be used by dynamic code generators to register an 
.eh_frame section, which contains one or more Call Frame Information records, 
each consisting of a Common Information Entry record followed by one or more 
Frame Description Entry records. This libunwind library also provides 
__register_frame
and __deregister_frame functions, but they are effectively aliases for 
__unw_add_dynamic_fde and __unw_remove_dynamic_fde and thus can only take a 
single FDE.

This patch adds __unw_add_dynamic_eh_frame_section and 
__unw_remove_dynamic_eh_frame_section functions which explicitly use the 
.eh_frame format. Clients such as the ORCv2 platform and runtime can check for 
these functions and use them if unwinding is being
provided by libunwind, or fall back to __register_frame and __deregister_frame 
if unwinding is provided by libgcc.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111863

Files:
  libunwind/src/DwarfParser.hpp
  libunwind/src/libunwind.cpp
  libunwind/src/libunwind_ext.h


Index: libunwind/src/libunwind_ext.h
===
--- libunwind/src/libunwind_ext.h
+++ libunwind/src/libunwind_ext.h
@@ -51,6 +51,9 @@
 extern void __unw_add_dynamic_fde(unw_word_t fde);
 extern void __unw_remove_dynamic_fde(unw_word_t fde);
 
+extern void __unw_add_dynamic_eh_frame_section(unw_word_t eh_frame_start);
+extern void __unw_remove_dynamic_eh_frame_section(unw_word_t eh_frame_start);
+
 #if defined(_LIBUNWIND_ARM_EHABI)
 extern const uint32_t* decode_eht_entry(const uint32_t*, size_t*, size_t*);
 extern _Unwind_Reason_Code _Unwind_VRS_Interpret(_Unwind_Context *context,
Index: libunwind/src/libunwind.cpp
===
--- libunwind/src/libunwind.cpp
+++ libunwind/src/libunwind.cpp
@@ -292,6 +292,35 @@
   // fde is own mh_group
   
DwarfFDECache::removeAllIn((LocalAddressSpace::pint_t)fde);
 }
+
+void __unw_add_dynamic_eh_frame_section(unw_word_t eh_frame_start) {
+  // The eh_frame section start serves as the mh_group
+  unw_word_t mh_group = eh_frame_start;
+  CFI_Parser::CIE_Info cieInfo;
+  CFI_Parser::FDE_Info fdeInfo;
+  auto p = (LocalAddressSpace::pint_t)eh_frame_start;
+  while (true) {
+if (CFI_Parser::decodeFDE(
+LocalAddressSpace::sThisAddressSpace, p, , ,
+true) == NULL) {
+  
DwarfFDECache::add((LocalAddressSpace::pint_t)mh_group,
+fdeInfo.pcStart, fdeInfo.pcEnd,
+fdeInfo.fdeStart);
+  p += fdeInfo.fdeLength;
+} else if (CFI_Parser::parseCIE(
+   LocalAddressSpace::sThisAddressSpace, p, ) == NULL) 
{
+  p += cieInfo.cieLength;
+} else
+  return;
+  }
+}
+
+void __unw_remove_dynamic_eh_frame_section(unw_word_t eh_frame_start) {
+  // The eh_frame section start serves as the mh_group
+  DwarfFDECache::removeAllIn(
+  (LocalAddressSpace::pint_t)eh_frame_start);
+}
+
 #endif // defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
 #endif // !defined(__USING_SJLJ_EXCEPTIONS__)
 
Index: libunwind/src/DwarfParser.hpp
===
--- libunwind/src/DwarfParser.hpp
+++ libunwind/src/DwarfParser.hpp
@@ -154,7 +154,8 @@
   uintptr_t sectionLength, pint_t fdeHint, FDE_Info 
*fdeInfo,
   CIE_Info *cieInfo);
   static const char *decodeFDE(A , pint_t fdeStart,
-   FDE_Info *fdeInfo, CIE_Info *cieInfo);
+   FDE_Info *fdeInfo, CIE_Info *cieInfo,
+   bool useCIEInfo = false);
   static bool parseFDEInstructions(A , const FDE_Info ,
const CIE_Info , pint_t upToPC,
int arch, PrologInfo *results);
@@ -165,7 +166,8 @@
 /// Parse a FDE into a CIE_Info and an FDE_Info
 template 
 const char *CFI_Parser::decodeFDE(A , pint_t fdeStart,
- FDE_Info *fdeInfo, CIE_Info *cieInfo) {
+ FDE_Info *fdeInfo, CIE_Info *cieInfo,
+ bool useCIEInfo) {
   pint_t p = fdeStart;
   pint_t cfiLength = (pint_t)addressSpace.get32(p);
   p += 4;
@@ -181,9 +183,14 @@
 return "FDE is really a CIE"; // this is a CIE not an FDE
   pint_t nextCFI = p + cfiLength;
   pint_t cieStart = p - ciePointer;
-  const char *err = parseCIE(addressSpace, cieStart, cieInfo);
-  if (err != NULL)
-return err;
+  if (useCIEInfo) {
+if