From e1794592f096d3a7303da03abdc56da0b8928f6c Mon Sep 17 00:00:00 2001 From: Kishan P Rao Date: Mon, 15 Jun 2026 17:02:15 +0200 Subject: [PATCH 1/3] allow unbounded attribute count using annotation list --- .../main/cpp/backends/crashpad-backend.cpp | 79 +++++++++++++------ 1 file changed, 54 insertions(+), 25 deletions(-) diff --git a/backtrace-library/src/main/cpp/backends/crashpad-backend.cpp b/backtrace-library/src/main/cpp/backends/crashpad-backend.cpp index 1b49f087..6fb948fd 100644 --- a/backtrace-library/src/main/cpp/backends/crashpad-backend.cpp +++ b/backtrace-library/src/main/cpp/backends/crashpad-backend.cpp @@ -2,8 +2,11 @@ #include "handler/handler_main.h" #include "handler/crash_report_upload_thread.h" #include "backtrace-native.h" +#include "client/annotation.h" #include #include +#include +#include extern std::string thread_id; extern std::atomic_bool initialized; @@ -56,6 +59,50 @@ namespace { "Started Crashpad upload thread for offline native reports"); } + // Stores each attribute as a crashpad StringAnnotation in the global + // AnnotationList (unbounded), replacing the legacy simple_annotations + // dictionary that was limited to 64 entries. Once set, an annotation is + // permanently referenced by crashpad's global list, so annotations and + // their name buffers are allocated once and never freed. + constexpr crashpad::Annotation::ValueSizeType kAnnotationValueMaxSize = 4096; + using DynamicAnnotation = crashpad::StringAnnotation; + + std::unordered_map g_annotations; + + // Caller must hold attribute_synchronization. + DynamicAnnotation* GetOrCreateAnnotation(const std::string& key) { + auto it = g_annotations.find(key); + if (it != g_annotations.end()) { + return it->second; + } + char* nameCopy = new char[key.size() + 1]; + std::memcpy(nameCopy, key.c_str(), key.size() + 1); + DynamicAnnotation* annotation = new DynamicAnnotation(nameCopy); + g_annotations.emplace(key, annotation); + return annotation; + } + + void SetAnnotation(const char* rawKey, const char* rawValue) { + if (rawKey == nullptr || rawKey[0] == '\0') { + return; + } + const std::lock_guard lock(attribute_synchronization); + DynamicAnnotation* annotation = GetOrCreateAnnotation(std::string(rawKey)); + // Set(const char*) uses strncpy; the StringPiece overload uses std::copy. + annotation->Set(base::StringPiece(rawValue != nullptr ? rawValue : "")); + } + + void ClearAnnotation(const char* rawKey) { + if (rawKey == nullptr || rawKey[0] == '\0') { + return; + } + const std::lock_guard lock(attribute_synchronization); + auto it = g_annotations.find(std::string(rawKey)); + if (it != g_annotations.end()) { + it->second->Clear(); + } + } + } // namespace std::vector @@ -325,23 +372,14 @@ void DumpWithoutCrashCrashpad(jstring message, jboolean set_main_thread_as_fault crashpad::CaptureContext(&context); // set dump message for single report - crashpad::SimpleStringDictionary *annotations = NULL; - if (message != NULL || set_main_thread_as_faulting_thread == true) { JNIEnv *env = GetJniEnv(); if (env == nullptr) { __android_log_print(ANDROID_LOG_ERROR, "Backtrace-Android", "Cannot initialize JNIEnv"); return; } - const std::lock_guard lock(attribute_synchronization); - crashpad::CrashpadInfo *info = crashpad::CrashpadInfo::GetCrashpadInfo(); - annotations = info->simple_annotations(); - if (!annotations) { - annotations = new crashpad::SimpleStringDictionary(); - info->set_simple_annotations(annotations); - } if (set_main_thread_as_faulting_thread == true) { - annotations->SetKeyValue("_mod_faulting_tid", thread_id); + SetAnnotation("_mod_faulting_tid", thread_id.c_str()); } if (message != NULL) { // user can't override error.message - exception message that Crashpad/crash-reporting tool @@ -349,14 +387,14 @@ void DumpWithoutCrashCrashpad(jstring message, jboolean set_main_thread_as_fault // report and after creating a dump, method will clean up this attribute. jboolean isCopy; const char *rawMessage = env->GetStringUTFChars(message, &isCopy); - annotations->SetKeyValue("error.message", rawMessage); + SetAnnotation("error.message", rawMessage); env->ReleaseStringUTFChars(message, rawMessage); } } client->DumpWithoutCrash(&context); - if (annotations != NULL) { - annotations->RemoveKey("error.message"); + if (message != NULL) { + ClearAnnotation("error.message"); } } @@ -372,22 +410,13 @@ void AddAttributeCrashpad(jstring key, jstring value) { return; } - const std::lock_guard lock(attribute_synchronization); - crashpad::CrashpadInfo *info = crashpad::CrashpadInfo::GetCrashpadInfo(); - crashpad::SimpleStringDictionary *annotations = info->simple_annotations(); - if (!annotations) { - annotations = new crashpad::SimpleStringDictionary(); - info->set_simple_annotations(annotations); - } - jboolean isCopy; const char *crashpadKey = env->GetStringUTFChars(key, &isCopy); const char *crashpadValue = env->GetStringUTFChars(value, &isCopy); - if (crashpadKey && crashpadValue) - annotations->SetKeyValue(crashpadKey, crashpadValue); + SetAnnotation(crashpadKey, crashpadValue); - env->ReleaseStringUTFChars(key, crashpadKey); - env->ReleaseStringUTFChars(value, crashpadValue); + if (crashpadKey) env->ReleaseStringUTFChars(key, crashpadKey); + if (crashpadValue) env->ReleaseStringUTFChars(value, crashpadValue); } void AddAttachmentCrashpad(jstring jattachment) { From c5a86388b7332819e746c1ce660a52138b496477 Mon Sep 17 00:00:00 2001 From: Kishan P Rao Date: Thu, 2 Jul 2026 16:35:58 +0200 Subject: [PATCH 2/3] Update doc, mention empty value & not unbounded limit, provide warning if number of entries cross crashpad max --- .../main/cpp/backends/crashpad-backend.cpp | 24 ++++++++++++++----- .../library/base/BacktraceBase.java | 3 +++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/backtrace-library/src/main/cpp/backends/crashpad-backend.cpp b/backtrace-library/src/main/cpp/backends/crashpad-backend.cpp index 6fb948fd..3659c705 100644 --- a/backtrace-library/src/main/cpp/backends/crashpad-backend.cpp +++ b/backtrace-library/src/main/cpp/backends/crashpad-backend.cpp @@ -3,6 +3,7 @@ #include "handler/crash_report_upload_thread.h" #include "backtrace-native.h" #include "client/annotation.h" +#include "snapshot/snapshot_constants.h" #include #include #include @@ -59,11 +60,11 @@ namespace { "Started Crashpad upload thread for offline native reports"); } - // Stores each attribute as a crashpad StringAnnotation in the global - // AnnotationList (unbounded), replacing the legacy simple_annotations - // dictionary that was limited to 64 entries. Once set, an annotation is - // permanently referenced by crashpad's global list, so annotations and - // their name buffers are allocated once and never freed. + // Stores attributes as crashpad StringAnnotations, replacing the 64-entry + // simple_annotations dictionary. The snapshot reader reads at most + // crashpad::kMaxNumberOfAnnotations per module, so SetAnnotation drops new + // keys past that. Annotations and name buffers are allocated once and never + // freed (the global list references them for the process lifetime). constexpr crashpad::Annotation::ValueSizeType kAnnotationValueMaxSize = 4096; using DynamicAnnotation = crashpad::StringAnnotation; @@ -87,7 +88,18 @@ namespace { return; } const std::lock_guard lock(attribute_synchronization); - DynamicAnnotation* annotation = GetOrCreateAnnotation(std::string(rawKey)); + std::string key(rawKey); + // Drop new keys past the read limit; updates to existing keys are fine. + if (g_annotations.find(key) == g_annotations.end() + && g_annotations.size() >= crashpad::kMaxNumberOfAnnotations) { + __android_log_print( + ANDROID_LOG_WARN, + "Backtrace-Android", + "Dropping native attribute '%s': reached crashpad's annotation read limit", + rawKey); + return; + } + DynamicAnnotation* annotation = GetOrCreateAnnotation(key); // Set(const char*) uses strncpy; the StringPiece overload uses std::copy. annotation->Set(base::StringPiece(rawValue != nullptr ? rawValue : "")); } diff --git a/backtrace-library/src/main/java/backtraceio/library/base/BacktraceBase.java b/backtrace-library/src/main/java/backtraceio/library/base/BacktraceBase.java index b977c932..c49819d5 100644 --- a/backtrace-library/src/main/java/backtraceio/library/base/BacktraceBase.java +++ b/backtrace-library/src/main/java/backtraceio/library/base/BacktraceBase.java @@ -357,6 +357,9 @@ public Map getAttributes() { * - is not an object (the attribute value is primitive type like String, or * Int) * + * Note: native crash reports omit attributes with an empty-string value + * (managed/JVM reports are unaffected). + * * @param key attribute name * @param value attribute value. */ From aa7521a38e81d1338a688fbeab30703318f4a742 Mon Sep 17 00:00:00 2001 From: Kishan P Rao Date: Tue, 7 Jul 2026 18:03:41 +0200 Subject: [PATCH 3/3] Cap added keys considering reserved internal keys, document empty string value behavior --- .../main/cpp/backends/crashpad-backend.cpp | 31 ++++++++++++++----- .../library/base/BacktraceBase.java | 2 +- .../library/interfaces/Client.java | 4 +++ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/backtrace-library/src/main/cpp/backends/crashpad-backend.cpp b/backtrace-library/src/main/cpp/backends/crashpad-backend.cpp index 3659c705..9d3af58e 100644 --- a/backtrace-library/src/main/cpp/backends/crashpad-backend.cpp +++ b/backtrace-library/src/main/cpp/backends/crashpad-backend.cpp @@ -61,13 +61,26 @@ namespace { } // Stores attributes as crashpad StringAnnotations, replacing the 64-entry - // simple_annotations dictionary. The snapshot reader reads at most - // crashpad::kMaxNumberOfAnnotations per module, so SetAnnotation drops new - // keys past that. Annotations and name buffers are allocated once and never + // simple_annotations dictionary. The crashpad snapshot reader reads at most + // crashpad::kMaxNumberOfAnnotations annotation objects per module, so + // SetAnnotation drops new user keys past that limit (internal keys keep + // reserved slots). Annotations and name buffers are allocated once and never // freed (the global list references them for the process lifetime). constexpr crashpad::Annotation::ValueSizeType kAnnotationValueMaxSize = 4096; using DynamicAnnotation = crashpad::StringAnnotation; + // Reserve slots for internal, per-report annotations so they are never + // crowded out by user attributes near the read limit. + constexpr size_t kReservedAnnotationSlots = 2; // error.message, _mod_faulting_tid + + bool IsValidAnnotationKey(const char* rawKey) { + return rawKey != nullptr && rawKey[0] != '\0'; + } + + bool IsReservedAnnotationKey(const std::string& key) { + return key == "error.message" || key == "_mod_faulting_tid"; + } + std::unordered_map g_annotations; // Caller must hold attribute_synchronization. @@ -84,14 +97,18 @@ namespace { } void SetAnnotation(const char* rawKey, const char* rawValue) { - if (rawKey == nullptr || rawKey[0] == '\0') { + if (!IsValidAnnotationKey(rawKey)) { return; } const std::lock_guard lock(attribute_synchronization); std::string key(rawKey); - // Drop new keys past the read limit; updates to existing keys are fine. - if (g_annotations.find(key) == g_annotations.end() - && g_annotations.size() >= crashpad::kMaxNumberOfAnnotations) { + const bool isNewKey = g_annotations.find(key) == g_annotations.end(); + // Cap user keys below the read limit so the reserved internal keys can + // always be registered; updates to existing keys are always allowed. + const size_t userAnnotationLimit = + crashpad::kMaxNumberOfAnnotations - kReservedAnnotationSlots; + if (isNewKey && !IsReservedAnnotationKey(key) + && g_annotations.size() >= userAnnotationLimit) { __android_log_print( ANDROID_LOG_WARN, "Backtrace-Android", diff --git a/backtrace-library/src/main/java/backtraceio/library/base/BacktraceBase.java b/backtrace-library/src/main/java/backtraceio/library/base/BacktraceBase.java index c49819d5..b6da9b07 100644 --- a/backtrace-library/src/main/java/backtraceio/library/base/BacktraceBase.java +++ b/backtrace-library/src/main/java/backtraceio/library/base/BacktraceBase.java @@ -358,7 +358,7 @@ public Map getAttributes() { * Int) * * Note: native crash reports omit attributes with an empty-string value - * (managed/JVM reports are unaffected). + * (managed reports are unaffected). * * @param key attribute name * @param value attribute value. diff --git a/backtrace-library/src/main/java/backtraceio/library/interfaces/Client.java b/backtrace-library/src/main/java/backtraceio/library/interfaces/Client.java index 2ac3095f..86c54f1c 100644 --- a/backtrace-library/src/main/java/backtraceio/library/interfaces/Client.java +++ b/backtrace-library/src/main/java/backtraceio/library/interfaces/Client.java @@ -23,6 +23,8 @@ public interface Client { * Adds new attributes to the client. * If the native integration is available and attributes are primitive type, * they will be added to the native reports. + * Note: native crash reports omit attributes with an empty-string value + * (managed reports are unaffected). * @param attributes client Attributes */ void addAttribute(Map attributes); @@ -31,6 +33,8 @@ public interface Client { * Adds new attribute to the client. * If the native integration is available and attributes are primitive type, * they will be added to the native reports. + * Note: native crash reports omit attributes with an empty-string value + * (managed reports are unaffected). * @param key attribute key * @param value attribute value */