1702662 - Hit MOZ_CRASH(Unable to find a bin!) at gfx/wr/webrender/src/texture_pack/guillotine.rs:33

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20210331-88275f615ea5 (--enable-debug)

Hit MOZ_CRASH(Unable to find a bin!) at gfx/wr/webrender/src/texture_pack/guillotine.rs:33

#0 0x7f82d8cacd15 in MOZ_Crash /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:246:3
#1 0x7f82d8cacd15 in RustMozCrash src/mozglue/static/rust/wrappers.cpp:17:3
#2 0x7f82d8caccc4 in mozglue_static::panic_hook::h3ffea86f75ae0747 src/mozglue/static/rust/lib.rs:89:9
#3 0x7f82d8cac69b in core::ops::function::Fn::call::had4c2414fa4ff0d5 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/core/src/ops/function.rs:70:5
#4 0x7f82d9cb8af5 in std::panicking::rust_panic_with_hook::h71e6a073d87de1f5 /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:595:17
#5 0x7f82d9cb8616 in std::panicking::begin_panic_handler::_$u7b$$u7b$closure$u7d$$u7d$::hd549436f6bb6dbb8 /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:497:13
#6 0x7f82d9cb47db in std::sys_common::backtrace::__rust_end_short_backtrace::h4e5f4b72b04174c3 /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys_common/backtrace.rs:141:18
#7 0x7f82d9cb8578 in rust_begin_unwind /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:493:5
#8 0x7f82d9d216f0 in core::panicking::panic_fmt::hcd56f7f635f62c74 /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/panicking.rs:92:14
#9 0x7f82d9d212d2 in core::option::expect_failed::h5086d7196b9e2f90 /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/option.rs:1292:5
#10 0x7f82d870713a in core::option::Option$LT$T$GT$::expect::hecb2b6321a90ae76 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/core/src/option.rs:349:21
#11 0x7f82d870713a in webrender::texture_pack::guillotine::FreeListBin::for_size::hc3a50aeca015cf83 src/gfx/wr/webrender/src/texture_pack/guillotine.rs:27:9
#12 0x7f82d870713a in webrender::texture_pack::guillotine::GuillotineAllocator::push::h07a5121057724fa8 src/gfx/wr/webrender/src/texture_pack/guillotine.rs:87:18
#13 0x7f82d86259e7 in webrender::texture_pack::guillotine::GuillotineAllocator::new::h465cbfa1965ec8d3 src/gfx/wr/webrender/src/texture_pack/guillotine.rs:77:13
#14 0x7f82d86259e7 in webrender::render_task_graph::RenderTaskGraphBuilder::end_frame::h269aaef9fd4caa73 src/gfx/wr/webrender/src/render_task_graph.rs:493:44
#15 0x7f82d857ae9b in webrender::frame_builder::FrameBuilder::build::h96abc7b333009a95 src/gfx/wr/webrender/src/frame_builder.rs:595:28
#16 0x7f82d86012a7 in webrender::render_backend::Document::build_frame::h2c2e068aaa76b2ca src/gfx/wr/webrender/src/render_backend.rs:625:25
#17 0x7f82d8612a83 in webrender::render_backend::RenderBackend::update_document::hf32d91e7e7aa8a54 src/gfx/wr/webrender/src/render_backend.rs:1526:41
#18 0x7f82d8608ce6 in webrender::render_backend::RenderBackend::prepare_transactions::h51608eba19522f94 src/gfx/wr/webrender/src/render_backend.rs:1378:28
#19 0x7f82d8608ce6 in webrender::render_backend::RenderBackend::process_api_msg::haf62d3c9d16ae515 src/gfx/wr/webrender/src/render_backend.rs:1234:17
#20 0x7f82d83ec7ad in webrender::render_backend::RenderBackend::run::ha973b52bb21082e2 src/gfx/wr/webrender/src/render_backend.rs:905:21
#21 0x7f82d83ec7ad in webrender::renderer::Renderer::new::_$u7b$$u7b$closure$u7d$$u7d$::h48c56f8a0fcd0e1a src/gfx/wr/webrender/src/renderer/mod.rs:1281:13
#22 0x7f82d83ec7ad in std::sys_common::backtrace::__rust_begin_short_backtrace::ha15bd84dad9f1dc3 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:125:18
#23 0x7f82d840e389 in std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h6844b16081fab287 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/thread/mod.rs:474:17
#24 0x7f82d840e389 in _$LT$std..panic..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$$LP$$RP$$GT$$GT$::call_once::h3212da61736926a3 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/panic.rs:344:9
#25 0x7f82d840e389 in std::panicking::try::do_call::hb5cb857f30c65ed7 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/panicking.rs:379:40
#26 0x7f82d840e389 in std::panicking::try::h26e0339b2e3e66ac /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/panicking.rs:343:19
#27 0x7f82d840e389 in std::panic::catch_unwind::h398fd3c1a7a47df6 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/panic.rs:431:14
#28 0x7f82d840e389 in std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::h3cdd8789b0e21f98 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/thread/mod.rs:473:30
#29 0x7f82d840e389 in core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::haa0410989cb7b6c8 /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
#30 0x7f82d9cc90e9 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
#31 0x7f82d9cc90e9 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
#32 0x7f82d9cc90e9 in std::sys::unix::thread::Thread::new::thread_start::hb5e40d3d934ebb7a /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/sys/unix/thread.rs:71:17
#33 0x7f82e5c13608 in start_thread /build/glibc-eX1tMB/glibc-2.31/nptl/pthread_create.c:477:8
#34 0x7f82e57dc292 in clone /build/glibc-eX1tMB/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Comment 1

