Skip to content

Commit

Permalink
feat(ndk): add bugsnag_refresh_symbol_table to force a reparse of d…
Browse files Browse the repository at this point in the history
…ynamically linked modules
  • Loading branch information
lemnik committed Aug 10, 2022
1 parent 1441f8b commit 3ca097b
Show file tree
Hide file tree
Showing 17 changed files with 74 additions and 5 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

## TBD

### Enhancements

* Introduced `bugsnag_refresh_symbol_table` and `BugsnagNDK.refreshSymbolTable` to allow NDK apps to force a refresh of cached
debug information used during a native crash. This new API is only applicable if you are using `dlopen` or `System.loadLibrary`
after startup, and experiencing native crashes with missing symbols.
[#1731](/~https://github.com/bugsnag/bugsnag-android/pull/1731)

### Bug fixes

* Non-List Collections are now correctly handled as OPAQUE values for NDK metadata
Expand Down
8 changes: 8 additions & 0 deletions bugsnag-plugin-android-ndk/src/main/assets/include/bugsnag.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ void bugsnag_add_on_error(bsg_on_error on_error);
*/
void bugsnag_remove_on_error();

/**
* Refresh cached symbol tables within Bugsnag. This can be used to force a
* refresh of the cached symbols used during stack unwinding. This is only
* useful if you are using `System.loadLibrary` or `dlopen` after your
* application startup is complete.
*/
void bugsnag_refresh_symbol_table();

#ifdef __cplusplus
}
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ internal class NdkPlugin : Plugin {

private external fun getBinaryArch(): String

private var nativeBridge: NativeBridge? = null
private var client: Client? = null

var nativeBridge: NativeBridge? = null
private set

private fun initNativeBridge(client: Client): NativeBridge {
val nativeBridge = NativeBridge()
client.addObserver(nativeBridge)
Expand Down Expand Up @@ -72,3 +74,6 @@ internal class NdkPlugin : Plugin {
return 0
}
}

internal val Client.ndkPlugin: NdkPlugin?
get() = getPlugin(NdkPlugin::class.java) as NdkPlugin?
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.bugsnag.android.ndk

import com.bugsnag.android.Bugsnag
import com.bugsnag.android.ndkPlugin

object BugsnagNDK {
@JvmStatic
fun refreshSymbolTable() {
if (Bugsnag.isStarted()) {
val client = Bugsnag.getClient()
client.ndkPlugin?.nativeBridge?.refreshSymbolTable()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class NativeBridge : StateObserver {
external fun addFeatureFlag(name: String, variant: String?)
external fun clearFeatureFlag(name: String)
external fun clearFeatureFlags()
external fun refreshSymbolTable()

override fun onStateChange(event: StateEvent) {
if (isInvalidMessage(event)) return
Expand Down
12 changes: 12 additions & 0 deletions bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ void bugsnag_remove_on_error() {
}
}

void bugsnag_refresh_symbol_table() { bsg_unwinder_refresh(); }

bool bsg_run_on_error() {
bsg_on_error on_error = bsg_global_env->on_error;
if (on_error != NULL) {
Expand Down Expand Up @@ -495,6 +497,10 @@ Java_com_bugsnag_android_ndk_NativeBridge_updateIsLaunching(
bugsnag_app_set_is_launching(&bsg_global_env->next_event, new_value);
bsg_update_next_run_info(bsg_global_env);
release_env_write_lock();

if (!new_value) {
bugsnag_refresh_symbol_table();
}
}

JNIEXPORT void JNICALL
Expand Down Expand Up @@ -806,6 +812,12 @@ Java_com_bugsnag_android_ndk_NativeBridge_clearFeatureFlags(JNIEnv *env,
release_env_write_lock();
}

JNIEXPORT void JNICALL
Java_com_bugsnag_android_ndk_NativeBridge_refreshSymbolTable(JNIEnv *env,
jobject thiz) {
bugsnag_refresh_symbol_table();
}

#ifdef __cplusplus
}
#endif
14 changes: 13 additions & 1 deletion bugsnag-plugin-android-ndk/src/main/jni/utils/stack_unwinder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void bsg_unwinder_init() {
}
attempted_init = true;

auto crash_time_maps = new unwindstack::LocalMaps();
auto crash_time_maps = new unwindstack::LocalUpdatableMaps();
if (crash_time_maps->Parse()) {
std::shared_ptr<unwindstack::Memory> crash_time_memory(
new unwindstack::MemoryLocal);
Expand All @@ -73,6 +73,18 @@ void bsg_unwinder_init() {
}
}

void bsg_unwinder_refresh(void) {
if (crash_time_unwinder == nullptr) {
return;
}

auto *crash_time_maps = dynamic_cast<unwindstack::LocalUpdatableMaps *>(
crash_time_unwinder->GetMaps());
if (crash_time_maps != nullptr) {
crash_time_maps->Reparse(nullptr);
}
}

ssize_t bsg_unwind_crash_stack(bugsnag_stackframe stack[BUGSNAG_FRAMES_MAX],
siginfo_t *info, void *user_context) {
if (crash_time_unwinder == nullptr || unwinding_crash_stack) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ extern "C" {
*/
void bsg_unwinder_init(void);

/**
* Refresh the stack unwinder. This can be called to force a refresh of any
* cached data within the unwinder.
*/
void bsg_unwinder_refresh(void);

/**
* Unwind a stack in a terminating context. If info and a user context pointer
* are provided, the exception stack will be walked. Otherwise, the current
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ cmake_minimum_required(VERSION 3.4.1)

add_library(cxx-scenarios-bugsnag SHARED
src/main/cpp/cxx-scenarios-bugsnag.cpp
src/main/cpp/CXXExternalStackElementScenario.cpp
src/main/cpp/CXXExceptionSmokeScenario.cpp)

set_target_properties(cxx-scenarios-bugsnag PROPERTIES
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <jni.h>
#include <cstdio>
#include <dlfcn.h>
#include <bugsnag.h>

extern "C" {
// defined in libs/[ABI]/libmonochrome.so
Expand All @@ -10,6 +11,7 @@ JNIEXPORT int JNICALL
Java_com_bugsnag_android_mazerunner_scenarios_CXXExternalStackElementScenario_crash(
JNIEnv *env, jobject instance, jint counter) {
void *monochrome = dlopen("libmonochrome.so", RTLD_GLOBAL);
bugsnag_refresh_symbol_table();
*(void**)(&something_innocuous) = dlsym(monochrome, "something_innocuous");

printf("Captain, why are we out here chasing comets?\n%d\n", counter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
public class CXXExternalStackElementScenario extends Scenario {

static {
System.loadLibrary("cxx-scenarios");
System.loadLibrary("bugsnag-ndk");
System.loadLibrary("cxx-scenarios-bugsnag");
}

public native void crash(int counter);
Expand Down
1 change: 0 additions & 1 deletion features/fixtures/mazerunner/cxx-scenarios/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ add_library(cxx-scenarios SHARED
src/main/cpp/CXXAbortScenario.cpp
src/main/cpp/CXXCallNullFunctionPointerScenario.cpp
src/main/cpp/CXXDereferenceNullScenario.cpp
src/main/cpp/CXXExternalStackElementScenario.cpp
src/main/cpp/CXXImproperTypecastScenario.cpp
src/main/cpp/CXXInvalidRethrow.cpp
src/main/cpp/CXXStackoverflowScenario.cpp
Expand Down
3 changes: 2 additions & 1 deletion features/full_tests/native_crash_handling.feature
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ Feature: Native crash reporting
And the exception "type" equals "c"
And the first significant stack frames match:
| something_innocuous | libmonochrome.so | (ignore) |
| Java_com_bugsnag_android_mazerunner_scenarios_CXXExternalStackElementScenario_crash | CXXExternalStackElementScenario.cpp | 18 |
| Java_com_bugsnag_android_mazerunner_scenarios_CXXExternalStackElementScenario_crash | CXXExternalStackElementScenario.cpp | 20 |
And the "codeIdentifier" of stack frame 0 is not null

Scenario: Call null function pointer
A null pointer should be the first element of a stack trace,
Expand Down

0 comments on commit 3ca097b

Please sign in to comment.