526046 - SkipList Generate better signatures for crashes related to unhandled Obj-C exceptions

Status: RESOLVED FIXED

(Socorro :: General, task)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Sam, Stuart, and I have been talking about the way Socorro/crash-stats is currently lumping together many unrelated crashes related to Obj-C exceptions and how to make the signatures for such crashes useful.

On 10.5/10.6, these exception-related crashes look like:

0 CoreFoundation        ___TERMINATING_DUE_TO_UNCAUGHT_EXCEPTION___  	
1 libobjc.A.dylib       objc_exception_throw 	
2 CoreFoundation        -[NSException raise] 	
3 <various>             <actual problem code>

Here, frame 0 adds no value and should be ignored/skipped.  Frames 1 and 2 tell us the crash comes from an exception, and we want to know that (exceptions can be thrown directly from objc_exception_throw and -[NSException raise], so we need to look for and prepend/compound both and can't ignore one).  Then we want Frame 3, which tells us what bit of code is causing the exception.

The desired signature from the above crash would look like

objc_exception_throw | -[NSException raise] | <actual problem code>

with possible variants (depending on how the particular exception flow happened)

objc_exception_throw | <actual problem code>

-[NSException raise] | <actual problem code>

On 10.4, these crashes look like:

0 <something, often raw address or objc_msgSend>  	
1 Foundation    _NSRaiseError 	
2 Foundation    +[NSException raise:format:] 	
3 <various>     <actual problem code>

Here, Frame 0 is something that's likely to be ignored/skipped already (certain raw addresses) or something that we already make part of a "compound signature"/prepend (objc_msgSend).  Frame 1 doesn't tell us anything useful and can be skipped.  Frame 2, though, tells us we're crashing from an exception (we want to know that), and we want that compounded with/prepended to Frame 3, which tells us what bit of code is causing the exception.

The desired signature from the above crash would look like

objc_msgSend | +[NSException raise:format:] | <actual problem code>

with possible variants (depending on what frame 0 was)

+[NSException raise:format:] | <actual problem code>

<raw address not currently skipped> | +[NSException raise:format:] | <actual problem code>

In summary, when encountering these frames, please

Skip/Ignore:
* ___TERMINATING_DUE_TO_UNCAUGHT_EXCEPTION___
* _NSRaiseError

Prepend/Compound with following frame:
* objc_exception_throw
* -[NSException raise]
* +[NSException raise:format:]

These changes will help create better exception-related signatures so that we can easily see where they're actually coming from and have crashes grouped appropriately.  Thanks!

Comment 1

[Stuart Morgan]

For completeness, we should also prepend/compound:
  +[NSException raise:format:arguments:]
in case it shows up.

Comment 2

[(not currently active) Ted Mielczarek]

Lars gets stuck dealing with these.

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Any chance we can get this for the next Socorro update rollout?

Comment 4

[Austin King [:ozten]]

Picking up for 1.5.
It sounds like a CrashKill priority, but it sounds like more than a config change.

Comment 5

[Frank Griswold [:griswolf] [:fgriswold]]

Actually, after a quick read, I think this is just a config change, albeit not quite like the usual ones. I'll look at it more closely later today or tomorrow.

Comment 6

[Austin King [:ozten]]

(In reply to comment #5)
Is this just a config change?

Comment 7

[Frank Griswold [:griswolf] [:fgriswold]]

Still haven't taken time to be sure. 80% 'yes'

Comment 8

[Austin King [:ozten]]

Do we know if this is just a config change yet?

Comment 9

[Frank Griswold [:griswolf] [:fgriswold]]

We can get close to the desired results (usefully close) with just a config change:

"___TERMINATING_DUE_TO_UNCAUGHT_EXCEPTION___" ==> irrelevantSignatureRegEx
"objc_exception_throw" ==> prefixSignatureRegex
"-[NSException raise]" ==> prefixSignatureRegex
"_NSRaiseError" ==> irrelevantSignatureRegEx
"+[NSException raise:format:]" ==> prefixSignatureRegex

This does not examine pairs of adjacent frames, as is requested above, so some of the prefixSignatureRegex elements may be seen when they are irrelevant (based on the next frame). That level of signature munging would require a code change, not just a config change. Suggest we go for the config change asap, and reconsider the code change after we see if this is helpful 'enough'.

Comment 10

[Frank Griswold [:griswolf] [:fgriswold]]

Change per comment #1:
add:

+[NSException raise:format:arguments:] ==> prefixSignatureRegex

Assumption: These strings are literal. For example, ':format:' is actually spelled ':','f','o','r','m','a','t',':'. Similarly for ':arguments"'

Comment 11

[Stuart Morgan]

Yes, that's a literal string (Objective-C uses method signatures that name the parameters).

Comment 12

[Frank Griswold [:griswolf] [:fgriswold]]

Resolved per completion of bug 543792