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

VERIFIED FIXED in 69

Status

Socorro
Backend
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: Benjamin Smedberg, Assigned: lars)

Tracking

unspecified
x86_64
Windows 7
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [config changes])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
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.
(Reporter)

Comment 2

5 years ago
--input-json=<file> ? That sounds reasonable, if it's not hard for the processor to produce.
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".
(Reporter)

Comment 4

5 years ago
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.
Assignee: nobody → lars
(Reporter)

Comment 5

5 years ago
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"
(Reporter)

Updated

5 years ago
Depends on: 946799
(Assignee)

Comment 6

5 years ago
Created attachment 8343963 [details] [review]
the PR to add the raw_crash json to the stackwalker
(Reporter)

Updated

5 years ago
Summary: Exclude the breakpad reserved memory block from the smallest-vm-block analysis → Exclude the breakpad reserved memory block from the largest-vm-block analysis

Comment 7

5 years ago
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
(Reporter)

Comment 8

5 years ago
All the PRs were merged, so I think this is FIXED.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 9

5 years ago
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'
Whiteboard: [config changes]
Target Milestone: --- → 69
See Also: → bug 949006
(Assignee)

Comment 10

5 years ago
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",
(Reporter)

Comment 11

5 years ago
v/fixed in production
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.