Add unloaded modules and process/thread data to minidumps

RESOLVED FIXED in Firefox 54

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ting, Assigned: ting)

Tracking

(Blocks 1 bug)

Trunk
mozilla54
Unspecified
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
While checking the crashes of bug 1322554, I realized that we don't have unloaded modules and TEB/PEB in minidumps, which might be helpful for debugging.  So file this bug to include MiniDumpWithUnloadedModules | MiniDumpWithProcessThreadData.

However, I am not so sure is this worth the increased dump size.
(Assignee)

Comment 1

2 years ago
Posted patch wip (obsolete) — Splinter Review
The size of minidump by crashing the parent process with :bsmedberg's crashfirefox-intentionally on my Win10:

  With the WIP:    ~610KB
  Without the WIP: ~381KB
Attachment #8830625 - Flags: feedback?(ted)
Attachment #8830625 - Attachment mime type: text/x-patch → text/plain
Attachment #8830625 - Attachment is patch: true
Comment on attachment 8830625 [details] [diff] [review]
wip

Review of attachment 8830625 [details] [diff] [review]:
-----------------------------------------------------------------

I'm fine with the concept. Maybe we should just turn it on for nightlies for a while and see what happens to the size distribution of minidumps? If they go up drastically we can always switch it off. How about wrapping this in #ifdef NIGHTLY_BUILD while we test?

FWIW, it looks like Chromium enabled these a while ago:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-reviews/JcMDWj0OMog

I don't know if they're still using that or not.
Attachment #8830625 - Flags: feedback?(ted) → feedback+
Assignee: nobody → janus926
(Assignee)

Updated

2 years ago
Attachment #8830625 - Attachment is obsolete: true

Comment 4

2 years ago
mozreview-review
Comment on attachment 8832721 [details]
Bug 1334027 - Add unloaded modules and process/thread data to minidumps.

https://reviewboard.mozilla.org/r/108924/#review110274

::: toolkit/crashreporter/nsExceptionHandler.cpp:1509
(Diff revision 1)
>        if (major > 6 || (major == 6 && minor > 1) ||
>            (major == 6 && minor == 1 && revision >= 7600)) {
>          minidump_type = MiniDumpWithFullMemoryInfo;
>        }
> +#ifdef NIGHTLY_BUILD
> +      // TODO: Remove the NIGHTLY_BUILD wrapping if the increased size is

If you want to do this you should file a followup bug to track it so it doesn't get lost.
Attachment #8832721 - Flags: review?(ted) → review+
(Assignee)

Updated

2 years ago
Blocks: 1337688

Comment 5

2 years ago
Pushed by tchou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3305ef5e8faa
Add unloaded modules and process/thread data to minidumps. r=ted

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3305ef5e8faa
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.