[Tyson Smith [:tsmith]]

Attachment: prefs.js

Comment 2

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/_14bT6o-FcsG3oAK1BuZyg/index.html

Comment 3

[Mayank Bansal]

https://crash-stats.mozilla.org/report/index/9f44d99d-2a3e-46a0-989b-94fd90210402

Comment 4

[Mayank Bansal]

2021-04-02T11:28:00.225000: INFO : Narrowed integration regression window from [ef6207cd, be0f7947] (3 builds) to [59344712, be0f7947] (2 builds) (~1 steps left)
2021-04-02T11:28:00.237000: DEBUG : Starting merge handling...
2021-04-02T11:28:00.237000: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=be0f7947d378a4764a10bd3e676e859146efa445&full=1
2021-04-02T11:28:00.238000: DEBUG : redo: attempt 1/3
2021-04-02T11:28:00.238000: DEBUG : redo: retry: calling _default_get with args: ('https://hg.mozilla.org/integration/autoland/json-pushes?changeset=be0f7947d378a4764a10bd3e676e859146efa445&full=1',), kwargs: {}, attempt #1
2021-04-02T11:28:00.240000: DEBUG : urllib3.connectionpool: Resetting dropped connection: hg.mozilla.org
2021-04-02T11:28:02.541000: DEBUG : urllib3.connectionpool: https://hg.mozilla.org:443 "GET /integration/autoland/json-pushes?changeset=be0f7947d378a4764a10bd3e676e859146efa445&full=1 HTTP/1.1" 200 None
2021-04-02T11:28:02.593000: DEBUG : Found commit message:
Bug 1700539 - Allow negative scale in ScaleOffset type. r=gfx-reviewers,kvark

Previously, a transform with a negative scale would result in a
new coordinate system being created when updating the spatial
tree.

This meant that a primitive in that space with a clip in a parent
space would create a clip mask (and was thus unable to be promoted
to a compositor surface).

This change allows negative scales to be part of the same coord
system (since they are still axis-aligned with the parent) and
avoid a clip mask in this case.

Differential Revision: https://phabricator.services.mozilla.com/D109580

2021-04-02T11:28:02.593000: DEBUG : Did not find a branch, checking all integration branches
2021-04-02T11:28:02.595000: INFO : The bisection is done.
2021-04-02T11:28:02.597000: INFO : Stopped

---

Comment 5

[Mayank Bansal]

Attachment: reduced testcase.html

Comment 6

[Glenn Watson [:gw]]

