Page MenuHomePhabricator

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

Authored by rprichard on Oct 15 2019, 12:37 AM.



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

Diff Detail

Event Timeline

rprichard created this revision.Oct 15 2019, 12:37 AM

FWIW, I tested the "extest" benchmark from 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.)

danalbert accepted this revision.Oct 15 2019, 9:13 PM
This revision is now accepted and ready to land.Oct 15 2019, 9:13 PM
srhines accepted this revision.Oct 17 2019, 3:13 PM
This revision was automatically updated to reflect the committed changes.

Hello, we tried rolling libunwind in chrome, and this commit makes chrome crashy: /

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.

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 21 2020, 1:28 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

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.