Skip to content

Commit

Permalink
fix(unwind): fallback to dladdr in the crash unwinder when a stack …
Browse files Browse the repository at this point in the history
…frames `filename` or `method` cannot be located in our cached data
  • Loading branch information
lemnik committed Apr 29, 2022
1 parent 882cce9 commit 3db6a57
Show file tree
Hide file tree
Showing 11 changed files with 39 additions and 19 deletions.
22 changes: 12 additions & 10 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,18 @@ test-fixtures:
@cp features/fixtures/mazerunner/app/build/outputs/apk/release/fixture.apk build/fixture.apk
# copy the unstripped scenarios libs (for stack unwinding validation)
# grabs the newest files as the likely just built ones
LIB_CXX_BUGSNAG=$$(ls -dtr features/fixtures/mazerunner/cxx-scenarios-bugsnag/build/intermediates/cxx/RelWithDebInfo/* | head -n 1) && \
LIB_CXX=$$(ls -dtr features/fixtures/mazerunner/cxx-scenarios/build/intermediates/cxx/RelWithDebInfo/* | head -n 1) && \
cp $$LIB_CXX/obj/x86_64/libcxx-scenarios.so build/libcxx-scenarios-x86_64.so && \
cp $$LIB_CXX/obj/x86/libcxx-scenarios.so build/libcxx-scenarios-x86.so && \
cp $$LIB_CXX/obj/arm64-v8a/libcxx-scenarios.so build/libcxx-scenarios-arm64.so && \
cp $$LIB_CXX/obj/armeabi-v7a/libcxx-scenarios.so build/libcxx-scenarios-arm32.so && \
cp $$LIB_CXX_BUGSNAG/obj/x86_64/libcxx-scenarios-bugsnag.so build/libcxx-scenarios-bugsnag-x86_64.so && \
cp $$LIB_CXX_BUGSNAG/obj/x86/libcxx-scenarios-bugsnag.so build/libcxx-scenarios-bugsnag-x86.so && \
cp $$LIB_CXX_BUGSNAG/obj/arm64-v8a/libcxx-scenarios-bugsnag.so build/libcxx-scenarios-bugsnag-arm64.so && \
cp $$LIB_CXX_BUGSNAG/obj/armeabi-v7a/libcxx-scenarios-bugsnag.so build/libcxx-scenarios-bugsnag-arm32.so
LIB_CXX_BUGSNAG_64=$$(dirname `ls -dtr features/fixtures/mazerunner/cxx-scenarios-bugsnag/build/intermediates/cxx/RelWithDebInfo/*/obj/x86_64 | head -n 1`) && \
LIB_CXX_BUGSNAG_32=$$(dirname `ls -dtr features/fixtures/mazerunner/cxx-scenarios-bugsnag/build/intermediates/cxx/RelWithDebInfo/*/obj/armeabi-v7a | head -n 1`) && \
LIB_CXX_64=$$(dirname `ls -dtr features/fixtures/mazerunner/cxx-scenarios/build/intermediates/cxx/RelWithDebInfo/*/obj/x86_64 | head -n 1`) && \
LIB_CXX_32=$$(dirname `ls -dtr features/fixtures/mazerunner/cxx-scenarios/build/intermediates/cxx/RelWithDebInfo/*/obj/armeabi-v7a | head -n 1`) && \
cp $$LIB_CXX_64/x86_64/libcxx-scenarios.so build/libcxx-scenarios-x86_64.so && \
cp $$LIB_CXX_32/x86/libcxx-scenarios.so build/libcxx-scenarios-x86.so && \
cp $$LIB_CXX_64/arm64-v8a/libcxx-scenarios.so build/libcxx-scenarios-arm64.so && \
cp $$LIB_CXX_32/armeabi-v7a/libcxx-scenarios.so build/libcxx-scenarios-arm32.so && \
cp $$LIB_CXX_BUGSNAG_64/x86_64/libcxx-scenarios-bugsnag.so build/libcxx-scenarios-bugsnag-x86_64.so && \
cp $$LIB_CXX_BUGSNAG_32/x86/libcxx-scenarios-bugsnag.so build/libcxx-scenarios-bugsnag-x86.so && \
cp $$LIB_CXX_BUGSNAG_64/arm64-v8a/libcxx-scenarios-bugsnag.so build/libcxx-scenarios-bugsnag-arm64.so && \
cp $$LIB_CXX_BUGSNAG_32/armeabi-v7a/libcxx-scenarios-bugsnag.so build/libcxx-scenarios-bugsnag-arm32.so

# And the minimal (no NDK or ANR plugin) test fixture
@./gradlew -PMINIMAL_FIXTURE=true -PTEST_FIXTURE_NAME=fixture-minimal.apk -p=features/fixtures/mazerunner/ assembleRelease -x check
Expand Down
18 changes: 18 additions & 0 deletions bugsnag-plugin-android-ndk/src/main/jni/utils/stack_unwinder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "string.h"

#include <dlfcn.h>
#include <unwindstack/LocalUnwinder.h>
#include <unwindstack/Maps.h>
#include <unwindstack/MemoryLocal.h>
Expand Down Expand Up @@ -73,6 +74,23 @@ ssize_t bsg_unwind_crash_stack(bugsnag_stackframe stack[BUGSNAG_FRAMES_MAX],
sizeof(stack[frame_count].filename));
bsg_strncpy(stack[frame_count].method, frame.function_name.c_str(),
sizeof(stack[frame_count].method));

// if the filename or method name cannot be found - try fallback to dladdr
// to find them
static Dl_info info;
if ((frame.map_name.empty() || frame.function_name.empty()) &&
dladdr((void *)frame.pc, &info) != 0) {

if (info.dli_fname != nullptr) {
bsg_strncpy(stack[frame_count].filename, (char *)info.dli_fname,
sizeof(stack[frame_count].filename));
}
if (info.dli_sname != nullptr) {
bsg_strncpy(stack[frame_count].method, (char *)info.dli_sname,
sizeof(stack[frame_count].method));
}
}

frame_count++;
}
unwinding_crash_stack = false;
Expand Down
9 changes: 3 additions & 6 deletions features/fixtures/mazerunner/cxx-scenarios/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,8 @@ add_library(cxx-scenarios SHARED
src/main/cpp/bugsnag-java-scenarios.cpp)

set_target_properties(cxx-scenarios PROPERTIES
CXX_STANDARD 11
CXX_STANDARD_REQUIRED YES)
CXX_STANDARD 11
CXX_STANDARD_REQUIRED YES)

add_library(lib_monochrome SHARED IMPORTED)
set_target_properties(lib_monochrome PROPERTIES IMPORTED_LOCATION
${CMAKE_SOURCE_DIR}/libs/${ANDROID_ABI}/libmonochrome.so)
target_link_libraries(cxx-scenarios lib_monochrome)
target_link_libraries(cxx-scenarios)

Empty file.
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
#include <jni.h>
#include <cstdio>
#include <dlfcn.h>

extern "C" {
// defined in libs/[ABI]/libmonochrome.so
int something_innocuous(int input);
int (*something_innocuous)(int input);

JNIEXPORT int JNICALL
Java_com_bugsnag_android_mazerunner_scenarios_CXXExternalStackElementScenario_crash(
JNIEnv *env, jobject instance, jint counter) {
void *monochrome = dlopen("libmonochrome.so", RTLD_GLOBAL);
*(void**)(&something_innocuous) = dlsym(monochrome, "something_innocuous");

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

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

Expand Down
2 changes: 1 addition & 1 deletion features/full_tests/native_crash_handling.feature
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ 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 | 14 |
| Java_com_bugsnag_android_mazerunner_scenarios_CXXExternalStackElementScenario_crash | CXXExternalStackElementScenario.cpp | 18 |

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

0 comments on commit 3db6a57

Please sign in to comment.