630230 - Add items including mozalloc_handle_oom/nsTArray to prefixSignatureRegEx

Status: RESOLVED FIXED

(Socorro :: General, task)

Description

[Benjamin Smedberg]

Please add the following items to prefixSignatureRegEx:

mozalloc_handle_oom
nsTArray_base<.*
nsTArray<.*
KiFastSystemCallRet
(Nt|Zw)?WaitForSingleObject(Ex)?
(Nt|Zw)?WaitForMultipleObjects(Ex)?
NtUserWaitMessage
NtUserMessageCall

These are needed to make process classifying and resolving the important bits of bug 627727.

Comment 1

[Benjamin Smedberg]

Attachment: Add these to the default config, which may be the production config, and make the config easier to manage using a list

Here's the patch. I don't have a testing environment, so I'd like somebody else to take this from here, please!

Comment 2

[Laura Thomson :laura]

Lars, can you review/commit please, then reassign to jabba to get this into prod.

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 508436 [details] [diff] [review]
Add these to the default config, which may be the production config, and make the config easier to manage using a list

Looks fine to me (and more readable to boot).

Comment 4

[Jim Mathies [:jimm]]

Could we also add 'RtlEnterCriticalSection' to this list? From what I understand of this bug, the list being modified here includes api calls we skip over for the top crash signature. If that's the case, adding RtlEnterCriticalSection would help with a crash stack like this:

https://crash-stats.mozilla.com/report/index/ea82dd4c-934a-4c54-a596-aa3962110201

Comment 5

[Jim Mathies [:jimm]]

Also, ignoring 'WaitForSingleObjectExImplementation' would be good.

https://crash-stats.mozilla.com/report/index/a54c84f9-25bf-4ed4-bc37-ff4212110201

Comment 6

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

:jimm - I'm concerned that your requests are for the wrong configuration parameter.  You speak of ignoring frame signatures.  The 'prefixSignatureRegEx' being modified in this bug is for frame signatures that are candidates for inclusion into compound signatures.  Is that what you intended?  

See http://code.google.com/p/socorro/wiki/SignatureGeneration for more information about how the various regular expressions are used to create signatures.

Comment 7

[Benjamin Smedberg]

Let's land this as-is and deal with jimm's requests in a followup.

Comment 8

[Jim Mathies [:jimm]]

(In reply to comment #6)
> :jimm - I'm concerned that your requests are for the wrong configuration
> parameter.  You speak of ignoring frame signatures.  The 'prefixSignatureRegEx'
> being modified in this bug is for frame signatures that are candidates for
> inclusion into compound signatures.  Is that what you intended?  
> 
> See http://code.google.com/p/socorro/wiki/SignatureGeneration for more
> information about how the various regular expressions are used to create
> signatures.

Sounds like what I'm looking for here is 'irrelevantSignatureRegEx'. Basically, as we walk down the stack, I want to ignore certain frames until we get to something useful, so that these crashes are broken out on crash-stats. So for example:

0 	ntdll.dll 	KiFastSystemCallRet 	
1 	ntdll.dll 	NtWaitForSingleObject 	
2 	KERNELBASE.dll 	WaitForSingleObjectEx 	
3 	kernel32.dll 	WaitForSingleObjectExImplementation 	
4 	kernel32.dll 	WaitForSingleObject 	
5 	wdmaud.drv 	CJob::Wait

I'd like crash-stats to walk down the stack to 'CJob::Wait', and from there generate a signature. I'm a little confused on prefixSignatureRegEx, this sounds like a trigger that starts recording compound signatures? Since I have no idea what 'CJob::Wait' will be in every case, I can't add that to a list.

Comment 9

[(not currently active) Ted Mielczarek]

It's for signatures that we want appended, like 'free', so you can tell that the caller is crashing in 'free', but still assign blame to the caller.

Comment 10

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

:jimm - It sounds like you want all of 'KiFastSystemCallRet', 'NtWaitForSingleObject', 'WaitForSingleObjectEx', 'WaitForSingleObjectExImplementation', and 'WaitForSingleObject' added to the irrelevantSignatureRegEx.  That's the only way that you'll get Socorro to start with frame 5 for signature generation.

I think you'll have to get "buy in" from the rest of the developers as adding 'KiFastSystemCallRet' to irrelevantSignatureRegEx could have a major impact.  From what I understand, 'KiFastSystemCallRet' is the top of the stack for hang detector.  Adding it to irrelevantSignatureRegEx would eliminate that signature from all future crashes.  Would that cause confusion?

If you wish to proceed, please file a separate bug.  That way we can avoiding hiding important discussion under the wrong topic name.

Comment 11

[(not currently active) Ted Mielczarek]

I don't think this is a problem. 'KiFastSystemCallRet' is an extremely generic thing that just means "you're waiting for the Windows kernel to return from a system call". The actual calls below that are always what's of interest.

Comment 12

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

This change is now live in production.  You should see the change in signature processing for all crashes processed after 2/2/11 17:38

Comment 13

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

Pretty sure this change caused bug 631633.

Comment 14

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

Comment on attachment 508436 [details] [diff] [review]
Add these to the default config, which may be the production config, and make the config easier to manage using a list

just cleaning up my review queue - I recognize that this is now moot.