crash near null in [@ nsBlockFrame::StealFrame]
Categories
(Core :: Layout: Block and Inline, defect)
Tracking
()
| 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)
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)
|
Reporter | |
Comment 1•1 year ago
|
||
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'
|
|
Reporter | |
Updated•1 year ago
|
|
|
Reporter | |
Updated•1 year ago
|
|
||
Comment 2•1 year ago
|
||
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
|
||
Comment 3•1 year ago
|
||
Regressed by: Bug 1839223 - Remove nsMappedAttributes. r=smaug
|
||
Comment 4•1 year ago
|
||
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.
|
||
Updated•1 year ago
|
|
Assignee | |
Updated•1 year ago
|
|
|
Assignee | |
Comment 6•1 year ago
|
||
This is drive-by. We already rely on was_restyled not being null so make
that explicit.
|
|
Assignee | |
Comment 7•1 year ago
|
||
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
|
|
Assignee | |
Comment 8•1 year ago
|
||
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
|
||
Comment 11•1 year ago
|
||
Backed out 3 changesets (Bug 1841128) for causing failures in 1385656.html
Log: https://treeherder.mozilla.org/logviewer?job_id=421882451&repo=autoland&lineNumber=9335
Backout: https://hg.mozilla.org/integration/autoland/rev/588c7386d4aeb3ec9b0c5056e5f855977568e2e3
|
|
Assignee | |
Updated•1 year ago
|
|
||
Comment 12•1 year ago
|
||
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
|
|
||
Comment 13•1 year ago
|
||
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
|
|
||
Comment 15•1 year ago
|
||
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.
|
|
||
Comment 16•1 year ago
|
||
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.
|
||
Comment 17•1 year ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/09f6f651b86d
https://hg.mozilla.org/mozilla-central/rev/5c564fc0473a
https://hg.mozilla.org/mozilla-central/rev/43977d30a10a
https://hg.mozilla.org/mozilla-central/rev/803ccb6d1160
Upstream PR merged by moz-wptsync-bot
|
|
||
Comment 19•1 year ago
|
||
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.
|
||
Comment 20•1 year ago
|
||
| bugherder | ||
|
|
||
Comment 21•1 year ago
|
||
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-firefox116towontfix.
For more information, please visit BugBot documentation.
|
|
Assignee | |
Comment 22•1 year ago
|
||
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
|
|
Assignee | |
Updated•1 year ago
|
Comment on attachment 9342333 [details]
Bug 1841128 - Simplify first-line reparenting a bit. r=#layout,#style
Approved for 116.0b4
|
|
||
Comment 24•1 year ago
|
||
| uplift | ||
https://hg.mozilla.org/releases/mozilla-beta/rev/87cd09c81495
|
|
||
Comment 25•1 year ago
|
||
| uplift | ||
https://hg.mozilla.org/releases/mozilla-beta/rev/d59e19237835
|
||
Updated•1 year ago
|
|
||
Comment 26•1 year ago
|
||
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.
Description
•