Closed Bug 946381 Opened 11 years ago Closed 11 years ago

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

Categories

(Socorro :: Backend, task)

x86_64
Windows 7
task
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: benjamin, Assigned: lars)

References

Details

(Whiteboard: [config changes])

Attachments

(1 file)

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.
--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".
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
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"
Depends on: 946799
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
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
All the PRs were merged, so I think this is FIXED.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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
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",
v/fixed in production
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: