Closed Bug 1334027 Opened 4 years ago Closed 4 years ago

Add unloaded modules and process/thread data to minidumps

Categories

(Toolkit :: Crash Reporting, defect)

Unspecified
Windows
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: ting, Assigned: ting)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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.
Attached 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
Attachment #8830625 - Attachment is obsolete: true
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+
Blocks: 1337688
Pushed by tchou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3305ef5e8faa
Add unloaded modules and process/thread data to minidumps. r=ted
https://hg.mozilla.org/mozilla-central/rev/3305ef5e8faa
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.