Use libunwind instead of libbacktrace to build stack traces. This fixes a segfault issue with platforms using libexecinfo and is generally more portable.

smessmer · smessmer · commit 807f0dc01b5f · 2019-03-25T09:29:58.000-07:00
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -86,6 +86,50 @@ references:
         cmake --version
         /usr/local/bin/$CC --version
         /usr/local/bin/$CXX --version
+  upgrade_libunwind_pre: &upgrade_libunwind_pre
+    restore_cache:
+      keys:
+        # Find the most recent cache from any branch
+        - v1_upgrade_libunwind_cache_{{ checksum "/tmp/_build_env_vars" }}_{{ arch }}
+  upgrade_libunwind_post: &upgrade_libunwind_post
+    save_cache:
+      key: v1_upgrade_libunwind_cache_{{ checksum "/tmp/_build_env_vars" }}_{{ arch }}
+      paths:
+        - /tmp/libunwind-1.2.1
+  upgrade_libunwind: &upgrade_libunwind
+    run:
+      name: Upgrade Libunwind
+      command: |
+        # We need to install libunwind manually because Circle CI uses Ubuntu 14.04 and the default libunwind version
+        # on that Ubuntu suffers from http://savannah.nongnu.org/bugs/?43752.
+        # This isn't an issue for any later version of Ubuntu.
+
+        # Detect number of CPU cores
+        export NUMCORES=`nproc`
+        echo Using $NUMCORES cores
+        # Download and prepare libunwind (only if not already present from cache)
+        if [ ! -d "/tmp/libunwind-1.2.1" ]; then
+          echo "Didn't find libunwind in cache. Downloading and building."
+          wget -O /tmp/libunwind-1.2.1.tar.gz http://download.savannah.nongnu.org/releases/libunwind/libunwind-1.2.1.tar.gz
+          if [ $(sha512sum /tmp/libunwind-1.2.1.tar.gz | awk '{print $1;}') == "af7c280d2a963779a4a2711887618bc96383011e4e5d52e4085aa7fb351e55e357468f6ff85e66a216f1c6826538f498335a917a5970575c93be74c96316319b" ]; then
+            echo Correct sha512sum
+          else
+            echo Wrong sha512sum
+            sha512sum /tmp/libunwind-1.2.1.tar.gz
+            exit 1
+          fi
+          echo Extracting...
+          tar -xf /tmp/libunwind-1.2.1.tar.gz -C /tmp
+          rm -rf /tmp/libunwind-1.2.1.tar.gz
+          cd /tmp/libunwind-1.2.1
+          ./configure
+          make -j${NUMCORES}
+        else
+          echo Found libunwind in cache. Use cache and build.
+        fi
+        # Compile and install libunwind (if cached, this should be fast)
+        cd /tmp/libunwind-1.2.1
+        sudo make -j${NUMCORES} install
   upgrade_boost_pre: &upgrade_boost_pre
     restore_cache:
       keys:
@@ -189,6 +233,9 @@ references:
     - <<: *container_setup_pre
     - <<: *container_setup
     - <<: *container_setup_post
+    - <<: *upgrade_libunwind_pre
+    - <<: *upgrade_libunwind
+    - <<: *upgrade_libunwind_post
     - <<: *upgrade_boost_pre
     - <<: *upgrade_boost
     - <<: *upgrade_boost_post
@@ -489,6 +536,9 @@ jobs:
     - <<: *container_setup_pre
     - <<: *container_setup
     - <<: *container_setup_post
+    - <<: *upgrade_libunwind_pre
+    - <<: *upgrade_libunwind
+    - <<: *upgrade_libunwind_post
     - <<: *upgrade_boost_pre
     - <<: *upgrade_boost
     - <<: *upgrade_boost_post
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -7,7 +7,8 @@ cmake_policy(SET CMP0054 NEW)
 
 project(cryfs)
 
-include(cmake-utils/utils.cmake)
+list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR}/cmake-utils)
+include(utils)
 
 require_gcc_version(5.0)
 require_clang_version(4.0)
diff --git a/ChangeLog.txt b/ChangeLog.txt
@@ -7,6 +7,7 @@ Fixed bugs:
 
 Compatibility:
 * Fixed some incompatibilities with systems using the musl libc
+* Use libunwind instead of libbacktrace to build stack traces. This fixes a segfault issue with platforms using libexecinfo and is generally more portable.
 
 Other:
 * Updated to crypto++ 8.1
