1791606 - Assertion failure: !focusedTarget || focusedTarget == &target (If the focused target has been found on X axis, the target should be same), at /layout/generic/ScrollSnap.cpp:450

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Jason Kratzer [:jkratzer]]

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

Testcase can be reproduced using the following commands:

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

Assertion failure: !focusedTarget || focusedTarget == &target (If the focused target has been found on X axis, the target should be same), at /layout/generic/ScrollSnap.cpp:450

    ==1218563==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f6b8d17824e bp 0x7ffc2d4395a0 sp 0x7ffc2d439440 T1218563)
    ==1218563==The signal is caused by a WRITE memory access.
    ==1218563==Hint: address points to the zero page.
        #0 0x7f6b8d17824e in GetCandidateInLastTargets /layout/generic/ScrollSnap.cpp:447:11
        #1 0x7f6b8d17824e in mozilla::ScrollSnapUtils::GetSnapPointForResnap(mozilla::layers::ScrollSnapInfo const&, nsRect const&, nsPoint const&, mozilla::UniquePtr<mozilla::ScrollSnapTargetIds, mozilla::DefaultDelete<mozilla::ScrollSnapTargetIds> > const&, nsIContent const*) /layout/generic/ScrollSnap.cpp:499:17
        #2 0x7f6b8d203f9f in mozilla::ScrollFrameHelper::NeedsResnap() /layout/generic/nsGfxScrollFrame.cpp:8083:10
        #3 0x7f6b8d203cda in nsHTMLScrollFrame::DidReflow(nsPresContext*, mozilla::ReflowInput const*) /layout/generic/nsGfxScrollFrame.cpp:1529:15
        #4 0x7f6b8d1aea5c in nsContainerFrame::FinishReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput const&, mozilla::ReflowInput const*, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags) /layout/generic/nsContainerFrame.cpp:1154:14
        #5 0x7f6b8d1a192b in nsBlockReflowContext::PlaceBlock(mozilla::ReflowInput const&, bool, nsLineBox*, nsCollapsingMargin&, mozilla::OverflowAreas&, nsReflowStatus const&) /layout/generic/nsBlockReflowContext.cpp:430:3
        #6 0x7f6b8d19d8ba in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowState&, nsLineList_iterator, bool*) /layout/generic/nsBlockFrame.cpp:4183:15
        #7 0x7f6b8d19a83e in nsBlockFrame::ReflowLine(mozilla::BlockReflowState&, nsLineList_iterator, bool*) /layout/generic/nsBlockFrame.cpp:3379:5
        #8 0x7f6b8d194655 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowState&) /layout/generic/nsBlockFrame.cpp:2896:9
        #9 0x7f6b8d18fb35 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /layout/generic/nsBlockFrame.cpp:1472:3
        #10 0x7f6b8d1b3d95 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /layout/generic/nsContainerFrame.cpp:1005:14
        #11 0x7f6b8d1b2e8d in nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /layout/generic/nsCanvasFrame.cpp:794:7
        #12 0x7f6b8d1b3d95 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /layout/generic/nsContainerFrame.cpp:1005:14
        #13 0x7f6b8d1fd199 in nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput&, bool, bool, mozilla::ReflowOutput*) /layout/generic/nsGfxScrollFrame.cpp:838:3
        #14 0x7f6b8d1fdd8f in nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput&, mozilla::ReflowOutput const&) /layout/generic/nsGfxScrollFrame.cpp:974:3
        #15 0x7f6b8d2020e6 in nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /layout/generic/nsGfxScrollFrame.cpp:1399:3
        #16 0x7f6b8d1846a6 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /layout/generic/nsContainerFrame.cpp:1045:14
        #17 0x7f6b8d183e0c in mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /layout/generic/ViewportFrame.cpp:375:7
        #18 0x7f6b8d080079 in mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*) /layout/base/PresShell.cpp:9638:11
        #19 0x7f6b8d0a380f in mozilla::PresShell::ProcessReflowCommands(bool) /layout/base/PresShell.cpp:9810:24
        #20 0x7f6b8d089854 in DoFlushLayout /layout/base/PresShell.cpp:9880:10
        #21 0x7f6b8d089854 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /layout/base/PresShell.cpp:4409:11
        #22 0x7f6b8d04e603 in FlushPendingNotifications /builds/worker/workspace/obj-build/dist/include/mozilla/PresShell.h:1470:5
        #23 0x7f6b8d04e603 in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsRefreshDriver::IsExtraTick) /layout/base/nsRefreshDriver.cpp:2599:20
        #24 0x7f6b8d05d812 in operator() /layout/base/nsRefreshDriver.cpp:1771:25
        #25 0x7f6b8d05d812 in mozilla::detail::RunnableFunction<nsRefreshDriver::EnsureTimerStarted(nsRefreshDriver::EnsureTimerStartedFlags)::$_1>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:531:5
        #26 0x7f6b87d6086e in mozilla::RunnableTask::Run() /xpcom/threads/TaskController.cpp:538:16
        #27 0x7f6b87d38d89 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /xpcom/threads/TaskController.cpp:851:26
        #28 0x7f6b87d37913 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /xpcom/threads/TaskController.cpp:683:15
        #29 0x7f6b87d37b83 in mozilla::TaskController::ProcessPendingMTTask(bool) /xpcom/threads/TaskController.cpp:461:36
        #30 0x7f6b87d64116 in operator() /xpcom/threads/TaskController.cpp:187:37
        #31 0x7f6b87d64116 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:531:5
        #32 0x7f6b87d4d9df in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1205:16
        #33 0x7f6b87d53fed in NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:465:10
        #34 0x7f6b88934646 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:85:21
        #35 0x7f6b88859ca7 in MessageLoop::RunInternal() /ipc/chromium/src/base/message_loop.cc:381:10
        #36 0x7f6b88859bb2 in RunHandler /ipc/chromium/src/base/message_loop.cc:374:3
        #37 0x7f6b88859bb2 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:356:3
        #38 0x7f6b8cd157b8 in nsBaseAppShell::Run() /widget/nsBaseAppShell.cpp:150:27
        #39 0x7f6b8ef0bd0b in XRE_RunAppShell() /toolkit/xre/nsEmbedFunctions.cpp:880:20
        #40 0x7f6b8893553a in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:235:9
        #41 0x7f6b88859ca7 in MessageLoop::RunInternal() /ipc/chromium/src/base/message_loop.cc:381:10
        #42 0x7f6b88859bb2 in RunHandler /ipc/chromium/src/base/message_loop.cc:374:3
        #43 0x7f6b88859bb2 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:356:3
        #44 0x7f6b8ef0b223 in XRE_InitChildProcess(int, char**, XREChildData const*) /toolkit/xre/nsEmbedFunctions.cpp:739:34
        #45 0x55b05248db39 in content_process_main /browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
        #46 0x55b05248db39 in main /browser/app/nsBrowserApp.cpp:359:18
        #47 0x7f6b9ee5fd8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
        #48 0x7f6b9ee5fe3f in __libc_start_main csu/../csu/libc-start.c:392:3
        #49 0x55b0524638dc in _start (/home/jkratzer/builds/mc-debug/firefox-bin+0x168dc) (BuildId: b1d395243da9b2b84533ddb248fa0c0bd65c19ef)
    
    UndefinedBehaviorSanitizer can not provide additional info.
    SUMMARY: UndefinedBehaviorSanitizer: SEGV /layout/generic/ScrollSnap.cpp:447:11 in GetCandidateInLastTargets
    ==1218563==ABORTING

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: Testcase

Comment 2

[Bugmon [:jkratzer for issues]]

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

Start: 307dd529e2a3dc6f6191a40d35c1c9e9fd94f727 (20220630220030)
End: 1f99e4a5113539ba2b85689d04f66fd0261db0a8 (20220701020433)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=307dd529e2a3dc6f6191a40d35c1c9e9fd94f727&tochange=1f99e4a5113539ba2b85689d04f66fd0261db0a8

Comment 3

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

The severity field is not set for this bug.
:alaskanemily, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

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

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

Comment 5

[Hiroyuki Ikezoe (:hiro)]

The focused content in question has two different nsIFrames;

              line@7fbac175b628 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,forced-break,clear-before:none,clear-after:none(x=153.6, y=58, w=56.6333, h=24) ink-overflow=(x=150.6, y=54, w=64.3833, h=97.0667) scr-overflow=(x=153.6, y=57, w=56.6333, h=91.0667) <
                Inline(i id=id_0)(1)@7fbac175b3f0 parent=7fbac175b1e8 next=7fbabb6be020 next-in-flow=7fbabb6be020 (x=153.6, y=57, w=56.6333, h=91.0667) ink-overflow=(x=-3, y=-3, w=64.3833, h=97.0667) scr-overflow=(x=0, y=0, w=56.6333, h=91.0667) [content=7fbac1304820] [cs=7fbabb6c23e8] <
                  Text(0)"\n"@7fbac175b498 parent=7fbac175b3f0 next-in-flow=7fbac175bda8 (x=56.6333, y=1, w=0, h=24) ink-overflow=(x=-1.25, y=0, w=6, h=24) scr-overflow=(x=0, y=0, w=0, h=24) [content=7fbac1305500] [cs=7fbabb6c24d8:-moz-text] [run=7fbac1fbc400][0,1,F] 
                >
              >
              line@7fbabb6be0c8 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,forced-break,clear-before:none,clear-after:none(x=153.6, y=82, w=70.9667, h=24) ink-overflow=(x=150.6, y=78, w=76.9667, h=97.0667) scr-overflow=(x=153.6, y=81, w=70.9667, h=91.0667) <
                Inline(i id=id_0)(1)@7fbabb6be020 parent=7fbac175b1e8 next=7fbac175b538 prev-in-flow=7fbac175b3f0 (x=153.6, y=81, w=70.9667, h=91.0667) ink-overflow=(x=-3, y=-3, w=76.9667, h=97.0667) scr-overflow=(x=0, y=0, w=70.9667, h=91.0667) [content=7fbac1304820] [cs=7fbabb6c23e8] <
                  Text(0)"    \n"@7fbac175bda8 parent=7fbabb6be020 prev-in-flow=7fbac175b498 (x=0, y=1, w=14.3333, h=24) ink-overflow=(x=-1.25, y=0, w=20.3333, h=24) scr-overflow=(x=0, y=0, w=14.3333, h=24) [content=7fbac1305500] [cs=7fbabb6c24d8:-moz-text] [run=7fbabb31e670][1,5,T] 
                >
              >

