1271529 - Add entries to prefix_signature_re for Rust panics

Status: RESOLVED DUPLICATE of bug 1373272

(Socorro :: Signature, task)

Description

[Nicholas Nethercote [inactive]]

Rust panics currently all get merged into a single signature. We need to do some regex stuff in Socorro to get more frames pulled into crash signatures. The hard part is working out exactly what to include.

Here are some example crash reports involving Rust panics.

https://crash-stats.mozilla.com/report/index/d05f717e-3198-4f6b-bd5c-1bfee2160509

> 0 	XUL 	rust_panic 	
> 1 	XUL 	fmt::num::_$LT$impl$GT$::fmt::hae8058ba3dd6d5c8iHU 	
> 2 	XUL 	fmt::_$LT$impl$GT$::fmt::h4df0c7957bab19b88pW 	
> 3 	XUL 	sys_common::unwind::begin_unwind_inner::h766be80a74d04fd1dPs 	
> 4 	XUL 	sys_common::unwind::begin_unwind_inner::h766be80a74d04fd1dPs 	
> 5 	XUL 	str54286 	
> 6 	XUL 	str54581

https://crash-stats.mozilla.com/report/index/3e67a61a-5aa3-4080-993e-b23622160501

> 0 	XUL 	rust_panic 	
> 1 	XUL 	fmt::num::RadixFmt$LT$isize$C$$u20$Radix$GT$.fmt..Display::fmt::h2b8187f207a16fc1UST 	
> 2 	XUL 	fmt::Arguments$LT$$u27$a$GT$.Debug::fmt::h7448e19c39f327afuCV 	
> 3 	XUL 	const35784 	
> 4 	XUL 	sys_common::unwind::begin_unwind_inner::h8870c6d052d6d406qas 	
> 5 	XUL 	str49837

https://crash-stats.mozilla.com/report/index/6125cfbb-05bc-487d-a0db-8400c2160504

> 0 	XUL 	rust_panic 	
> 1 	XUL 	sys_common::unwind::begin_unwind_inner::h83fec1ce131695c5dPs 	
> 2 	XUL 	sys_common::unwind::begin_unwind_fmt::ha676a2adc0d109c8jOs 	
> 3 	XUL 	rust_begin_unwind 	
> 4 	XUL 	panicking::panic_fmt::h648451da899e8314LFK 	
> 5 	XUL 	mp4parse_read

So we definitely want these two:

- rust_panic
- sys_common::unwind::begin_unwind_inner::.*

But I'm not sure about the fmt:: entries, and how loose we need the matching to be. I also don't know what the "const" and "str" entries mean.

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Nicholas Nethercote [:njn] from comment #0)
> I also don't know what the "const" and "str" entries mean.

Those look like stack scanning gone wrong — when breakpad has no other way to unwind, it picks up anything on the stack that looks like a return address.  So it can yield stack frames that already returned (the fmt:: frames look like that), and apparently also picked up some pointers into .rodata in some cases.  (It can also cause the more precise unwinding tactics to be applied to a partially-overwritten stack frame and yield additional garbage, and that can be seen happening here in the "Raw Dump" tab on crash-stats, but none of those turned up anything with a name, so they're not in the listings pasted above.)

The first two traces use stack scanning immediately after the initial rust_panic context, so they're not very trustworthy.  The third one uses frame pointer unwinding up to mp4parse_read, so that's at least a vague approximation of how things will look once bug 1268617 and related fixes start being used.  I don't know offhand if Socorro uses the "trust" key from the stack frames, but if that's why it's not including anything past "rust_panic" there then that will hopefully be fixed soon.

And as for that third stack… it seems to me that, given "rust_panic", those next 4 frames don't add any information, and the next meaningful one is mp4parse_read; i.e., we'd want to suppress the panic internals while including frames after them, which I assume we already have some way to do for Gecko C++ assertion/panic stuff?  But note that https://github.com/rust-lang/rust/pull/32900 just changed most of the names in question.

Comment 2

[Nicholas Nethercote [inactive]]

> And as for that third stack… it seems to me that, given "rust_panic", those
> next 4 frames don't add any information, and the next meaningful one is
> mp4parse_read; i.e., we'd want to suppress the panic internals while
> including frames after them, which I assume we already have some way to do
> for Gecko C++ assertion/panic stuff?

If I understand the question correctly, the way to do it is via these regexp lists, such as prefix_signature_re, which give Socorro control over how stack frames get added to the crash signature -- you can use these lists to ignore certain stack frames, or to include them in the signature but also request that the subsequent stack frame be considered.

Comment 3

[[DEACTIVATED] Adrian Gaudebert]

Sorry for taking this long getting to this. I have recently been working on making it a lot easier for users to edit those signatures lists themselves. We now have them in separate text files in our repo, and I wrote some documentation to help people make changes there. All of that is here: https://github.com/mozilla/socorro/tree/master/socorro/siglists

Nicholas, would you like to give it a try and make those changes yourself? We can meet in London to do that if you want. Feedback on that process would be appreciated of course, we are doing this to make the whole process of updating the skip lists easier and faster for everyone. :)

Comment 4

[Nicholas Nethercote [inactive]]

I see the file. I'll do it once we work out the hard part, which is deciding exactly how to filter these...

Comment 5

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

It looks like we still don't have any correctly unwound Rust crashes on crash-stats yet, or at least nothing that comes up in a search for rust_panic, but locally produced/processed crashes might be usable for figuring out the signature generation settings (I haven't tried).

Comment 6

[(not currently active) Ted Mielczarek]

So from our discussion in the oxidation meeting in London, the consensus was that the stacks are being unwound correctly, we will just get some incorrect symbols in frames from libstd/libcore because we don't have debug symbols there. I can't actually find any `rust_panic` reports from any builds that have the fix for bug 1268617, so I'm not totally sure how to proceed here.

I did have an insight at the end of the oxidation meeting--if we do want to handle all `rust_panic` crashes similarly, we probably want to put that frame into the "Signature Sentinels" list:
https://github.com/mozilla/socorro/tree/master/socorro/siglists#signature-sentinels

Comment 7

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

This crash has a signature of "rust_panic":

https://crash-stats.mozilla.com/report/index/eba8d291-9e87-4ff4-ada5-f63c00180321

It's been two years since the discussion. Do we still want to change signature generation for crashes like this one? If so, what do we want to do?

Comment 8

[(not currently active) Ted Mielczarek]

Those are all from really old versions of Firefox before we fixed debug symbols in the Rust standard library code. It looks like bug 1373272 fixed the current behavior here.