[libunwind][Android] Fix findUnwindSections for ARM EHABI Bionic

[rprichard]

Summary

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 and D30681). The
special case was restored in 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.)

[rprichard]

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.)

[rprichard]

Aside: This change is also tracked in Android's Gerrit review tool: https://android-review.googlesource.com/c/platform/external/libunwind_llvm/+/1142642

[thakis]

Hello, we tried rolling libunwind in chrome, and this commit makes chrome crashy: https://bugs.chromium.org/p/chromium/issues/detail?id=1056216 / https://bugs.chromium.org/p/chromium/issues/detail?id=1055188

If this breaks chrome, chances are it breaks something else as well.

Not sure what good next steps are. For us it'd be great if this could be backed out while we figure things out so that we can try rolling to trunk and possibly finding more bugs.

Maybe hop over to the first crbug and ask questions. torne can get you a prebuilt binary and/or try things for you if needed.

[rprichard]

I commented on the Chromium bug. Chromium has a stack sampler component that overrides dl_unwind_find_exidx with an implementation that outputs a number-of-bytes instead of number-of-entries. It looks like we're fixing the overriding dl_unwind_find_exidx to use number-of-entries, so we don't need to revert this change.

libunwind/src/AddressSpace.hpp

 
 #if _LIBUNWIND_USE_DLADDR
 #include <dlfcn.h>
 #if defined(__unix__) && defined(__ELF__) && defined(_LIBUNWIND_HAS_COMMENT_LIB_PRAGMA)
 #pragma comment(lib, "dl")
 #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 {
    bool checkKeyMgrRegisteredFDEs(uintptr_t targetAddr, void *&fde);
 }
 #endif
 
 #include "libunwind.h"
#elif defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND) && defined(_WIN32)
   }
   return false;
 #elif defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32)
   // Don't even bother, since Windows has functions that do all this stuff
   // for us.
   (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)
   struct dl_iterate_cb_data {
     LocalAddressSpace *addressSpace;
     UnwindInfoSections *sects;
     uintptr_t targetAddr;
   };

libunwind/src/UnwindCursor.hpp

 
 template <typename A, typename R> bool UnwindCursor<A, R>::isSignalFrame() {
   return _isSignalFrame;
 }
 
 #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;
 
   typedef typename A::pint_t value_type;
   typedef typename A::pint_t* pointer;
   typedef typename A::pint_t& reference;
   typedef size_t size_type;