Fix non-windows backtrace implementation

src/cpp-utils/CMakeLists.txt

@@ -58,7 +58,10 @@ find_package(Backtrace REQUIRED)
 target_include_directories(${PROJECT_NAME} PUBLIC ${Backtrace_INCLUDE_DIRS})
 target_link_libraries(${PROJECT_NAME} PUBLIC ${Backtrace_LIBRARIES})
 
-target_link_libraries(${PROJECT_NAME} PUBLIC pthread)
+find_package(Threads REQUIRED)
+target_link_libraries(${PROJECT_NAME} PUBLIC ${CMAKE_THREAD_LIBS_INIT})
+
+target_link_libraries(${PROJECT_NAME} PUBLIC ${CMAKE_DL_LIBS})
 
 # TODO From Crypto++ 5.7 on, it should support cmake with find_package() instead of find_library().
 find_library_with_path(CryptoPP cryptopp CRYPTOPP_LIB_PATH)

src/cpp-utils/assert/backtrace.h

@@ -9,7 +9,7 @@ namespace cpputils {
 
     //TODO Refactor (for example: RAII or at least try{}finally{} instead of  free())
     //TODO Use the following? https://github.com/bombela/backward-cpp
-    void showBacktraceOnSigSegv();
+    void showBacktraceOnCrash();
 }
 
 #endif

src/cpp-utils/assert/backtrace_nonwindows.cpp

@@ -2,12 +2,14 @@
 
 #include "backtrace.h"
 #include <execinfo.h>
-#include <signal.h>
+#include <csignal>
 #include <iostream>
 #include <unistd.h>
 #include <cxxabi.h>
 #include <string>
 #include <sstream>
+#include <string>
+#include <dlfcn.h>
 #include "../logging/logging.h"
 
 // TODO Add file and line number on non-windows
@@ -22,33 +24,53 @@ namespace {
     std::string demangle(const string &mangledName) {
         string result;
         int status = -10;
-        char *demangledName = abi::__cxa_demangle(mangledName.c_str(), NULL, NULL, &status);
-        if (status == 0) {
-            result = demangledName;
-        } else {
-            result = mangledName;
+        char *demangledName = nullptr;
+        try {
+            demangledName = abi::__cxa_demangle(mangledName.c_str(), NULL, NULL, &status);
+            if (status == 0) {
+                result = demangledName;
+            } else {
+                result = "[demangling error " + std::to_string(status) + "]" + mangledName;
+            }
+            free(demangledName);
+            return result;
+        } catch (...) {
+            free(demangledName);
+            throw;
         }
-        free(demangledName);
-        return result;
     }
 
-    std::string pretty(const string &backtraceLine) {
-        size_t startMangledName = backtraceLine.find('(');
-        size_t endMangledName = backtraceLine.find('+');
-        if (startMangledName == string::npos || endMangledName == string::npos) {
-            return backtraceLine;
+    void pretty_print(std::ostream& str, const void *addr) {
+        Dl_info info;
+        if (0 == dladdr(addr, &info)) {
+            str << "[failed parsing line]";
+        } 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;
         }
-        return demangle(backtraceLine.substr(startMangledName + 1, endMangledName - startMangledName - 1)) + ": (" +
-               backtraceLine.substr(0, startMangledName) + backtraceLine.substr(endMangledName);
     }
 
     string backtrace_to_string(void *array[], size_t size) {
         ostringstream result;
-        char **ptr = backtrace_symbols(array, size);
         for (size_t i = 0; i < size; ++i) {
-            result << pretty(ptr[i]) << "\n";
+            result << "#" << std::dec << i << " ";
+            pretty_print(result, array[i]);
+            result << "\n";
         }
-        free(ptr);
         return result.str();
     }
 }
@@ -65,15 +87,27 @@ namespace {
         LOG(ERR, "SIGSEGV\n{}", backtrace());
         exit(1);
     }
+    void sigill_handler(int) {
+        LOG(ERR, "SIGILL\n{}", backtrace());
+        exit(1);
+    }
+    void sigabrt_handler(int) {
+        LOG(ERR, "SIGABRT\n{}", backtrace());
+        exit(1);
+    }
+    void set_handler(int signum, void(*handler)(int)) {
+        auto result = signal(signum, handler);
+        if (SIG_ERR == result) {
+            LOG(ERR, "Failed to set signal {} handler. Errno: {}", signum, errno);
+        }
+    }
 }
 
-	void showBacktraceOnSigSegv() {
-		auto result = signal(SIGSEGV, sigsegv_handler);
-		if (SIG_ERR == result) {
-			LOG(ERR, "Failed to set sigsegv signal handler. Errno: {}", errno);
-		}
+	void showBacktraceOnCrash() {
+		set_handler(SIGSEGV, &sigsegv_handler);
+		set_handler(SIGABRT, &sigabrt_handler);
+		set_handler(SIGILL, &sigill_handler);
 	}
-
 }
 
 #endif

src/cpp-utils/assert/backtrace_windows.cpp

@@ -115,7 +115,7 @@ namespace cpputils {
 		return backtrace_to_string(&context);
 	}
 
-	void showBacktraceOnSigSegv() {
+	void showBacktraceOnCrash() {
 		SetUnhandledExceptionFilter(TopLevelExceptionHandler);
 	}
 

src/cryfs-cli/Cli.cpp

@@ -374,7 +374,7 @@ namespace cryfs {
     }
 
     int Cli::main(int argc, const char *argv[], unique_ref<HttpClient> httpClient) {
-        cpputils::showBacktraceOnSigSegv();
+        cpputils::showBacktraceOnCrash();
 
         try {
             _showVersion(std::move(httpClient));

test/cpp-utils/assert/backtrace_test.cpp

@@ -1,4 +1,5 @@
 #include <gmock/gmock.h>
+#include <csignal>
 #include "cpp-utils/assert/backtrace.h"
 
 using std::string;
@@ -16,17 +17,62 @@ TEST(BacktraceTest, ContainsTopLevelLine) {
 }
 
 namespace {
-void cause_sigsegv() {
-    cpputils::showBacktraceOnSigSegv();
+void nullptr_access() {
+    cpputils::showBacktraceOnCrash();
     int* ptr = nullptr;
-    int a = *ptr;
-    (void)a;
+    *ptr = 5;
 }
+void raise_signal(int signal) {
+    cpputils::showBacktraceOnCrash();
+    ::raise(signal);
+}
+}
+
+TEST(BacktraceTest, ShowBacktraceOnNullptrAccess) {
+    EXPECT_DEATH(
+        nullptr_access(),
+        "cpputils::backtrace"
+    );
 }
 
 TEST(BacktraceTest, ShowBacktraceOnSigSegv) {
     EXPECT_DEATH(
-        cause_sigsegv(),
-        "cpputils::backtrace"
+            raise_signal(SIGSEGV),
+            "cpputils::backtrace"
+    );
+}
+
+TEST(BacktraceTest, ShowBacktraceOnSigAbrt) {
+    EXPECT_DEATH(
+            raise_signal(SIGABRT),
+            "cpputils::backtrace"
+    );
+}
+
+TEST(BacktraceTest, ShowBacktraceOnSigIll) {
+    EXPECT_DEATH(
+            raise_signal(SIGILL),
+            "cpputils::backtrace"
+    );
+}
+
+TEST(BacktraceTest, ShowBacktraceOnSigSegv_ShowsCorrectSignalName) {
+    EXPECT_DEATH(
+            raise_signal(SIGSEGV),
+            "SIGSEGV"
+    );
+}
+
+TEST(BacktraceTest, ShowBacktraceOnSigAbrt_ShowsCorrectSignalName) {
+    EXPECT_DEATH(
+            raise_signal(SIGABRT),
+            "SIGABRT"
+    );
+}
+
+TEST(BacktraceTest, ShowBacktraceOnSigIll_ShowsCorrectSignalName) {
+    EXPECT_DEATH(
+            raise_signal(SIGILL),
+            "SIGILL"
     );
 }