1514746 - rust panics shouldn't have "GeckoCrashOOL" signature

Status: RESOLVED FIXED

(Socorro :: Signature, task)

Description

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/mozilla-central/rev/35f916cb6452 added a GeckoCrashOOL function that gets called on the rust panic path.  It shouldn't be getting in the way of signature generation.

Comment 1

[Julien Cristau [:jcristau]]

Attachment: GitHub Pull Request

Comment 2

[[github robot]]

Commits pushed to master at https://github.com/mozilla-services/socorro

https://github.com/mozilla-services/socorro/commit/331033fdd4e4e47acb2b4926c105ef680ca47ab9
Bug 1514746 - add std::panicking::begin_panic<T> to sentinels

https://github.com/mozilla-services/socorro/commit/033a6c200a0cd186cac5d58f6a79f64afa43d82e
Merge pull request #4747 from jcristau/geckocrashool

Bug 1514746 - add std::panicking::begin_panic<T> to the sentinels list

Comment 3

[Will Kahn-Greene [:willkg] ET needinfo? me]

Now that the changes are merged, this will land in stage. We're in a change freeze, so nothing will be deployed to prod until January. At that point, we can reprocess the affected crashes.

Marking as FIXED and adding a reminder to reprocess.

Thank you!

Comment 5

[Will Kahn-Greene [:willkg] ET needinfo? me]

I pushed the code to prod and reprocessed crashes with GeckoCrashOOL in them. There were 609 of them.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Thanks!

Comment 7

[Chris Peterson [:cpeterson]]

Julien, I'm renaming GeckoCrashOOL to GeckoCrash in bug 1507049. (The function name GeckoCrash is a little misleading because it is actually used to report Rust panics.) Based on your change here to add std::panicking::begin_panic<T> to the sentinels list, it looks like I won't need to GeckoCrash to the irrelevant_signature_re.txt list (because the signature generation algorithm will skip everything until it find the std::panicking::begin_panic<T> sentinel like in bp-70f7c1da-90a5-409c-8b9b-63bac0181116). Is that correct?

I'm also renaming MOZ_CrashOOL to MOZ_Crash (MOZ_CrashOOL's original name). I see that MOZ_Crash is already on the irrelevant_signature_re.txt list, but MOZ_CrashOOL is not.

It looks like there are plenty of unhelpful signatures like MOZ_CrashOOL or random names like ?MOZ_CrashOOL@@[e-mail].13761241159606428771:

https://crash-stats.mozilla.com/search/?signature=~MOZ_CrashOOL&date=%3E%3D2018-08-07T23%3A54%3A28.000Z&date=%3C2019-02-07T22%3A54%3A28.000Z&_facets=signature&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature

What function names would need to be skipped or declared a sentinel to give (for example) bp-0c353de7-41ce-4829-9495-231610190207 a useful signature? Should MOZ_CrashOOL be skipped? Should std::panicking::continue_panic_fmt be declared a sentinel like std::panicking::begin_panic_fmt is?

Comment 8

[Julien Cristau [:jcristau]]

Yes, adding std::panicking::continue_panic_fmt to the sentinel list should help for that crash:

Crash id: 0c353de7-41ce-4829-9495-231610190207
Original: ?MOZ_CrashOOL@@[e-mail].13761241159606428771
New: style::matching::PrivateMatchMethods::replace_rules_internal<T>
Same?: False

That seems to be a minority of the MOZ_CrashOOL signatures though, there's a bunch of cases like https://crash-stats.mozilla.com/report/index/ffd8fcc7-d0a4-4122-b6d5-6b28c0190207 with no good stack (facet on proto signature can be useful).