i#2399 libunwind: Use drcallstack for better Linux callstacks (#2431)

Updates DR to 53af6c7 for the new drcallstack library. Adds a new option -callstack_use_unwind which is on by default for Linux. This uses drcallstack's libunwind-based callstack walk, which fixes problems of missing frames. If the starting PC is not in a module, the old callstack walking is used. Updates malloc replacement contexts to include the PC as of the same point as the captured stack pointer, for proper libunwind input. Issue: #823, #2399, #2392, #1222 Fixes #2392
DynamoRIO · Oct 18, 2021 · a0cf46b · a0cf46b
1 parent 5d12994
commit a0cf46b
Show file tree

Hide file tree

Showing 18 changed files with 265 additions and 22 deletions.
diff --git a/.github/workflows/ci-aarchxx.yml b/.github/workflows/ci-aarchxx.yml
@@ -58,11 +58,21 @@ jobs:
 
     # Install cross-compilers for cross-compiling Linux build.
     # We are not yet testing aarch64 but we will be soon.
+    # Unfortunately there is no libunwind-dev or libunwind cross-compile
+    # package so we unpack the native versions and copy their files.
     - name: Create Build Environment
       run: |
         sudo apt-get update
         sudo apt-get -y install doxygen jsonlint \
           g++-arm-linux-gnueabihf g++-aarch64-linux-gnu
+        sudo add-apt-repository 'deb [arch=armhf] http://ports.ubuntu.com/ubuntu-ports focal main'
+        mkdir ../extract
+        pushd ../extract
+        apt download libunwind8:armhf libunwind-dev:armhf liblzma5:armhf
+        for i in *.deb; do dpkg-deb -x $i .; done
+        for i in include lib; do sudo rsync -av ./usr/${i}/arm-linux-gnueabihf/ /usr/arm-linux-gnueabihf/${i}/; done
+        sudo rsync -av ./lib/arm-linux-gnueabihf/ /usr/arm-linux-gnueabihf/lib/
+        popd
 
     - name: Run Suite
       working-directory: ${{ github.workspace }}

diff --git a/.github/workflows/ci-clang.yml b/.github/workflows/ci-clang.yml
@@ -56,11 +56,23 @@ jobs:
     - name: Fetch Sources
       run: git fetch --no-tags --depth=1 origin master
 
-    # Install multilib for non-cross-compiling Linux build:
+    # Install multilib for non-cross-compiling Linux build.
+    # GA CI uses packages.microsoft.com which is missing i386 packages, and
+    # attempts at using apt with us.archive-ubuntu.com hit dep issues:
+    # so we manually install the libunwind i386 packages we need.
     - name: Create Build Environment
       run: |
         sudo apt-get update
-        sudo apt-get -y install doxygen jsonlint g++-multilib
+        sudo apt-get -y install doxygen jsonlint g++-multilib libunwind-dev
+        sudo add-apt-repository 'deb [arch=i386] http://us.archive.ubuntu.com/ubuntu focal main'
+        mkdir ../extract
+        pushd ../extract
+        apt download libunwind8:i386 libunwind-dev:i386 liblzma5:i386
+        for i in *.deb; do dpkg-deb -x $i .; done
+        sudo rsync -av ./usr/lib/i386-linux-gnu/ /lib32/
+        sudo rsync -av ./lib/i386-linux-gnu/ /lib32/
+        sudo rsync -av ./usr/include/i386-linux-gnu/ /usr/include/
+        popd
 
     # Downgrade from cmake 3.20 to avoid 32-bit toolchain problems (DRi#4830).
     - name: Downgrade cmake

diff --git a/.github/workflows/ci-package.yml b/.github/workflows/ci-package.yml
@@ -55,11 +55,23 @@ jobs:
     - name: Fetch Sources
       run: git fetch --no-tags --depth=1 origin master
 
-    # Install multilib for non-cross-compiling Linux build:
+    # Install multilib for non-cross-compiling Linux build.
+    # GA CI uses packages.microsoft.com which is missing i386 packages, and
+    # attempts at using apt with us.archive-ubuntu.com hit dep issues:
+    # so we manually install the libunwind i386 packages we need.
     - name: Create Build Environment
       run: |
         sudo apt-get update
