1758127 - Hit MOZ_CRASH(called `Option::unwrap()` on a `None` value) at gfx/wr/webrender/src/batch.rs:1362

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Jason Kratzer [:jkratzer]]

Testcase found while fuzzing mozilla-central rev b01b8627f45f (built with: --enable-debug --enable-fuzzing).

Testcase can be reproduced using the following commands:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch --build b01b8627f45f --debug --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html

Hit MOZ_CRASH(called `Option::unwrap()` on a `None` value) at gfx/wr/webrender/src/batch.rs:1362

    ==823809==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f7cd4116945 bp 0x7f7c424ef440 sp 0x7f7c424ef430 T823942)
    ==823809==The signal is caused by a WRITE memory access.
    ==823809==Hint: address points to the zero page.
        #0 0x7f7cd4116945 in MOZ_Crash /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:261:3
        #1 0x7f7cd4116945 in RustMozCrash /mozglue/static/rust/wrappers.cpp:18:3
        #2 0x7f7cd41167a4 in mozglue_static::panic_hook::h773f18c382903796 /mozglue/static/rust/lib.rs:91:9
        #3 0x7f7cd411630b in core::ops::function::Fn::call::ha1de6d8c8d2b790f /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/ops/function.rs:70:5
        #4 0x7f7cd4f04d44 in std::panicking::rust_panic_with_hook::h1a5ea2d6c23051aa /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/panicking.rs:610:17
        #5 0x7f7cd4f04a11 in std::panicking::begin_panic_handler::_$u7b$$u7b$closure$u7d$$u7d$::h07f549390938b73f /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/panicking.rs:500:13
        #6 0x7f7cd4f00923 in std::sys_common::backtrace::__rust_end_short_backtrace::h5ec3758a92cfb00d /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/sys_common/backtrace.rs:139:18
        #7 0x7f7cd4f04778 in rust_begin_unwind /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/panicking.rs:498:5
        #8 0x7f7cca8d83e0 in core::panicking::panic_fmt::h3a79a6a99affe1d5 /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panicking.rs:116:14
        #9 0x7f7cca8d832c in core::panicking::panic::h97167cd315d19cd4 /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panicking.rs:48:5
        #10 0x7f7cd39e4720 in core::option::Option$LT$T$GT$::unwrap::h0bd9deeaa6970f43 /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/option.rs:729:21
        #11 0x7f7cd39e4720 in webrender::batch::BatchBuilder::add_prim_to_batch::_$u7b$$u7b$closure$u7d$$u7d$::hff59fbc6943b522e /gfx/wr/webrender/src/batch.rs:1362:64
        #12 0x7f7cd3b8f0d8 in webrender::resource_cache::ResourceCache::fetch_glyphs::h8f8afebd3b612084 /gfx/wr/webrender/src/resource_cache.rs:1154:13
        #13 0x7f7cd39dccca in webrender::batch::BatchBuilder::add_prim_to_batch::h21a746ddf2e30849 /gfx/wr/webrender/src/batch.rs:1275:17
        #14 0x7f7cd3a64f10 in webrender::batch::BatchBuilder::add_pic_to_batch::hfb104a41b7ab2090 /gfx/wr/webrender/src/batch.rs:876:17
        #15 0x7f7cd3a64f10 in webrender::frame_builder::build_render_pass::hce295e36482740f9 /gfx/wr/webrender/src/frame_builder.rs:943:9
        #16 0x7f7cd3a64f10 in webrender::frame_builder::FrameBuilder::build::he9c3d4392d7dc5fa /gfx/wr/webrender/src/frame_builder.rs:655:28
        #17 0x7f7cd3adf926 in webrender::render_backend::Document::build_frame::hdb47e483955b9990 /gfx/wr/webrender/src/render_backend.rs:493:25
        #18 0x7f7cd3af5f4a in webrender::render_backend::RenderBackend::update_document::hddcfd3ccea10d6f7 /gfx/wr/webrender/src/render_backend.rs:1387:41
        #19 0x7f7cd3aeae70 in webrender::render_backend::RenderBackend::prepare_transactions::hb5ea8d5add0fff26 /gfx/wr/webrender/src/render_backend.rs:1236:28
        #20 0x7f7cd3aeae70 in webrender::render_backend::RenderBackend::process_api_msg::h420b35c10dc36626 /gfx/wr/webrender/src/render_backend.rs:1088:17
        #21 0x7f7cd3b1d52c in webrender::render_backend::RenderBackend::run::h2e623193e95a1225 /gfx/wr/webrender/src/render_backend.rs:758:21
        #22 0x7f7cd3b1d52c in webrender::renderer::Renderer::new::_$u7b$$u7b$closure$u7d$$u7d$::h71d03fd1931e9bd6 /gfx/wr/webrender/src/renderer/mod.rs:1328:13
        #23 0x7f7cd3b1d52c in std::sys_common::backtrace::__rust_begin_short_backtrace::haa681597b7253c7c /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/sys_common/backtrace.rs:123:18
        #24 0x7f7cd38b2dee in std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h2164df2487f01841 /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/thread/mod.rs:477:17
        #25 0x7f7cd38b2dee in _$LT$core..panic..unwind_safe..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$$LP$$RP$$GT$$GT$::call_once::h0247046a4e5f6a61 /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panic/unwind_safe.rs:271:9
        #26 0x7f7cd38b2dee in std::panicking::try::do_call::h34f69670c47ed47f /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/panicking.rs:406:40
        #27 0x7f7cd38b2dee in std::panicking::try::hb1c8891e6e1c3b28 /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/panicking.rs:370:19
        #28 0x7f7cd38b2dee in std::panic::catch_unwind::he0e01a26201bb699 /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/panic.rs:133:14
        #29 0x7f7cd38b2dee in std::thread::Builder::spawn_unchecked::_$u7b$$u7b$closure$u7d$$u7d$::h6c42819231972e85 /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/thread/mod.rs:476:30
        #30 0x7f7cd38b2dee in core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::hbe8f8a7be2039d20 /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/ops/function.rs:227:5
        #31 0x7f7cd4f10992 in _$LT$alloc..boxed..Box$LT$F$C$A$GT$$u20$as$u20$core..ops..function..FnOnce$LT$Args$GT$$GT$::call_once::h49b6c7c5155a2296 /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/alloc/src/boxed.rs:1854:9
        #32 0x7f7cd4f10992 in _$LT$alloc..boxed..Box$LT$F$C$A$GT$$u20$as$u20$core..ops..function..FnOnce$LT$Args$GT$$GT$::call_once::ha8b5234bfeb15105 /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/alloc/src/boxed.rs:1854:9
        #33 0x7f7cd4f10992 in std::sys::unix::thread::Thread::new::thread_start::h6f207dd842d64859 /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/sys/unix/thread.rs:108:17
        #34 0x7f7ce1496608 in start_thread /build/glibc-sMfBJT/glibc-2.31/nptl/pthread_create.c:477:8
        #35 0x7f7ce105d162 in __clone /build/glibc-sMfBJT/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
    
    UndefinedBehaviorSanitizer can not provide additional info.
    SUMMARY: UndefinedBehaviorSanitizer: SEGV /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:261:3 in MOZ_Crash
    ==823809==ABORTING

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: Testcase

