[libunwind] fix dynamic .eh_frame registration

[SihangZhu]

Fix this issue #76957
Libgcc provides __register_frame to register a dynamic .eh_frame section, while __unw_add_dynamic_eh_frame_section can be used to do the same in libunwind. However, the address after dynamic .eh_frame are padding with 0 value, it will be identified as
legal CIE. And __unw_add_dynamic_eh_frame_section will continue to parse subsequent addresses until illegal memory or other sections are accessed.
This patch adds length formal parameter for dynamic registration.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-libunwind

Author: None (SihangZhu)

Changes

Fix this issue #76957
Libgcc provides __register_frame to register a dynamic .eh_frame section, while __unw_add_dynamic_eh_frame_section can be used to do the same in libunwind. However, the address after dynamic .eh_frame are padding with 0 value, it will be identified as
legal CIE. And __unw_add_dynamic_eh_frame_section will continue to parse subsequent addresses until illegal memory or other sections are accessed.
This patch adds length formal parameter for dynamic registration.

---

Full diff: https://github.com/llvm/llvm-project/pull/77185.diff

2 Files Affected:

• (modified) libunwind/src/libunwind.cpp (+3-2)
• (modified) libunwind/src/libunwind_ext.h (+1-1)

diff --git a/libunwind/src/libunwind.cpp b/libunwind/src/libunwind.cpp
index cd610377b63de8..7d78d167b83434 100644
--- a/libunwind/src/libunwind.cpp
+++ b/libunwind/src/libunwind.cpp
@@ -318,13 +318,14 @@ void __unw_remove_dynamic_fde(unw_word_t fde) {
   DwarfFDECache<LocalAddressSpace>::removeAllIn((LocalAddressSpace::pint_t)fde);
 }
 
-void __unw_add_dynamic_eh_frame_section(unw_word_t eh_frame_start) {
+void __unw_add_dynamic_eh_frame_section(unw_word_t eh_frame_start, size_t length) {
   // The eh_frame section start serves as the mh_group
   unw_word_t mh_group = eh_frame_start;
   CFI_Parser<LocalAddressSpace>::CIE_Info cieInfo;
   CFI_Parser<LocalAddressSpace>::FDE_Info fdeInfo;
   auto p = (LocalAddressSpace::pint_t)eh_frame_start;
-  while (true) {
+  auto end = p + length;
+  while (p < end) {
     if (CFI_Parser<LocalAddressSpace>::decodeFDE(
             LocalAddressSpace::sThisAddressSpace, p, &fdeInfo, &cieInfo,
             true) == NULL) {
diff --git a/libunwind/src/libunwind_ext.h b/libunwind/src/libunwind_ext.h
index 28db43a4f6eef2..1bfb595c46130f 100644
--- a/libunwind/src/libunwind_ext.h
+++ b/libunwind/src/libunwind_ext.h
@@ -55,7 +55,7 @@ extern void __unw_iterate_dwarf_unwind_cache(void (*func)(
 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_add_dynamic_eh_frame_section(unw_word_t eh_frame_start, size_t length);
 extern void __unw_remove_dynamic_eh_frame_section(unw_word_t eh_frame_start);
 
 #ifdef __APPLE__

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[arichardson]

While I think this is a reasonable change, I don't think we can change this function signature without breaking the ABI. So it probably needs to be a new function instead.

[lhames]

As @arichardson noted this would break the ABI.

The best solution is to fix the implementation of __unw_add_dynamic_eh_frame_section (and __unw_remove_dynamic_eh_frame_section) as described in #76957 (comment).

[SihangZhu]

As @arichardson noted this would break the ABI.

The best solution is to fix the implementation of __unw_add_dynamic_eh_frame_section (and __unw_remove_dynamic_eh_frame_section) as described in #76957 (comment).

thx for advice.

[SihangZhu]

@lhames Could you check again, I have fixed the implementation as you decribed

[lhames]

Hi @SihangZhu. Sorry for the delay — will try to review this tomorrow.

[lhames]

Ok -- local testing seems to confirm that this is the right fix. We need to add a testcase for this API, but that can happen as a follow-up.