Closed Bug 1707744 Opened 4 years ago Closed 3 years ago

Hit MOZ_CRASH(called `Option::unwrap()` on a `None` value) at /builds/worker/checkouts/gecko/third_party/rust/euclid/src/size.rs:285

Categories

(Core :: Graphics: WebRender, defect)

defect

Tracking

()

VERIFIED FIXED
90 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox88 --- unaffected
firefox89 --- unaffected
firefox90 --- verified

People

(Reporter: tsmith, Assigned: nical)

References

(Blocks 2 open bugs, Regression)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [bugmon:bisected,confirmed])

Crash Data

Attachments

(2 files)

Attached file testcase.html (deleted) —

Found while fuzzing m-c 20210423-9b7bac5af873 (--enable-debug --enable-fuzzing)

Hit MOZ_CRASH(called Option::unwrap() on a None value) at /builds/worker/checkouts/gecko/third_party/rust/euclid/src/size.rs:285

#0 0x7fc700637575 in MOZ_Crash /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:256:3
#1 0x7fc700637575 in RustMozCrash src/mozglue/static/rust/wrappers.cpp:17:3
#2 0x7fc700637524 in mozglue_static::panic_hook::h478557d7509e77f5 src/mozglue/static/rust/lib.rs:89:9
#3 0x7fc700636efb in core::ops::function::Fn::call::h3d42afac2264c64f /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/core/src/ops/function.rs:70:5
#4 0x7fc701658ef5 in std::panicking::rust_panic_with_hook::h71e6a073d87de1f5 /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:595:17
#5 0x7fc7016589e6 in std::panicking::begin_panic_handler::_$u7b$$u7b$closure$u7d$$u7d$::hd549436f6bb6dbb8 /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:495:13
#6 0x7fc701654bdb in std::sys_common::backtrace::__rust_end_short_backtrace::h4e5f4b72b04174c3 /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys_common/backtrace.rs:141:18
#7 0x7fc701658978 in rust_begin_unwind /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:493:5
#8 0x7fc7016c1af0 in core::panicking::panic_fmt::hcd56f7f635f62c74 /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/panicking.rs:92:14
#9 0x7fc7016c1a3c in core::panicking::panic::h07405d6be4bce887 /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/panicking.rs:50:5
#10 0x7fc70006355e in webrender::scene_building::SceneBuilder::build_item::h0b1c12c11bfac5d7 src/gfx/wr/webrender/src/scene_building.rs
#11 0x7fc700046dda in webrender::scene_building::SceneBuilder::build_all::h51872254ed9e7607 src/gfx/wr/webrender/src/scene_building.rs:803:25
#12 0x7fc700042093 in webrender::scene_building::SceneBuilder::build::h56c097327dee4138 src/gfx/wr/webrender/src/scene_building.rs:561:9
#13 0x7fc700042093 in webrender::scene_builder_thread::SceneBuilderThread::process_transaction::h7f2e82f3648ab675 src/gfx/wr/webrender/src/scene_builder_thread.rs:665:25
#14 0x7fc700033b04 in webrender::scene_builder_thread::SceneBuilderThread::run::_$u7b$$u7b$closure$u7d$$u7d$::h8bdb705b0c3d972e src/gfx/wr/webrender/src/scene_builder_thread.rs:318:36
#15 0x7fc700033b04 in core::iter::adapters::map::map_try_fold::_$u7b$$u7b$closure$u7d$$u7d$::h9f2d5f6788ed6ea8 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/core/src/iter/adapters/map.rs:87:28
#16 0x7fc700033b04 in core::iter::traits::iterator::Iterator::try_fold::hd34ccf2b6f068f9e /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/core/src/iter/traits/iterator.rs:1983:21
#17 0x7fc700033b04 in _$LT$core..iter..adapters..map..Map$LT$I$C$F$GT$$u20$as$u20$core..iter..traits..iterator..Iterator$GT$::try_fold::haa31aebd04934bc5 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/core/src/iter/adapters/map.rs:113:9
#18 0x7fc700033b04 in alloc::vec::source_iter_marker::_$LT$impl$u20$alloc..vec..spec_from_iter..SpecFromIter$LT$T$C$I$GT$$u20$for$u20$alloc..vec..Vec$LT$T$GT$$GT$::from_iter::hfe762fe24140954b /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/alloc/src/vec/source_iter_marker.rs:60:20
#19 0x7fc700033b04 in _$LT$alloc..vec..Vec$LT$T$GT$$u20$as$u20$core..iter..traits..collect..FromIterator$LT$T$GT$$GT$::from_iter::h4b939600b2803d45 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:2196:9
#20 0x7fc700033b04 in core::iter::traits::iterator::Iterator::collect::h33e25d00782e18b0 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/core/src/iter/traits/iterator.rs:1765:9
#21 0x7fc700033b04 in webrender::scene_builder_thread::SceneBuilderThread::run::hce3a6357122a239c src/gfx/wr/webrender/src/scene_builder_thread.rs:317:67
#22 0x7fc6ffd79cdc in webrender::renderer::Renderer::new::_$u7b$$u7b$closure$u7d$$u7d$::h7b5f0ec06ff895e8 src/gfx/wr/webrender/src/renderer/mod.rs:1212:13
#23 0x7fc6ffd79cdc in std::sys_common::backtrace::__rust_begin_short_backtrace::h226c57af29c9b4e0 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:125:18
#24 0x7fc6ffd9c559 in std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::ha4677264de486b03 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/thread/mod.rs:474:17
#25 0x7fc6ffd9c559 in _$LT$std..panic..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$$LP$$RP$$GT$$GT$::call_once::h753b4f06d310cb62 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/panic.rs:344:9
#26 0x7fc6ffd9c559 in std::panicking::try::do_call::h98c1b20892542b6f /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/panicking.rs:379:40
#27 0x7fc6ffd9c559 in std::panicking::try::hedc6cb4b54b39457 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/panicking.rs:343:19
#28 0x7fc6ffd9c559 in std::panic::catch_unwind::h82540a0debded83a /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/panic.rs:431:14
#29 0x7fc6ffd9c559 in std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::hd776dbb7ae2eb4ab /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/thread/mod.rs:473:30
#30 0x7fc6ffd9c559 in core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::hf04a58c686900591 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
#31 0x7fc7016694e9 in _$LT$alloc..boxed..Box$LT$F$C$A$GT$$u20$as$u20$core..ops..function..FnOnce$LT$Args$GT$$GT$::call_once::h61144a2be4ee36d8 /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/alloc/src/boxed.rs:1521:9
#32 0x7fc7016694e9 in _$LT$alloc..boxed..Box$LT$F$C$A$GT$$u20$as$u20$core..ops..function..FnOnce$LT$Args$GT$$GT$::call_once::hcf5d395fdd120c17 /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/alloc/src/boxed.rs:1521:9
#33 0x7fc7016694e9 in std::sys::unix::thread::Thread::new::thread_start::hb5e40d3d934ebb7a /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys/unix/thread.rs:71:17
#34 0x7fc70d58c608 in start_thread /build/glibc-eX1tMB/glibc-2.31/nptl/pthread_create.c:477:8
#35 0x7fc70d155292 in clone /build/glibc-eX1tMB/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Flags: in-testsuite?

