1493630 - Crash in std::panicking::rust_panic_with_hook

Status: RESOLVED WORKSFORME

(Core :: Networking, defect, P2)

Description

[Gabriele Svelto [:gsvelto]]

This bug was filed from the Socorro interface and is
report bp-a101f4a1-3862-4771-a52d-10dd10180924.
=============================================================

Top 2 frames of crashing thread:

0 xul.dll static void std::panicking::rust_panic_with_hook src/libstd/panicking.rs:521
1 xul.dll exp2 

=============================================================

Very shallow stack unfortunately (corrupted stack maybe?). The volume is rather high though.

Comment 1

[Gabriele Svelto [:gsvelto]]

Mike, Ted, any clues as to what could be causing this?

Comment 2

[(not currently active) Ted Mielczarek]

I don't have access to my Windows machine this week, but someone should open this dump in Visual Studio or WinDbg and see if we get a better stack out of those. Note the `MOZ_CRASH` reason field does say:
byte index 227636000 is out of bounds of `euc-word-view.officeapps.live.com`

So it looks like a bad index into a `&str` in some Rust code.

Comment 3

[Selena Deckelmann :selenamarie :selena]

Anthony -- is there anyone else on your team that can have a look?

Comment 4

[[:philipp]]

there is a whole range of different "moz crash reason" fields for these crash reports:
https://crash-stats.mozilla.com/search/?signature=%3Dstd%3A%3Apanicking%3A%3Arust_panic_with_hook&date=%3E%3D2018-08-01&_facets=moz_crash_reason#facet-moz_crash_reason

Comment 5

[Selena Deckelmann :selenamarie :selena]

Anthony - please find an owner for investigating and putting this bug to bed if it is not actionable. Thanks!

Comment 6

[(not currently active) Ted Mielczarek]

I downloaded the dump from the crash in comment 0. WinDBG doesn't give us much more here, although it does show inlined frames:
 # ChildEBP RetAddr  
00 0dafef44 68e741d1 xul!std::panicking::rust_panic_with_hook(void)+0x186 [C:\projects\rust\src\libstd\panicking.rs @ 521]
01 0dafef78 68e74f75 xul!std::panicking::continue_panic_fmt(void)+0x51 [C:\projects\rust\src\libstd\panicking.rs @ 426]
02 0dafef7c 68e8022a xul!std::panicking::rust_begin_panic(void)+0x5 [C:\projects\rust\src\libstd\panicking.rs @ 337]
03 0dafefa4 68e7a397 xul!core::panicking::panic_fmt(void)+0x3a [C:\projects\rust\src\libcore\panicking.rs @ 92]
04 00000000 00000000 xul!core::str::slice_error_fail(void)+0x147 [C:\projects\rust\src\libcore\str\mod.rs @ 2094]

Comment 7

[Gabriele Svelto [:gsvelto]]

As :philipp pointed out in comment 4 there's a whole bunch of different reasons for this signature. We should probably put the contents of MozCrashReason in the signature and reprocess these crashes if we want to make heads or tails out of them.

Comment 8

[(not currently active) Ted Mielczarek]

FWIW, for the crash in comment 0 I ran it through my tool that dumps anything on the stack that looks like a potential return address, and farther down the stack we find:

0xdaff020 xul.dll!static struct str* core::str::traits::{{impl}}::index::{{closure}}(struct closure) [https://github.com/rust-lang/rust/blob/9634041f0e8c0f3191d2867311276f19d0a42564/src/libcore/str/mod.rs#L1942 + 18]
0xdaff030 xul.dll!static union core::result::Result<(url::host::Host<alloc::string::String>, url::parser::Input), url::parser::ParseError> url::parser::Parser::parse_host(url::parser::SchemeType) [https://hg.mozilla.org/releases/mozilla-beta/annotate/503cd479184519f50341493317e1aacf0d0d35ec/third_party/rust/url/src/parser.rs#l886 + 24]
0xdaff048 xul.dll!static union core::result::Result<(url::host::Host<alloc::string::String>, url::parser::Input), url::parser::ParseError> url::parser::Parser::parse_host(url::parser::SchemeType) [https://hg.mozilla.org/releases/mozilla-beta/annotate/503cd479184519f50341493317e1aacf0d0d35ec/third_party/rust/url/src/parser.rs#l841 + 23]
0xdaff078 xul.dll!static union core::result::Result<(core::option::Option<u32>, core::option::Option<u32>), url::parser::ParseError> url::parser::Parser::parse_query_and_fragment(unsigned int, struct url::parser::Input) [https://hg.mozilla.org/releases/mozilla-beta/annotate/503cd479184519f50341493317e1aacf0d0d35ec/third_party/rust/url/src/parser.rs#l1122 + 11]
0xdaff0b4 xul.dll!static union core::result::Result<url::Url, url::parser::ParseError> url::parser::Parser::after_double_slash(struct url::parser::Input, url::parser::SchemeType, unsigned int) [https://hg.mozilla.org/releases/mozilla-beta/annotate/503cd479184519f50341493317e1aacf0d0d35ec/third_party/rust/url/src/parser.rs#l754 + 27]
0xdaff0c4 xul.dll!static union core::result::Result<url::Url, url::parser::ParseError> url::parser::Parser::after_double_slash(struct url::parser::Input, url::parser::SchemeType, unsigned int) [https://hg.mozilla.org/releases/mozilla-beta/annotate/503cd479184519f50341493317e1aacf0d0d35ec/third_party/rust/url/src/parser.rs#l745 + 15]
0xdaff0ec xul.dll!static core::result::Result url::host::{{impl}}::fmt<alloc::string::String>(union url::host::Host<alloc::string::String> *, struct core::fmt::Formatter *) [https://hg.mozilla.org/releases/mozilla-beta/annotate/503cd479184519f50341493317e1aacf0d0d35ec/third_party/rust/url/src/host.rs#l183 + 0]

Which looks pretty sensible and matches the crash reason in comment 2.

Comment 9

[Valentin Gosu [:valentin] (he/him)]

(In reply to Ted Mielczarek [:ted] [:ted.mielczarek] from comment #8)
> FWIW, for the crash in comment 0 I ran it through my tool that dumps
> anything on the stack that looks like a potential return address, and
> farther down the stack we find:

Thanks for the stack, Ted. Can we get some of the C++ stack trace from the dump?

Comment 10

[(not currently active) Ted Mielczarek]

Attachment: dumpstack output

Here's the full output from that tool. (dumpstack from https://github.com/luser/rust-minidump/tree/master/minidump-tools ) It can be a little hard to interpret because it just goes through the stack memory on the crashing thread a word at a time and outputs any value that resolves to an address inside a known function.

Comment 11

[Valentin Gosu [:valentin] (he/him)]

(In reply to Ted Mielczarek [:ted] [:ted.mielczarek] from comment #10)
> (dumpstack from https://github.com/luser/rust-minidump/tree/master/minidump-tools ) 

Awesome tool. I bookmarked it :D

> Created attachment 9026350 [details]
> dumpstack output
> 
> Here's the full output from that tool. 

OK, so it seems the call is coming from ClientIsValidPrincipalInfo where we use MozURL to compare origins.
Now I just have to figure out why this is is panicking :)

Comment 12

[Julien Cristau [:jcristau] (back April 22)]

(In reply to Gabriele Svelto [:gsvelto] from comment #7)
> As :philipp pointed out in comment 4 there's a whole bunch of different
> reasons for this signature. We should probably put the contents of
> MozCrashReason in the signature and reprocess these crashes if we want to
> make heads or tails out of them.

Will, I don't suppose something like the above would be possible?  Or somehow figure out why these stacks are so bad?

Comment 13

[Julien Cristau [:jcristau] (back April 22)]

Out of the crashes in the last week with this signature, about half look like
"byte index XXX is out of bounds of `XXX`"

Half of the rest are of the form
"called `Result::unwrap()` on an `Err` value: FailedAllocationError { reason: "out of memory when allocating RawTable", allocation_info: Some(AllocationInfo { size: XXX, alignment: 4 }) }"

And as far as I can tell they're all win32.

Comment 14

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

Julien: Socorro explicitly sanitizes MozCrashReason values because they can contain anything and some of that is definitely PII. There's a bug for fixing how MozCrashReason is set in the case of Rust crashes such that the value is static and not dynamically generated. I don't recall the bug number and can't seem to find it.

There's a signature generation rule that stomps on the signature in the case where the parent id isn't the same as the child id:

https://github.com/mozilla-services/socorro/blob/935ff45f896e65e75732bc35cd63f41685f8ee2d/socorro/signature/rules.py#L796

Maybe we can add a signature generation rule that adds an indicator to the signature based on the MozCrashReason value. For example:

   https://crash-stats.mozilla.com/report/index/286a2a6f-311d-49c9-b3eb-dd56b0181125

could become:

   out of bounds | std::panicking::rust_panic_with_hook

If something like that is interesting to look into, we should do it in a new bug.

Comment 15

[Julien Cristau [:jcristau] (back April 22)]

So far it looks like this is gone in 65 (no crash in early beta builds).

Comment 16

[Julien Cristau [:jcristau] (back April 22)]

Let's call this one fixed.