-        sudo apt-get -y install doxygen jsonlint g++-multilib
+        sudo apt-get -y install doxygen jsonlint g++-multilib libunwind-dev
+        sudo add-apt-repository 'deb [arch=i386] http://us.archive.ubuntu.com/ubuntu focal main'
+        mkdir ../extract
+        pushd ../extract
+        apt download libunwind8:i386 libunwind-dev:i386 liblzma5:i386
+        for i in *.deb; do dpkg-deb -x $i .; done
+        sudo rsync -av ./usr/lib/i386-linux-gnu/ /lib32/
+        sudo rsync -av ./lib/i386-linux-gnu/ /lib32/
+        sudo rsync -av ./usr/include/i386-linux-gnu/ /usr/include/
+        popd
 
     # Downgrade from cmake 3.20 to avoid 32-bit toolchain problems (DRi#4830).
     - name: Downgrade cmake

diff --git a/.github/workflows/ci-x86.yml b/.github/workflows/ci-x86.yml
@@ -61,11 +61,23 @@ jobs:
     - name: Fetch Sources
       run: git fetch --no-tags --depth=1 origin master
 
-    # Install multilib for non-cross-compiling Linux build:
+    # Install multilib for non-cross-compiling Linux build.
+    # GA CI uses packages.microsoft.com which is missing i386 packages, and
+    # attempts at using apt with us.archive-ubuntu.com hit dep issues:
+    # so we manually install the libunwind i386 packages we need.
     - name: Create Build Environment
       run: |
         sudo apt-get update
-        sudo apt-get -y install doxygen jsonlint g++-multilib
+        sudo apt-get -y install doxygen jsonlint g++-multilib libunwind-dev
+        sudo add-apt-repository 'deb [arch=i386] http://us.archive.ubuntu.com/ubuntu focal main'
+        mkdir ../extract
+        pushd ../extract
+        apt download libunwind8:i386 libunwind-dev:i386 liblzma5:i386
+        for i in *.deb; do dpkg-deb -x $i .; done
+        sudo rsync -av ./usr/lib/i386-linux-gnu/ /lib32/
+        sudo rsync -av ./lib/i386-linux-gnu/ /lib32/
+        sudo rsync -av ./usr/include/i386-linux-gnu/ /usr/include/
+        popd
 
     # Downgrade from cmake 3.20 to avoid 32-bit toolchain problems (DRi#4830).
     - name: Downgrade cmake

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -404,6 +404,14 @@ if (UNIX)
     # see comments above about adding configure.h
     set(DEFINES ${DEFINES} -DHAVE_ASM_I386)
   endif (HAVE_ASM_I386)
+  if (LINUX)
+    check_include_files("libunwind.h" HAVE_LIBUNWIND_H)
+    if (HAVE_LIBUNWIND_H)
+      set(DEFINES ${DEFINES} -DHAVE_LIBUNWIND_H)
+    endif ()
+  else ()
+    set(HAVE_LIBUNWIND_H OFF)
+  endif ()
 endif (UNIX)
 
 if (UNIX)
@@ -1294,6 +1302,9 @@ use_DynamoRIO_extension(${client_target} drx_static)
 use_DynamoRIO_extension(${client_target} drutil_static)
 use_DynamoRIO_extension(${client_target} drwrap_static)
 use_DynamoRIO_extension(${client_target} drreg_static)
+if (HAVE_LIBUNWIND_H)
+  use_DynamoRIO_extension(${client_target} drcallstack_static)
+endif ()
 # DRi#1829: we discussed invoking drcov as a separate client, which would require using
 # all shared ext libs.  We ended up making drcovlib.
 use_DynamoRIO_extension(${client_target} drcovlib_static)
@@ -1865,6 +1876,9 @@ if (BUILD_TOOL_TESTS)
     use_DynamoRIO_extension(unit_tests drutil_static)
     use_DynamoRIO_extension(unit_tests drwrap_static)
     use_DynamoRIO_extension(unit_tests drsyms_static)
+    if (HAVE_LIBUNWIND_H)
+      use_DynamoRIO_extension(unit_tests drcallstack_static)
+    endif ()
     use_DynamoRIO_extension(unit_tests drcovlib_static)
     target_link_libraries(unit_tests drsyscall_int)
     target_link_libraries(unit_tests umbra_int)

diff --git a/common/alloc_replace.c b/common/alloc_replace.c
@@ -640,11 +640,11 @@ get_replace_native_caller(void *drcontext)
 #endif
 
 /* This must be inlined to get an xsp that's in the call chain */
