[libunwind][Haiku] Fix signal frame unwinding #135367
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libunwind Author: Trung Nguyen (trungnt2910) ChangesThe current unwinding implementation on Haiku is messy and broken.
This commit strips all references to private headers and ports a working signal frame implementation. It has been tested against Full diff: https://github.com/llvm/llvm-project/pull/135367.diff 2 Files Affected:
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.
|
|
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
trungnt2910
force-pushed
the
dev/trungnt2910/libunwind
branch
5 times, most recently
from
06e16ff to
116b60a
Compare
brad0
changed the title
trungnt2910
force-pushed
the
dev/trungnt2910/libunwind
branch
from
116b60a to
bc84623
Compare
trungnt2910
changed the title
|
Apologize for the confusing naming, somehow the tag prefixes were missing when I applied the patch from my test repo to the LLVM monorepo. |
X547
reviewed
Apr 12, 2025
|
Tested on x86_64? |
|
Yes, tested on This is also the same implementation used in my debugger ports for Haiku. |
Does it build on r1beta5? |
Theoretically yes, since I did not require any new Haiku nightly feature for the implementation. The unwinding might not fully work due to missing CFE information in |
trungnt2910
force-pushed
the
dev/trungnt2910/libunwind
branch
from
bc84623 to
0ce9ca6
Compare
|
Submitted a new implementation that uses Haiku's recent special handling of the This removes the need for both private headers and re-declarations of private symbols. |
trungnt2910
force-pushed
the
dev/trungnt2910/libunwind
branch
from
0ce9ca6 to
755d699
Compare
trungnt2910
force-pushed
the
dev/trungnt2910/libunwind
branch
from
755d699 to
464d992
Compare
|
cc @MaskRay |
|
@trungnt2910 It would be good to also fix i386 while you're at it. |
|
You mean adding support for From what I understand about the linked issue, x86 is failing due to some signal frame unwinding function prototypes got leaked to general Haiku code, This PR guards both under |
Yes.
I wasn't sure if that was the case or not. That's what it looks like, but I wasn't sure if it built on i386 and whether it has been build tested to ensure that is the case. At least building is still a step forward from not being able to build, which AFAIK, that is the case out of the box on i386 at the moment. |
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.
trungnt2910
force-pushed
the
dev/trungnt2910/libunwind
branch
from
464d992 to
7d36309
Compare
|
The latest commit is known to build well on |
|
Yes, that is fine. Thanks. |
|
cc @MaskRay |
|
Tried to give x86 unwinding a shot. The general idea is correct, but it is currently impossible to unwind past a |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/38/builds/3653 Here is the relevant piece of the build log for the reference |
svkeerthy
pushed a commit
that referenced
this pull request
May 29, 2025
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.
google-yfyang
pushed a commit
to google-yfyang/llvm-project
that referenced
this pull request
May 29, 2025
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.
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current unwinding implementation on Haiku is messy and broken.
This commit strips all references to private headers and ports a working signal frame implementation. It has been tested against
tests/signal_unwind.pass.cppand can go pass the signal frame.