1827009 - Crash [@ nsLineLayout::GetHangFrom]

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jason Kratzer [:jkratzer]]

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

Testcase can be reproduced using the following commands:

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

[@ nsLineLayout::GetHangFrom]

    ==158558==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000014 (pc 0x7f6c8c93e051 bp 0x7fff441b7980 sp 0x7fff441b7980 T158558)
    ==158558==The signal is caused by a READ memory access.
    ==158558==Hint: address points to the zero page.
        #0 0x7f6c8c93e051 in nsLineLayout::GetHangFrom(nsLineLayout::PerSpanData const*, bool) /layout/generic/nsLineLayout.cpp:3041:75
        #1 0x7f6c8c93e235 in nsLineLayout::TextAlignLine(nsLineBox*, bool) /layout/generic/nsLineLayout.cpp:3087:12
        #2 0x7f6c8c828a63 in nsBlockFrame::PlaceLine(mozilla::BlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFloatManager::SavedState*, nsFlowAreaRect&, int&, bool*) /layout/generic/nsBlockFrame.cpp:5177:15
        #3 0x7f6c8c8272cf in nsBlockFrame::DoReflowInlineFrames(mozilla::BlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) /layout/generic/nsBlockFrame.cpp:4676:12
        #4 0x7f6c8c822fb1 in nsBlockFrame::ReflowInlineFrames(mozilla::BlockReflowState&, nsLineList_iterator, bool*) /layout/generic/nsBlockFrame.cpp:4423:9
        #5 0x7f6c8c81f447 in nsBlockFrame::ReflowLine(mozilla::BlockReflowState&, nsLineList_iterator, bool*) /layout/generic/nsBlockFrame.cpp:3407:5
        #6 0x7f6c8c819834 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowState&) /layout/generic/nsBlockFrame.cpp:2921:9
        #7 0x7f6c8c814bcb in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /layout/generic/nsBlockFrame.cpp:1483:3
        #8 0x7f6c8c825a91 in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowState&) /layout/generic/nsBlockReflowContext.cpp:290:11
        #9 0x7f6c8c821d94 in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowState&, nsLineList_iterator, bool*) /layout/generic/nsBlockFrame.cpp:4060:11
        #10 0x7f6c8c81f501 in nsBlockFrame::ReflowLine(mozilla::BlockReflowState&, nsLineList_iterator, bool*) /layout/generic/nsBlockFrame.cpp:3404:5
        #11 0x7f6c8c819834 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowState&) /layout/generic/nsBlockFrame.cpp:2921:9
        #12 0x7f6c8c814bcb in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /layout/generic/nsBlockFrame.cpp:1483:3
        #13 0x7f6c8c838e8a 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:889:14
        #14 0x7f6c8c838349 in nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /layout/generic/nsCanvasFrame.cpp:755:7
        #15 0x7f6c8c838e8a 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:889:14
        #16 0x7f6c8c882bee in nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput&, bool, bool, mozilla::ReflowOutput*) /layout/generic/nsGfxScrollFrame.cpp:937:3
        #17 0x7f6c8c8838d7 in nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput&, mozilla::ReflowOutput const&) /layout/generic/nsGfxScrollFrame.cpp:1070:3
        #18 0x7f6c8c888288 in nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /layout/generic/nsGfxScrollFrame.cpp:1504:3
        #19 0x7f6c8c8094c7 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /layout/generic/nsContainerFrame.cpp:929:14
        #20 0x7f6c8c808c24 in mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /layout/generic/ViewportFrame.cpp:385:7
        #21 0x7f6c8c7022e1 in mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*) /layout/base/PresShell.cpp:9624:11
        #22 0x7f6c8c7266af in mozilla::PresShell::ProcessReflowCommands(bool) /layout/base/PresShell.cpp:9802:22
        #23 0x7f6c8c70b725 in DoFlushLayout /layout/base/PresShell.cpp:9873:10
        #24 0x7f6c8c70b725 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /layout/base/PresShell.cpp:4350:11
        #25 0x7f6c8c6cf03e in FlushPendingNotifications /builds/worker/workspace/obj-build/dist/include/mozilla/PresShell.h:1470:5
        #26 0x7f6c8c6cf03e in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsRefreshDriver::IsExtraTick) /layout/base/nsRefreshDriver.cpp:2657:20
        #27 0x7f6c8c6dec92 in operator() /layout/base/nsRefreshDriver.cpp:1780:25
        #28 0x7f6c8c6dec92 in mozilla::detail::RunnableFunction<nsRefreshDriver::EnsureTimerStarted(nsRefreshDriver::EnsureTimerStartedFlags)::$_1>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:548:5
        #29 0x7f6c8709b495 in mozilla::RunnableTask::Run() /xpcom/threads/TaskController.cpp:553:16
        #30 0x7f6c870965e8 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /xpcom/threads/TaskController.cpp:869:26
        #31 0x7f6c8709515a in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /xpcom/threads/TaskController.cpp:700:15
        #32 0x7f6c870954e5 in mozilla::TaskController::ProcessPendingMTTask(bool) /xpcom/threads/TaskController.cpp:464:36
        #33 0x7f6c8709ead6 in operator() /xpcom/threads/TaskController.cpp:191:37
        #34 0x7f6c8709ead6 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_3>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:548:5
        #35 0x7f6c870b4517 in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1239:16
        #36 0x7f6c870baa7d in NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:479:10
        #37 0x7f6c87cfa683 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:85:21
        #38 0x7f6c87c1c758 in MessageLoop::RunInternal() /ipc/chromium/src/base/message_loop.cc:369:10
        #39 0x7f6c87c1c661 in RunHandler /ipc/chromium/src/base/message_loop.cc:362:3
        #40 0x7f6c87c1c661 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:344:3
        #41 0x7f6c8c355118 in nsBaseAppShell::Run() /widget/nsBaseAppShell.cpp:148:27
        #42 0x7f6c8e5aaaeb in XRE_RunAppShell() /toolkit/xre/nsEmbedFunctions.cpp:738:20
        #43 0x7f6c87cfb549 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:235:9
        #44 0x7f6c87c1c758 in MessageLoop::RunInternal() /ipc/chromium/src/base/message_loop.cc:369:10
        #45 0x7f6c87c1c661 in RunHandler /ipc/chromium/src/base/message_loop.cc:362:3
        #46 0x7f6c87c1c661 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:344:3
        #47 0x7f6c8e5aa638 in XRE_InitChildProcess(int, char**, XREChildData const*) /toolkit/xre/nsEmbedFunctions.cpp:673:34
        #48 0x5615fa6b4f20 in content_process_main /browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
        #49 0x5615fa6b4f20 in main /browser/app/nsBrowserApp.cpp:353:18
        #50 0x7f6c9be29d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
        #51 0x7f6c9be29e3f in __libc_start_main csu/../csu/libc-start.c:392:3
        #52 0x5615fa68b588 in _start (/home/jkratzer/builds/m-c-20230407094736-fuzzing-debug/firefox-bin+0x5b588) (BuildId: c6943f509a84fb958e3958ed1427ffb00676a8b4)
    
    UndefinedBehaviorSanitizer can not provide additional info.
    SUMMARY: UndefinedBehaviorSanitizer: SEGV /layout/generic/nsLineLayout.cpp:3041:75 in nsLineLayout::GetHangFrom(nsLineLayout::PerSpanData const*, bool)
    ==158558==ABORTING

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: Testcase

