1163831 - Collapse all <...> in crash signatures to <T>

Status: RESOLVED FIXED

(Socorro :: General, task)

Description

[(Away)]

The full type of IntRect, in the debug symbols, is:
mozilla::gfx::BaseRect<int,mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits>,mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits>,mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>,mozilla::gfx::IntMarginTyped<mozilla::gfx::UnknownUnits> >

As with any data structure rename, some spurious "new" signatures spiked after bug 1158122. But in this case the new ones are so long that they get cut off. For example: bp-96522516-106f-449f-90f7-4a2182150509 is in IntRect::IsEmpty, but you'd never know that from the signature. I can't even tell whether they are all IsEmpty, or if there are multiple crash sites getting put in the same truncated bucket.

What can we do about this?

Comment 1

[Robert Kaiser]

I more and more start to believe that we should change signature generation somewhat. IMHO, we probably (in classic signatures generated from a C/C++ stack) should strip off everything in normal or angle brackets.

Comment 2

[(Away)]

Stripping bracketed content could also help reduce the COMDAT-folding explosion we see in cases like nsCOMPtr<Fifty_random_signatures_that_all_mean_the_same_crash>.

I am not yet sure whether we want to do this for all bracketed text or just based on a list. Perhaps we could add a "rewrite list" in addition to the skip-list and ignore-list. For starters we might do IntRect/nsCOMPtr/nsTArray.

On the other hand, I think I'm willing to try your idea first (just delete it all) and see whether it causes any problems.

Comment 3

[Robert Kaiser]

I'd really like to see if signatures would be better to handle if we just strip all pieces in rounded or angle brackets. In theory, it was a good idea to have them, but signatures get very long, and even the rounded brackets (argument lists of functions) create more difficulty than they help with, as it's frequent enough that types get changed or arguments added/removed while the crashes remain the same. I think I never really saw it helping in terms of pointing to a specific overloaded function.

Comment 4

[(not currently active) Ted Mielczarek]

We already do a small form of this, in bug 481445 we decided to replace integer template parameters with "int". I think replacing all template parameters with T in signatures would probably be alright. As dmajor points out in comment 2 we often get the wrong function name anyway, since the identical function bodies have been COMDAT folded together, so nsCOMPtr<T>::foo() would actually be an improvement in many cases. For cases where it's not, presumably we could still leave the original function name as provided by Breakpad in the stack so that it'd be clearer what's going on.

On the Socorro side this fix would go into CSignatureToolBase:
https://github.com/mozilla/socorro/blob/47b804ed6b1bd1efa399d151e4cc02ac0d094fad/socorro/processor/signature_utilities.py#L67

Comment 5

[Robert Kaiser]

It looks like David, Ted and I agree that it would be good to collapse all signature pieces between < and > into <T> - lars, can we get that implemented?

Comment 6

[(Away)]

As a test case, Alpha<Bravo<Charlie>, Delta>::Echo<Foxtrot> should become Alpha<T>::Echo<T>

Comment 7

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

we currently have a normalization rule that applies to templates with integer parameters:

   f<3>(x, y, z) -->  f<int>(x, y, z)

does this new rule supersede this older template rule?  

   f<3>(x, y, z) -->  f<T>(x, y, z)

Comment 8

[(Away)]

I'd be fine with that.

Comment 9

[[github robot]]

Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/f1b5fcd223aba6b774c1698f67e4fa7ed96a3cd3
Merge pull request #2832 from twobraids/collapse_templates_in_signatures

fixes Bug 1163831 - collapse templates in signatures

Comment 10

[Mats Palmgren (inactive)]

This change should be announced on dev-platform so that we can avoid confusion
like in bug 1167557 where we thought the crash had magically stopped occurring.

I suspect we have many other bugs on file that are affected so everyone needs
to be aware of this so we don't close bugs that still occurs.

Comment 11

[Mats Palmgren (inactive)]

BTW, would it be possible to update Bugzilla signatures that has template args
with and *additional* signature that collapses the args to "T" to catch new
crashes with the new signature scheme?

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Oh wow. Thanks!  I will mention this on mailing lists and in the channel meeting.

Comment 13

[(Away)]

While we're at it we should mention that bug 1171437 is coming soon as well.

Comment 14

[Robert Kaiser]

(In reply to Mats Palmgren (:mats) from comment #10)
> This change should be announced on dev-platform

It will be but there is another even larger change to signatures coming in the next 2 weeks and I wanted to talk about both in the same email but had very little time before my vacation and Whistler.

Comment 15

[Robert Kaiser]

(In reply to David Major [:dmajor] from comment #13)
> While we're at it we should mention that bug 1171437 is coming soon as well.

That's exactly my plan, things just got short in the week before I left for vacation and Whistler.

(In reply to Mats Palmgren (:mats) from comment #11)
> BTW, would it be possible to update Bugzilla signatures that has template
> args
> with and *additional* signature that collapses the args to "T" to catch new
> crashes with the new signature scheme?

Feel free to write up something for it and endure all the bugmail annoyance that comes with it. I will only update bugs that are currently interesting and will do so only once we activated bug 1171437 as well.

Comment 16

[Mats Palmgren (inactive)]

I don't see why you can't announce these changes right now even if not all of them
are deployed yet.  We almost closed bug 1167557 as WFM b/c we were completely
unaware of this change.  I think you need to notify people *before* massive changes
like this.

Comment 17

[Robert Kaiser]

Because I needed some fricking vacation and you are getting me back into the stressed, grumpy situation I was in before I had some and pushing me into not having any relaxing effects of it left. Thanks for that.

Comment 18

[Robert Kaiser]

All that said, now that I cooled down a bit and am not stuck in travel stress, thanks for the pointer and we will do better in communication for future changes that will affect a larger amount of signatures.