[compiler-rt] 46c7fc2 - [libFuzzer] Fix unwinding for Fuchsia
Marco Vanotti via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 21 15:47:24 PST 2019
Author: Marco Vanotti
Date: 2019-11-21T15:47:07-08:00
New Revision: 46c7fc22cfb14a5b4445261c3ae849242f4d2ff9
URL: https://github.com/llvm/llvm-project/commit/46c7fc22cfb14a5b4445261c3ae849242f4d2ff9
DIFF: https://github.com/llvm/llvm-project/commit/46c7fc22cfb14a5b4445261c3ae849242f4d2ff9.diff
LOG: [libFuzzer] Fix unwinding for Fuchsia
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 current 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.
Reviewers: mcgrathr, jakehehrlich, phosek, kcc, aarongreen
Subscribers: aprantl, kristof.beyls, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69579
Added:
Modified:
compiler-rt/lib/fuzzer/FuzzerUtilFuchsia.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/fuzzer/FuzzerUtilFuchsia.cpp b/compiler-rt/lib/fuzzer/FuzzerUtilFuchsia.cpp
index 79fd950bbf97..d869c3ec235e 100644
--- a/compiler-rt/lib/fuzzer/FuzzerUtilFuchsia.cpp
+++ b/compiler-rt/lib/fuzzer/FuzzerUtilFuchsia.cpp
@@ -76,6 +76,23 @@ void InterruptHandler() {
Fuzzer::StaticInterruptCallback();
}
+// CFAOffset is used to reference the stack pointer before entering the
+// trampoline (Stack Pointer + CFAOffset = prev Stack Pointer). Before jumping
+// to the trampoline we copy all the registers onto the stack. We need to make
+// sure that the new stack has enough space to store all the registers.
+//
+// The trampoline holds CFI information regarding the registers stored in the
+// stack, which is then used by the unwinder to restore them.
+#if defined(__x86_64__)
+// In x86_64 the crashing function might also be using the red zone (128 bytes
+// on top of their rsp).
+constexpr size_t CFAOffset = 128 + sizeof(zx_thread_state_general_regs_t);
+#elif defined(__aarch64__)
+// In aarch64 we need to always have the stack pointer aligned to 16 bytes, so we
+// make sure that we are keeping that same alignment.
+constexpr size_t CFAOffset = (sizeof(zx_thread_state_general_regs_t) + 15) & -(uintptr_t)16;
+#endif
+
// For the crash handler, we need to call Fuzzer::StaticCrashSignalCallback
// without POSIX signal handlers. To achieve this, we use an assembly function
// to add the necessary CFI unwinding information and a C function to bridge
@@ -140,7 +157,6 @@ void InterruptHandler() {
OP_NUM(27) \
OP_NUM(28) \
OP_NUM(29) \
- OP_NUM(30) \
OP_REG(sp)
#else
@@ -148,14 +164,17 @@ void InterruptHandler() {
#endif
// Produces a CFI directive for the named or numbered register.
+// The value used refers to an assembler immediate operand with the same name
+// as the register (see ASM_OPERAND_REG).
#define CFI_OFFSET_REG(reg) ".cfi_offset " #reg ", %c[" #reg "]\n"
-#define CFI_OFFSET_NUM(num) CFI_OFFSET_REG(r##num)
+#define CFI_OFFSET_NUM(num) CFI_OFFSET_REG(x##num)
-// Produces an assembler input operand for the named or numbered register.
+// Produces an assembler immediate operand for the named or numbered register.
+// This operand contains the offset of the register relative to the CFA.
#define ASM_OPERAND_REG(reg) \
- [reg] "i"(offsetof(zx_thread_state_general_regs_t, reg)),
+ [reg] "i"(offsetof(zx_thread_state_general_regs_t, reg) - CFAOffset),
#define ASM_OPERAND_NUM(num) \
- [r##num] "i"(offsetof(zx_thread_state_general_regs_t, r[num])),
+ [x##num] "i"(offsetof(zx_thread_state_general_regs_t, r[num]) - CFAOffset),
// Trampoline to bridge from the assembly below to the static C++ crash
// callback.
@@ -168,7 +187,16 @@ static void StaticCrashHandler() {
}
// Creates the trampoline with the necessary CFI information to unwind through
-// to the crashing call stack. The attribute is necessary because the function
+// to the crashing call stack:
+// * Defining the CFA so that it points to the stack pointer at the point
+// of crash.
+// * Storing all registers at the point of crash in the stack and refer to them
+// via CFI information (relative to the CFA).
+// * Setting the return column so the unwinder knows how to continue unwinding.
+// * (x86_64) making sure rsp is aligned before calling StaticCrashHandler.
+// * Calling StaticCrashHandler that will trigger the unwinder.
+//
+// The __attribute__((used)) is necessary because the function
// is never called; it's just a container around the assembly to allow it to
// use operands for compile-time computed constants.
__attribute__((used))
@@ -181,16 +209,21 @@ void MakeTrampoline() {
".cfi_signal_frame\n"
#if defined(__x86_64__)
".cfi_return_column rip\n"
- ".cfi_def_cfa rsp, 0\n"
+ ".cfi_def_cfa rsp, %c[CFAOffset]\n"
FOREACH_REGISTER(CFI_OFFSET_REG, CFI_OFFSET_NUM)
+ "mov %%rsp, %%rbp\n"
+ ".cfi_def_cfa_register rbp\n"
+ "andq $-16, %%rsp\n"
"call %c[StaticCrashHandler]\n"
"ud2\n"
#elif defined(__aarch64__)
".cfi_return_column 33\n"
- ".cfi_def_cfa sp, 0\n"
- ".cfi_offset 33, %c[pc]\n"
+ ".cfi_def_cfa sp, %c[CFAOffset]\n"
FOREACH_REGISTER(CFI_OFFSET_REG, CFI_OFFSET_NUM)
- "bl %[StaticCrashHandler]\n"
+ ".cfi_offset 33, %c[pc]\n"
+ ".cfi_offset 30, %c[lr]\n"
+ "bl %c[StaticCrashHandler]\n"
+ "brk 1\n"
#else
#error "Unsupported architecture for fuzzing on Fuchsia"
#endif
@@ -202,8 +235,10 @@ void MakeTrampoline() {
: FOREACH_REGISTER(ASM_OPERAND_REG, ASM_OPERAND_NUM)
#if defined(__aarch64__)
ASM_OPERAND_REG(pc)
+ ASM_OPERAND_REG(lr)
#endif
- [StaticCrashHandler] "i" (StaticCrashHandler));
+ [StaticCrashHandler] "i" (StaticCrashHandler),
+ [CFAOffset] "i" (CFAOffset));
}
void CrashHandler(zx_handle_t *Event) {
@@ -269,17 +304,14 @@ void CrashHandler(zx_handle_t *Event) {
// onto the stack and jump into a trampoline with CFI instructions on how
// to restore it.
#if defined(__x86_64__)
- uintptr_t StackPtr =
- (GeneralRegisters.rsp - (128 + sizeof(GeneralRegisters))) &
- -(uintptr_t)16;
+ uintptr_t StackPtr = GeneralRegisters.rsp - CFAOffset;
__unsanitized_memcpy(reinterpret_cast<void *>(StackPtr), &GeneralRegisters,
sizeof(GeneralRegisters));
GeneralRegisters.rsp = StackPtr;
GeneralRegisters.rip = reinterpret_cast<zx_vaddr_t>(CrashTrampolineAsm);
#elif defined(__aarch64__)
- uintptr_t StackPtr =
- (GeneralRegisters.sp - sizeof(GeneralRegisters)) & -(uintptr_t)16;
+ uintptr_t StackPtr = GeneralRegisters.sp - CFAOffset;
__unsanitized_memcpy(reinterpret_cast<void *>(StackPtr), &GeneralRegisters,
sizeof(GeneralRegisters));
GeneralRegisters.sp = StackPtr;
More information about the llvm-commits
mailing list