[libcxx-commits] [PATCH] D68972: [libunwind][Android] Fix findUnwindSections for ARM EHABI Bionic

[Ryan Prichard via Phabricator via libcxx-commits]

rprichard created this revision.
Herald added subscribers: libcxx-commits, christof, kristof.beyls, srhines.
Herald added a project: libc++.
rprichard added reviewers: thomasanderson, srhines, danalbert, ed, keith.walker.arm.
rprichard added a comment.
Herald added a reviewer: mclow.lists.

FWIW, I tested the "extest" benchmark from https://github.com/android/ndk/issues/1062 on arm32. Using dl_unwind_find_exidx, the benchmark throws 10,000 exceptions in about 860ms. Using dl_iterate_phdr, the run-time varies, and I saw run-times from 3000-6000ms. (The unwinder has an optimization where it compares dlpi_addr to the target PC, and that makes EH overhead depend on the layout of shared libraries in memory.)


Fix the arm_section_length count. The meaning of the arm_section_length
field changed from num-of-elements to num-of-bytes when the
dl_unwind_find_exidx special case was removed (D30306 <https://reviews.llvm.org/D30306> and D30681 <https://reviews.llvm.org/D30681>). The
special case was restored in D39468 <https://reviews.llvm.org/D39468>, but that patch didn't account for the
change in arm_section_length's meaning.

That patch worked when it was applied to the NDK's fork of libunwind,
because it never removed the special case in the first place, and the
special case is probably disabled in the Android platform's copy of
libunwind, because __ANDROID_API__ is greater than 21.

Turn the dl_unwind_find_exidx special case on unconditionally for Bionic.
Bionic's dl_unwind_find_exidx is much faster than using dl_iterate_phdr.
(e.g. Bionic stores exidx info on an internal soinfo object.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68972

Files:
  libunwind/src/AddressSpace.hpp
  libunwind/src/UnwindCursor.hpp


Index: libunwind/src/UnwindCursor.hpp
===================================================================
--- libunwind/src/UnwindCursor.hpp
+++ libunwind/src/UnwindCursor.hpp
@@ -1222,11 +1222,6 @@
 #endif // defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
 
 #if defined(_LIBUNWIND_ARM_EHABI)
-struct EHABIIndexEntry {
-  uint32_t functionOffset;
-  uint32_t data;
-};
-
 template<typename A>
 struct EHABISectionIterator {
   typedef EHABISectionIterator _Self;
Index: libunwind/src/AddressSpace.hpp
===================================================================
--- libunwind/src/AddressSpace.hpp
+++ libunwind/src/AddressSpace.hpp
@@ -32,6 +32,13 @@
 #endif
 #endif
 
+#if defined(_LIBUNWIND_ARM_EHABI)
+struct EHABIIndexEntry {
+  uint32_t functionOffset;
+  uint32_t data;
+};
+#endif
+
 #ifdef __APPLE__
 #include <mach-o/getsect.h>
 namespace libunwind {
@@ -462,12 +469,13 @@
   (void)targetAddr;
   (void)info;
   return true;
-#elif defined(_LIBUNWIND_ARM_EHABI) && defined(__BIONIC__) &&                  \
-    (__ANDROID_API__ < 21)
+#elif defined(_LIBUNWIND_ARM_EHABI) && defined(__BIONIC__)
+  // For ARM EHABI, Bionic didn't implement dl_iterate_phdr until API 21. After
+  // API 21, dl_iterate_phdr exists, but dl_unwind_find_exidx is much faster.
   int length = 0;
   info.arm_section =
       (uintptr_t)dl_unwind_find_exidx((_Unwind_Ptr)targetAddr, &length);
-  info.arm_section_length = (uintptr_t)length;
+  info.arm_section_length = (uintptr_t)length * sizeof(EHABIIndexEntry);
   if (info.arm_section && info.arm_section_length)
     return true;
 #elif defined(_LIBUNWIND_ARM_EHABI) || defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68972.224963.patch
Type: text/x-patch
Size: 1652 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20191015/41e6cf66/attachment.bin>