1426090 - Use MiniDumpWithIndirectlyReferencedMemory for nightly on Windows

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, enhancement)

Description

[Cervantes Yu [:cyu] [:cervantes]]

In bug 1286802, we added heap regions referenced from the crash context to minidumps. This bug is to take a further step by including heap regions referenced from stack objects.

We can implement this by ourselves or use the MiniDumpWithIndirectlyReferencedMemory minidump type for Windows and implement on our own on other platforms.

The cost of this is the size of minidumps. I made some tests and it increases the dump size from ~1200 KB to ~2100 KB. I think this is worthy given that this will increase our ability to debug.

Comment 1

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: Use minidump type 'MiniDumpWithIndirectlyReferencedMemory' for nightly on Windows

Comment 2

[Cervantes Yu [:cyu] [:cervantes]]

If we land this, we can consider reverting bug 1286802 or removing the Windows part of it and add the support for other platforms. I have a WIP for stack scanning to include heap regions but just cannot decide how much to scan. It seems that scanning the whole stack is not too bad w.r.t. the increase of minidump size given that the implementation in dbghelp.dll includes heap regions referenced from stack of *all* threads and the size increase is within 2x.

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 8937681 [details] [diff] [review]
Use minidump type 'MiniDumpWithIndirectlyReferencedMemory' for nightly on Windows

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

I guess I have to transfer my rubber-stamp from the real world to bugzilla, eh?

Comment 4

[(not currently active) Ted Mielczarek]

(In reply to Cervantes Yu [:cyu] [:cervantes] from comment #2)
> If we land this, we can consider reverting bug 1286802 or removing the
> Windows part of it and add the support for other platforms.

You should do whatever you think is best here.

Comment 5

[Cervantes Yu [:cyu] [:cervantes]]

https://treeherder.mozilla.org/logviewer.html#?job_id=152342488&repo=try&lineNumber=11201
Win7 test failure:

12:48:18     INFO -  PID 6048 | 2017-12-19 12:48:18: minidump.cc:2917: ERROR: MinidumpMemoryList could not store memory region 308/331, 0x86bc2d8+0x3d28
12:48:18     INFO -  PID 6048 | 2017-12-19 12:48:18: minidump.cc:4979: ERROR: GetStream could not read stream type 5

This means that there are overlapping memory regions included in the minidump to fail the check of our minidump processor.

Don't use MiniDumpWithIndirectlyReferencedMemory on Win7 or earlier:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a224fdab3e37eee0b3967d88fbadf5d9d3f8326

I'll land this if the tests are green.

Comment 6

[Cervantes Yu [:cyu] [:cervantes]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ea35726ecdf874f09ddff50b0fec140d05ebad7c
Bug 1426090 - Use minidump type 'MiniDumpWithIndirectlyReferencedMemory' for nightly on Windows. r=ted

Comment 7

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/ea35726ecdf8