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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