Nical, any ideas what kind of inputs would result in this panic occurring in the texture allocator?

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

tracking+ for 88. Let's backout bug 1700539 from Beta if we can't get this sorted soon.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

(In reply to Glenn Watson [:gw] from comment #6)

Nical, any ideas what kind of inputs would result in this panic occurring in the texture allocator?

Does the pernosco session from comment 2 help answer this?

Comment 10

[Glenn Watson [:gw]]

I will attempt to have a look at this today, if I can - it would be a shame to lose the performance benefit that this patch provides.

Comment 11

[Glenn Watson [:gw]]

Attachment: Bug 1702662 - Fix scale_factors calculation in presence in negative ScaleOffset.

The scale_factors method for ScaleOffset needs to take the absolute
value of the scale field, which is the same way that scale factors
are calculated in the slower transform based path. These scale factors
are used to determine the appropriate size of an offscreen surface in
some cases.

Comment 12

[Glenn Watson [:gw]]

There is a lot to unpack here, due to varying bug signatures / repro steps / bugzilla bugs, which I try to explain below.

TL;DR: I believe the patch attached here will fix all panics related to the regression from https://phabricator.services.mozilla.com/D109580 that we are currently seeing on nightly / beta.

There are two bugzilla bugs with related information:

Bug #1702662:

I was able to reproduce a panic on the test case in both comment 0 and also comment 5. However, the panic I was seeing was not the reported one in the bug title, instead it was a panic in picture.rs.

However, the attached patch in this bug fixes both test cases for me, and I am able to run them without any other panics. Possibilities include:
(1) Code changed somewhat recently so that the panic occurs in a different place.
(2) Reported crash signature is incorrect.
(3) That bug still exists and I can't repro it locally, but both test cases caused this unrelated panic that my patch fixes.

None of those seem particularly likely. However, it is very likely that the bug fixed by the patch could cause the referenced panic in texture_cache.rs, so I suspect that despite the panic signature not matching, this patch will completely fix this bug.

Bug #1699854:

This is a fuzzing bug that was reported some time ago, and (I believe) was detected before the original ScaleOffset patch landed, in which case it couldn't have been caused by that, and must be something else.

However, comment 13 includes a test case which does reproduce locally for me, with that panic signature, but which is definitely caused by the regression from the ScaleOffset patch, and I can confirm is fixed by the patch attached to this bug.

So I think here that the extra repro in comment 13 will be fixed, but the original fuzzing bug that I still cannot repro locally probably still exists (though this is much less important / common than the other repros).

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19b0de3b7106
Fix scale_factors calculation in presence in negative ScaleOffset. r=gfx-reviewers,jrmuizel

Comment 14

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/19b0de3b7106

Comment 15

[Glenn Watson [:gw]]

Comment on attachment 9213393 [details]
Bug 1702662 - Fix scale_factors calculation in presence in negative ScaleOffset.

Beta/Release Uplift Approval Request

• User impact if declined: Fixes panics in WR on a number of pages
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Per repro steps in bug
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's a verified fix for a panic. Alternative is to revert regressing patch from beta, but that would regress performance, esp. of webgl content.
• String changes made/needed:

Comment 16

[Bugmon [:jkratzer for issues]]

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

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9213393 [details]
Bug 1702662 - Fix scale_factors calculation in presence in negative ScaleOffset.

Crash fix looks good on Nightly and I'm not seeing any new spikes in recent builds. Approved for Desktop 88.0b7 & Fenix 88.0.0-beta.4. Thanks for including a test.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/00e297efab52c222b3ea4990f4138735c03dba25

Comment 19

[Alexandru Trif, Desktop QA [:atrif]]

Reproduced the issue on Ubuntu 18.04 with m-c 88275f615ea5 (20210331164215) and by using the both attached test cases and prefs file locally. Here are the terminal threads: link.

I can no longer reproduce the issue on Ubuntu 18.04 and Windows 10x64 using Firefox 88.0b7 (20210404164138) ASAN Bof build and the attached prefs. The test cases are properly loaded and no errors are presented.