[PATCH] D90898: [libunwind] Unwind through aarch64/Linux sigreturn frame

Thu Jan 14 16:17:00 PST 2021

rprichard added a comment.

In D90898#2496725 <https://reviews.llvm.org/D90898#2496725>, @compnerd wrote:

> In D90898#2384775 <https://reviews.llvm.org/D90898#2384775>, @rprichard wrote:
>
>> In D90898#2379387 <https://reviews.llvm.org/D90898#2379387>, @compnerd wrote:
>>
>>> Could you add a bit of a comment explaining why the XOM case is not protected against?
>>
>> I think I can expand on the comment.
>>
>>> libunwind can be used for generating backtraces during regular execution (e.g. profiling) and crashing if the XOM is in use seems unfortunate.
>>
>> For profiling, I assume the unwind info needs to be accurate at every instruction? I know that's definitely not the case for AArch64 <https://bugs.llvm.org/show_bug.cgi?id=47142>, but I thought it's still only mostly-accurate on other archs? I don't *really* know that unwinding through a signal handler is important for Android, but I don't know how to rule it out either, and we do have a test for it <https://android.googlesource.com/platform/bionic/+/6fc751cca894fb496fb5d854260151bf443229ac/tests/stack_unwinding_test.cpp#114>. I think the test predated Android's libunwindstack unwinder. Maybe someone could be using `_Unwind_Backtrace` in a segfault handler, and that probably avoids the async-unwind-info problem most of the time. But I also don't think that's how third-party Android crash dumpers work.
>
> Sure, though, libunwind is generic and is used outside of android as well, including other AArch64 targets, so we do need to consider those use cases while making changes.

Makes sense. Thanks for the review!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90898