Skip to content

Commit

Permalink
fix: Only set stack registers / exit unwind early on 32-bit
Browse files Browse the repository at this point in the history
While the stack enhancements added in #378 are intended only to be run
on 32-bit devices and gated to the __arm__ build, when 64-bit devices
are running the 32-bit library, the stacktraces for those devices were
unintentionally affected. This is the case when an app is configured to
only run 32-bit binaries (Android cannot run both 64-bit and 32-bit
binaries at once), and many popular libraries (e.g. React Native) only
distribute 32-bit libraries.

To further improve the stack quality on 32-bit, switches the underlying
unwinder from gnu unwind to non-gnu libunwind. Improves frame resolution
and does not have the crash in unwind when setting registers.

Fixes #382
  • Loading branch information
kattrali committed Nov 7, 2018
1 parent 03f37a4 commit 1e9d8a9
Show file tree
Hide file tree
Showing 10 changed files with 657 additions and 58 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

## TBD

### Bug fixes

* [NDK] Fix regression in 4.9.0 which truncated stacktraces on 64-bit devices to
a single frame
[#383](https://github.com/bugsnag/bugsnag-android/pull/383)

## 4.9.1 (2018-11-01)

### Bug fixes
Expand Down
7 changes: 7 additions & 0 deletions ndk/src/main/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ add_library( # Specifies the name of the library.
include_directories(
jni/
jni/deps
jni/external/libunwind/include
)

target_include_directories(bugsnag-ndk PRIVATE ${BUGSNAG_DIR}/assets/include)
Expand All @@ -47,3 +48,9 @@ set_target_properties(bugsnag-ndk
COMPILE_OPTIONS
-Werror -Wall -pedantic)

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()
13 changes: 11 additions & 2 deletions ndk/src/main/java/com/bugsnag/android/ndk/NativeBridge.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ public class NativeBridge implements Observer {
private static final Lock lock = new ReentrantLock();
private static final AtomicBoolean installed = new AtomicBoolean(false);

public static native void install(String reportingDirectory, boolean autoNotify, int apiLevel);
public static native void install(String reportingDirectory, boolean autoNotify, int apiLevel,
boolean is32bit);

public static native void deliverReportAtPath(String filePath);

Expand Down Expand Up @@ -216,7 +217,15 @@ private void handleInstallMessage(Object arg) {
return;
}
String reportPath = reportDirectory + UUID.randomUUID().toString() + ".crash";
install(reportPath, true, Build.VERSION.SDK_INT);
String[] abis = (String[])NativeInterface.getDeviceData().get("cpuAbi");
boolean is32bit = true;
for (String abi : abis) {
if (abi.contains("64")) {
is32bit = false;
break;
}
}
install(reportPath, true, Build.VERSION.SDK_INT, is32bit);
installed.set(true);
} finally {
lock.unlock();
Expand Down
4 changes: 2 additions & 2 deletions ndk/src/main/jni/bugsnag_ndk.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ bsg_unwinder bsg_configured_unwind_style() {

JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install(
JNIEnv *env, jobject _this, jstring _report_path, jboolean auto_notify,
jint _api_level) {
jint _api_level, jboolean is32bit) {
bsg_environment *bugsnag_env = calloc(1, sizeof(bsg_environment));
bugsnag_env->unwind_style = bsg_get_unwind_type((int)_api_level);
bugsnag_env->unwind_style = bsg_get_unwind_type((int)_api_level, (bool)is32bit);
bugsnag_env->report_header.big_endian =
htonl(47) == 47; // potentially too clever, see man 3 htonl
bugsnag_env->report_header.version = BUGSNAG_REPORT_VERSION;
Expand Down
55 changes: 55 additions & 0 deletions ndk/src/main/jni/external/libunwind/include/__libunwind_config.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
//===------------------------- __libunwind_config.h -----------------------===//
//
// The LLVM Compiler Infrastructure
//
// This file is dual licensed under the MIT and the University of Illinois Open
// Source Licenses. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
#ifndef ____LIBUNWIND_CONFIG_H__
#define ____LIBUNWIND_CONFIG_H__
#if defined(__arm__) && !defined(__USING_SJLJ_EXCEPTIONS__) && \
!defined(__ARM_DWARF_EH__)
#define _LIBUNWIND_ARM_EHABI 1
#else
#define _LIBUNWIND_ARM_EHABI 0
#endif
#if defined(_LIBUNWIND_IS_NATIVE_ONLY)
# if defined(__i386__)
# define _LIBUNWIND_TARGET_I386 1
# define _LIBUNWIND_CONTEXT_SIZE 8
# define _LIBUNWIND_CURSOR_SIZE 19
# elif defined(__x86_64__)
# define _LIBUNWIND_TARGET_X86_64 1
# define _LIBUNWIND_CONTEXT_SIZE 21
# define _LIBUNWIND_CURSOR_SIZE 33
# elif defined(__ppc__)
# define _LIBUNWIND_TARGET_PPC 1
# define _LIBUNWIND_CONTEXT_SIZE 117
# define _LIBUNWIND_CURSOR_SIZE 128
# elif defined(__aarch64__)
# define _LIBUNWIND_TARGET_AARCH64 1
# define _LIBUNWIND_CONTEXT_SIZE 66
# define _LIBUNWIND_CURSOR_SIZE 78
# elif defined(__arm__)
# define _LIBUNWIND_TARGET_ARM 1
# define _LIBUNWIND_CONTEXT_SIZE 60
# define _LIBUNWIND_CURSOR_SIZE 67
# elif defined(__or1k__)
# define _LIBUNWIND_TARGET_OR1K 1
# define _LIBUNWIND_CONTEXT_SIZE 16
# define _LIBUNWIND_CURSOR_SIZE 28
# else
# error "Unsupported architecture."
# endif
#else // !_LIBUNWIND_IS_NATIVE_ONLY
# define _LIBUNWIND_TARGET_I386 1
# define _LIBUNWIND_TARGET_X86_64 1
# define _LIBUNWIND_TARGET_PPC 1
# define _LIBUNWIND_TARGET_AARCH64 1
# define _LIBUNWIND_TARGET_ARM 1
# define _LIBUNWIND_TARGET_OR1K 1
# define _LIBUNWIND_CONTEXT_SIZE 128
# define _LIBUNWIND_CURSOR_SIZE 140
#endif // _LIBUNWIND_IS_NATIVE_ONLY
#endif // ____LIBUNWIND_CONFIG_H__
Loading

0 comments on commit 1e9d8a9

Please sign in to comment.