Page MenuHomePhabricator

Improve crash unwinding on Fuchsia

Authored by aarongreen on Jun 22 2018, 4:53 PM.



Fuchsia doesn't have signals; instead it expects processes to have a dedicated exception thread that binds to the process' exception port and waits for exception packets to be delivered. On the other hand, libFuzzer and sanitizer_common use expect to collect crash information via libunwind from the same thread that caused the exception.

The long term fix is to improve support for remote unwinding in libunbwind, plumb this through sanitizer_common and libFuzzer, and handle the exception exclusively on the exception thread. In the meantime, this revision has the exception thread "resurrect" the crashing thread by:

  • saving its general purpose register state onto the crashing thread's stack,
  • setting the crashing thread's program counter to an assembly trampoline with the CFI information needed by libunwind, and
  • resuming the crashed thread.

Diff Detail

Event Timeline

aarongreen created this revision.Jun 22 2018, 4:53 PM
Herald added subscribers: Restricted Project, llvm-commits, chrib. · View Herald Transcript
phosek added inline comments.Jun 25 2018, 5:51 PM

Do you care about atexit hooks being executed? I think this should probably use _exit instead.


I'd use an explicit #elif defined(__aarch64__) and #error in the #else case. You never know when someone tries to compile this on some other architecture.

aarongreen marked 2 inline comments as done.

Addressed Petr's comment, and modified the visibility of CrashTrampolineAsm and MakeTrampoline to suppress their warnings.

Almost there, but some nits.


Clarify in the comment that this appears to have external linkage to C++ and that's why it's not in the anonymous namespace, but the assembly definition inside MakeTrampoline(), below, actually defines the symbol with internal linkage only.


You don't want this printf for the success case in production code.


typo: s/and/an/


typo: trailing slash should be period


Alternatively, Fuchsia may in future actually implement basic signal handling for the machine trap signals.


lr is not special, it's just an alias for x30 so I'd use OP_NUM(30) here.
Only sp (and pc) are really special.


This can use offsetof(..., r[num]) instead of local arithmetic. It's not standard but it works in Clang and GCC and always will.


Make this static (no extern "C") and pass it into the asm.


Clarify that this function is never really used as such and that's why the attribute is necessary.
It's just a container around the asm so it can use operands for compile-time computed constants.


No need for a variable here.


This can be ".cfi_startproc simple" because you supply all the details explicitly.


This can be "call %c[StaticCrashHandler]" with the operand being "i' (StaticCrashHandler).


This can be "bl %[StaticCrashHandler]" with the operand being "i' (StaticCrashHandler).
Make this the last operand and it solves to comma-eating problem too.


Just use "i" (0) here.


Can use initialized member decls and default constructor.
I'd make a ScopedHandle instead and just have two of them.


This should use __unsanitized_memcpy from <zircon/sanitizer.h>.
Otherwise it's theoretically possible for this thread to reenter the sanitizer runtime and that could go badly.


Also needs rounded down to modulo 16.


typo: s/its/it's/

aarongreen marked 18 inline comments as done.

Addressed Roland's comments.

mcgrathr accepted this revision.Jul 14 2018, 12:30 AM

Looks great!


superfluous ;

This revision is now accepted and ready to land.Jul 14 2018, 12:30 AM
aarongreen marked an inline comment as done.

Scrubbed superfluous semicolon.

This revision was automatically updated to reflect the committed changes.