Closed Bug 1318943 Opened 5 years ago Closed 5 years ago

Crash when parsing "chrome:" as a URI (Crash in xul.dll@0x5eff6 | xul.dll@0x61eea | mozilla::net::RustURL::GetFilePath)

Categories

(Core :: Networking, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
firefox52 --- unaffected
firefox53 + disabled
firefox54 --- fixed

People

(Reporter: mao, Assigned: valentin)

References

()

Details

(Keywords: crash, regression, Whiteboard: [necko-active])

Crash Data

This bug was filed from the Socorro interface and is 
report bp-c880e122-6474-431e-aef3-e688d2161120.
=============================================================
See Also: → 1318978
Sorry, crashing the parent process notwithstanding e10s turns out to be really simple.
Just issue a HTTP redirect to "chrome:", like (in PHP)

<?php
header('Location: chrome:')

See (warning: you'll crash the whole browser immediately)
https://maone.net/test/bugs/chrome-scheme-crash/
[Tracking Requested - why for this release]: Parent process crash.
Keywords: regression
Depends on: 1318432
Can confirm that this gets fixed by https://github.com/servo/rust-url/pull/242

However, bug 1318432 is the correct fix to land for this crash. Once my rust-url PR merges we can update the rust-url version to pull this in, but we should avoid shipping rust-url in nightly enabled by default for now anyway.
Duplicate of this bug: 1318978
Assignee: nobody → valentin.gosu
Crash Signature: [@ xul.dll@0x5eff6 | xul.dll@0x61eea | mozilla::net::RustURL::GetFilePath] Any attempt to parse "chrome:" as a URI causes this crash. To reproduce, just navigate data:text/html,<a href="chrome:"> Parsing the href attribute to normalize crashe… → [@ xul.dll@0x5eff6 | xul.dll@0x61eea | mozilla::net::RustURL::GetFilePath] [@ xul.dll@0x613fd | xul.dll@0x64a3c | xul.dll@0x649d3 | xul.dll@0x6a4eb | xul.dll@0x6a367 | rust_url_capi::rusturl_get_path ]
Whiteboard: [necko-active]
Crash Signature: [@ xul.dll@0x5eff6 | xul.dll@0x61eea | mozilla::net::RustURL::GetFilePath] [@ xul.dll@0x613fd | xul.dll@0x64a3c | xul.dll@0x649d3 | xul.dll@0x6a4eb | xul.dll@0x6a367 | rust_url_capi::rusturl_get_path ] → [@ xul.dll@0x5eff6 | xul.dll@0x61eea | mozilla::net::RustURL::GetFilePath] [@ xul.dll@0x613fd | xul.dll@0x64a3c | xul.dll@0x649d3 | xul.dll@0x6a4eb | xul.dll@0x6a367 | rust_url_capi::rusturl_get_path ] [@ xul.dll@0x613fd | xul.dll@0x649dc | xul.dll@0x64…
Ted, this is a crash signature with some xul.dll@0xBLAH entries. Does that indicate missing symbols? Is it a weird Rust-only thing? Seems like something you might be interested in...
Flags: needinfo?(ted)
(In reply to Nicholas Nethercote [:njn] from comment #6)
> Ted, this is a crash signature with some xul.dll@0xBLAH entries. Does that
> indicate missing symbols? Is it a weird Rust-only thing? Seems like
> something you might be interested in...

See also bug 1319298, which is about improving the crash signature for these crashes on Mac and Linux.
It's definitely not missing symbols in the way we usually talk about them (there are very clearly symbols for xul.dll in the frames below), but something is not right there. Given that this is a crash in Rust code it seems pretty likely that something is going wrong with symbol dumping for the Rust functions.
Flags: needinfo?(ted)
Interestingly frame 2 is clearly calling rusturl_get_path (from the crash report in comment 0):
https://hg.mozilla.org/mozilla-central/annotate/57a8cde3f08c/netwerk/base/RustURL.cpp#l477

and the symbol file for libxul has an entry for that function (I downloaded it from the link on the Modules tab):
FUNC 3a7a0 88 0 rust_url_capi::rusturl_get_path

...but it's at a different offset. I'll grab the PDB file and run it through dump_syms locally to see if I can figure out what's going on here.
Carrying over tracking flag from the duplicate.
Crash Signature: xul.dll@0x64973 | xul.dll@0x6a4db | xul.dll@0x6a357 | rust_url_capi::rusturl_get_path ] → xul.dll@0x64973 | xul.dll@0x6a4db | xul.dll@0x6a357 | rust_url_capi::rusturl_get_path ] [@ mozalloc_abort | abort | __rust_start_panic]
Crash Signature: xul.dll@0x64973 | xul.dll@0x6a4db | xul.dll@0x6a357 | rust_url_capi::rusturl_get_path ] [@ mozalloc_abort | abort | __rust_start_panic] → xul.dll@0x64973 | xul.dll@0x6a4db | xul.dll@0x6a357 | rust_url_capi::rusturl_get_path ] [@ xul.dll@0x613fd | xul.dll@0x64a5c | xul.dll@0x649f3 | xul.dll@0x6a50b | xul.dll@0x6a387 | rust_url_capi::rusturl_get_path] [@ mozalloc_abort | abort | __rust_sta…
Crash Signature: xul.dll@0x64973 | xul.dll@0x6a4db | xul.dll@0x6a357 | rust_url_capi::rusturl_get_path ] [@ xul.dll@0x613fd | xul.dll@0x64a5c | xul.dll@0x649f3 | xul.dll@0x6a50b | xul.dll@0x6a387 | rust_url_capi::rusturl_get_path] [@ mozalloc_abort | abort | __rust_sta… → xul.dll@0x64973 | xul.dll@0x6a4db | xul.dll@0x6a357 | rust_url_capi::rusturl_get_path ] [@ xul.dll@0x613fd | xul.dll@0x64a5c | xul.dll@0x649f3 | xul.dll@0x6a50b | xul.dll@0x6a387 | rust_url_capi::rusturl_get_path] [@xul.dll@0x613fd | xul.dll@0x64a5c | …
See Also: → 1320134
I updated to the latest nightly, flipped on the network.standard-url.enable-rust pref, and clicked the test URL and was able to produce this crash:
https://crash-stats.mozilla.com/report/index/95603eb6-4f4d-49a1-9752-3ff1c2161129

I had theorized (on IRC, apparently) that the top frames without symbols were because we didn't have symbols for libstd. Since bug 1318531 landed we now have symbols for those frames.
> I had theorized (on IRC, apparently) that the top frames without symbols
> were because we didn't have symbols for libstd. Since bug 1318531 landed we
> now have symbols for those frames.

Excellent. The signature for that crash is [@ std::panicking::rust_panic_with_hook]. Once https://github.com/mozilla/socorro/pull/3601 lands (just waiting for the Socorro freeze to end, should be any day now) any such future crashes will get a better signature due to better handling of the Rust panic frames via the skiplists.
(In reply to Nicholas Nethercote [:njn] from comment #12)
> > I had theorized (on IRC, apparently) that the top frames without symbols
> > were because we didn't have symbols for libstd. Since bug 1318531 landed we
> > now have symbols for those frames.
> 
> Excellent. The signature for that crash is [@
> std::panicking::rust_panic_with_hook]. Once
> https://github.com/mozilla/socorro/pull/3601 lands (just waiting for the
> Socorro freeze to end, should be any day now) any such future crashes will
> get a better signature due to better handling of the Rust panic frames via
> the skiplists.

So it looks like that pull landed - so do we just need to reprocess the signatures?
I got a slightly different crash signature when I reproduced on Windows - [@ rust_url_capi::rusturl_get_path ].

Should we close out this bug if it fixed by the pull in Comment 4?
Manish: did you update the in-tree rust-url to fix this? If so we can mark this FIXED.
(In reply to Marcia Knous [:marcia - use ni] from comment #14)
> I got a slightly different crash signature when I reproduced on Windows - [@
> rust_url_capi::rusturl_get_path ].

Thanks for checking, that's expected as the result of bug 1318531 + bug 1319298.
Crash Signature: [@ xul.dll@0x5eff6 | xul.dll@0x61eea | mozilla::net::RustURL::GetFilePath] [@ xul.dll@0x613fd | xul.dll@0x64a3c | xul.dll@0x649d3 | xul.dll@0x6a4eb | xul.dll@0x6a367 | rust_url_capi::rusturl_get_path ] [@ xul.dll@0x613fd | xul.dll@0x649dc | xul.dll@0x64… → [@ rust_url_capi::rusturl_get_path ] [@ xul.dll@0x5eff6 | xul.dll@0x61eea | mozilla::net::RustURL::GetFilePath] [@ xul.dll@0x613fd | xul.dll@0x64a3c | xul.dll@0x649d3 | xul.dll@0x6a4eb | xul.dll@0x6a367 | rust_url_capi::rusturl_get_path ] [@ xul.dll@0x…
Duplicate of this bug: 1323776
Crash Signature: rust_url_capi::rusturl_get_path] [@xul.dll@0x613fd | xul.dll@0x64a5c | xul.dll@0x649f3 | xul.dll@0x6a50b | xul.dll@0x6a387 | rust_url_capi::rusturl_get_path] [@ mozalloc_abort | abort | __rust_start_panic] → rust_url_capi::rusturl_get_path] [@xul.dll@0x613fd | xul.dll@0x64a5c | xul.dll@0x649f3 | xul.dll@0x6a50b | xul.dll@0x6a387 | rust_url_capi::rusturl_get_path] [@ mozalloc_abort | abort | __rust_start_panic] [@ mozalloc_abort | abort | __rust_start_panic…
Hi Valentin, we looked at this during regression triage today. Any traction on this issue?
Flags: needinfo?(valentin.gosu)
Depends on: 1339809
Should be fixed now that we updated rust-url in bug 1339809.
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(valentin.gosu)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.