-#define INITIALIZE_MCONTEXT_FOR_REPORT(mc) do {            \
-    /* assumption: we only need xsp and xbp initialized */ \
-    (mc)->size = sizeof(*(mc));                            \
-    (mc)->flags = DR_MC_CONTROL | DR_MC_INTEGER;           \
-    get_stack_registers(&MC_SP_REG(mc), &MC_FP_REG(mc)); \
+#define INITIALIZE_MCONTEXT_FOR_REPORT(mc) do {                        \
+    /* Assumption: we only need xsp, xbp, and pc initialized. */       \
+    (mc)->size = sizeof(*(mc));                                        \
+    (mc)->flags = DR_MC_CONTROL | DR_MC_INTEGER;                       \
+    get_unwind_registers(&MC_SP_REG(mc), &MC_FP_REG(mc), &((mc)->pc)); \
 } while (0)
 
 #ifdef WINDOWS

diff --git a/common/asm_utils.h b/common/asm_utils.h
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2010-2013 Google, Inc.  All rights reserved.
+ * Copyright (c) 2010-2021 Google, Inc.  All rights reserved.
  * Copyright (c) 2007-2010 VMware, Inc.  All rights reserved.
  * **********************************************************/
 
@@ -27,6 +27,10 @@
 void
 get_stack_registers(reg_t *xsp OUT, reg_t *xbp OUT);
 
+/* Returns the current values of xsp and xbp */
+void
+get_unwind_registers(reg_t *xsp OUT, reg_t *xbp OUT, app_pc *xip OUT);
+
 #ifdef UNIX
 ptr_int_t
 raw_syscall(uint sysnum, uint num_args, ...);

diff --git a/common/asm_utils_arm.asm b/common/asm_utils_arm.asm
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2012-2015 Google, Inc.  All rights reserved.
+ * Copyright (c) 2012-2021 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /* Dr. Memory: the memory debugger
@@ -70,6 +70,18 @@ GLOBAL_LABEL(FUNCNAME:)
 #undef FUNCNAME
 
 
+/* void get_unwind_registers(reg_t *sp OUT, reg_t *fp OUT, app_pc *pc OUT) */
+#define FUNCNAME get_unwind_registers
+        DECLARE_FUNC(FUNCNAME)
+GLOBAL_LABEL(FUNCNAME:)
+        str      sp, [REG_R0]
+        str      fp, [REG_R1]
+        str      lr, [REG_R2]
+        bx       lr
+        END_FUNC(FUNCNAME)
+#undef FUNCNAME
+
+
 #ifdef LINUX
 /* Straight from DynamoRIO.
  * Signature: raw_syscall(sysnum, num_args, arg1, arg2, ...)

diff --git a/common/asm_utils_x86.asm b/common/asm_utils_x86.asm
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2012-2014 Google, Inc.  All rights reserved.
+ * Copyright (c) 2012-2021 Google, Inc.  All rights reserved.
  * **********************************************************/
 
 /* Dr. Memory: the memory debugger
@@ -89,6 +89,36 @@ GLOBAL_LABEL(FUNCNAME:)
 #undef FUNCNAME
 
 
+/* void get_unwind_registers(reg_t *xsp OUT, reg_t *xbp OUT, app_pc *xip OUT);
+ *
+ * Returns the caller's values of xsp and xbp and xip.
+ */
+#define FUNCNAME get_unwind_registers
+        DECLARE_FUNC(FUNCNAME)
+GLOBAL_LABEL(FUNCNAME:)
+        /* we don't bother to be SEH64 compliant */
+        mov      REG_XAX, ARG1
+        mov      REG_XCX, ARG2
+        mov      PTRSZ [REG_XCX], REG_XBP
+        mov      REG_XCX, ARG3
+        mov      REG_XDX, PTRSZ [REG_XSP]
+        mov      PTRSZ [REG_XCX], REG_XDX
+        /* The caller may not use push to call us: it may allocate its
+         * max frame up front.  However, all current uses are from functions
+         * with large frames themselves, so our removals here are not a problem.
+         */
+#if defined(X64) && defined(WINDOWS)
+        add      REG_XCX, 32 + ARG_SZ   /* remove frame space + retaddr */
+#elif !defined(X64)
+        add      REG_XCX, 3 * ARG_SZ    /* remove args + retaddr */
+
+#endif
+        mov      PTRSZ [REG_XAX], REG_XCX
+        ret
+        END_FUNC(FUNCNAME)
+#undef FUNCNAME
+
+
 #ifdef LINUX
 /* Straight from DynamoRIO as a faster fix than continually tweaking the
  * C routines raw_syscall_N_args() since i#199 has complex issues and is