In this specific case, the scroll-snap-type is block proximity so that the snap target points for both nsIFrams are same, thus we can ignore either of one of them in CollectScrollPositionsForSnap(). But if the scroll-snap-type is inline and if the scroll-snap-align is center, things are more complicated, we need to handle split-nsIFrame cases in both CollectScrollPositionsForSnap and AppendScrollPositionsForSnap. That will not be an easy task as far as I can tell, I mean with my current knowledge.

Setting S3:P3 for now, if there are a bunch of such cases in the wild (scroll snapping fails in such cases), we will bump up the priority.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Note that this bug is marked as "regressed by bug 1530253", indeed the assertion was added in bug 1530253, but in fact the assertion just caught a case that we haven't been handling properly since the beginning of our scroll snap implementation, i.e. since our old scroll snap implementation.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Should the assert be downgraded to NS_ASSERTION if we wanna live with it? Otherwise it unnecessarily breaks blocks fuzzers etc.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

Should the assert be downgraded to NS_ASSERTION if we wanna live with it? Otherwise it unnecessarily breaks blocks fuzzers etc.

Thanks for the suggestion. Indeed we should downgrade it for fuzzers.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1791606 - Downgrade the assertion when we found multiple SnapTargets for the same focused content. r?emilio

The assertion is valid, but fixing it would be a bit of work (bug 1798240),
so now we degrade the assertion to make it harmless on fuzzers works.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89ae75f45eaa
Downgrade the assertion when we found multiple SnapTargets for the same focused content. r=emilio

Comment 11

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/89ae75f45eaa

Comment 12

[Bugmon [:jkratzer for issues]]

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