diff --git a/README.md b/README.md
@@ -56,14 +56,15 @@ Requirements
   - libFUSE version >= 2.8.6 (including development headers), on Mac OS X instead install osxfuse from https://osxfuse.github.io/
   - Python >= 2.7
   - OpenMP
+  - Libunwind
 
 You can use the following commands to install these requirements
 
         # Ubuntu
-        $ sudo apt-get install git g++ cmake make libcurl4-openssl-dev libboost-filesystem-dev libboost-system-dev libboost-chrono-dev libboost-program-options-dev libboost-thread-dev libssl-dev libfuse-dev python
+        $ sudo apt install git g++ cmake make libcurl4-openssl-dev libboost-filesystem-dev libboost-system-dev libboost-chrono-dev libboost-program-options-dev libboost-thread-dev libssl-dev libfuse-dev python libunwind-dev
 
         # Fedora
-        sudo dnf install git gcc-c++ cmake make libcurl-devel boost-devel boost-static openssl-devel fuse-devel python
+        sudo dnf install git gcc-c++ cmake make libcurl-devel boost-devel boost-static openssl-devel fuse-devel python libunwind-devel
 
         # Macintosh
         brew install cmake boost openssl libomp
diff --git a/cmake-utils/FindLibunwind.cmake b/cmake-utils/FindLibunwind.cmake
@@ -0,0 +1,44 @@
+# Taken from https://github.com/monero-project/monero/blob/31bdf7bd113c2576fe579ef3a25a2d8fef419ffc/cmake/FindLibunwind.cmake
+# modifications:
+# - remove linkage against gcc_eh because it was causing segfaults in various of our unit tests
+
+# - Try to find libunwind
+# Once done this will define
+#
+#  LIBUNWIND_FOUND - system has libunwind
+#  LIBUNWIND_INCLUDE_DIR - the libunwind include directory
+#  LIBUNWIND_LIBRARIES - Link these to use libunwind
+#  LIBUNWIND_DEFINITIONS - Compiler switches required for using libunwind
+
+# Copyright (c) 2006, Alexander Dymo, <adymo@kdevelop.org>
+#
+# Redistribution and use is allowed according to the terms of the BSD license.
+# For details see the accompanying COPYING-CMAKE-SCRIPTS file.
+
+find_path(LIBUNWIND_INCLUDE_DIR libunwind.h
+        /usr/include
+        /usr/local/include
+        )
+
+find_library(LIBUNWIND_LIBRARIES NAMES unwind )
+if(NOT LIBUNWIND_LIBRARIES STREQUAL "LIBUNWIND_LIBRARIES-NOTFOUND")
+    if (CMAKE_COMPILER_IS_GNUCC)
+        set(LIBUNWIND_LIBRARIES "${LIBUNWIND_LIBRARIES}")
+    endif()
+endif()
+
+# some versions of libunwind need liblzma, and we don't use pkg-config
+# so we just look whether liblzma is installed, and add it if it is.
+# It might not be actually needed, but doesn't hurt if it is not.
+# We don't need any headers, just the lib, as it's privately needed.
+message(STATUS "looking for liblzma")
+find_library(LIBLZMA_LIBRARIES lzma )
+if(NOT LIBLZMA_LIBRARIES STREQUAL "LIBLZMA_LIBRARIES-NOTFOUND")
+    message(STATUS "liblzma found")
+    set(LIBUNWIND_LIBRARIES "${LIBUNWIND_LIBRARIES};${LIBLZMA_LIBRARIES}")
+endif()
+
+include(FindPackageHandleStandardArgs)
+find_package_handle_standard_args(Libunwind "Could not find libunwind" LIBUNWIND_INCLUDE_DIR LIBUNWIND_LIBRARIES)
+# show the LIBUNWIND_INCLUDE_DIR and LIBUNWIND_LIBRARIES variables only in the advanced view
+mark_as_advanced(LIBUNWIND_INCLUDE_DIR LIBUNWIND_LIBRARIES )
diff --git a/src/cpp-utils/CMakeLists.txt b/src/cpp-utils/CMakeLists.txt
@@ -61,13 +61,14 @@ set(SOURCES
 
 add_library(${PROJECT_NAME} STATIC ${SOURCES})
 
-
-if(NOT MSVC)
-  find_package(Backtrace REQUIRED)
-  target_include_directories(${PROJECT_NAME} PUBLIC ${Backtrace_INCLUDE_DIRS})
-  target_link_libraries(${PROJECT_NAME} PUBLIC ${Backtrace_LIBRARIES})
-else()
-  target_link_libraries(${PROJECT_NAME} PUBLIC DbgHelp)
+if(MSVC)
+    target_link_libraries(${PROJECT_NAME} PUBLIC DbgHelp)
+elseif(NOT APPLE)
+    # note: We use the libunwind code on Apple, but we don't seem to need these lines to link against it.
+    find_package(Libunwind REQUIRED)
+    target_include_directories(${PROJECT_NAME} PRIVATE ${LIBUNWIND_INCLUDE_DIR})
+    target_link_libraries(${PROJECT_NAME} PRIVATE ${LIBUNWIND_LIBRARIES})
+    target_compile_definitions(${PROJECT_NAME} PRIVATE ${LIBUNWIND_DEFINITIONS})
 endif()
 
 if (NOT MSVC)
diff --git a/src/cpp-utils/assert/backtrace_nonwindows.cpp b/src/cpp-utils/assert/backtrace_nonwindows.cpp
@@ -1,18 +1,15 @@
 #if !defined(_MSC_VER)
 
-#include "backtrace.h"
-#include <execinfo.h>
 #include <csignal>
-#include <iostream>
-#include <unistd.h>
 #include <cxxabi.h>
-#include <string>
 #include <sstream>
-#include <string>
-#include <dlfcn.h>
+
 #include "../logging/logging.h"
 #include <cpp-utils/process/SignalHandler.h>
 
+#define UNW_LOCAL_ONLY
+#include <libunwind.h>
+
 // TODO Add file and line number on non-windows
 
 using std::string;
@@ -30,7 +27,12 @@ namespace {
             demangledName = abi::__cxa_demangle(mangledName.c_str(), NULL, NULL, &status);
             if (status == 0) {
                 result = demangledName;
+            } else if (status == -2) {
+                // mangledName was not a c++ mangled name, probably because it's a C name like for static
+                // initialization or stuff. Let's just return the name instead.
+                result = mangledName;
             } else {
+                // other error
                 result = "[demangling error " + std::to_string(status) + "]" + mangledName;
             }
             free(demangledName);
@@ -41,46 +43,55 @@ namespace {
         }
     }
 
-    void pretty_print(std::ostream& str, const void *addr) {
-        Dl_info info;
-        if (0 == dladdr(addr, &info)) {
-            str << "[failed parsing line]";
+    void pretty_print(std::ostringstream& str, unw_cursor_t* cursor) {
+        constexpr unsigned int MAXNAMELEN=256;
+        char name[MAXNAMELEN];
+        unw_word_t offp = 0, ip = 0;
+
+        int status = unw_get_reg(cursor, UNW_REG_IP, &ip);
+        if (0 != status) {
+            str << "[unw_get_reg error: " << status << "]: ";
         } else {
-            if (nullptr == info.dli_fname) {
-                str << "[no dli_fname]";
-            } else {
-                str << info.dli_fname;
-            }
-            str << ":" << std::hex << info.dli_fbase << " ";
-            if (nullptr == info.dli_sname) {
-                str << "[no symbol name]";
-            } else if (info.dli_sname[0] == '_') {
-                // is a mangled name
-                str << demangle(info.dli_sname);
-            } else {
-                // is not a mangled name
-                str << info.dli_sname;
-            }
-            str << " : " << std::hex << info.dli_saddr;
+            str << "0x" << std::hex << ip << ": ";
         }
-    }
 
-    string backtrace_to_string(void *array[], size_t size) {
-        ostringstream result;
-        for (size_t i = 0; i < size; ++i) {
-            result << "#" << std::dec << i << " ";
-            pretty_print(result, array[i]);
-            result << "\n";
+        status = unw_get_proc_name(cursor, name, MAXNAMELEN, &offp);
+        if (0 != status) {
+            str << "[unw_get_proc_name error: " << status << "]";
+        } else {
+            str << demangle(name);
         }
-        return result.str();
+        str << " +0x" << std::hex << offp;
     }
 }
 
 	string backtrace() {
-		constexpr unsigned int MAX_SIZE = 100;
-		void *array[MAX_SIZE];
-		size_t size = ::backtrace(array, MAX_SIZE);
-		return backtrace_to_string(array, size);
+        std::ostringstream result;
+
+        unw_context_t uc;
+        int status = unw_getcontext(&uc);
+        if (0 != status) {
+            return "[unw_getcontext error: " + std::to_string(status) + "]";
+        }
+
+        unw_cursor_t cursor;
+        status = unw_init_local(&cursor, &uc);
+        if (0 != status) {
+            return "[unw_init_local error: " + std::to_string(status) + "]";
+        }
+
+
+        size_t line = 0;
+        while ((status = unw_step(&cursor)) > 0) {
+            result << "#" << std::dec << (line++) << " ";
+            pretty_print(result, &cursor);
+            result << "\n";
+        }
+        if (status != 0) {
+            result << "[unw_step error :" << status << "]";
+        }
+
+        return result.str();
 	}
 
 namespace {
diff --git a/src/cpp-utils/process/SignalCatcher.cpp b/src/cpp-utils/process/SignalCatcher.cpp
@@ -88,7 +88,8 @@ class SignalCatcherImpl final {
     , _registerer(signal, this)
     , _handler(signal) {
         // note: the order of the members ensures that:
-        //  - when registering the signal handler fails, the registerer will be destroyed, unregistering the signal_occurred_flag,
+        //  - when registering the signal handler, the SignalCatcher impl already has a valid _signal_occurred_flag set.
+        //  - when registering the signal handler fails, the _registerer will be destroyed again, unregistering this SignalCatcherImpl,
         //    i.e. there is no leak.
 
         // Allow only the set of signals that is supported on all platforms, see for Windows: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/signal?view=vs-2017
diff --git a/src/cpp-utils/process/SignalHandler.cpp b/src/cpp-utils/process/SignalHandler.cpp
@@ -1 +1,66 @@
 #include "SignalHandler.h"
+
+#if !defined(_MSC_VER)
+
+namespace cpputils {
+namespace detail {
+namespace {
+
+std::atomic<bool> already_checked_for_libunwind_bug(false);
+
+}
+
+sigset_t _sigemptyset() {
+    sigset_t result;
+    int error = sigemptyset(&result);
+    if (0 != error) {
+        throw std::runtime_error("Error calling sigemptyset. Errno: " + std::to_string(errno));
+    }
+    return result;
+}
+
+void _sigmask(sigset_t* new_value, sigset_t* old_value) {
+    int error = pthread_sigmask(SIG_SETMASK, new_value, old_value);
+    if (0 != error) {
+        throw std::runtime_error("Error calling pthread_sigmask. Errno: " + std::to_string(errno));
+    }
+}
+
+// Check that we're not running into http://savannah.nongnu.org/bugs/?43752
+void check_against_libunwind_bug() {
+    if (already_checked_for_libunwind_bug.exchange(true)) {
+        return;
+    }
+
+    // set new signal handler
+    sigset_t old_value = _sigemptyset();
+    sigset_t new_value = _sigemptyset();
+
+    _sigmask(&new_value, &old_value);
+
+    sigset_t before_exception = _sigemptyset();
+    _sigmask(nullptr, &before_exception);
+
+    // throw an exception
+    try {
+        throw std::runtime_error("Some exception");
+    } catch (const std::exception &e) {}
+
+    sigset_t after_exception = _sigemptyset();
+    _sigmask(nullptr, &after_exception);
+
+    // reset to old signal handler
+    _sigmask(&old_value, nullptr);
+
+    // check that the exception didn't screw up the signal mask
+    if (0 != std::memcmp(&before_exception, &after_exception, sizeof(sigset_t))) {  // NOLINT(cppcoreguidelines-pro-type-union-access)
+        ASSERT(false,
+               "Throwing an exception screwed up the signal mask. You likely ran into this bug: http://savannah.nongnu.org/bugs/?43752 . Please build CryFS against a newer version of libunwind or build libunwind with --disable-cxx-exceptions.");
+    }
+}
+
+
+}
+}
+
+#endif
diff --git a/src/cpp-utils/process/SignalHandler.h b/src/cpp-utils/process/SignalHandler.h
@@ -20,11 +20,17 @@ using SignalHandlerFunction = void(int);
 
 #if !defined(_MSC_VER)
 
+namespace detail {
+void check_against_libunwind_bug();
+}
+
 template<SignalHandlerFunction* handler>
 class SignalHandlerRAII final {
 public:
     explicit SignalHandlerRAII(int signal)
             : _old_handler(), _signal(signal) {
+        detail::check_against_libunwind_bug();
+
         struct sigaction new_signal_handler{};
         std::memset(&new_signal_handler, 0, sizeof(new_signal_handler));
         new_signal_handler.sa_handler = handler;  // NOLINT(cppcoreguidelines-pro-type-union-access)
diff --git a/test/cpp-utils/assert/backtrace_test.cpp b/test/cpp-utils/assert/backtrace_test.cpp
