Merge dc3fd25 into 991e628

CHANGELOG.md

@@ -2,11 +2,17 @@
 
 ## 4.X.X (TBD)
 
+### Enhancements
+
+* [NDK] Improve support for C++ exceptions, adding the exception class name
+  and description to reports and improving the stacktrace quality
+  [#412](https://github.com/bugsnag/bugsnag-android/pull/412)
+
 ### Bug fixes
 
 * Cache result of device root check
   [#411](https://github.com/bugsnag/bugsnag-android/pull/411)
-  
+
 * Prevent unnecessary free disk calculations on initialisation
  [#409](https://github.com/bugsnag/bugsnag-android/pull/409)
 

features/fixtures/mazerunner/src/main/cpp/entrypoint.cpp

@@ -4,6 +4,23 @@
 #include <signal.h>
 #include <time.h>
 
+#include <stdexcept>
+
+bool __attribute__((noinline)) run_away(bool value) {
+  if (value)
+    throw new std::runtime_error("How about NO");
+
+  return false;
+}
+
+bool __attribute__((noinline)) run_back(int value, int boundary) {
+  printf("boundary: %d\n", boundary);
+  if (value > -boundary)
+    throw 42;
+
+  return false;
+}
+
 extern "C" {
 
 static char * __attribute__((used)) somefakefunc(void) {};
@@ -44,6 +61,22 @@ int crash_write_read_only_mem(int counter) {
   return counter / 14;
 }
 
+int __attribute__((noinline)) throw_an_object(bool value, int boundary) {
+  if (value) {
+    printf("Now we know what they mean by 'advanced' tactical training: %d", boundary);
+    return (int)run_back(value, boundary);
+  }
+  return boundary * 2;
+}
+
+int __attribute__((noinline)) trigger_an_exception(bool value) {
+  printf("Shields up! Rrrrred alert!.\n");
+  if (value)
+    return (int)run_away(value);
+  else
+    return 405;
+}
+
 char *crash_improper_cast(int counter) {
   reporter_t *report = (reporter_t *)counter;
 
@@ -161,6 +194,22 @@ Java_com_bugsnag_android_mazerunner_scenarios_CXXNullPointerScenario_crash(JNIEn
   printf("This one here: %ld\n", (long) crash_null_pointer(x > 0));
 }
 
+JNIEXPORT void JNICALL
+Java_com_bugsnag_android_mazerunner_scenarios_CXXExceptionScenario_crash(JNIEnv *env,
+                                                                         jobject instance) {
+  int x = 61;
+  printf("This one here: %ld\n", (long) trigger_an_exception(x > 0));
+  printf("This one here: %ld\n", (long) throw_an_object(x > 0, x));
+}
+
+JNIEXPORT void JNICALL
+Java_com_bugsnag_android_mazerunner_scenarios_CXXThrowSomethingScenario_crash(JNIEnv *env,
+                                                                              jobject instance,
+                                                                              jint num) {
+  printf("This one here: %ld\n", (long) throw_an_object((num - 10) > 0, num));
+  printf("This one here: %ld\n", (long) trigger_an_exception(num > 0));
+}
+
 JNIEXPORT void JNICALL
 Java_com_bugsnag_android_mazerunner_scenarios_CXXStackoverflowScenario_crash(JNIEnv *env,
                                                                              jobject instance,

features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CXXExceptionScenario.java

@@ -0,0 +1,33 @@
+package com.bugsnag.android.mazerunner.scenarios;
+
+import android.content.Context;
+
+import com.bugsnag.android.Configuration;
+
+import android.support.annotation.NonNull;
+
+public class CXXExceptionScenario extends Scenario {
+
+    static {
+        System.loadLibrary("bugsnag-ndk");
+        System.loadLibrary("monochrome");
+        System.loadLibrary("entrypoint");
+    }
+
+    public native void crash();
+
+    public CXXExceptionScenario(@NonNull Configuration config, @NonNull Context context) {
+        super(config, context);
+        config.setAutoCaptureSessions(false);
+    }
+
+    @Override
+    public void run() {
+        super.run();
+        String metadata = getEventMetaData();
+        if (metadata != null && metadata.equals("non-crashy")) {
+            return;
+        }
+        crash();
+    }
+}

features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/CXXThrowSomethingScenario.java

@@ -0,0 +1,33 @@
+package com.bugsnag.android.mazerunner.scenarios;
+
+import android.content.Context;
+
+import com.bugsnag.android.Configuration;
+
+import android.support.annotation.NonNull;
+
+public class CXXThrowSomethingScenario extends Scenario {
+
+    static {
+        System.loadLibrary("bugsnag-ndk");
+        System.loadLibrary("monochrome");
+        System.loadLibrary("entrypoint");
+    }
+
+    public native void crash(int num);
+
+    public CXXThrowSomethingScenario(@NonNull Configuration config, @NonNull Context context) {
+        super(config, context);
+        config.setAutoCaptureSessions(false);
+    }
+
+    @Override
+    public void run() {
+        super.run();
+        String metadata = getEventMetaData();
+        if (metadata != null && metadata.equals("non-crashy")) {
+            return;
+        }
+        crash(23);
+    }
+}

features/native_crash_handling.feature

@@ -210,3 +210,33 @@ Feature: Native crash reporting
         And the first significant stack frame methods and files should match:
             | something_innocuous | libmonochrome.so |
             | Java_com_bugsnag_android_mazerunner_scenarios_CXXExternalStackElementScenario_crash | libentrypoint.so |
+
+    Scenario: Throwing an exception in C++
+        When I run "CXXExceptionScenario"
+        And I configure the app to run in the "non-crashy" state
+        And I relaunch the app
+        Then I should receive a request
+        And the request payload contains a completed native report
+        And the event "severity" equals "error"
+        And the event "unhandled" is true
+        And the exception "errorClass" equals "PSt13runtime_error"
+        And the exception "message" equals "How about NO"
+        And the first significant stack frame methods and files should match:
+            | run_away(bool)       | libentrypoint.so |
+            | trigger_an_exception | libentrypoint.so |
+            | Java_com_bugsnag_android_mazerunner_scenarios_CXXExceptionScenario_crash | libentrypoint.so |
+
+    Scenario: Throwing an object in C++
+        When I run "CXXThrowSomethingScenario"
+        And I configure the app to run in the "non-crashy" state
+        And I relaunch the app
+        Then I should receive a request
+        And the request payload contains a completed native report
+        And the event "severity" equals "error"
+        And the event "unhandled" is true
+        And the exception "errorClass" equals "i"
+        And the exception "message" equals "42"
+        And the first significant stack frame methods and files should match:
+            | run_back(int, int) | libentrypoint.so |
+            | throw_an_object    | libentrypoint.so |
+            | Java_com_bugsnag_android_mazerunner_scenarios_CXXThrowSomethingScenario_crash | libentrypoint.so |

features/steps/build_steps.rb

@@ -86,10 +86,15 @@
   stacktrace.each_with_index do |item, index|
     next if expected_index >= expected_frame_values.length
     expected_frame = expected_frame_values[expected_index]
-    next if item["method"].start_with? "bsg_"
-    next if item["file"].start_with? "/system/"
-
-    assert_equal(expected_frame[0], item["method"])
+    method = `c++filt -_ _#{item["method"]}`.chomp
+    method = item["method"] if method == "_#{item["method"]}"
+    next if method.start_with? "bsg_" or
+            method.start_with? "std::" or
+            method.start_with? "__cxx" or
+            item["file"].start_with? "/system/" or
+            item["file"].end_with? "libbugsnag-ndk.so"
+
+    assert_equal(expected_frame[0], method)
     assert(item["file"].end_with?(expected_frame[1]), "'#{item["file"]}' in frame #{index} does not end with '#{expected_frame[1]}'")
     expected_index += 1
   end

ndk/build.gradle

@@ -22,6 +22,12 @@ android {
             abiFilters 'arm64-v8a', 'armeabi-v7a', 'armeabi', 'x86', 'x86_64'
         }
         consumerProguardFiles 'proguard-rules.pro'
+        externalNativeBuild {
+            cmake {
+                arguments '-DANDROID_CPP_FEATURES=exceptions',
+                          '-DANDROID_STL=c++_static'
+            }
+        }
     }
 
     externalNativeBuild {

ndk/src/main/CMakeLists.txt

@@ -11,6 +11,7 @@ add_library( # Specifies the name of the library.
              jni/metadata.c
              jni/report.c
              jni/handlers/signal_handler.c
+             jni/handlers/cpp_handler.cpp
              jni/utils/crash_info.c
              jni/utils/stack_unwinder.c
              jni/utils/stack_unwinder_libcorkscrew.c

ndk/src/main/jni/bugsnag_ndk.c

@@ -7,6 +7,7 @@
 #include <string.h>
 
 #include "handlers/signal_handler.h"
+#include "handlers/cpp_handler.h"
 #include "metadata.h"
 #include "report.h"
 #include "utils/serializer.h"
@@ -54,6 +55,7 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install(
 
   if ((bool)auto_notify) {
     bsg_handler_install_signal(bugsnag_env);
+    bsg_handler_install_cpp(bugsnag_env);
   }
 
   // populate metadata from Java layer

ndk/src/main/jni/bugsnag_ndk.h

@@ -43,6 +43,10 @@ typedef struct {
      * from being processed simultaneously
      */
     bool handling_crash;
+    /**
+     * true if a handler has completed crash handling
+     */
+    bool crash_handled;
 } bsg_environment;
 
 bsg_unwinder bsg_configured_unwind_style();

ndk/src/main/jni/handlers/cpp_handler.cpp

@@ -0,0 +1,112 @@
+#include "cpp_handler.h"
+#include <cxxabi.h>
+#include <exception>
+#include <pthread.h>
+#include <stdexcept>
+#include <string>
+
+#include "../utils/crash_info.h"
+#include "../utils/serializer.h"
+#include "../utils/string.h"
+/**
+ * Previously installed termination handler
+ */
+std::terminate_handler bsg_global_terminate_previous;
+/**
+ * Global shared context for Bugsnag reports
+ */
+static bsg_environment *bsg_global_env;
+
+/**
+ * C++ exception handler
+ */
+void bsg_handle_cpp_terminate();
+
+bool bsg_handler_install_cpp(bsg_environment *env) {
+  static pthread_mutex_t bsg_cpp_handler_config = PTHREAD_MUTEX_INITIALIZER;
+  pthread_mutex_lock(&bsg_cpp_handler_config);
+  bsg_global_terminate_previous = std::set_terminate(bsg_handle_cpp_terminate);
+  bsg_global_env = env;
+
+  pthread_mutex_unlock(&bsg_cpp_handler_config);
+  return true;
+}
+
+void bsg_handler_uninstall_cpp() {
+  if (bsg_global_env == NULL)
+    return;
+  std::set_terminate(bsg_global_terminate_previous);
+  bsg_global_env = NULL;
+}
+
+void bsg_write_current_exception_message(char *message, size_t length) {
+  try {
+    throw;
+  } catch (std::exception &exc) {
+    bsg_strncpy(message, (char *)exc.what(), length);
+  } catch (std::exception *exc) {
+    bsg_strncpy(message, (char *)exc->what(), length);
+  } catch (std::string obj) {
+    bsg_strncpy(message, (char *)obj.c_str(), length);
+  } catch (char *obj) {
+    snprintf(message, length, "%s", obj);
+  } catch (char obj) {
+    snprintf(message, length, "%c", obj);
+  } catch (short obj) {
+    snprintf(message, length, "%d", obj);
+  } catch (int obj) {
+    snprintf(message, length, "%d", obj);
+  } catch (long obj) {
+    snprintf(message, length, "%ld", obj);
+  } catch (long long obj) {
+    snprintf(message, length, "%lld", obj);
+  } catch (long double obj) {
+    snprintf(message, length, "%Lf", obj);
+  } catch (double obj) {
+    snprintf(message, length, "%f", obj);
+  } catch (float obj) {
+    snprintf(message, length, "%f", obj);
+  } catch (unsigned char obj) {
+    snprintf(message, length, "%u", obj);
+  } catch (unsigned short obj) {
+    snprintf(message, length, "%u", obj);
+  } catch (unsigned int obj) {
+    snprintf(message, length, "%u", obj);
+  } catch (unsigned long obj) {
+    snprintf(message, length, "%lu", obj);
+  } catch (unsigned long long obj) {
+    snprintf(message, length, "%llu", obj);
+  } catch (...) {
+    // no way to describe what this is
+  }
+}
+
+void bsg_handle_cpp_terminate() {
+  if (bsg_global_env == NULL || bsg_global_env->handling_crash)
+    return;
+
+  bsg_global_env->handling_crash = true;
+  bsg_populate_report_as(bsg_global_env);
+  bsg_global_env->next_report.exception.frame_count = bsg_unwind_stack(
+      bsg_global_env->unwind_style,
+      bsg_global_env->next_report.exception.stacktrace, NULL, NULL);
+
+  std::type_info *tinfo = __cxxabiv1::__cxa_current_exception_type();
+  if (tinfo != NULL) {
+    bsg_strncpy(bsg_global_env->next_report.exception.name,
+                (char *)tinfo->name(),
+                sizeof(bsg_global_env->next_report.exception.name));
+  }
+  size_t message_length = sizeof(bsg_global_env->next_report.exception.message);
+  char message[message_length];
+  bsg_write_current_exception_message(message, message_length);
+  bsg_strncpy(bsg_global_env->next_report.exception.message, (char *)message,
+              message_length);
+
+  bsg_serialize_report_to_file(bsg_global_env);
+  bsg_global_env->crash_handled = true;
+  bsg_handler_uninstall_cpp();
+  if (bsg_global_terminate_previous != NULL) {
+    bsg_global_terminate_previous();
+  }
+}