Closed Bug 1841128 Opened 1 year ago Closed 1 year ago

crash near null in [@ nsBlockFrame::StealFrame]

Categories

(Core :: Layout: Block and Inline, defect)

defect

Tracking

()

VERIFIED FIXED
117 Branch
Tracking Status
firefox-esr102 --- unaffected
firefox-esr115 --- unaffected
firefox114 --- unaffected
firefox115 --- unaffected
firefox116 --- verified
firefox117 --- verified

People

(Reporter: tsmith, Assigned: emilio)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: crash, regression, testcase, Whiteboard: [bugmon:bisected,confirmed], [wptsync upstream])

Crash Data

Attachments

(4 files)

Attached file testcase.html

Found while fuzzing m-c 20230629-d18ee9401610 (--enable-address-sanitizer --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -a --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html
==16078==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f6b6049225b bp 0x7ffc0bef8b70 sp 0x7ffc0bef89e0 T0)
==16078==The signal is caused by a READ memory access.
==16078==Hint: address points to the zero page.
    #0 0x7f6b6049225b in begin /builds/worker/checkouts/gecko/layout/generic/nsLineBox.h:1145:25
    #1 0x7f6b6049225b in nsBlockFrame::StealFrame(nsIFrame*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:6721:47
    #2 0x7f6b60491c96 in nsContainerFrame::DeleteNextInFlowChild(nsIFrame*, bool) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:1390:3
    #3 0x7f6b60492ba6 in nsBlockFrame::DeleteNextInFlowChild(nsIFrame*, bool) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:6760:23
    #4 0x7f6b6048d2d3 in nsBlockFrame::DoRemoveOutOfFlowFrame(nsIFrame*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:6132:25
    #5 0x7f6b6048cb6f in nsBlockFrame::RemoveFrame(mozilla::FrameChildListID, nsIFrame*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:5862:5
    #6 0x7f6b60775f14 in nsPlaceholderFrame::DestroyFrom(nsIFrame*, mozilla::PostFrameDestroyData&) /builds/worker/checkouts/gecko/layout/generic/nsPlaceholderFrame.cpp:164:11
    #7 0x7f6b604b8d10 in Destroy /builds/worker/checkouts/gecko/layout/generic/nsIFrame.h:657:5
    #8 0x7f6b604b8d10 in nsContainerFrame::RemoveFrame(mozilla::FrameChildListID, nsIFrame*) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:186:19
    #9 0x7f6b602d2832 in RemoveFrame /builds/worker/checkouts/gecko/layout/base/nsFrameManager.cpp:119:18
    #10 0x7f6b602d2832 in nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, nsCSSFrameConstructor::RemoveFlags) /builds/worker/checkouts/gecko/layout/base/nsCSSFrameConstructor.cpp:7561:5
    #11 0x7f6b602c9b9b in nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind) /builds/worker/checkouts/gecko/layout/base/nsCSSFrameConstructor.cpp:8543:7
    #12 0x7f6b60237cfe in mozilla::RestyleManager::ProcessRestyledFrames(nsStyleChangeList&) /builds/worker/checkouts/gecko/layout/base/RestyleManager.cpp:1607:25
    #13 0x7f6b60243828 in mozilla::RestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /builds/worker/checkouts/gecko/layout/base/RestyleManager.cpp:3179:9
    #14 0x7f6b601f3c68 in ProcessPendingRestyles /builds/worker/checkouts/gecko/layout/base/RestyleManager.cpp:3264:3
    #15 0x7f6b601f3c68 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:4330:39
    #16 0x7f6b60159c01 in FlushPendingNotifications /builds/worker/workspace/obj-build/dist/include/mozilla/PresShell.h:1464:5
    #17 0x7f6b60159c01 in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsRefreshDriver::IsExtraTick) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2636:22
    #18 0x7f6b6016ecac in TickDriver /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:373:13
    #19 0x7f6b6016ecac in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver>>&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:351:7
    #20 0x7f6b6016e98e in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:367:5
    #21 0x7f6b6016e601 in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:911:5
    #22 0x7f6b6016d886 in mozilla::VsyncRefreshDriverTimer::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:825:5
    #23 0x7f6b6016c4ab in mozilla::VsyncRefreshDriverTimer::NotifyVsyncOnMainThread(mozilla::VsyncEvent const&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:746:5
    #24 0x7f6b6016bb32 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsyncTimerOnMainThread() /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:592:14
    #25 0x7f6b6016b6e5 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::VsyncEvent const&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:549:9
    #26 0x7f6b5e51bb2b in mozilla::dom::VsyncMainChild::RecvNotify(mozilla::VsyncEvent const&, float const&) /builds/worker/checkouts/gecko/dom/ipc/VsyncMainChild.cpp:66:15
    #27 0x7f6b5eae2e74 in mozilla::dom::PVsyncChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PVsyncChild.cpp:220:78
    #28 0x7f6b5e8a9fe1 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PContentChild.cpp:8741:32
    #29 0x7f6b562408b5 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1811:25
    #30 0x7f6b5623c23f in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::UniquePtr<IPC::Message, mozilla::DefaultDelete<IPC::Message>>) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1736:9
    #31 0x7f6b5623d679 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1536:3
    #32 0x7f6b5623ebf3 in mozilla::ipc::MessageChannel::MessageTask::Run() /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1634:14
    #33 0x7f6b545f3d7a in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:555:16
    #34 0x7f6b545dedce in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:880:26
    #35 0x7f6b545dbd17 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:704:15
    #36 0x7f6b545dc5ff in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:491:36
    #37 0x7f6b545fb9b1 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:218:37
    #38 0x7f6b545fb9b1 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.h:548:5
    #39 0x7f6b54625ac3 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1199:16
    #40 0x7f6b546334d4 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:480:10
    #41 0x7f6b56249e0e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
    #42 0x7f6b56075b4a in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:370:10
    #43 0x7f6b56075b4a in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
    #44 0x7f6b56075b4a in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
    #45 0x7f6b5f8271e9 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
    #46 0x7f6b656b464e in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:717:20
    #47 0x7f6b56075b4a in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:370:10
    #48 0x7f6b56075b4a in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
    #49 0x7f6b56075b4a in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
    #50 0x7f6b656b3d08 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:652:34
    #51 0x561c5d8c06de in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
    #52 0x561c5d8c06de in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:375:18
    #53 0x7f6b7b029d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #54 0x7f6b7b029e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #55 0x561c5d7e9d18 in _start (/home/user/workspace/browsers/m-c-20230629153145-fuzzing-asan-opt/firefox+0x107d18) (BuildId: 2a3850bd69b94e06c49f072f049e0b542673ee1a)
Flags: in-testsuite?

Using the UBSan null check gives the following error:
layout/generic/nsBlockFrame.cpp:6720:48: runtime error: member access within null pointer of type 'FrameLines'

Keywords: assertioncrash
Crash Signature: [@ nsLineList::begin ]

Verified bug as reproducible on mozilla-central 20230629214838-c5ac7e957828.
The bug appears to have been introduced in the following build range:

Start: 19c777f3b143ca86c79eb065fef63b104200fe97 (20230622134915)
End: b9e51e69d0451a93822b61cc15d72666124f02dd (20230622144102)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=19c777f3b143ca86c79eb065fef63b104200fe97&tochange=b9e51e69d0451a93822b61cc15d72666124f02dd

Keywords: regression
Whiteboard: [bugmon:bisected,confirmed]

Regressed by: Bug 1839223 - Remove nsMappedAttributes. r=smaug

Regressed by: 1839223

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

:emilio, since you are the author of the regressor, bug 1839223, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Flags: needinfo?(emilio)

Blerg, this is bug 1402218 all over again.

See Also: → 1402218
Depends on: 1841496
See Also: → 1465474
Assignee: nobody → emilio
Flags: needinfo?(emilio)

This is drive-by. We already rely on was_restyled not being null so make
that explicit.

This doesn't change behavior, but avoids some useless work when
reparenting frames that used to be in display: none subtrees.

