[libunwind] Fix signal frame unwinding (PR #135367)
Trung Nguyen via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 11 06:46:48 PDT 2025
https://github.com/trungnt2910 updated https://github.com/llvm/llvm-project/pull/135367
>From d213e68a98b5c92656122aa13bf853b77e6c0b7a Mon Sep 17 00:00:00 2001
From: Trung Nguyen <57174311+trungnt2910 at users.noreply.github.com>
Date: Fri, 11 Apr 2025 23:11:04 +1000
Subject: [PATCH] Fix signal frame unwinding
The current unwinding implementation on Haiku is messy and broken.
1. It searches weird paths for private headers, which is breaking builds
in consuming projects, such as dotnet/runtime.
2. It does not even work, due to relying on incorrect private offsets.
This commit strips all references to private headers and ports a
working signal frame implementation. It has been tested against
`tests/signal_unwind.pass.cpp` and can go pass the signal frame.
---
libunwind/src/CMakeLists.txt | 16 ---
libunwind/src/UnwindCursor.hpp | 227 ++++++++++++++++++++++++---------
2 files changed, 167 insertions(+), 76 deletions(-)
diff --git a/libunwind/src/CMakeLists.txt b/libunwind/src/CMakeLists.txt
index d69013e5dace1..70bd3a017cda7 100644
--- a/libunwind/src/CMakeLists.txt
+++ b/libunwind/src/CMakeLists.txt
@@ -118,22 +118,6 @@ if (HAIKU)
add_compile_flags("-D_DEFAULT_SOURCE")
add_compile_flags("-DPT_GNU_EH_FRAME=PT_EH_FRAME")
-
- find_path(LIBUNWIND_HAIKU_PRIVATE_HEADERS
- "commpage_defs.h"
- PATHS ${CMAKE_SYSTEM_INCLUDE_PATH}
- PATH_SUFFIXES "/private/system"
- NO_DEFAULT_PATH
- REQUIRED)
-
- include_directories(SYSTEM "${LIBUNWIND_HAIKU_PRIVATE_HEADERS}")
- if (LIBUNWIND_TARGET_TRIPLE)
- if (${LIBUNWIND_TARGET_TRIPLE} MATCHES "^x86_64")
- include_directories(SYSTEM "${LIBUNWIND_HAIKU_PRIVATE_HEADERS}/arch/x86_64")
- endif()
- else()
- include_directories(SYSTEM "${LIBUNWIND_HAIKU_PRIVATE_HEADERS}/arch/${CMAKE_SYSTEM_PROCESSOR}")
- endif()
endif ()
string(REPLACE ";" " " LIBUNWIND_COMPILE_FLAGS "${LIBUNWIND_COMPILE_FLAGS}")
diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index ca9927edc9990..1c545978673b4 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -41,6 +41,14 @@
#define _LIBUNWIND_CHECK_LINUX_SIGRETURN 1
#endif
+#if defined(_LIBUNWIND_TARGET_HAIKU) && defined(_LIBUNWIND_TARGET_X86_64)
+#include <OS.h>
+#include <elf.h>
+#include <image.h>
+#include <signal.h>
+#define _LIBUNWIND_CHECK_HAIKU_SIGRETURN 1
+#endif
+
#include "AddressSpace.hpp"
#include "CompactUnwinder.hpp"
#include "config.h"
@@ -1015,7 +1023,7 @@ class UnwindCursor : public AbstractUnwindCursor{
template <typename Registers> int stepThroughSigReturn(Registers &) {
return UNW_STEP_END;
}
-#elif defined(_LIBUNWIND_TARGET_HAIKU)
+#elif defined(_LIBUNWIND_CHECK_HAIKU_SIGRETURN)
bool setInfoForSigReturn();
int stepThroughSigReturn();
#endif
@@ -2559,7 +2567,7 @@ int UnwindCursor<A, R>::stepWithTBTable(pint_t pc, tbtable *TBTable,
template <typename A, typename R>
void UnwindCursor<A, R>::setInfoBasedOnIPRegister(bool isReturnAddress) {
#if defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN) || \
- defined(_LIBUNWIND_TARGET_HAIKU)
+ defined(_LIBUNWIND_CHECK_HAIKU_SIGRETURN)
_isSigReturn = false;
#endif
@@ -2684,7 +2692,7 @@ void UnwindCursor<A, R>::setInfoBasedOnIPRegister(bool isReturnAddress) {
#endif // #if defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
#if defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN) || \
- defined(_LIBUNWIND_TARGET_HAIKU)
+ defined(_LIBUNWIND_CHECK_HAIKU_SIGRETURN)
if (setInfoForSigReturn())
return;
#endif
@@ -2760,63 +2768,6 @@ int UnwindCursor<A, R>::stepThroughSigReturn(Registers_arm64 &) {
_isSignalFrame = true;
return UNW_STEP_SUCCESS;
}
-
-#elif defined(_LIBUNWIND_TARGET_HAIKU) && defined(_LIBUNWIND_TARGET_X86_64)
-#include <commpage_defs.h>
-#include <signal.h>
-
-extern "C" {
-extern void *__gCommPageAddress;
-}
-
-template <typename A, typename R>
-bool UnwindCursor<A, R>::setInfoForSigReturn() {
-#if defined(_LIBUNWIND_TARGET_X86_64)
- addr_t signal_handler =
- (((addr_t *)__gCommPageAddress)[COMMPAGE_ENTRY_X86_SIGNAL_HANDLER] +
- (addr_t)__gCommPageAddress);
- addr_t signal_handler_ret = signal_handler + 45;
-#endif
- pint_t pc = static_cast<pint_t>(this->getReg(UNW_REG_IP));
- if (pc == signal_handler_ret) {
- _info = {};
- _info.start_ip = signal_handler;
- _info.end_ip = signal_handler_ret;
- _isSigReturn = true;
- return true;
- }
- return false;
-}
-
-template <typename A, typename R>
-int UnwindCursor<A, R>::stepThroughSigReturn() {
- _isSignalFrame = true;
- pint_t sp = _registers.getSP();
-#if defined(_LIBUNWIND_TARGET_X86_64)
- vregs *regs = (vregs *)(sp + 0x70);
-
- _registers.setRegister(UNW_REG_IP, regs->rip);
- _registers.setRegister(UNW_REG_SP, regs->rsp);
- _registers.setRegister(UNW_X86_64_RAX, regs->rax);
- _registers.setRegister(UNW_X86_64_RDX, regs->rdx);
- _registers.setRegister(UNW_X86_64_RCX, regs->rcx);
- _registers.setRegister(UNW_X86_64_RBX, regs->rbx);
- _registers.setRegister(UNW_X86_64_RSI, regs->rsi);
- _registers.setRegister(UNW_X86_64_RDI, regs->rdi);
- _registers.setRegister(UNW_X86_64_RBP, regs->rbp);
- _registers.setRegister(UNW_X86_64_R8, regs->r8);
- _registers.setRegister(UNW_X86_64_R9, regs->r9);
- _registers.setRegister(UNW_X86_64_R10, regs->r10);
- _registers.setRegister(UNW_X86_64_R11, regs->r11);
- _registers.setRegister(UNW_X86_64_R12, regs->r12);
- _registers.setRegister(UNW_X86_64_R13, regs->r13);
- _registers.setRegister(UNW_X86_64_R14, regs->r14);
- _registers.setRegister(UNW_X86_64_R15, regs->r15);
- // TODO: XMM
-#endif
-
- return UNW_STEP_SUCCESS;
-}
#endif // defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN) &&
// defined(_LIBUNWIND_TARGET_AARCH64)
@@ -3032,6 +2983,162 @@ int UnwindCursor<A, R>::stepThroughSigReturn(Registers_s390x &) {
#endif // defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN) &&
// defined(_LIBUNWIND_TARGET_S390X)
+#if defined(_LIBUNWIND_CHECK_HAIKU_SIGRETURN)
+
+#if defined(B_HAIKU_32_BIT)
+typedef Elf32_Sym elf_sym;
+#define ELF_ST_TYPE ELF32_ST_TYPE
+#elif defined(B_HAIKU_64_BIT)
+typedef Elf64_Sym elf_sym;
+#define ELF_ST_TYPE ELF64_ST_TYPE
+#endif
+
+// Private syscall declared as a weak symbol to prevent ABI breaks.
+extern "C" status_t __attribute__((weak))
+_kern_read_kernel_image_symbols(image_id id, elf_sym *symbolTable,
+ int32 *_symbolCount, char *stringTable,
+ size_t *_stringTableSize, addr_t *_imageDelta);
+
+static addr_t signalHandlerBegin = 0;
+static addr_t signalHandlerEnd = 0;
+
+template <typename A, typename R>
+bool UnwindCursor<A, R>::setInfoForSigReturn() {
+ if (signalHandlerBegin == 0) {
+ // If we do not have the addresses yet, find them now.
+
+ // Determine if the private function is there and usable.
+ if (_kern_read_kernel_image_symbols == nullptr) {
+ signalHandlerBegin = (addr_t)-1;
+ return false;
+ }
+
+ // Get the system commpage and enumerate its symbols.
+ image_id commpageImage = -1;
+ image_info info;
+ int32 cookie = 0;
+ while (get_next_image_info(B_SYSTEM_TEAM, &cookie, &info) == B_OK) {
+ if (strcmp(info.name, "commpage") == 0) {
+ commpageImage = info.id;
+ break;
+ }
+ }
+ if (commpageImage == -1) {
+ signalHandlerBegin = (addr_t)-1;
+ return false;
+ }
+
+ // Separate loop to get the process commpage,
+ // which is in a different address from the system commpage.
+ addr_t commpageAddress = -1;
+ cookie = 0;
+ while (get_next_image_info(B_CURRENT_TEAM, &cookie, &info) == B_OK) {
+ if (strcmp(info.name, "commpage") == 0) {
+ commpageAddress = (addr_t)info.text;
+ break;
+ }
+ }
+
+ // The signal handler function address is
+ // defined in the system commpage symbols.
+
+ // First call to get the memory required.
+ int32 symbolCount = 0;
+ size_t stringTableSize = 0;
+ if (_kern_read_kernel_image_symbols(commpageImage, nullptr, &symbolCount,
+ nullptr, &stringTableSize,
+ nullptr) < B_OK) {
+ signalHandlerBegin = (addr_t)-1;
+ return false;
+ }
+
+ size_t memorySize = symbolCount * sizeof(elf_sym) + stringTableSize + 1;
+ void *buffer = malloc(memorySize);
+ if (buffer == nullptr) {
+ // No more memory. This is a temporary failure, we can try again later.
+ return false;
+ }
+ memset(buffer, 0, memorySize);
+
+ elf_sym *symbols = (elf_sym *)buffer;
+ char *stringTable = (char *)buffer + symbolCount * sizeof(elf_sym);
+ if (_kern_read_kernel_image_symbols(commpageImage, symbols, &symbolCount,
+ stringTable, &stringTableSize,
+ nullptr) < B_OK) {
+ free(buffer);
+ signalHandlerBegin = (addr_t)-1;
+ return false;
+ }
+
+ for (int32 i = 0; i < symbolCount; ++i) {
+ char *name = stringTable + symbols[i].st_name;
+ if (strcmp(name, "commpage_signal_handler") == 0) {
+ signalHandlerBegin = commpageAddress + symbols[i].st_value;
+ signalHandlerEnd = signalHandlerBegin + symbols[i].st_size;
+ break;
+ }
+ }
+ free(buffer);
+
+ if (signalHandlerBegin == 0) {
+ signalHandlerBegin = (addr_t)-1;
+ return false;
+ }
+ } else if (signalHandlerBegin == (addr_t)-1) {
+ // We have found, and failed.
+ return false;
+ }
+ pint_t pc = static_cast<pint_t>(this->getReg(UNW_REG_IP));
+ if (pc >= (pint_t)signalHandlerBegin && pc < (pint_t)signalHandlerEnd) {
+ _info = {};
+ _info.start_ip = signalHandlerBegin;
+ _info.end_ip = signalHandlerEnd;
+ _isSigReturn = true;
+ return true;
+ }
+ return false;
+}
+
+template <typename A, typename R>
+int UnwindCursor<A, R>::stepThroughSigReturn() {
+ _isSignalFrame = true;
+
+#if defined(_LIBUNWIND_TARGET_X86_64)
+ // Layout of the stack before function call:
+ // - signal_frame_data
+ // + siginfo_t (public struct, fairly stable)
+ // + ucontext_t (public struct, fairly stable)
+ // - mcontext_t -> Offset 0x70, this is what we want.
+ // - frame->ip (8 bytes)
+ // - frame->bp (8 bytes). Not written by the kernel,
+ // but the signal handler has a "push %rbp" instruction.
+ pint_t bp = this->getReg(UNW_X86_64_RBP);
+ vregs *regs = (vregs *)(bp + 0x70);
+
+ _registers.setRegister(UNW_REG_IP, regs->rip);
+ _registers.setRegister(UNW_REG_SP, regs->rsp);
+ _registers.setRegister(UNW_X86_64_RAX, regs->rax);
+ _registers.setRegister(UNW_X86_64_RDX, regs->rdx);
+ _registers.setRegister(UNW_X86_64_RCX, regs->rcx);
+ _registers.setRegister(UNW_X86_64_RBX, regs->rbx);
+ _registers.setRegister(UNW_X86_64_RSI, regs->rsi);
+ _registers.setRegister(UNW_X86_64_RDI, regs->rdi);
+ _registers.setRegister(UNW_X86_64_RBP, regs->rbp);
+ _registers.setRegister(UNW_X86_64_R8, regs->r8);
+ _registers.setRegister(UNW_X86_64_R9, regs->r9);
+ _registers.setRegister(UNW_X86_64_R10, regs->r10);
+ _registers.setRegister(UNW_X86_64_R11, regs->r11);
+ _registers.setRegister(UNW_X86_64_R12, regs->r12);
+ _registers.setRegister(UNW_X86_64_R13, regs->r13);
+ _registers.setRegister(UNW_X86_64_R14, regs->r14);
+ _registers.setRegister(UNW_X86_64_R15, regs->r15);
+ // TODO: XMM
+#endif // defined(_LIBUNWIND_TARGET_X86_64)
+
+ return UNW_STEP_SUCCESS;
+}
+#endif // defined(_LIBUNWIND_CHECK_HAIKU_SIGRETURN)
+
template <typename A, typename R> int UnwindCursor<A, R>::step(bool stage2) {
(void)stage2;
// Bottom of stack is defined is when unwind info cannot be found.
More information about the cfe-commits
mailing list