1769559 - Assertion failure: !node || !node->GetApzc(), at /gfx/layers/apz/src/APZCTreeManager.cpp:2436

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect)

Description

[Jason Kratzer [:jkratzer]]

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

Testcase can be reproduced using the following commands:

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

Assertion failure: !node || !node->GetApzc(), at /gfx/layers/apz/src/APZCTreeManager.cpp:2436

    ==1354742==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fa4ecc564db bp 0x7fa49ab5cea0 sp 0x7fa49ab5ce00 T1354887)
    ==1354742==The signal is caused by a WRITE memory access.
    ==1354742==Hint: address points to the zero page.
        #0 0x7fa4ecc564db in mozilla::layers::APZCTreeManager::UpdateZoomConstraints(mozilla::layers::ScrollableLayerGuid const&, mozilla::Maybe<mozilla::layers::ZoomConstraints> const&) /gfx/layers/apz/src/APZCTreeManager.cpp:2435:3
        #1 0x7fa4ecca2751 in applyImpl<mozilla::layers::APZCTreeManager, void (mozilla::layers::APZCTreeManager::*)(const mozilla::layers::ScrollableLayerGuid &, const mozilla::Maybe<mozilla::layers::ZoomConstraints> &), StoreCopyPassByConstLRef<mozilla::layers::ScrollableLayerGuid>, StoreCopyPassByConstLRef<mozilla::Maybe<mozilla::layers::ZoomConstraints> >, 0UL, 1UL> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1147:12
        #2 0x7fa4ecca2751 in apply<mozilla::layers::APZCTreeManager, void (mozilla::layers::APZCTreeManager::*)(const mozilla::layers::ScrollableLayerGuid &, const mozilla::Maybe<mozilla::layers::ZoomConstraints> &)> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1153:12
        #3 0x7fa4ecca2751 in mozilla::detail::RunnableMethodImpl<mozilla::layers::APZCTreeManager*, void (mozilla::layers::APZCTreeManager::*)(mozilla::layers::ScrollableLayerGuid const&, mozilla::Maybe<mozilla::layers::ZoomConstraints> const&), true, (mozilla::RunnableKind)0, mozilla::layers::ScrollableLayerGuid, mozilla::Maybe<mozilla::layers::ZoomConstraints> >::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1200:13
        #4 0x7fa4ecc657b6 in mozilla::layers::APZUpdater::ProcessQueue() /gfx/layers/apz/src/APZUpdater.cpp:462:23
        #5 0x7fa4ecc65a4c in mozilla::layers::APZUpdater::ProcessPendingTasks(mozilla::wr::WrWindowId const&) /gfx/layers/apz/src/APZUpdater.cpp:132:14
        #6 0x7fa4ecc68279 in apz_run_updater /gfx/layers/apz/src/APZUpdater.cpp:534:3
        #7 0x7fa4f4a2a00b in webrender::scene_builder_thread::SceneBuilderThread::run::h5f0377b0a80d441a /gfx/wr/webrender/src/scene_builder_thread.rs:390:17
        #8 0x7fa4f49ac217 in webrender::renderer::Renderer::new::_$u7b$$u7b$closure$u7d$$u7d$::hd1d7971c37fea216 /gfx/wr/webrender/src/renderer/mod.rs:1249:13
        #9 0x7fa4f49ac217 in std::sys_common::backtrace::__rust_begin_short_backtrace::h3257ab80ae9131b6 /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys_common/backtrace.rs:122:18
        #10 0x7fa4f4732f1e in std::thread::Builder::spawn_unchecked_::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::hdbd1235dd5346e86 /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/thread/mod.rs:498:17
        #11 0x7fa4f4732f1e in _$LT$core..panic..unwind_safe..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$$LP$$RP$$GT$$GT$::call_once::hd7ffe928d346f5ad /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/panic/unwind_safe.rs:271:9
        #12 0x7fa4f4732f1e in std::panicking::try::do_call::h357d801da377dcc4 /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:492:40
        #13 0x7fa4f4732f1e in std::panicking::try::he262630b8eb1c967 /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:456:19
        #14 0x7fa4f4732f1e in std::panic::catch_unwind::hb3a8fd2de72aef50 /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panic.rs:137:14
        #15 0x7fa4f4732f1e in std::thread::Builder::spawn_unchecked_::_$u7b$$u7b$closure$u7d$$u7d$::h98be28c902162b64 /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/thread/mod.rs:497:30
        #16 0x7fa4f4732f1e in core::ops::function::FnOnce::call_once$u7b$$u7b$vtable.shim$u7d$$u7d$::hb061268df47cab5f /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/ops/function.rs:227:5
        #17 0x7fa4f5d69a02 in _$LT$alloc..boxed..Box$LT$F$C$A$GT$$u20$as$u20$core..ops..function..FnOnce$LT$Args$GT$$GT$::call_once::hf70ac038171e3e1a /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/alloc/src/boxed.rs:1853:9
        #18 0x7fa4f5d69a02 in _$LT$alloc..boxed..Box$LT$F$C$A$GT$$u20$as$u20$core..ops..function..FnOnce$LT$Args$GT$$GT$::call_once::he6690128792365ad /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/alloc/src/boxed.rs:1853:9
        #19 0x7fa4f5d69a02 in std::sys::unix::thread::Thread::new::thread_start::ha07928d93d5a5ec9 /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys/unix/thread.rs:108:17
        #20 0x7fa502460608 in start_thread /build/glibc-SzIz7B/glibc-2.31/nptl/pthread_create.c:477:8
        #21 0x7fa502027132 in __clone /build/glibc-SzIz7B/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
    
    UndefinedBehaviorSanitizer can not provide additional info.
    SUMMARY: UndefinedBehaviorSanitizer: SEGV /gfx/layers/apz/src/APZCTreeManager.cpp:2435:3 in mozilla::layers::APZCTreeManager::UpdateZoomConstraints(mozilla::layers::ScrollableLayerGuid const&, mozilla::Maybe<mozilla::layers::ZoomConstraints> const&)
    ==1354742==ABORTING

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: Testcase