Comment 2

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20220304152603-49b96467418f.
The bug appears to have been introduced in the following build range:

Start: 2b3545abbf55f49144afae64bd9ac4b75d28d817 (20210618033124)
End: d8dea0ed3b970ae1ec4e07b9e21514bc00c8f2ba (20210618033757)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2b3545abbf55f49144afae64bd9ac4b75d28d817&tochange=d8dea0ed3b970ae1ec4e07b9e21514bc00c8f2ba

Comment 3

[Mayank Bansal]

https://crash-stats.mozilla.org/report/index/f5da4ff6-2198-45cc-badf-43b880220305

Comment 4

[Jamie Nicol [:jnicol] out of office until 6th Jan]

transform_point2d() is panicking because the perspective transform results in W < 0. As I understand it we shouldn't be using the TRANSFORM_GLYPHS path when we have a perspective transform (see here). However, in this case the perspective amount is small enough that has_perspective_component() returns false.

However, due to the incredibly large letter spacing in the test case, even with the small amount of perspective we still get some glyphs where W < 0.

I think we can probably just change the bounding rect calculation in those cases to ignore those glyphs. In this case the glyphs are way off screen anyway, and we'll intersect the bounds with the pic_coverage_rect which should clip that out. I'm pretty sure the rest of this calculation is incorrect in this case though, eg we don't scale the size of the gylphs at all. But it's a niche test case so maybe we don't have to worry about that..

But I wonder if we may run in to some other issues when we treat the transform as not having a perspective component even though it does?

Glenn, Lee, any thoughts?

Comment 5

[Lee Salzman [:lsalzman]]

Attachment: Bug 1758127 - Don't use tight bounding rect if glyph transform fails. r?jnicol

Comment 6

[Glenn Watson [:gw]]

The patch Lee pasted does seem like a reasonable workaround, I think.

Comment 7

[Jamie Nicol [:jnicol] out of office until 6th Jan]

Yep that's roughly what I had in mind, thanks for writing it Lee! I was wondering if we anticipate any issues elsewhere as a result of treating perspective transforms as non-perspective, though. Or if we care that the rendering will be incorrect here, even though we'll avoid the crash with the patch. (My hunch is probably not, at least until we see an actual website that causes this rather than a fuzzed test case)

Comment 8

[Lee Salzman [:lsalzman]]

In the spirit of "first, do no harm", until and if we come up with a testcase that demonstrates a legitimate use-case where fonts are being rendered wrong because of the permissiveness, I'd rather err on the side of keeping the math permissive with small values in the transform that are functionally zero.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/42a5cf2bfccd
Don't use tight bounding rect if glyph transform fails. r=jnicol

Comment 10

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

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

Comment 11

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

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

Comment 12

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/42a5cf2bfccd

Comment 13

[Bugmon [:jkratzer for issues]]

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