Skip to content

[libunwind][X86-64] Handle Linux sigreturn trampoline when DWARF info is missing #103473

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

al13n321
Copy link

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 ClickHouse/libunwind#32

@al13n321 al13n321 requested a review from a team as a code owner August 13, 2024 21:26
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented Aug 13, 2024

@llvm/pr-subscribers-libunwind

Author: Michael Kolupaev (al13n321)

Changes

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 ClickHouse/libunwind#32


Full diff: https://github.com/llvm/llvm-project/pull/103473.diff

1 Files Affected:

  • (modified) libunwind/src/UnwindCursor.hpp (+95-1)
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 <errno.h>
 #include <signal.h>
 #include <sys/syscall.h>
@@ -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 <typename Registers> bool setInfoForSigReturn(Registers &) {
     return false;
@@ -2917,6 +2921,96 @@ int UnwindCursor<A, R>::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 <typename A, typename R>
+bool UnwindCursor<A, R>::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<pint_t>(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<const uint8_t *>(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 <typename A, typename R>
+int UnwindCursor<A, R>::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_SUCCESS;
+}
+#endif // defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN) &&
+       // defined(_LIBUNWIND_TARGET_X86_64)
+
 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.

Copy link

github-actions bot commented Aug 29, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff e21a1737f3523488a04169096fa27d0914a142a7 7cad11fb5e860f7b6cce3fd7b390275fa57cc990 --extensions hpp -- libunwind/src/UnwindCursor.hpp
View the diff from clang-format here.
diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index f077aafee7..1877e6f6e9 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -33,8 +33,7 @@
 #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_X86_64))
 #include <errno.h>
 #include <signal.h>
@@ -3110,7 +3109,7 @@ int UnwindCursor<A, R>::stepThroughSigReturn(Registers_x86_64 &) {
   //     ...
   //   };
   const size_t idx_map[17] = {13, 12, 14, 11, 9, 8, 10, 15, 0,
-                              1, 2, 3, 4, 5, 6, 7, 16};
+                              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);

@thevar1able
Copy link
Contributor

Ping

@DavidSpickett
Copy link
Collaborator

Can someone from @llvm/reviewers-libcxx help with the FreeBSD failure? Seems unrelated.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants