Update NDK unwinding logic (#1605)

bugsnag-android-core/src/main/jni/root_detection.c

@@ -59,7 +59,7 @@ static inline bool is_path_writable(const char* path) {
 static inline bool can_create_file(const char* path) {
   unlink(path);
 
-  const int fd = open(path, O_CREAT);
+  const int fd = open(path, O_CREAT, O_RDONLY);
   if (fd < 0) {
     return false;
   }

bugsnag-plugin-android-ndk/CMakeLists.txt

@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.4.1)
+cmake_minimum_required(VERSION 3.8.0)
 project(TEST)
 add_subdirectory(src/main)
 

bugsnag-plugin-android-ndk/src/androidTest/java/com/bugsnag/android/ndk/UnwindTest.kt

@@ -0,0 +1,45 @@
+package com.bugsnag.android.ndk
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.fail
+import org.junit.Test
+
+class UnwindTest {
+
+    @Test
+    fun testUnwindForNotify() {
+        val frames = unwindForNotify()
+        // the top of the stack includes the unwinder itself, etc, so first
+        // scan for the expected contents, which must be a sequence
+        var offset = frames.indexOf("unwind_func_four")
+        if (offset == -1) {
+            fail("did not find initial stack frame in list: $frames")
+        }
+        assertEquals("unwind_func_four", frames[offset])
+        assertEquals("unwind_func_three", frames[offset + 1])
+        assertEquals("unwind_func_two", frames[offset + 2])
+        assertEquals("unwind_func_one", frames[offset + 3])
+        assertEquals("Java_com_bugsnag_android_ndk_UnwindTest_unwindForNotify", frames[offset + 4])
+    }
+
+    @Test
+    fun testUnwindForCrash() {
+        val frames = unwindForCrash()
+        // the top of the stack includes the unwinder itself, etc, so first
+        // scan for the expected contents, which must be a sequence
+        var offset = frames.indexOf("unwind_func_four")
+        if (offset == -1) {
+            fail("did not find initial stack frame in list: $frames")
+        }
+        assertEquals("unwind_func_four", frames[offset])
+        assertEquals("unwind_func_three", frames[offset + 1])
+        assertEquals("unwind_func_two", frames[offset + 2])
+        assertEquals("unwind_func_one", frames[offset + 3])
+        assertEquals("Java_com_bugsnag_android_ndk_UnwindTest_unwindForCrash", frames[offset + 4])
+    }
+
+    // unwind a known set of functions and validate the method names in the
+    // stack
+    external fun unwindForNotify(): List<String>
+    external fun unwindForCrash(): List<String>
+}

bugsnag-plugin-android-ndk/src/main/CMakeLists.txt

@@ -20,11 +20,7 @@ add_library( # Specifies the name of the library.
     jni/utils/serializer/event_reader.c
     jni/utils/serializer/event_writer.c
     jni/utils/serializer/json_writer.c
-    jni/utils/stack_unwinder.c
-    jni/utils/stack_unwinder_libunwindstack.cpp
-    jni/utils/stack_unwinder_libcorkscrew.c
-    jni/utils/stack_unwinder_libunwind.c
-    jni/utils/stack_unwinder_simple.c
+    jni/utils/stack_unwinder.cpp
     jni/utils/serializer.c
     jni/utils/string.c
     jni/utils/threads.c
@@ -34,7 +30,6 @@ add_library( # Specifies the name of the library.
 include_directories(
     jni
     jni/deps
-    jni/external/libunwind/include
     jni/external/libunwindstack-ndk/include
              )
 
@@ -47,14 +42,9 @@ target_link_libraries( # Specifies the target library.
 
 set_target_properties(bugsnag-ndk
                       PROPERTIES
-                      COMPILE_OPTIONS
-                      -Werror -Wall -pedantic)
+                      COMPILE_OPTIONS -Werror -Wall -pedantic
+                      CXX_STANDARD 17
+                      CXX_STANDARD_REQUIRED YES)
 
 add_subdirectory(jni/external/libunwindstack-ndk/cmake)
 target_link_libraries(bugsnag-ndk unwindstack)
-if(${ANDROID_ABI} STREQUAL "armeabi" OR ${ANDROID_ABI} STREQUAL "armeabi-v7a")
-    add_library(libunwind STATIC IMPORTED)
-    set_target_properties(libunwind PROPERTIES IMPORTED_LOCATION
-        ${ANDROID_NDK}/sources/cxx-stl/llvm-libc++/libs/${ANDROID_ABI}/libunwind.a)
-    target_link_libraries(bugsnag-ndk libunwind)
-endif()

bugsnag-plugin-android-ndk/src/main/jni/bugsnag.c

@@ -136,8 +136,7 @@ void bugsnag_notify_env(JNIEnv *env, const char *name, const char *message,
 
   bugsnag_stackframe stacktrace[BUGSNAG_FRAMES_MAX];
   memset(stacktrace, 0, sizeof(stacktrace));
-  ssize_t frame_count =
-      bsg_unwind_stack(bsg_configured_unwind_style(), stacktrace, NULL, NULL);
+  ssize_t frame_count = bsg_unwind_concurrent_stack(stacktrace, NULL, NULL);
 
   // create StackTraceElement array
   jtrace = bsg_safe_new_object_array(env, frame_count,

bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c

@@ -38,24 +38,6 @@ static void release_env_write_lock(void) {
   pthread_mutex_unlock(&bsg_global_env_write_mutex);
 }
 
-bsg_unwinder bsg_configured_unwind_style() {
-  if (bsg_global_env != NULL)
-    return bsg_global_env->unwind_style;
-
-  return BSG_CUSTOM_UNWIND;
-}
-
-/**
- * Get the configured unwind style for async-safe environments such as signal
- * handlers.
- */
-bsg_unwinder bsg_configured_signal_unwind_style() {
-  if (bsg_global_env != NULL)
-    return bsg_global_env->signal_unwind_style;
-
-  return BSG_CUSTOM_UNWIND;
-}
-
 void bugsnag_add_on_error(bsg_on_error on_error) {
   if (bsg_global_env != NULL) {
     bsg_global_env->on_error = on_error;
@@ -155,9 +137,7 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install(
   }
 
   bsg_environment *bugsnag_env = calloc(1, sizeof(bsg_environment));
-  bsg_set_unwind_types((int)_api_level, (bool)is32bit,
-                       &bugsnag_env->signal_unwind_style,
-                       &bugsnag_env->unwind_style);
+  bsg_unwinder_init();
   bugsnag_env->report_header.big_endian =
       htonl(47) == 47; // potentially too clever, see man 3 htonl
   bugsnag_env->report_header.version = BUGSNAG_EVENT_VERSION;
@@ -721,20 +701,10 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateMetadata(
   release_env_write_lock();
 }
 
-// Unwind the stack using the configured unwind style for signal handlers.
-// This function gets exposed via
-// Java_com_bugsnag_android_ndk_NativeBridge_getSignalUnwindStackFunction()
-static ssize_t
-bsg_unwind_stack_signal(bugsnag_stackframe stacktrace[BUGSNAG_FRAMES_MAX],
-                        siginfo_t *info, void *user_context) __asyncsafe {
-  return bsg_unwind_stack(bsg_configured_signal_unwind_style(), stacktrace,
-                          info, user_context);
-}
-
 JNIEXPORT jlong JNICALL
 Java_com_bugsnag_android_ndk_NativeBridge_getSignalUnwindStackFunction(
     JNIEnv *env, jobject thiz) {
-  return (jlong)bsg_unwind_stack_signal;
+  return (jlong)bsg_unwind_crash_stack;
 }
 
 JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addFeatureFlag(

bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.h

@@ -16,14 +16,6 @@ extern "C" {
 #endif
 
 typedef struct {
-  /**
-   * Unwinding style used for signal-safe handling
-   */
-  bsg_unwinder signal_unwind_style;
-  /**
-   * Preferred unwinding style
-   */
-  bsg_unwinder unwind_style;
   /**
    * Records the version of the bugsnag NDK report being serialized to disk.
    */
@@ -76,12 +68,6 @@ typedef struct {
   bsg_thread_send_policy send_threads;
 } bsg_environment;
 
-/**
- * Get the configured unwind style for non-async-safe environments.
- * DO NOT USE THIS IN A SIGNAL HANDLER!
- */
-bsg_unwinder bsg_configured_unwind_style();
-
 /**
  * Invokes the user-supplied on_error callback, if it has been set. This allows
  * users to mutate the bugsnag_event payload before it is persisted to disk, and