commit f86dd185939a0a81dbbaea4f26d95f299d26811b
author David Benjamin <davidben@google.com> Tue Aug 29 16:51:53 2023 -0400
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Aug 30 15:53:09 2023 +0000
tree a8fbeaf79c3fb95f79d7903ae9f05b7607c14022
parent e3d9b69e8c6f6b78120006282c020c71803a8075

Initialize libunwind cursors from the signal handler's ucontext_t

unw_context_t is, at least on x86_64, the same type as ucontext_t.
Passing that into unw_init_local doesn't work, but there's a
unw_init_local2 in libunwind 1.3.0 or later, which has a flag for this
case.

This avoids needing to unwind past the signal handler stack frames,
which is both simpler and faster. (Shaved around 10 seconds off running
all the unwind tests on my machine.)

Change-Id: I09c130e76682d63e51b7b9de9ff5b91415e26f32
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/62867
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>

CMakeLists.txt

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1529526..78f9b59 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -56,7 +56,7 @@
 if(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND NOT CMAKE_CROSSCOMPILING)
   find_package(PkgConfig QUIET)
   if (PkgConfig_FOUND)
-    pkg_check_modules(LIBUNWIND libunwind-generic)
+    pkg_check_modules(LIBUNWIND libunwind-generic>=1.3.0)
     if(LIBUNWIND_FOUND)
       add_definitions(-DBORINGSSL_HAVE_LIBUNWIND)
     else()

crypto/test/abi_test.cc

diff --git a/crypto/test/abi_test.cc b/crypto/test/abi_test.cc
index 2338258..7c84d88 100644
--- a/crypto/test/abi_test.cc
+++ b/crypto/test/abi_test.cc
@@ -302,7 +302,7 @@
 class UnwindCursor {
  public:
   explicit UnwindCursor(unw_context_t *ctx) : ctx_(ctx) {
-    int ret = InitAtSignalFrame(&cursor_);
+    int ret = unw_init_local2(&cursor_, ctx_, UNW_INIT_SIGNAL_FRAME);
     if (ret < 0) {
       FatalError("Error getting unwind context: ", unw_strerror(ret));
     }
@@ -367,7 +367,7 @@
     // constructor.
     unw_cursor_t cursor;
     unw_word_t off;
-    if (InitAtSignalFrame(&cursor) != 0 ||
+    if (unw_init_local2(&cursor, ctx_, UNW_INIT_SIGNAL_FRAME) != 0 ||
         unw_get_proc_name(&cursor, starting_ip_buf_, sizeof(starting_ip_buf_),
                           &off) != 0) {
       StrCatSignalSafe(starting_ip_buf_, "0x", WordToHex(starting_ip_).data());
@@ -389,30 +389,6 @@
     return UnwindStatus(msg == nullptr ? "unknown error" : msg);
   }
 
-  int InitAtSignalFrame(unw_cursor_t *cursor) {
-    // Work around a bug in libunwind which breaks rax and rdx recovery. This
-    // breaks functions which temporarily use rax as the CFA register. See
-    // https://git.savannah.gnu.org/gitweb/?p=libunwind.git;a=commit;h=819bf51bbd2da462c2ec3401e8ac9153b6e725e3
-    OPENSSL_memset(cursor, 0, sizeof(*cursor));
-    int ret = unw_init_local(cursor, ctx_);
-    if (ret < 0) {
-      return ret;
-    }
-    for (;;) {
-      ret = unw_is_signal_frame(cursor);
-      if (ret < 0) {
-        return ret;
-      }
-      if (ret != 0) {
-        return 0;  // Found the signal frame.
-      }
-      ret = unw_step(cursor);
-      if (ret < 0) {
-        return ret;
-      }
-    }
-  }
-
   int GetReg(crypto_word_t *out, unw_regnum_t reg) {
     unw_word_t val;
     int ret = unw_get_reg(&cursor_, reg, &val);
@@ -694,10 +670,11 @@
 
 static pthread_t g_main_thread;
 
-static void TrapHandler(int sig) {
+static void TrapHandler(int sig, siginfo_t *info, void *ucontext_v) {
   // Note this is a signal handler, so only async-signal-safe functions may be
   // used here. See signal-safety(7). libunwind promises local unwind is
   // async-signal-safe.
+  ucontext_t *ucontext = static_cast<ucontext_t*>(ucontext_v);
 
   // |pthread_equal| is not listed as async-signal-safe, but this is clearly an
   // oversight.
@@ -705,13 +682,7 @@
     FatalError("SIGTRAP on background thread");
   }
 
-  unw_context_t ctx;
-  int ret = unw_getcontext(&ctx);
-  if (ret < 0) {
-    FatalError("Error getting unwind context: ", unw_strerror(ret));
-  }
-
-  UnwindCursor cursor(&ctx);
+  UnwindCursor cursor(ucontext);
   CheckUnwind(&cursor);
 }
 
@@ -727,7 +698,8 @@
   struct sigaction trap_action;
   OPENSSL_memset(&trap_action, 0, sizeof(trap_action));
   sigemptyset(&trap_action.sa_mask);
-  trap_action.sa_handler = TrapHandler;
+  trap_action.sa_flags = SA_SIGINFO;
+  trap_action.sa_sigaction = TrapHandler;
   if (sigaction(SIGTRAP, &trap_action, NULL) != 0) {
     perror("sigaction");
     abort();