Comment 2

[Bugmon [:jkratzer for issues]]

Unable to reproduce bug 1827009 using build mozilla-central 20230407094736-3b54fd2a69ea. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 3

[David Shin[:dshin]]

Seems like an issue specifically with pre-wrap + min-content to force wrapping + first-letter, so S3 seems ok.
(Interestingly enough, launching the testcase by launching it as an argument to Firefox crashes it, but loading it from a running instance doesn't. It shows the page momentarily as well before the crash)

Comment 4

[Emilio Cobos Álvarez (:emilio)]

This is a regression from bug 1712703. Jonathan can you take a look?

Comment 5

[Jonathan Kew [:jfkthame]]

I can't seem to reproduce this locally - I suspect it may be sensitive to the exact timing of load/reflow etc. But it looks like the issue must be that lastText->GetTextRun() is returning null, as the frame's textrun hasn't necessarily been created yet.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1827009 - Don't assume the textrun has necessarily been created already. r=#layout-reviewers

Comment 7

[Jonathan Kew [:jfkthame]]

David, if you're able to reproduce this crash locally, could you confirm the attached patch fixes it? Thanks!

Comment 8

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

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

Comment 9

[David Shin[:dshin]]

Couldn't repro on local build, just on mozregression, but I found that you can change zoom to trigger this. With the patch, the crash no longer triggers.

Comment 10

[Jonathan Kew [:jfkthame]]

Oh, cool - thanks for the testing! I just Lando'd it, so we should be good here.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3652be19f534
Don't assume the textrun has necessarily been created already. r=emilio

Comment 12

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (dynamically changes font-size)

This testcase makes a dynamic change to font-size (similar to the full-page-zoom action that dshin mentioned). It crashes for me in latest Nightly.

jfkthame, maybe you could morph this into a reftest-wait / double-requestAnimationFrame-based test to land as a followup here?

Comment 13

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/3652be19f534

Comment 14

[Jonathan Kew [:jfkthame]]

(In reply to Daniel Holbert [:dholbert] from comment #12)

Created attachment 9327994 [details]
testcase 2 (dynamically changes font-size)

This testcase makes a dynamic change to font-size (similar to the full-page-zoom action that dshin mentioned). It crashes for me in latest Nightly.

jfkthame, maybe you could morph this into a reftest-wait / double-requestAnimationFrame-based test to land as a followup here?

Thanks, that insta-crashes for me, even without using timeout(), so we can easily make it into a reftest (which fails with a crash before the patch).

Comment 15

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1827009 - Add testcase as a reftest. r=dholbert

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77823ce3741b
Add testcase as a reftest. r=dholbert

Comment 17

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

The patch landed in nightly and beta is affected.
:jfkthame, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox113 to wontfix.

For more information, please visit auto_nag documentation.

Comment 18

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

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

Comment 19

[Jonathan Kew [:jfkthame]]

Comment on attachment 9327894 [details]
Bug 1827009 - Don't assume the textrun has necessarily been created already. r=#layout-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Possible crash with content using white-space:pre-wrap and other properties
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple fix to ensure textrun is present before accessing it
• String changes made/needed:
• Is Android affected?: Yes

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/77823ce3741b

Comment 21

[Pascal Chevrel:pascalc (PTO until Sept 2)]

Comment on attachment 9327894 [details]
Bug 1827009 - Don't assume the textrun has necessarily been created already. r=#layout-reviewers

Approved for 113 beta 4, thanks.

Comment 22

[Pascal Chevrel:pascalc (PTO until Sept 2)]

https://hg.mozilla.org/releases/mozilla-beta/rev/b1e02be2eb4d
https://hg.mozilla.org/releases/mozilla-beta/rev/3a73e9ef13a1