397199 - display assertion info from minidumps (Crash reason: No crash)

Status: RESOLVED FIXED

(Socorro :: General, task, P2)

Description

[Doug Wright]

Despite what Socorro is claiming for the linked URL and a half-dozen others I've had over the last few days Minefield most definitely did crash ;-)

Comment 1

[(not currently active) Ted Mielczarek]

This is mostly due to http://code.google.com/p/google-breakpad/issues/detail?id=170.  What you hit was the "invalid parameter handler".  Some code called a CRT function with a bad parameter.  Breakpad doesn't see that as a crash, since it's not an OS-level exception.  We don't really report that well currently.  Interestinly enough, tortoisesvn.dll is in the stack for your crash, so it looks like Tortoise is the reason you're crashing.

Comment 2

[Doug Wright]

(In reply to comment #1)
> This is mostly due to
> http://code.google.com/p/google-breakpad/issues/detail?id=170.  What you hit
> was the "invalid parameter handler".  Some code called a CRT function with a
> bad parameter.  Breakpad doesn't see that as a crash, since it's not an
> OS-level exception.  We don't really report that well currently.  Interestinly
> enough, tortoisesvn.dll is in the stack for your crash, so it looks like
> Tortoise is the reason you're crashing.
> 

All the relevant crashes occurred around me trying to use the Save dialog, which Tortoise integrates into so that could well be the cause. Tortoise 1.4.3 fixed a Vista/Explorer crash bug so maybe I should upgrade and see if that stops my crashing.

Either way, I'd expect to see something like 'Crash reason: Unable to identify', rather than 'no crash' which is *never ever* going to be correct as the reason for a crash...

Comment 3

[(not currently active) Ted Mielczarek]

Well ideally we'd say Crash Reason: CRT invalid parameter handler called or something like that, but yeah, it's not great.

Comment 4

[(not currently active) Ted Mielczarek]

This isn't completely a Socorro bug, but it's in the Breakpad processor, so it fits better there than here.

Comment 5

[Michael Morgan [:morgamic]]

This is tied to the need for report statuses or error codes in the reports table.  Bumping priority.

Comment 6

[(not currently active) Ted Mielczarek]

I don't think it is, it's more about a piece of info that we're not getting out of minidump_stackwalk right now.

Comment 7

[Damon Sicore (:damons)]

Affects CrashKill efforts.  Blocking.

Comment 8

[Michael Morgan [:morgamic]]

Ted - have you looked at this?  How can we get a win?

Comment 9

[(not currently active) Ted Mielczarek]

I think we can just hack minidump_stackwalk to spit out the assertion text, and change the processor to accept that and store it in the "crash reason" field. That should be the quickest way to get something useful out of this. I'll patch this soon.

Comment 10

[Damon Sicore (:damons)]

Not blocking now, but dammit, want it.

Comment 11

[Michael Morgan [:morgamic]]

Ted, can this be done by 12/1?

Comment 12

[(not currently active) Ted Mielczarek]

I can get the minidump_stackwalk part done very easily, if you can get someone else to fix the processor to accept the data. We should just be able to do what I described in comment 9.

Comment 13

[Michael Morgan [:morgamic]]

Just Do It (tm)!

Comment 14

[(not currently active) Ted Mielczarek]

Apparently I had a patch to implement 99% of this over two years ago(!):
http://code.google.com/p/google-breakpad/issues/detail?id=170

I'll finish that last 1% and get it landed in Breakpad. I think it can be implemented without any Socorro changes.

Comment 15

[(not currently active) Ted Mielczarek]

Posted for review upstream:
http://breakpad.appspot.com/45001

Comment 16

[(not currently active) Ted Mielczarek]

Ok, landed upstream. This just needs a minidump_stackwalk update in production. Filed bug 532438 on that.

http://code.google.com/p/google-breakpad/source/detail?r=433

Comment 17

[(not currently active) Ted Mielczarek]

Note that the only thing this change will do is change "Crash reason" from "No crash" to something like "Pure virtual function called". The stack and signature will remain the same.

Comment 18

[Michael Morgan [:morgamic]]

Did you file an IT bug for doing this?

Comment 19

[Michael Morgan [:morgamic]]

Nevermind - bug 532438 is the IT bug.

Comment 20

[(not currently active) Ted Mielczarek]

Looks good:
http://crash-stats.mozilla.com/report/index/44118c67-3304-4f20-ba35-cb2672091203