946381 - Exclude the breakpad reserved memory block from the largest-vm-block analysis

Status: VERIFIED FIXED

(Socorro :: Backend, task)

Description

[Benjamin Smedberg]

The now-working reserved memory block for breakpad has the unfortunate side effect of making the smallest-vm-block numbers much harder to deal with. Because this block is free at the time the minidump is taken, it is reported as free memory when in fact at the time of the crash it was not free memory.

It's pretty trivial to annotate the location and size of the reserved memory block. This should allow us to exclude it from the MDSW calculation *except* that currently we don't pass any annotation data to MDSW.

Lars, how feasible would it be to pass two annotation args to MDSW if they are present in the metadata? Probably a single flag of the form --reserveinfo=addr,size would be fine.

Comment 1

[(not currently active) Ted Mielczarek]

If you wanted to give yourself more future flexibility, we could just add an --input-json=... parameter. We're already using jsoncpp to produce the JSON output, so parsing a JSON input file for input parameters wouldn't be hard.

Comment 2

[Benjamin Smedberg]

--input-json=<file> ? That sounds reasonable, if it's not hard for the processor to produce.

Comment 3

[(not currently active) Ted Mielczarek]

Yeah. Ideally you could just feed the entire extra-data-json into mdsw, so it would have access to all the client-produced annotations. That would make it trivial to do additional analyses like "crash was at a poisoned address".

Comment 4

[Benjamin Smedberg]

Lars, I've written an untested draft of the Socorro-side changes we need for this here: https://github.com/bsmedberg/socorro/compare/exclude-reservedVM?expand=1

There are two commits:

* a change to the stackwalk so that it accepts a new command-line flag --raw-json <file> and uses that to exclude the reserved breakpad memory from its calculations. I have built and tested this change.
* a change to the processor to pass in the raw JSON as a file. This is completely unbuilt and untested.

Comment 5

[Benjamin Smedberg]

For testing, I am using the crash bp-1e78caf6-bbf9-4a4d-8649-63a352131129 and adding the following keys to the raw JSON:

"BreakpadReserveAddress": "18446744073600368640",
"BreakpadReserveSize": "10616832"

Before these changes: "largest_free_vm_block" : "0xa20000"
After these changes: "largest_free_vm_block" : "0x600000"

Comment 6

[K Lars Lohn [:lars] [:klohn]]

Attachment: the PR to add the raw_crash json to the stackwalker

Comment 7

[[github robot]]

Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/e6f8cba00d3dd0ffbdab0fd7a6d9b8212be845a7
Bug 946381, stackwalker part - exclude the breakpad reserved memory region from the calculation of the smallest available VM block

Comment 8

[Benjamin Smedberg]

All the PRs were merged, so I think this is FIXED.

Comment 9

[K Lars Lohn [:lars] [:klohn]]

config change summary:  the line in processor.ini that specifies the syntax of the stackwalker invocation needs to just be allowed to be the default.  Comment out the processor.ini:271

# stackwalk_command_line='$minidump_stackwalk_pathname --pipe $dumpfilePathname $processor_symbols_pathname_list 2>/dev/null'

Comment 10

[K Lars Lohn [:lars] [:klohn]]

I attest that this is working properly in staging:

diff bbc81131-cedc-4b8b-b470-885282131211.proc.json fb4ae9db-ebe9-4c78-9784-c0b142131211.proc.json4,5c4,5
<    "uuid" : "bbc81131-cedc-4b8b-b470-885282131211",
<    "startedDateTime" : "2013-12-11 17:28:32.215671",
---
>    "uuid" : "fb4ae9db-ebe9-4c78-9784-c0b142131211",
>    "startedDateTime" : "2013-12-11 17:28:25.093447",
106c106
<    "date_processed" : "2013-12-11 17:28:16.438212",
---
>    "date_processed" : "2013-12-11 17:28:16.430011",
116c116
<    "completeddatetime" : "2013-12-11 17:28:33.226524",
---
>    "completeddatetime" : "2013-12-11 17:28:26.109742",
10244c10244
<       "largest_free_vm_block" : "0xa20000",
---
>       "largest_free_vm_block" : "0x600000",

Comment 11

[Benjamin Smedberg]

v/fixed in production