[libunwind] Fix signal frame unwinding (PR #135367)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 11 06:34:39 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libunwind
Author: Trung Nguyen (trungnt2910)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/135367.diff
2 Files Affected:
- (modified) libunwind/src/CMakeLists.txt (-16)
- (modified) libunwind/src/UnwindCursor.hpp (+163-60)
``````````diff
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..fc1f8e91724b2 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 <elf.h>
+#include <image.h>
+#include <signal.h>
+#include <OS.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,158 @@ 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.
``````````
</details>
https://github.com/llvm/llvm-project/pull/135367
More information about the cfe-commits
mailing list