Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Native crashes caused by calls to LOG_ALWAYS_FATAL have details missing in the reports #2121

Closed
zacharee opened this issue Dec 27, 2024 · 3 comments
Labels
needs discussion Requires internal analysis/discussion

Comments

@zacharee
Copy link

zacharee commented Dec 27, 2024

Describe the bug

I've noticed a few native crashes in my app reported on the Bugsnag dashboard with the SIGABRT error code. Looking at the stack trace, the app is killed because some native code made a call to LOG_ALWAYS_FATAL (which is a convenience wrapper for __android_log_assert), which terminates the process once it logs whatever message it was given. Unfortunately, when Bugsnag reports the error, it only has the stack trace and the final error code, and not the actual message that was passed to LOG_ALWAYS_FATAL. This makes it difficult to know what could be causing the issue leading to the process termination.

Example stacktrace:

Steps to reproduce

I'm not sure how to reproduce a real error, but calling __android_log_assert with a message should be a way to reproduce that the message isn't available in the Bugsnag dashboard.

Environment

  • Android version: Any
  • Bugsnag version: 6.10.0
  • Emulator or physical device: Physical
Example stacktrace:

There should be a message similar to Failed to set damage region on surface %p, error=%s attached here.
https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/libs/hwui/renderthread/EglManager.cpp;l=610?q=eglmanager.cpp

SIGABRT: Abort program

0  libc.so +0x534c4     abort
1  libart.so +0x930918  art::Runtime::Abort(char const*)
2  libbase.so +0x160f8  android::base::SetAborter(std::__1::function<void (char const*)>&&)::$_0::__invoke(char const*)
3  liblog.so +0x6f10    __android_log_assert
4  libhwui.so +0x288584 android::uirenderer::renderthread::EglManager::damageFrame(android::uirenderer::renderthread::Frame const&, SkRect const&)
5  libhwui.so +0x27c85c _ZN7android10uirenderer12skiapipeline18SkiaOpenGLPipeline4drawERKNS0_12renderthread5FrameERK6SkRectS9_RKNS0_13LightGeometryEPNS0_16LayerUpdateQueueERKNS0_4RectEbRKNS0_9LightInfoERKNSt3__16vectorINS_2spINS0_10RenderNodeEEENSL_9allocatorISP_EEEEPNS0_19Frame
6  libhwui.so +0x283b68 android::uirenderer::renderthread::CanvasContext::draw()
7  libhwui.so +0x2865d0 std::__1::__function::__func<android::uirenderer::renderthread::DrawFrameTask::postAndWait()::$_0, std::__1::allocator<android::uirenderer::renderthread::DrawFrameTask::postAndWait()::$_0>, void ()>::operator()() [clone .c1671e787f244890c877724752face20]
8  libhwui.so +0x2765b0 android::uirenderer::WorkQueue::process()
9  libhwui.so +0x296a68 android::uirenderer::renderthread::RenderThread::threadLoop()
10 libutils.so +0x1343c android::Thread::_threadLoop(void*)
11 libc.so +0xc1b48     __pthread_start(void*)
12 libc.so +0x54cfc     __start_thread
@agrzegorzewski
Copy link

Hi @zacharee

Thanks for raising this.

The __android_log_assert function aborts the program by raising SIGABRT using abort(). From our initial investigation, it appears that the log message may not be included in the error visible on BugSnag, as it’s not possible to pass parameters to abort.

We are discussing this further internally, and we’ll make sure to post any updates on this thread.

As a temporary workaround, you can manually report errors to BugSnag by setting a custom aborter using __android_set_log_aborter:<https://developer.android.com/ndk/reference/group/logging#__android_log_set_aborter >

Below is an example implementation of a custom aborter function. This is not an official implementation. You can use it when setting up a custom aborter function, and while this solution appears to be robust, we can’t guarantee it is 100% reliable.

Please note that you have to add the on_error callback when aborting to not catch the SIGABRT that's being raised after, as this would give 2 events in BugSnag. This callback is added right before the program aborts through the __android_log_assert function, so it does not modify BugSnag’s behavior otherwise.

#include <jni.h>
#include <android/log.h>
#include "bugsnag.h"
#include <cstdlib>
#include <cstring>

bool bugsnag_on_error(void *event) {
    char* error_class = bugsnag_error_get_error_class(event);
    if (strcmp(error_class, "SIGABRT") == 0) {
        return false;
    }
}

void aborter(const char *abort_message) {
    bugsnag_notify("NativeCrash", abort_message, BSG_SEVERITY_ERR);
    bugsnag_add_on_error(bugsnag_on_error);
    std::abort();
}

extern "C"
JNIEXPORT void JNICALL
Java_com_example_myapplication_MainActivity_triggerNativeCrash(JNIEnv *env, jobject thiz) {
    __android_log_set_aborter(aborter);
    __android_log_assert("0", "NativeCrash", "Native crash triggered deliberately!");
}

@agrzegorzewski agrzegorzewski added the needs discussion Requires internal analysis/discussion label Dec 31, 2024
@zacharee
Copy link
Author

zacharee commented Jan 2, 2025

Thanks for the code sample! I modified it to try to just report the error instead of aborting and to check the API level since __android_log_set_aborter() was added in API 30. Posting here for reference.

// Requires `android.defaultConfig.externalNativeBuild.cmake.arguments.add("-DANDROID_WEAK_API_DEFS=ON")` 
// in build.gradle to work with a minSdk below 30.

#include <jni.h>
#include <android/log.h>
#include "bugsnag.h"
#include <cstdlib>
#include <cstring>

bool bugsnag_on_error(void *event) {
    char* error_class = bugsnag_error_get_error_class(event);

    return strcmp(error_class, "SIGABRT") != 0;
}

void aborter(const char *abort_message) {
    bugsnag_notify("NativeCrash", abort_message, BSG_SEVERITY_ERR);
    bugsnag_add_on_error(bugsnag_on_error);
    std::abort();
}

extern "C"
JNIEXPORT void JNICALL
Java_com_example_myapplication_MainActivity_setUpAborter(JNIEnv *env, jobject thiz) {
    if (__builtin_available(android 30, *)) {
        __android_log_set_aborter(aborter);
    }
}

@clr182
Copy link

clr182 commented Jan 6, 2025

Hi @zacharee

Thanks for sharing. I'm going to close this ticket out, but please feel free to re-ping if you come across any further issues and have any follow up questions.

@clr182 clr182 closed this as completed Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Requires internal analysis/discussion
Projects
None yet
Development

No branches or pull requests

3 participants