Crash stack walks should ignore infinite recursion stack frames |
||||||||
Issue descriptionWhen infinite recursion causes a crash, the crash backtrace will only show the most recent stack frames and then a bunch of frames from the recursion. To debug you need to see the first stack frames (to see what kicked off the recursion). It makes sense that the systems that generate stack frames for debugging purposes (minidump_stackwalk, crsym?) don't want to be overwhelmed by tons of stack frames, but given that most of the stack frames in an infinite recursion are the same, it seems like they can ignore these redundant stack frames and return the interesting ones at the beginning and end of the stack.
,
Jun 22 2017
,
Jun 22 2017
Adding a sample minidump that has the problem.
,
Jun 28 2017
@shrike: the attached minidump doesn't seem to have any signs of recursion or stack trace truncation, are you sure it's the right dump? Also, if you have any additional examples it would help.
,
Jun 28 2017
I'm sure it's an example of the problem. Are you thinking it's not because you're looking at the results of running it through minidump_stackwalk? There are limits in the minidump_stackwalk code that cause it to clip its output. I hacked on that code to make the limits very large - when I run the minidump through my hacked minidump_stackwalk I get the attached output which contains a stack with 24846 stack frames. The minidump in c#3 is also 9Gb, which is very large for a minidump file).
,
Jun 28 2017
Thanks, I'll take another look. Also, I found a few other examples: https://crash.corp.google.com/browse?q=ReportID%3D%2701d2335480000000%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D https://crash.corp.google.com/browse?q=ReportID%3D%270f1a0e6480000000%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D https://crash.corp.google.com/browse?q=ReportID%3D%2710e80ab300000000%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D https://crash.corp.google.com/browse?q=ReportID%3D%2715af2b3480000000%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D
,
Jul 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/breakpad/breakpad/+/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b commit 01431c2f61aa2af1804f1e139da9bc7c4afa9e7b Author: Leonard Mosescu <mosescu@chromium.org> Date: Wed Jul 12 17:53:15 2017 Handle very large stack traces The main motivation for this change is to handle very large stack traces, normally the result of infinite recursion. This part is actually fairly simple, relaxing a few self-imposed limits on how many frames we can unwind and the max size for stack memory. Relaxing these limits requires stricter and more consistent checks for stack unwinding. There are a number of unwinding invariants that apply to all the platforms: 1. stack pointer (and frame pointer) must be within the stack memory (frame pointer, if preset, must point to the right frame too) 2. unwinding must monotonically increase SP (except for the first frame unwind, this must be a strict increase) 3. Instruction pointer (return address) must point to a valid location 4. stack pointer (and frame pointer) must be appropriately aligned This change is focused on 2), which is enough to guarantee that the unwinding doesn't get stuck in an infinite loop. 1) is implicitly validated part of accessing the stack memory (explicit checks might be nice though). 4) is ABI specific and while it may be valuable in catching suspicious frames is not in the scope of this change. 3) is also an interesting check but thanks to just-in-time compilation it's more complex than just calling StackWalker::InstructionAddressSeemsValid() and we don't want to drop parts of the callstack due to an overly conservative check. Bug: chromium:735989 Change-Id: I9aaba77c7fd028942d77c87d51b5e6f94e136ddd Reviewed-on: https://chromium-review.googlesource.com/563771 Reviewed-by: Mark Mentovai <mark@chromium.org> Reviewed-by: Ivan Penkov <ivanpe@chromium.org> [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/processor/stackwalker_sparc.cc [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/processor/stackwalker_ppc64.cc [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/processor/stackwalker_amd64.cc [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/processor/stackwalker_amd64.h [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/processor/stackwalker_mips.cc [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/processor/stackwalker.cc [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/processor/stackwalker_arm.cc [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/processor/stackwalker_ppc.cc [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/processor/stackwalker_x86.cc [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/google_breakpad/processor/minidump.h [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/processor/microdump_processor_unittest.cc [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/google_breakpad/processor/stackwalker.h [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/processor/stackwalker_arm64.cc [modify] https://crrev.com/01431c2f61aa2af1804f1e139da9bc7c4afa9e7b/src/processor/minidump.cc
,
Jul 17 2017
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by mark@chromium.org
, Jun 22 2017