A Pernosco session is available here: https://pernos.co/debug/k95Kx--8VxakxB_UgciioA/index.html

Ironically, it panics on checking the fast path. It doesn't actually need that to_i32 for the logic

if segment_rect.to_i32().is_empty() {

Nical, given that this is in optimize_linear_gradient, I assume you'd be interested to look?
Obviously, we can just do try_cast instead of cast.
But more importantly, we should reconsider this whole idea of exposing cast() and making it so easy to use, since it panics. We shouldn't make the panicing code easy to write.

Blocks: wr-fuzz
Severity: -- → S3
Flags: needinfo?(nical.bugzilla)

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20210426213158-6f8320a4798f.
The bug appears to have been introduced in the following build range:

Start: c00239b6c35155149b2e2d8caa02c1c96d21e546 (20210422093115)
End: f733b37f9626548ae83564a3fd43ddfa972977d9 (20210422104010)
Pushlog: https://hg.mozilla.org/mozilla-unified/pushloghtml?fromchange=c00239b6c35155149b2e2d8caa02c1c96d21e546&tochange=f733b37f9626548ae83564a3fd43ddfa972977d9

Whiteboard: [bugmon:bisected,confirmed]
Crash Signature: [@ webrender::scene_building::SceneBuilder::build_item ]
Assignee: nobody → nical.bugzilla
Flags: needinfo?(nical.bugzilla)

:nical, since this bug contains a bisection range, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Flags: needinfo?(nical.bugzilla)

(In reply to Dzmitry Malyshau [:kvark] from comment #2)

Ironically, it panics on checking the fast path. It doesn't actually need that to_i32 for the logic

if segment_rect.to_i32().is_empty() {

Nical, given that this is in optimize_linear_gradient, I assume you'd be interested to look?
Obviously, we can just do try_cast instead of cast.

It's not as simple as bailing out if casting fails. We have gradient endpoints stops that are so fat appart that their difference can't be represented with 32bits integers, but the primitive bounds are sane and something should be shown. Applying the clip rect before casting would help. The fuzzer might still be able to come up with a test case with both very far endpoints and very large gradient bounds but it will have a harder time because gecko makes an effort to clip primitives to the viewport.

But more importantly, we should reconsider this whole idea of exposing cast() and making it so easy to use, since it panics. We shouldn't make the panicing code easy to write.

I disagree. This is a rather extreme case but 99.9% of the time you know or expect to be working with reasonable ranges of values and should be able to do simple casts conveniently. If we didn't have an infallible cast we'd have written .unwrap() here (and in all other similar places that come to mind) because it was reasonable to expect i32 to be a big enough representation, resulting in the same bug. This particular case is not very different from all arithmetic operations that don't return an Option even though their result might not be representable.

I think that the assertion+fuzzing combo we already have here is the most pragmatic way to deal with this.

Flags: needinfo?(nical.bugzilla)
Regressed by: 1702228
Has Regression Range: --- → yes

Large segment bounds trip an assertion when casting coordinates to integers. Clipping early also reduces the amount of cached pixels.

Set release status flags based on info from the regressing bug 1702228

Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6fc108de5c8f Avoid far gradient endpoints causing large gradient segments. r=gfx-reviewers,lsalzman
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20210503153234-cdcfe2f59d26.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Status: RESOLVED → VERIFIED
Keywords: bugmon
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: