This is an archive of the discontinued LLVM Phabricator instance.

[libFuzzer] Fix unwinding for Fuchsia
ClosedPublic

Authored by charco on Oct 29 2019, 12:05 PM.

Details

Summary

This commit fixes part of the issues with stack unwinding in fuchsia for
arm64 and x86_64. It consists of multiple fixes:

(1) The cfa_offset calculation was wrong, instead of pointing to the
previous stack pointer, it was pointing to the correct one. It worked in
most of the cases because the crashing functions already had a
prologue and had their cfa information relative to another register. The
fix consists on adding a constant that can be used to calculate the
crashing function's stack pointer, and base all the cfi information
relative to that offset.

(2) (arm64) Due to errors with the syntax for the dwarf information, most
of the OP_NUM macros were not working. The problem was that they were
referred to as r##NUM (like r14), when it should have been x##num
(like x14), or even without the x.

(3) (arm64) The link register was being considered a part of the main
registers (r30), when in the real struct it has its own field. Given
that the link register is in the same spot in the struct as r[30] would be,
and that C++ doesn't care about anything, the calculation was still correct.

(4) (x86_64) The stack doesn't need to be aligned to 16 bytes when we
jump to the trampoline function, but it needs to be before performing
call instructions. Encoding that logic in cfi information was tricky, so
we decided to make the cfa information relative to rbp and align rsp.
Note that this could have been done using another register directly,
but it seems cleaner to make a new fake stack frame.

There are some other minor changes like adding a brk 1 instruction in
arm64 to make sure that we never return to the crash trampoline (similar to
what we do in x86_64).

Sadly this commit does not fix unwinding for all use cases for arm64.
Crashing functions that do not add information related to the return column in
their cfi information will fail to unwind due to a bug in libunwinder.

Diff Detail

Event Timeline

charco created this revision.Oct 29 2019, 12:05 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 29 2019, 12:05 PM
Herald added subscribers: llvm-commits, Restricted Project, kristof.beyls, aprantl. · View Herald Transcript
phosek added inline comments.Nov 5 2019, 11:09 AM
compiler-rt/lib/fuzzer/FuzzerUtilFuchsia.cpp
89

s/cfa_offset/CFAOffset/ to follow LLVM naming conventions.

91

Nit: s/arm64/aarch64/ for consistency.

147

Can you leave this here as well?

194

Nit: be consistent about the case for CFI and CFA in comments.

charco updated this revision to Diff 227975.Nov 5 2019, 3:32 PM
charco marked 4 inline comments as done.

Make code consistent with llvm code style.

charco added a comment.Nov 5 2019, 3:33 PM

Thanks for the comments, Petr!

phosek accepted this revision.Nov 7 2019, 11:39 PM

LGTM

This revision is now accepted and ready to land.Nov 7 2019, 11:39 PM

@phosek would you land this change? I don't have land access

eep added a subscriber: eep.Nov 15 2019, 1:37 PM

Just a small note on the writeup in the comments.

compiler-rt/lib/fuzzer/FuzzerUtilFuchsia.cpp
80

Nit: sign error, SP + offset = SP_prev

charco updated this revision to Diff 230181.Nov 19 2019, 6:01 PM
charco marked 2 inline comments as done.

Update comments & commit message.

Thanks for the comments!

@phosek can you land this change?

compiler-rt/lib/fuzzer/FuzzerUtilFuchsia.cpp
80

Thanks for catching this!

This revision was automatically updated to reflect the committed changes.