Allow the minidump writer to collect handle data so that resulting dump contains information about …

Allow the minidump writer to collect handle data so that resulting dump contains
information about opened handles (!handle) and handle operations trace
(!htrace).

Depending on the dump type different amount of data is collected. For a full
dump all handles and complete handle trace are included to the dump. In the case
of a minidump the list of handle operations for a single handle value (the last
handle value that caused STATUS_INVALID_HANDLE exception) is recorded. In either
case the handle trace should be explicitly enabled by the client process (or by
a debugger) in order to produce any handle trace data.

BUG=131699

[alexeypa]

Please take a look.

[Mark Mentovai]

I don’t know much about the underlying win32 stuff here, but the code itself
looks reasonable. LGTM.

https://breakpad.appspot.com/410002/diff/1/client/windows/crash_generation/mi...
File client/windows/crash_generation/minidump_generator.cc (right):

https://breakpad.appspot.com/410002/diff/1/client/windows/crash_generation/mi...
client/windows/crash_generation/minidump_generator.cc:32: #include <avrfsdk.h>
Don’t mix C and C++ system headers. C system headers come first. Blank lines
between sections are helpful.

https://breakpad.appspot.com/410002/diff/1/client/windows/crash_generation/mi...
client/windows/crash_generation/minidump_generator.cc:33: #include <cassert>
<assert.h> (and in the C section), not <cassert>.

https://breakpad.appspot.com/410002/diff/1/client/windows/crash_generation/mi...
client/windows/crash_generation/minidump_generator.cc:108: if
(!verifier_module_) {
Are you sure about the ! here?

[alexeypa]

Please see my comment about pre-XP support. 

Could you check it in for me? I'm not a Breakpad committer.

https://breakpad.appspot.com/410002/diff/1/client/windows/crash_generation/mi...
File client/windows/crash_generation/minidump_generator.cc (right):

https://breakpad.appspot.com/410002/diff/1/client/windows/crash_generation/mi...
client/windows/crash_generation/minidump_generator.cc:32: #include <avrfsdk.h>
On 2012/07/03 14:10:16, Mark Mentovai wrote:
> Don’t mix C and C++ system headers. C system headers come first. Blank lines
> between sections are helpful.

Done.

https://breakpad.appspot.com/410002/diff/1/client/windows/crash_generation/mi...
client/windows/crash_generation/minidump_generator.cc:33: #include <cassert>
On 2012/07/03 14:10:16, Mark Mentovai wrote:
> <assert.h> (and in the C section), not <cassert>.

Done.

https://breakpad.appspot.com/410002/diff/1/client/windows/crash_generation/mi...
client/windows/crash_generation/minidump_generator.cc:108: if
(!verifier_module_) {
On 2012/07/03 14:10:16, Mark Mentovai wrote:
> Are you sure about the ! here?

Done.

https://breakpad.appspot.com/410002/diff/5001/client/windows/crash_generation...
File client/windows/crash_generation/minidump_generator.cc (right):

https://breakpad.appspot.com/410002/diff/5001/client/windows/crash_generation...
client/windows/crash_generation/minidump_generator.cc:134: return false;
verifier.dll is available starting from Windows XP/W2K3. Is Windows 2000 support
required?

[Mark Mentovai]

I’m reasonably confident nobody cares about win2k any more. I think Firefox
finally gave up on them last month.

[Mark Mentovai]

Breakpad r979.

[Ted Mielczarek]

On 2012/07/03 00:09:55, alexeypa wrote:
> Please take a look.

Is this significantly better than the stock MiniDumpWithHandleData feature that
it's worth the extra code?

[alexeypa]

On 2012/07/03 20:00:53, Ted Mielczarek wrote:
> On 2012/07/03 00:09:55, alexeypa wrote:
> > Please take a look.
> 
> Is this significantly better than the stock MiniDumpWithHandleData feature
that
> it's worth the extra code?

MiniDumpWithHandleData adds much more data (50-100KB and up). A handle
descriptor is 40 bytes plus an object name. A handle operation descriptor is 288
bytes. There are usually hundreds of opened handles and thousands of handle
operations.

The added code filters all that out and leaves a few handle operation entries. I
believe it is important for a 200-300KB dump.

[justin]

https://breakpad.appspot.com/410002/diff/5001/client/windows/crash_generation...
File client/windows/crash_generation/minidump_generator.cc (right):

https://breakpad.appspot.com/410002/diff/5001/client/windows/crash_generation...
client/windows/crash_generation/minidump_generator.cc:134: return false;
On 2012/07/03 16:40:12, alexeypa wrote:
> verifier.dll is available starting from Windows XP/W2K3. Is Windows 2000
support
> required?

Random ex-post-facto drive-by, but is this DLL guaranteed to be present on XP+?
If not, you'll want to fully qualify the path just to be safe from DLL hijack
issues.

[alexeypa]

https://breakpad.appspot.com/410002/diff/5001/client/windows/crash_generation...
File client/windows/crash_generation/minidump_generator.cc (right):

https://breakpad.appspot.com/410002/diff/5001/client/windows/crash_generation...
client/windows/crash_generation/minidump_generator.cc:134: return false;
On 2012/07/13 00:47:47, justin wrote:
> On 2012/07/03 16:40:12, alexeypa wrote:
> > verifier.dll is available starting from Windows XP/W2K3. Is Windows 2000
> support
> > required?
> 
> Random ex-post-facto drive-by, but is this DLL guaranteed to be present on
XP+?
> If not, you'll want to fully qualify the path just to be safe from DLL hijack
> issues.

It is installed by default on XP+ as far as I can tell.