-
Notifications
You must be signed in to change notification settings - Fork 6k
Gather demangled stack traces and report the same to console on crashes. #11431
Conversation
These should only be used on host binaries for more detailed crash reports. Installing the handler on targets (iOS/Android) may cause use to break existing crash reporting mechanisms users may have installed themselves in the process.
|
This should work on Darwin & Linux. I'll work on Windows next. Doing something like |
dnfield
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
Looks like you need to add the new files to the license golden file. |
|
/cc @jonahwilliams @zanderso this may be interesting for the tool to capture or treat specially - this will be super helpful if we see crashes in flutter_tester due to e.g. a segfault. /cc also @GaryQian |
| @@ -0,0 +1,132 @@ | |||
| // Copyright 2013 The Flutter Authors. All rights reserved. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2019?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use 2013 for all source files in the engine - keeps the license file in better shape.
|
|
||
| int status = 0; | ||
| size_t length = 0; | ||
| char* demangled = __cxxabiv1::__cxa_demangle( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Dart VM does an MSAN_UNPOISON() on the result of a similar function. See: https://github.com/dart-lang/sdk/blob/master/runtime/vm/native_symbol_linux.cc#L36.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment made me realize that backtrace printing stuff is not signal safe at all. I am closing this and reworking this to only use signal safe routines.
| static void ToggleSignalHandlers(bool set); | ||
|
|
||
| static void SignalHandler(int signal) { | ||
| // We are a crash signal handler. This can only happen once. Since we don't |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Things to think about, but which don't necessarily need to be solved in this PR:
- What (should) happen(s) here when a SIGSEGV was caused by a stack overflow?
- What (should) happen(s) here when a SIGABRT was caused by malloc heap corruption?
| // want to catch crashes while we are generating the crash reports, disable | ||
| // all set signal handlers to their default values before reporting the crash | ||
| // and re-raising the signal. | ||
| ToggleSignalHandlers(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this disable backtrace printing for all threads or only the calling thread?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Printing a backtrace and the signal handling are orthogonal. We will print the backtrace that caused this specific signal and re-raise it. Presumably, this will cause process termination. So there is nothing else to do on other threads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If ToggleSignalHandlers() only affects the calling thread, and two threads crash at the same time, then the backtrace output could be interleaved.
| @@ -0,0 +1,20 @@ | |||
| // Copyright 2013 The Flutter Authors. All rights reserved. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2019?
| @@ -0,0 +1,33 @@ | |||
| // Copyright 2013 The Flutter Authors. All rights reserved. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2019?
|
Closing because of signal unsafe-ness of the backtrace printing. Will rework it to be safe. |
These should only be used on host binaries for more detailed crash reports. Installing the handler on targets (iOS/Android) may cause use to break existing crash reporting mechanisms users may have installed themselves in the process.