Also fixes a clang-tidy warning.

Depends on D182767

The TLDR is that our first-line implementation is complete madness, but
I'm not fixing bug 1465474 yet.

The guarantee that first-line style reparenting relies on is that
reparenting shouldn't change reset properties (that's why we had all
this "style to inherit ignoring first-line" to inherit reset
properties). Bug 1839223 broke this guarantee in the case the mapped
attribute declarations are reused / mutated (which is something we want
to do for performance).

What's going on in the test-case is that the old style's rule-node
declarations are mutated by the mapped attributes code. Re-cascading the
old styles yields a different float property value.

When a style change that causes a reframe like this one happens, we
don't update the frame tree styles with the new styles: it'd be wrong
and also useless work.

First-line reparenting happens after we've recomputed the styles, but
before we've reframed as needed. In this intermediate state, the frame
remains with the old (floated) style, but the ::first-line reparenting
code isn't aware that this frame is going away, so it happily
re-cascades and updates the style anyways, creating a floated
frame that has float: none in its computed style information.

This causes the frame destruction code to crash.

This patch restores that guarantee that ::first-line doesn't change
reset properties by re-using the "reset properties cache" code-path.
This is also simpler since it avoids us having to compute the "parent
ignoring ::first-line" style.

The code is also more explicit with the new enum rather than just an
Option.

Depends on D182768

Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cb94d6c0916d Clean-up Servo_TakeChangeHint. r=TYLin https://hg.mozilla.org/integration/autoland/rev/67e3d9e33912 Minor clean-ups to first-line reparenting code. r=TYLin https://hg.mozilla.org/integration/autoland/rev/423c48eae15e Simplify first-line reparenting a bit. r=TYLin
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/40918 for changes under testing/web-platform/tests
Whiteboard: [bugmon:bisected,confirmed] → [bugmon:bisected,confirmed], [wptsync upstream]
Whiteboard: [bugmon:bisected,confirmed] → [bugmon:bisected,confirmed], [wptsync upstream]
Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/09f6f651b86d Clean-up Servo_TakeChangeHint. r=TYLin https://hg.mozilla.org/integration/autoland/rev/5c564fc0473a Minor clean-ups to first-line reparenting code. r=TYLin
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/43977d30a10a Simplify first-line reparenting a bit. r=TYLin
Upstream PR was closed without merging
Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/autoland/rev/ccb4027ee457 Tweak an assert that doesn't hold in presence of first-line reparenting but doesn't hurt correctness.
Pushed by imoraru@mozilla.com: https://hg.mozilla.org/mozilla-central/rev/803ccb6d1160 Tweak an assert that doesn't hold in presence of first-line reparenting but doesn't hurt correctness.
Upstream PR merged by moz-wptsync-bot

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

Status: RESOLVED → VERIFIED
Keywords: bugmon

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

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

For more information, please visit BugBot documentation.

Flags: needinfo?(emilio)

Comment on attachment 9342333 [details]
Bug 1841128 - Simplify first-line reparenting a bit. r=#layout,#style

Beta/Release Uplift Approval Request

  • User impact if declined: crashes, potentially security-sensitive.
  • 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: comment 0
  • List of other uplifts needed: none
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): While it's a decently-sized set of patches, it's pretty self-contained to ::first-line, and it's a simplification... So while definitely not risk-free, I think it's worth considering uplifting.
  • String changes made/needed: none
  • Is Android affected?: Yes
Flags: needinfo?(emilio)
Attachment #9342333 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9342333 [details]
Bug 1841128 - Simplify first-line reparenting a bit. r=#layout,#style

Approved for 116.0b4

Attachment #9342333 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Reproduced the issue on Firefox m-c 20230629 on Ubuntu 22.04 by following the STR provided in Comment 0.

The crash is no longer reproducible on Firefox NIghtly 20230712094940 and Firefox 116.0b4 (treeherder build) on the same system.

Flags: qe-verify+
Regressions: 1871513
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: