Fix inconsistencies in stack trace quality for C/C++ events #1611
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* removes "unwind style" logic in favor of a unified interface * updates C++ standard to c++17 (required for unwindstack's use of optional, etc) * splits unwinding into separate functions for signal/exception-time and user-generated events
Bumping is required to compile with c++17 language features Use latest LTS, per guidance for middleware vendors > NDK versions are largely compatible with each other, but occasionally there are changes that break compatibility. If you know that all of your users are using the same version of the NDK, it's best to use the same version that they do. Otherwise, use the newest version. https://developer.android.com/ndk/guides/middleware-vendors change requires update to root_detection to use 3-param open(2) [full ci]
reduce the surface of API, which expanded quite a bit with updated unwindstack. https://developer.android.com/ndk/guides/middleware-vendors debug builds export additional symbols used in the tests This change requires removal of code which re-throws types defined in binaries other than libbugsnag-ndk, because if the type has been hidden, then the throw invocation will trigger an immediate termination (skipping the crash report), or if the type is custom (e.g. a subclass of std::runtime_error, etc), then no useful message was detected anyway, as the type information is likely not available. Useful reading on the topic: * https://gcc.gnu.org/wiki/Visibility (specifically "Problems with C++ exceptions") * https://developer.android.com/ndk/guides/common-problems#rttiexceptions_not_working_across_library_boundaries * https://developer.android.com/ndk/guides/cpp-support#ic * https://en.cppreference.com/w/cpp/language/definition (section on ODR) Technically this lib should not export std:: symbols but this at least preserves existing behavior until its re-evaluated. [full ci]
Removes the double free scenario, as the exact crashing condition is dependent on undefined behavior, and thus can't assert an exact location. Refactors several existing tests to have their own files for their native components with the intent to: 1. Make the native functions easier to find - one big file is getting unwieldy 2. Make line number assertions stable, avoiding changes if unrelated tests change Avoids asserting on inconsistent behavior - for example CXXStackoverflowScenario always crashes in the same function but line info may not be available on arm32.
kattrali
force-pushed
the
integration/update-unwinder
branch
from
da24fae to
7e01a7a
Compare
Add expectations for the symbolication results of ndk stacktraces
kattrali
force-pushed
the
integration/update-unwinder
branch
from
b08128b to
0f1257f
Compare
lemnik
approved these changes
Mar 14, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changeset
Testing