Comment 2

[Bugmon [:jkratzer for issues]]

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

Start: a47e970555ccc1ecc6c7d8057ce171544080ca4b (20220513094118)
End: 4aa40d4543da126a325b799542bd3fa72777bb4b (20220513134313)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a47e970555ccc1ecc6c7d8057ce171544080ca4b&tochange=4aa40d4543da126a325b799542bd3fa72777bb4b

Comment 3

[Botond Ballo [:botond]]

(In reply to Bugmon [:jkratzer for issues] from comment #2)

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

Start: a47e970555ccc1ecc6c7d8057ce171544080ca4b (20220513094118)
End: 4aa40d4543da126a325b799542bd3fa72777bb4b (20220513134313)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a47e970555ccc1ecc6c7d8057ce171544080ca4b&tochange=4aa40d4543da126a325b799542bd3fa72777bb4b

Suspect bug 1423746 in that range.

Comment 4

[Botond Ballo [:botond]]

Confirmed using local builds that bug 1423746 is the regressing change.

Comment 5

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

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

Comment 6

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

:emilio, since you are the author of the regressor, bug 1423746, could you take a look?
For more information, please visit auto_nag documentation.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced test-case

Yeah so that bug is potentially related, in the sense that it changes the display list of the root scroll frame. However I only see the issue with invalid filters so I'm a bit confused.

To reproduce on a regular debug build you just need to open the attached test-case and open a new tab (ctrl+T).

We're hitting the !initialized code-path here because the filter isn't found. It seems we rely on building the WR commands for the descendants of the filter (if I remove that early-return then stuff just works).

Tim, seems you introduced this assertion, could you help me understand what is it intended to assert? I'm not familiar with the async zoom code and so on.

Comment 8

[Timothy Nikkel (:tnikkel)]

To be honest I don't remember why I added that assertion. Looking at the patch and the bug I added it in I think the reason is as follows. The patch changed how we searched the hittesting tree nodes from always skipping nodes that don't have an apzc (and asserting that the returned node had an azpc) to finding the first node that had the async zoom id on it. Probably in all but very unusual situations the node with the async zoom id on it does not have an apzc, so I replaced the assert to say we did not have apzc.

So then the question is if we think the displaylist, webrender scroll data, or hit testing tree are valid and as we expect for this testcase, and the code that was added with that assert in bug 1705622 would handle it properly. If the answer is yes to both we would probably want to remove the assert.

Comment 9

[Botond Ballo [:botond]]

(In reply to Timothy Nikkel (:tnikkel) from comment #8)

So then the question is if we think the displaylist, webrender scroll data, or hit testing tree are valid and as we expect for this testcase

The test case is unusual in that the RCD-RSF scroll metadata is on the async zoom container node (rather than on a descendant of it). This is something we used to disallow, but in bug 1553045 we started to allow it (though the motivation there seems Layers-specific), so at least structurally I don't think it currently violates APZ invariants.

and the code that was added with that assert in bug 1705622 would handle it properly

A quick read through the patches in bug 1705622 suggests that both APZCTreeManager::UpdateZoomConstraints() (which would get into here) and UpdateHitTestingTree() (which checks for an async zoom container here to push onto the zoom constraints stack, and later looks at the top of the stack here) would handle this situation fine.

==> It seems like the assertion should be safe to remove.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1769559 - Remove invalid assertion. r=tnikkel,botond

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/481c50c1fa9c
Remove invalid assertion. r=tnikkel

Comment 12

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/481c50c1fa9c

Comment 13

[Bugmon [:jkratzer for issues]]

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