Closed Bug 520678 Opened 15 years ago Closed 13 years ago

SkipList Add "nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int)" and "PL_DHashTableOperate" to ignore-frame-list

Categories

(Socorro :: General, task)

x86
All
task
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: sicking, Assigned: laura)

References

Details

Crashes that happen on nsTArray_base::ShiftData is probably due to calls on destroyed arrays, so the bug is likely one level up the callstack. So it would be great if we could add this function to the list of functions that we prepend using the [foo | bar | baz] syntax.
Lars: Are you the right assignee for this one?
Assignee: nobody → lars
Can we also add PL_DHashTableOperate.
Summary: Add nsTArray_base::ShiftData to ignore-frame-list → Add nsTArray_base::ShiftData and PL_DHashTableOperate to ignore-frame-list
We need to come up with a more efficient way to make these configuration changes.  All I can do is turn around and make a request to IT for the configuration change.  If you could direct the request first to IT, cc'ng me on the request, I could rubber stamp it and it would get into production more quickly.

A request to IT needs to specify that it is a "configuration change request for Socorro processor" - it needs to go on to say that a certain signature needs to be added to the 'prefixSignatureRegEx' configuration parameter.

It is important to give the exact signature when making the request. "nsTArray_base::ShiftData" doesn't appear to be correct.  Did you really mean, "nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int)"?

After writing all that I realize that I'll probably be stuck with writing these configuration requests forever... I've written Bug 521916 for this issue
I agree it'd be great if we could make this smoother. Not sure I followed the rest of your comment though, would you like me to reassign this to someone in IT (and update to the exact signature in the subject)? If so, who?
(Updating summary since that needs to happen no matter who does this)
Summary: Add nsTArray_base::ShiftData and PL_DHashTableOperate to ignore-frame-list → Add "nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int)" and "PL_DHashTableOperate" to ignore-frame-list
You need not take any additional action.  

By writing out what it takes to make a properly worded IT request, I realized that it was too complicated for anyone not versed in the Socorro internals.  So, I drew a deep breath and submitted the request myself.

You should continue to make these types of requests through me.
lars, any update?  I also posted a comment in Bug 519771 that I'm interested in thoughts on.   maybe this collective informantion is just some kind of future meta data that we try and add on top through some extra analysis path.
Target Milestone: --- → 1.2
This task was completed with an IT request in Bug 521916.  If there is any reason to reopen this bug, please feel free to do so.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Jonas, mind taking verification on this?  Thanks!
nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int) still does not appear to be on the skip list.

http://crash-stats.mozilla.com/topcrasher/byversion/Firefox/3.5.5/7

lists "memmove | nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int)" as one of the crashers (currently as #29)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
We have an error in our 'prefix' regex for nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int), so it was not being recognized. I'll try to get that fixed today.
Summary: Add "nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int)" and "PL_DHashTableOperate" to ignore-frame-list → SkipList Add "nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int)" and "PL_DHashTableOperate" to ignore-frame-list
is this working as expected now?  I always see "nsTArray_base::ShiftData..." as a prefix component of compound signatures when querying for the last few days.  I'm closing it again.  Reopen if necessary.

"nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int) | nsTArray<unsigned char>::~nsTArray<unsigned char>()"

"memmove | nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int) | nsObserverList::FillObserverArray(nsCOMArray<nsIObserver>&)"
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
(In reply to comment #12)
> is this working as expected now?  

Jonas, et al, 
I amm also wondering if this working correctly. Shouldn't socorro be giving a different signature for bp-18083e26-1154-43db-8ef9-54cd82100806 ??

Frank, Lars, can you tell us exactly what skiplist string was used. Note: I can't see the bugs at https://wiki.mozilla.org/Breakpad/SkipList#Changelog ... so I have no idea what skiplists have actually been implemented in the past year. 

Part of my reason for asking is I may need further differentiation of stacks via Bug 555599. But first I'd like to know what was implemented in this bug is working correctly.
Blocks: 555599
fgriswold, lars, please see comment 11 thru comment 13. the skiplist doesn't seem to be working. 
example 1 bp-18083e26-1154-43db-8ef9-54cd82100806 comment 13 above
example 2 bp-152833f6-faf1-4ad0-8e4b-453272091215 from bug 534858
Status: RESOLVED → REOPENED
OS: Mac OS X → All
Resolution: FIXED → ---
Current skiplist config for prefixes:
prefixSignatureRegEx.default = '@0x0|strchr|strstr|strlen|PL_strlen|strcmp|wcslen|memcpy|memmove|memcmp|malloc|realloc|.*free|arena_dalloc_small|arena_alloc|arena_dalloc|nsObjCExceptionLogAbort(\(.*?\)){0,1}|libobjc.A.dylib@0x1568.|nsCOMPtr_base::assign_from_qi(nsQueryInterface,nsIDconst&)|objc_msgSend|_purecall|nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int)|PL_DHashTableOperate|EtwEventEnabled|RtlpFreeHandleForAtom|RtlpDeCommitFreeBlock|RtlpAllocateAffinityIndex|RtlAddAccessAllowedAce|RtlQueryPerformanceFrequency|RtlpWaitOnCriticalSection|RtlpWaitForCriticalSection|_PR_MD_ATOMIC_(INC|DEC)REMENT|nsCOMPtr.*|nsRefPtr.*|operator new\([^,)]+\)|CFRelease'
Thanks Laura. Do the parenthesis need to be escaped in the string?
 nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int)
as in 
 nsTArray_base::ShiftData\(unsigned int, unsigned int, unsigned int, unsigned int\)


I'm unsure, because 

1. It's clear nsTArray_base::ShiftData... isn't being skipped, per http://crash-stats.mozilla.com/query/query?product=Firefox&version=ALL%3AALL&range_value=2&range_unit=days&date=08%2F26%2F2010+15%3A06%3A58&query_search=signature&query_type=contains&query=nsTArray_base%3A%3AShiftData%28unsigned+int%2C+unsigned+int%2C+unsigned+int%2C+unsigned+int%29&build_id=&process_type=any&hang_type=any&do_query=1 

2. But nsCOMPtr_base::assign_from_qi(nsQueryInterface,nsIDconst&) is working and has no escaped parenthesis.  per http://crash-stats.mozilla.com/query/query?product=Thunderbird&version=ALL%3AALL&range_value=2&range_unit=days&date=08%2F26%2F2010+14%3A58%3A39&query_search=signature&query_type=contains&query=nsCOMPtr_base%3A%3Aassign_from_qi&build_id=&process_type=any&hang_type=any&do_query=1
all literal parens within a skiplist rule should be escaped.  Since the rules are simply regular expressions and parens have their own meaning within regular expressions, the backslash is mandatory to match a literal '(' or ')'.  

The reason that the second signature in comment #16 is working is because it is matched by the rule 'nsCOMPtr.*'
This is no longer a problem in prod config - I don't know when it changed, but these no longer need escaping (expressions no longer contain parens).  Resolving.
Status: REOPENED → RESOLVED
Closed: 15 years ago13 years ago
Resolution: --- → FIXED
Target Milestone: 1.2 → 2.2.2
Assignee: lars → laura
(In reply to Laura Thomson :laura from comment #18)
> This is no longer a problem in prod config - I don't know when it changed,
> but these no longer need escaping (expressions no longer contain parens). 
> Resolving.

Bumping to QA verified. I'm seeing the same behavior as :laura.
Status: RESOLVED → VERIFIED
Component: Socorro → General
Product: Webtools → Socorro
You need to log in before you can comment on or make changes to this bug.