From eb27e2214315c28fceaf8a4839f39c894060bd96 Mon Sep 17 00:00:00 2001 From: aromanov Date: Tue, 11 Jul 2017 11:50:04 -0700 Subject: [PATCH 1/6] Updated *.mk files to build with a recent Breakpad. See also: https://chromium.googlesource.com/breakpad/breakpad/+/master/README.md --- jni/Android.mk | 2 +- jni/Application.mk | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/jni/Android.mk b/jni/Android.mk index 8b42669..3bdb741 100644 --- a/jni/Android.mk +++ b/jni/Android.mk @@ -14,5 +14,5 @@ include $(BUILD_SHARED_LIBRARY) ifneq ($(NDK_MODULE_PATH),) $(call import-module,google_breakpad) else - include $(LOCAL_PATH)/../../breakpad/android/google_breakpad/Android.mk + include $(LOCAL_PATH)/../../breakpad/src/android/google_breakpad/Android.mk endif diff --git a/jni/Application.mk b/jni/Application.mk index dd83bdc..1ecaefb 100644 --- a/jni/Application.mk +++ b/jni/Application.mk @@ -1,2 +1,4 @@ APP_STL := stlport_static APP_ABI := armeabi armeabi-v7a +APP_CFLAGS += -std=c++11 -D__STDC_LIMIT_MACROS + From 663e858773d408f24c433cd705526500f84c81d3 Mon Sep 17 00:00:00 2001 From: aromanov Date: Tue, 11 Jul 2017 11:53:08 -0700 Subject: [PATCH 2/6] Simulate a nontrivial crash worth investigation (stack frames and locals) --- jni/native.cpp | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/jni/native.cpp b/jni/native.cpp index 7141f8f..b9e9bb3 100644 --- a/jni/native.cpp +++ b/jni/native.cpp @@ -11,9 +11,36 @@ bool DumpCallback(const google_breakpad::MinidumpDescriptor& descriptor, return succeeded; } +void swap(int*& left, int*& right) { + int*tmp = left; + left = right; + right = tmp; +} + +void bubble(int** first, int** last) { + if (first < last) { + bubble(&first[1], last); + while (first < last) { + if (*first[0] > *first[1]) { + swap(first[0], first[1]); + } + ++first; + } + } +} + void Crash() { - volatile int* a = reinterpret_cast(NULL); - *a = 1; + const int count = 10; + const int last = count - 1; + int values[count] = { 12, 34, 5, 7, 218, 923, -1, 0, -1, 5}; + int* refs[count]; + memset(&refs, 0, sizeof(refs)); + // let's introduce an error here and skip the base index (0) + for (int i = 1; i < count; ++i) { + refs[i] = &values[i]; + } + // now let's do a bubble sort + bubble(&refs[0], &refs[last]); } extern "C" { From 032d551dc623a9617bf10c90b22362097bb29f16 Mon Sep 17 00:00:00 2001 From: aromanov Date: Tue, 11 Jul 2017 13:01:35 -0700 Subject: [PATCH 3/6] Separate crash simulation and breakpad wrappers --- jni/Android.mk | 2 +- jni/breakpad.cpp | 17 +++++++++++++++++ jni/breakpad.h | 6 ++++++ jni/crash.cpp | 34 +++++++++++++++++++++++++++++++++ jni/crash.h | 7 +++++++ jni/native.cpp | 49 ++++-------------------------------------------- 6 files changed, 69 insertions(+), 46 deletions(-) create mode 100644 jni/breakpad.cpp create mode 100644 jni/breakpad.h create mode 100644 jni/crash.cpp create mode 100644 jni/crash.h diff --git a/jni/Android.mk b/jni/Android.mk index 3bdb741..4c27bb8 100644 --- a/jni/Android.mk +++ b/jni/Android.mk @@ -4,7 +4,7 @@ include $(CLEAR_VARS) LOCAL_LDLIBS := -L$(SYSROOT)/usr/lib -llog LOCAL_MODULE := native -LOCAL_SRC_FILES := native.cpp +LOCAL_SRC_FILES := native.cpp breakpad.cpp crash.cpp LOCAL_STATIC_LIBRARIES += breakpad_client include $(BUILD_SHARED_LIBRARY) diff --git a/jni/breakpad.cpp b/jni/breakpad.cpp new file mode 100644 index 0000000..fe5cb87 --- /dev/null +++ b/jni/breakpad.cpp @@ -0,0 +1,17 @@ +#include "breakpad.h" +#include "client/linux/handler/exception_handler.h" +#include "client/linux/handler/minidump_descriptor.h" +#include + +static google_breakpad::ExceptionHandler* exceptionHandler; +bool DumpCallback(const google_breakpad::MinidumpDescriptor& descriptor, + void* context, + bool succeeded) { + printf("Dump path: %s\n", descriptor.path()); + return succeeded; +} + +void setUpBreakpad(const char* path) { + google_breakpad::MinidumpDescriptor descriptor(path); + exceptionHandler = new google_breakpad::ExceptionHandler(descriptor, NULL, DumpCallback, NULL, true, -1); +} diff --git a/jni/breakpad.h b/jni/breakpad.h new file mode 100644 index 0000000..8bc7966 --- /dev/null +++ b/jni/breakpad.h @@ -0,0 +1,6 @@ +#ifndef __BREAKPAD_H +#define __BREAKPAD_H + +void setUpBreakpad(const char* path); + +#endif /*__BREAKPAD_H*/ \ No newline at end of file diff --git a/jni/crash.cpp b/jni/crash.cpp new file mode 100644 index 0000000..6f9e9a4 --- /dev/null +++ b/jni/crash.cpp @@ -0,0 +1,34 @@ +#include "crash.h" +#include + +void swap(int*& left, int*& right) { + int*tmp = left; + left = right; + right = tmp; +} + +void bubble(int** first, int** last) { + if (first < last) { + bubble(&first[1], last); + while (first < last) { + if (*first[0] > *first[1]) { + swap(first[0], first[1]); + } + ++first; + } + } +} + +void CrashWithStack() { + const int count = 10; + const int last = count - 1; + int values[count] = { 12, 34, 5, 7, 218, 923, -1, 0, -1, 5}; + int* refs[count]; + memset(&refs, 0, sizeof(refs)); + // let's introduce an error here and skip the base index (0) + for (int i = 1; i < count; ++i) { + refs[i] = &values[i]; + } + // now let's do a bubble sort + bubble(&refs[0], &refs[last]); +} \ No newline at end of file diff --git a/jni/crash.h b/jni/crash.h new file mode 100644 index 0000000..76c7ce0 --- /dev/null +++ b/jni/crash.h @@ -0,0 +1,7 @@ +#ifndef __CRASH_H +#define __CRASH_H + +void CrashWithStack(); +void CrashWithHeap(); + +#endif /*__CRASH_H*/ \ No newline at end of file diff --git a/jni/native.cpp b/jni/native.cpp index b9e9bb3..f070293 100644 --- a/jni/native.cpp +++ b/jni/native.cpp @@ -1,58 +1,17 @@ #include -#include -#include "client/linux/handler/exception_handler.h" -#include "client/linux/handler/minidump_descriptor.h" -static google_breakpad::ExceptionHandler* exceptionHandler; -bool DumpCallback(const google_breakpad::MinidumpDescriptor& descriptor, - void* context, - bool succeeded) { - printf("Dump path: %s\n", descriptor.path()); - return succeeded; -} - -void swap(int*& left, int*& right) { - int*tmp = left; - left = right; - right = tmp; -} - -void bubble(int** first, int** last) { - if (first < last) { - bubble(&first[1], last); - while (first < last) { - if (*first[0] > *first[1]) { - swap(first[0], first[1]); - } - ++first; - } - } -} - -void Crash() { - const int count = 10; - const int last = count - 1; - int values[count] = { 12, 34, 5, 7, 218, 923, -1, 0, -1, 5}; - int* refs[count]; - memset(&refs, 0, sizeof(refs)); - // let's introduce an error here and skip the base index (0) - for (int i = 1; i < count; ++i) { - refs[i] = &values[i]; - } - // now let's do a bubble sort - bubble(&refs[0], &refs[last]); -} +#include "breakpad.h" +#include "crash.h" extern "C" { void Java_com_hockeyapp_breakapp_MainActivity_setUpBreakpad(JNIEnv* env, jobject obj, jstring filepath) { const char *path = env->GetStringUTFChars(filepath, 0); - google_breakpad::MinidumpDescriptor descriptor(path); - exceptionHandler = new google_breakpad::ExceptionHandler(descriptor, NULL, DumpCallback, NULL, true, -1); + setUpBreakpad(path); } void Java_com_hockeyapp_breakapp_MainActivity_nativeCrash(JNIEnv* env, jobject obj) { - Crash(); + CrashWithStack(); } } From 50c1bab03af5c4d125791c8af483f1e857826b0c Mon Sep 17 00:00:00 2001 From: aromanov Date: Tue, 11 Jul 2017 13:06:14 -0700 Subject: [PATCH 4/6] JNI_OnLoad --- jni/native.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jni/native.cpp b/jni/native.cpp index f070293..639e861 100644 --- a/jni/native.cpp +++ b/jni/native.cpp @@ -14,4 +14,7 @@ void Java_com_hockeyapp_breakapp_MainActivity_nativeCrash(JNIEnv* env, jobject o CrashWithStack(); } +jint JNI_OnLoad(JavaVM* vm, void* reserved) { + return JNI_VERSION_1_6; +} } From 9782a5b0289405648bdf8218f53c711ed1d0f233 Mon Sep 17 00:00:00 2001 From: aromanov Date: Tue, 11 Jul 2017 13:21:14 -0700 Subject: [PATCH 5/6] Allow "caterpillar" memory collection (crashed stack + immediately referenced pages) --- jni/breakpad.cpp | 166 ++++++++++++++++++- jni/breakpad.h | 2 +- jni/native.cpp | 4 +- src/com/hockeyapp/breakapp/MainActivity.java | 4 +- 4 files changed, 168 insertions(+), 8 deletions(-) diff --git a/jni/breakpad.cpp b/jni/breakpad.cpp index fe5cb87..9ab4b31 100644 --- a/jni/breakpad.cpp +++ b/jni/breakpad.cpp @@ -1,9 +1,152 @@ #include "breakpad.h" +#include +#include +#include +#include + +#define private public #include "client/linux/handler/exception_handler.h" #include "client/linux/handler/minidump_descriptor.h" -#include +#include "client/linux/dump_writer_common/ucontext_reader.h" +// static exception handler static google_breakpad::ExceptionHandler* exceptionHandler; + +// these values are compilation time constants +const size_t pt_size = sizeof(void*); +const size_t STACK_PAGES = 2; +const size_t BYTES_PRIOR = 256; + +// these statics are queried at run time but the queries are idempotent (the same results expected each time) +static size_t page_size; +static size_t page_half; +static size_t mem_count; + +// this only needs to be used once +static uintptr_t* sortedBuf = NULL; + +// this one is a dummy marker. whatever equals it may be discarded. +static google_breakpad::AppMemory dummyChunk; + +inline uintptr_t ToPage(uintptr_t address) { + return address & (-page_size); +} + +inline bool IsPageMapped(void* page_address) { + unsigned char vector; + return 0 /*OK*/ == mincore(page_address, page_size, &vector); +} + +inline bool IsPageMapped(uintptr_t page_address) { + return IsPageMapped(reinterpret_cast(page_address)); +} + +inline bool IsAddressMapped(uintptr_t address) { + return IsPageMapped(ToPage(address)); +} + +void EnsureBuf() { + if (sortedBuf == NULL) { + size_t sizeOfBuf = page_size * STACK_PAGES; + // could be malloc(sizeOfBuf) but I preferred a dedicated mapping + sortedBuf = reinterpret_cast(mmap(NULL, sizeOfBuf, + PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)); + // assert sortedBuf != null + } +} + +class HeapContext { +private: + google_breakpad::AppMemoryList* mList; + +public: + void Attach(google_breakpad::ExceptionHandler * eh) { + google_breakpad::AppMemoryList& memoryList = eh->app_memory_list_; + // preallocate mem_count dummy chunks + for (int i = 0; i < mem_count; ++i) { + memoryList.push_back(dummyChunk); + } + mList = &memoryList; + } + + void TryIncludePage(google_breakpad::AppMemoryList::reverse_iterator& chunk, bool allowMerge, uintptr_t& fault, uintptr_t page_address) { + if (page_address != fault) { + // try merge + if (allowMerge) { + uintptr_t last_address = reinterpret_cast(chunk->ptr) + chunk->length; + if (page_address >= last_address) { + if (IsPageMapped(page_address)) { + if (page_address > last_address) { + --chunk; + chunk->ptr = reinterpret_cast(page_address); + chunk->length = page_size; + } else { + chunk->length += page_size; + } + } else { + fault = page_address; + } + } + } else { + // map, then create chunk + if (IsPageMapped(page_address)) { + --chunk; + chunk->ptr = reinterpret_cast(page_address); + chunk->length = page_size; + } else { + fault = page_address; + } + } + } + } + + void CollectData(uintptr_t stackTop, uintptr_t stackSize) { + memcpy(sortedBuf, reinterpret_cast(stackTop), stackSize); + uintptr_t ptr_count = stackSize / pt_size; + uintptr_t* sortedPtr = sortedBuf; + uintptr_t* sortedEnd = sortedBuf + ptr_count; + std::sort(sortedPtr, sortedEnd); // qsort + + google_breakpad::AppMemoryList::reverse_iterator start = mList->rend(); + google_breakpad::AppMemoryList::reverse_iterator chunk = start; + uintptr_t fault = 0xffffffff; + while (sortedPtr != sortedEnd) { + uintptr_t stackAddr = *(sortedPtr++); + if (stackAddr >= BYTES_PRIOR) { + stackAddr -= BYTES_PRIOR; + } + uintptr_t stackPage = ToPage(stackAddr); + TryIncludePage(chunk, chunk != start, fault, stackPage); + if (stackAddr > stackPage + page_half) { + TryIncludePage(chunk, chunk != start, fault, stackPage + page_size); + } + } + } +}; + +bool HeapCallback(const void* crash_context, size_t crash_context_size, void* context) { + // << We have a different source of information for the crashing thread.>> -- Google + // * google-breakpad/src/client/linux/minidump_writer/minidump_writer.cc:322 + + google_breakpad::ExceptionHandler::CrashContext* cc = (google_breakpad::ExceptionHandler::CrashContext*) crash_context; + uintptr_t stackTop = ToPage(google_breakpad::UContextReader::GetStackPointer(&cc->context)); + + // http://man7.org/linux/man-pages/man2/mincore.2.html + if (IsPageMapped(stackTop)) { + uintptr_t stackSize = page_size; + + // extra page. keep in sync with STACK_PAGES + if (IsPageMapped(stackTop + page_size)) { + stackSize += page_size; + } + + HeapContext* userContext = reinterpret_cast(context); + userContext->CollectData(stackTop, stackSize); + } + + return false; // we haven't handled - only prepared the handling +} + bool DumpCallback(const google_breakpad::MinidumpDescriptor& descriptor, void* context, bool succeeded) { @@ -11,7 +154,24 @@ bool DumpCallback(const google_breakpad::MinidumpDescriptor& descriptor, return succeeded; } -void setUpBreakpad(const char* path) { +void setUpBreakpad(const char* path, bool moreDump) { google_breakpad::MinidumpDescriptor descriptor(path); - exceptionHandler = new google_breakpad::ExceptionHandler(descriptor, NULL, DumpCallback, NULL, true, -1); + // http://man7.org/linux/man-pages/man3/sysconf.3.html + page_size = sysconf(_SC_PAGESIZE); + page_half = page_size >> 1; + mem_count = page_size * STACK_PAGES / pt_size; + + dummyChunk.ptr = &dummyChunk; + dummyChunk.length = 0; + + // signal handler registration is an EH constructor side effect. upon exit, we are registered + if (moreDump) { + HeapContext* userContext = new HeapContext; + exceptionHandler = new google_breakpad::ExceptionHandler(descriptor, NULL, DumpCallback, userContext, true, -1); + exceptionHandler->set_crash_handler(HeapCallback); + userContext->Attach(exceptionHandler); + EnsureBuf(); + } else { + exceptionHandler = new google_breakpad::ExceptionHandler(descriptor, NULL, DumpCallback, NULL, true, -1); + } } diff --git a/jni/breakpad.h b/jni/breakpad.h index 8bc7966..81218bb 100644 --- a/jni/breakpad.h +++ b/jni/breakpad.h @@ -1,6 +1,6 @@ #ifndef __BREAKPAD_H #define __BREAKPAD_H -void setUpBreakpad(const char* path); +void setUpBreakpad(const char* path, bool moreDump); #endif /*__BREAKPAD_H*/ \ No newline at end of file diff --git a/jni/native.cpp b/jni/native.cpp index 639e861..b7f9514 100644 --- a/jni/native.cpp +++ b/jni/native.cpp @@ -5,9 +5,9 @@ extern "C" { -void Java_com_hockeyapp_breakapp_MainActivity_setUpBreakpad(JNIEnv* env, jobject obj, jstring filepath) { +void Java_com_hockeyapp_breakapp_MainActivity_setUpBreakpad(JNIEnv* env, jobject obj, jstring filepath, jboolean moreDump) { const char *path = env->GetStringUTFChars(filepath, 0); - setUpBreakpad(path); + setUpBreakpad(path, moreDump); } void Java_com_hockeyapp_breakapp_MainActivity_nativeCrash(JNIEnv* env, jobject obj) { diff --git a/src/com/hockeyapp/breakapp/MainActivity.java b/src/com/hockeyapp/breakapp/MainActivity.java index efb53bb..5114936 100644 --- a/src/com/hockeyapp/breakapp/MainActivity.java +++ b/src/com/hockeyapp/breakapp/MainActivity.java @@ -12,7 +12,7 @@ public class MainActivity extends Activity { private static final String HOCKEYAPP_ID = "98254247ac79b7cd96dbec27c53b7c9f"; - private native void setUpBreakpad(String filepath); + private native void setUpBreakpad(String filepath, boolean moreDump); private native void nativeCrash(); static { @@ -25,7 +25,7 @@ protected void onCreate(Bundle savedInstanceState) { setContentView(R.layout.activity_main); Constants.loadFromContext(this); - setUpBreakpad(Constants.FILES_PATH); + setUpBreakpad(Constants.FILES_PATH, true); NativeCrashManager.handleDumpFiles(this, HOCKEYAPP_ID); } From f976e0ea32c1a15a45553e9e317043f0df434d18 Mon Sep 17 00:00:00 2001 From: aromanov Date: Tue, 11 Jul 2017 13:57:14 -0700 Subject: [PATCH 6/6] Simulate an off-stack crash to demonstrate "moredump" reporting --- jni/crash.cpp | 21 +++++++++++++++++++- jni/native.cpp | 8 ++++++-- res/layout/activity_main.xml | 16 ++++++++++++++- src/com/hockeyapp/breakapp/MainActivity.java | 8 ++++++-- 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/jni/crash.cpp b/jni/crash.cpp index 6f9e9a4..4ab1dd8 100644 --- a/jni/crash.cpp +++ b/jni/crash.cpp @@ -1,5 +1,7 @@ #include "crash.h" #include +#include +#include void swap(int*& left, int*& right) { int*tmp = left; @@ -31,4 +33,21 @@ void CrashWithStack() { } // now let's do a bubble sort bubble(&refs[0], &refs[last]); -} \ No newline at end of file +} + +bool compare(int* left, int* right) { + return *left < *right; +} + +void CrashWithHeap() { + const int count = 10; + const int last = count - 1; + int values[count] = { 12, 34, 5, 7, 218, 923, -1, 0, -1, 5}; + std::vector refs(count); + // let's introduce an error here and skip the base index (0) + for (int i = 1; i < count; ++i) { + refs[i] = &values[i]; + } + // now let's do a sort with a custom comparator + std::sort(refs.begin(), refs.end(), compare); +} diff --git a/jni/native.cpp b/jni/native.cpp index b7f9514..42b245e 100644 --- a/jni/native.cpp +++ b/jni/native.cpp @@ -10,8 +10,12 @@ void Java_com_hockeyapp_breakapp_MainActivity_setUpBreakpad(JNIEnv* env, jobject setUpBreakpad(path, moreDump); } -void Java_com_hockeyapp_breakapp_MainActivity_nativeCrash(JNIEnv* env, jobject obj) { - CrashWithStack(); +void Java_com_hockeyapp_breakapp_MainActivity_nativeCrash(JNIEnv* env, jobject obj, jboolean withHeap) { + if (withHeap) { + CrashWithHeap(); + } else { + CrashWithStack(); + } } jint JNI_OnLoad(JavaVM* vm, void* reserved) { diff --git a/res/layout/activity_main.xml b/res/layout/activity_main.xml index cd91b79..f0f7ace 100644 --- a/res/layout/activity_main.xml +++ b/res/layout/activity_main.xml @@ -100,7 +100,21 @@ android:layout_marginBottom="10dip" android:layout_marginTop="0dp" android:onClick="onNativeCrashClicked" - android:text="C++ Crash" /> + android:text="C++ Crash (stack)" /> + +