[libunwind] [libunwind][X86-64] Handle Linux sigreturn trampoline when DWARF info is missing (PR #103473)
https://github.com/al13n321 created
https://github.com/llvm/llvm-project/pull/103473
Do for X86-64 what libunwind already does for AArch64, RISC-V, and S390X. GDB
does this too.
Useful for musl libc, which doesn't have DWARF unwind info for `__restore_rt`
trampoline, so libunwind couldn't unwind out of signal handlers. Now it can.
Tested manually in https://github.com/ClickHouse/libunwind/pull/32
>From f3e4787973daf1cd278a9615960bbb4b31d3c0c0 Mon Sep 17 00:00:00 2001
From: Michael Kolupaev
Date: Tue, 13 Aug 2024 21:16:08 +
Subject: [PATCH] [libunwind][X86-64] Handle Linux sigreturn trampoline when
DWARF info is missing
---
libunwind/src/UnwindCursor.hpp | 96 +-
1 file changed, 95 insertions(+), 1 deletion(-)
diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index ce6dced535e781..02cbd9891e4ff5 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -32,7 +32,7 @@
#if defined(_LIBUNWIND_TARGET_LINUX) &&
\
(defined(_LIBUNWIND_TARGET_AARCH64) || defined(_LIBUNWIND_TARGET_RISCV) ||
\
- defined(_LIBUNWIND_TARGET_S390X))
+ defined(_LIBUNWIND_TARGET_S390X) || defined(_LIBUNWIND_TARGET_X86_64))
#include
#include
#include
@@ -1003,6 +1003,10 @@ class UnwindCursor : public AbstractUnwindCursor{
#if defined(_LIBUNWIND_TARGET_S390X)
bool setInfoForSigReturn(Registers_s390x &);
int stepThroughSigReturn(Registers_s390x &);
+#endif
+#if defined(_LIBUNWIND_TARGET_X86_64)
+ bool setInfoForSigReturn(Registers_x86_64 &);
+ int stepThroughSigReturn(Registers_x86_64 &);
#endif
template bool setInfoForSigReturn(Registers &) {
return false;
@@ -2917,6 +2921,96 @@ int UnwindCursor::stepThroughSigReturn(Registers_s390x &) {
#endif // defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN) &&
// defined(_LIBUNWIND_TARGET_S390X)
+#if defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN) &&
\
+defined(_LIBUNWIND_TARGET_X86_64)
+template
+bool UnwindCursor::setInfoForSigReturn(Registers_x86_64 &) {
+ // Look for the sigreturn trampoline. The trampoline's body is two
+ // specific instructions (see below). Typically the trampoline comes from the
+ // vDSO or from libc.
+ //
+ // This special code path is a fallback that is only used if the trampoline
+ // lacks proper (e.g. DWARF) unwind info.
+ const uint8_t amd64_linux_sigtramp_code[9] = {
+0x48, 0xc7, 0xc0, 0x0f, 0x00, 0x00, 0x00, // mov rax, 15
+0x0f, 0x05// syscall
+ };
+ const size_t code_size = sizeof(amd64_linux_sigtramp_code);
+
+ // The PC might contain an invalid address if the unwind info is bad, so
+ // directly accessing it could cause a SIGSEGV.
+ unw_word_t pc = static_cast(this->getReg(UNW_REG_IP));
+ if (!isReadableAddr(pc))
+return false;
+ // If near page boundary, check the next page too.
+ if (((pc + code_size - 1) & 4095) != (pc & 4095) && !isReadableAddr(pc +
code_size - 1))
+return false;
+
+ const uint8_t *pc_ptr = reinterpret_cast(pc);
+ if (memcmp(pc_ptr, amd64_linux_sigtramp_code, code_size))
+return false;
+
+ _info = {};
+ _info.start_ip = pc;
+ _info.end_ip = pc + code_size;
+ _isSigReturn = true;
+ return true;
+}
+
+template
+int UnwindCursor::stepThroughSigReturn(Registers_x86_64 &) {
+ // In the signal trampoline frame, sp points to ucontext:
+ // struct ucontext {
+ // unsigned long uc_flags;
+ // struct ucontext *uc_link;
+ // stack_t uc_stack; // 24 bytes
+ // struct sigcontext uc_mcontext;
+ // ...
+ // };
+ const pint_t kOffsetSpToSigcontext = (8 + 8 + 24);
+ pint_t sigctx = _registers.getSP() + kOffsetSpToSigcontext;
+
+ // UNW_X86_64_* -> field in struct sigcontext_64.
+ // struct sigcontext_64 {
+ // __u64 r8; // 0
+ // __u64 r9; // 1
+ // __u64 r10; // 2
+ // __u64 r11; // 3
+ // __u64 r12; // 4
+ // __u64 r13; // 5
+ // __u64 r14; // 6
+ // __u64 r15; // 7
+ // __u64 di; // 8
+ // __u64 si; // 9
+ // __u64 bp; // 10
+ // __u64 bx; // 11
+ // __u64 dx; // 12
+ // __u64 ax; // 13
+ // __u64 cx; // 14
+ // __u64 sp; // 15
+ // __u64 ip; // 16
+ // ...
+ // };
+ const size_t idx_map[17] = {13, 12, 14, 11, 9, 8, 10, 15, 0, 1, 2, 3, 4, 5,
6, 7, 16};
+
+ for (int i = 0; i < 17; ++i) {
+uint64_t value = _addressSpace.get64(sigctx + idx_map[i] * 8);
+_registers.setRegister(i, value);
+ }
+
+ // The +1 story is the same as in DwarfInstructions::stepWithDwarf()
+ // (search for "returnAddress + cieInfo.isSignalFrame" or "Return address
points to the next instruction").
+ // This is probably not the right place for this because this function is
not necessarily used
+ // with DWARF. Need to research whether the other unwind methods have the
same +-1 situation or
+ // are off by one.
+ _registers.setI
[libunwind] [libunwind] Detect cycles of length 1 (PR #103476)
https://github.com/al13n321 created
https://github.com/llvm/llvm-project/pull/103476
If an unwind step leaves both the instruction pointer and the stack pointer
unchanged, stop unwinding. Otherwise we'd be stuck in a loop producing the same
stack frame over and over.
This happens with musl libc in __clone(), which has incorrect DWARF unwind
information.
GDB also handles it using cycle detection, but it checks for cycles of any
length rather than just length 1. That seems unnecessary here, at least until
anyone encounters such cycles in practice.
Tested manually in https://github.com/ClickHouse/libunwind/pull/33
>From cdac646034cf9f7bf6326bcd28a3626db8650ed7 Mon Sep 17 00:00:00 2001
From: Michael Kolupaev
Date: Tue, 13 Aug 2024 21:31:30 +
Subject: [PATCH] [libunwind] Detect cycles of length 1
---
libunwind/src/UnwindCursor.hpp | 11 +++
1 file changed, 11 insertions(+)
diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index ce6dced535e781..b6c51446cb7d47 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -2923,6 +2923,9 @@ template int UnwindCursor::step(bool stage2) {
if (_unwindInfoMissing)
return UNW_STEP_END;
+ unw_word_t previousIP = getReg(UNW_REG_IP);
+ unw_word_t previousSP = getReg(UNW_REG_SP);
+
// Use unwinding info to modify register set as if function returned.
int result;
#if defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN)
@@ -2951,6 +2954,14 @@ template int UnwindCursor::step(bool stage2) {
// update info based on new PC
if (result == UNW_STEP_SUCCESS) {
+// Detect cycles of length 1. In particular this happens in musl's
+// __clone(), which has incorrect DWARF unwind information.
+// We don't check all registers, so it's not strictly guaranteed that
+// unwinding would be stuck in a cycle, but seems like a reasonable
+// heuristic.
+if (getReg(UNW_REG_SP) == previousSP && getReg(UNW_REG_IP) == previousIP)
+ return UNW_EBADFRAME;
+
this->setInfoBasedOnIPRegister(true);
if (_unwindInfoMissing)
return UNW_STEP_END;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libunwind] Detect cycles of length 1 (PR #103476)
al13n321 wrote: The `clone` syscall, in newly created thread, adjusts stack pointer and stack contents, and there's logically no "caller" stack frame and no return address on the stack. `tools/add-cfi.x86_64.awk` doesn't know about any of that and autogenerates cfi as if it's just normal linear code and the syscall does nothing special. Below the syscall the cfi is just all incorrect, at least from child thread's point of view. It just happens to have the property that ip and sp are left unchanged by the unwind step. (I don't remember the details, but it's something like: the `clone` syscall puts the entry function pointer somewhere near the top of the stack, and the cfi happens to point to that offset from sp despite having incorrect cfa.) A principled fix would be to add manual cfi annotations in `musl/src/thread/x86_64/clone.s`. But I figured that since (1) gdb does cycle detection, and (2) it's still not fixed in musl; then this is considered acceptable, and maybe even intentional. (And, IIUC, musl people generally don't want programs to be able to unwind their own stack for debugging purposes. If I send a fix to musl, I imagine the answer may be "gdb already works, libunwind already doesn't work, and that's exactly how we want it", but may be worth a try.) https://github.com/llvm/llvm-project/pull/103476 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libunwind] Detect cycles of length 1 (PR #103476)
al13n321 wrote: Feel free to close this PR if such change is not going to be accepted. https://github.com/llvm/llvm-project/pull/103476 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] [libunwind][X86-64] Handle Linux sigreturn trampoline when DWARF info is missing (PR #103473)
https://github.com/al13n321 updated
https://github.com/llvm/llvm-project/pull/103473
>From 3beb6fc40ccc0b5726b360156063ef35e2b1db3f Mon Sep 17 00:00:00 2001
From: Michael Kolupaev
Date: Tue, 13 Aug 2024 21:16:08 +
Subject: [PATCH] [libunwind][X86-64] Handle Linux sigreturn trampoline when
DWARF info is missing
---
libunwind/src/UnwindCursor.hpp | 100 -
1 file changed, 99 insertions(+), 1 deletion(-)
diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index ca9927edc9990..b4ddcd2792bf2 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -33,7 +33,8 @@
#if defined(_LIBUNWIND_TARGET_LINUX) &&
\
(defined(_LIBUNWIND_TARGET_AARCH64) ||
\
defined(_LIBUNWIND_TARGET_LOONGARCH) ||
\
- defined(_LIBUNWIND_TARGET_RISCV) || defined(_LIBUNWIND_TARGET_S390X))
+ defined(_LIBUNWIND_TARGET_RISCV) || defined(_LIBUNWIND_TARGET_S390X) ||
\
+ defined(_LIBUNWIND_TARGET_S390X) || defined(_LIBUNWIND_TARGET_X86_64))
#include
#include
#include
@@ -1008,6 +1009,10 @@ class UnwindCursor : public AbstractUnwindCursor{
#if defined(_LIBUNWIND_TARGET_S390X)
bool setInfoForSigReturn(Registers_s390x &);
int stepThroughSigReturn(Registers_s390x &);
+#endif
+#if defined(_LIBUNWIND_TARGET_X86_64)
+ bool setInfoForSigReturn(Registers_x86_64 &);
+ int stepThroughSigReturn(Registers_x86_64 &);
#endif
template bool setInfoForSigReturn(Registers &) {
return false;
@@ -3032,6 +3037,99 @@ int UnwindCursor::stepThroughSigReturn(Registers_s390x &) {
#endif // defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN) &&
// defined(_LIBUNWIND_TARGET_S390X)
+#if defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN) &&
\
+defined(_LIBUNWIND_TARGET_X86_64)
+template
+bool UnwindCursor::setInfoForSigReturn(Registers_x86_64 &) {
+ // Look for the sigreturn trampoline. The trampoline's body is two
+ // specific instructions (see below). Typically the trampoline comes from the
+ // vDSO or from libc.
+ //
+ // This special code path is a fallback that is only used if the trampoline
+ // lacks proper (e.g. DWARF) unwind info.
+ const uint8_t amd64_linux_sigtramp_code[9] = {
+ 0x48, 0xc7, 0xc0, 0x0f, 0x00, 0x00, 0x00, // mov rax, 15
+ 0x0f, 0x05// syscall
+ };
+ const size_t code_size = sizeof(amd64_linux_sigtramp_code);
+
+ // The PC might contain an invalid address if the unwind info is bad, so
+ // directly accessing it could cause a SIGSEGV.
+ unw_word_t pc = static_cast(this->getReg(UNW_REG_IP));
+ if (!isReadableAddr(pc))
+return false;
+ // If near page boundary, check the next page too.
+ if (((pc + code_size - 1) & 4095) != (pc & 4095) &&
+ !isReadableAddr(pc + code_size - 1))
+return false;
+
+ const uint8_t *pc_ptr = reinterpret_cast(pc);
+ if (memcmp(pc_ptr, amd64_linux_sigtramp_code, code_size))
+return false;
+
+ _info = {};
+ _info.start_ip = pc;
+ _info.end_ip = pc + code_size;
+ _isSigReturn = true;
+ return true;
+}
+
+template
+int UnwindCursor::stepThroughSigReturn(Registers_x86_64 &) {
+ // In the signal trampoline frame, sp points to ucontext:
+ // struct ucontext {
+ // unsigned long uc_flags;
+ // struct ucontext *uc_link;
+ // stack_t uc_stack; // 24 bytes
+ // struct sigcontext uc_mcontext;
+ // ...
+ // };
+ const pint_t kOffsetSpToSigcontext = (8 + 8 + 24);
+ pint_t sigctx = _registers.getSP() + kOffsetSpToSigcontext;
+
+ // UNW_X86_64_* -> field in struct sigcontext_64.
+ // struct sigcontext_64 {
+ // __u64 r8; // 0
+ // __u64 r9; // 1
+ // __u64 r10; // 2
+ // __u64 r11; // 3
+ // __u64 r12; // 4
+ // __u64 r13; // 5
+ // __u64 r14; // 6
+ // __u64 r15; // 7
+ // __u64 di; // 8
+ // __u64 si; // 9
+ // __u64 bp; // 10
+ // __u64 bx; // 11
+ // __u64 dx; // 12
+ // __u64 ax; // 13
+ // __u64 cx; // 14
+ // __u64 sp; // 15
+ // __u64 ip; // 16
+ // ...
+ // };
+ const size_t idx_map[17] = {13, 12, 14, 11, 9, 8, 10, 15, 0,
+ 1, 2, 3, 4, 5, 6, 7, 16};
+
+ for (int i = 0; i < 17; ++i) {
+uint64_t value = _addressSpace.get64(sigctx + idx_map[i] * 8);
+_registers.setRegister(i, value);
+ }
+
+ // The +1 story is the same as in DwarfInstructions::stepWithDwarf()
+ // (search for "returnAddress + cieInfo.isSignalFrame" or "Return address
+ // points to the next instruction"). This is probably not the right place for
+ // this because this function is not necessarily used with DWARF. Need to
+ // research whether the other unwind methods have the same +-1 situation or
+ // are off by one.
+ _registers.setIP(_registers.getIP() + 1);
+
+ _isSignalFrame = true;
+ return UNW_STEP_SUCCE
