1663232 - crash near null in [@ nsFrameManager::CaptureFrameState]

Status: RESOLVED FIXED

(Core :: Layout: Floats, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

==46075==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f6c63b97c5c bp 0x7ffcc1808b10 sp 0x7ffcc1808b00 T0)
==46075==The signal is caused by a READ memory access.
==46075==Hint: address points to the zero page.
    #0 0x7f6c63b97c5b in nsIFrame::ChildLists() const /builds/worker/workspace/obj-build/dist/include/nsIFrame.h
    #1 0x7f6c63d0b7a9 in nsFrameManager::CaptureFrameState(nsIFrame*, nsILayoutHistoryState*) /gecko/layout/base/nsFrameManager.cpp:166:40
    #2 0x7f6c63d0ba17 in nsFrameManager::CaptureFrameState(nsIFrame*, nsILayoutHistoryState*) /gecko/layout/base/nsFrameManager.cpp:175:7
    #3 0x7f6c63d0ba17 in nsFrameManager::CaptureFrameState(nsIFrame*, nsILayoutHistoryState*) /gecko/layout/base/nsFrameManager.cpp:175:7
    #4 0x7f6c63d0ba17 in nsFrameManager::CaptureFrameState(nsIFrame*, nsILayoutHistoryState*) /gecko/layout/base/nsFrameManager.cpp:175:7
    #5 0x7f6c63d0ba17 in nsFrameManager::CaptureFrameState(nsIFrame*, nsILayoutHistoryState*) /gecko/layout/base/nsFrameManager.cpp:175:7
    #6 0x7f6c63d0ba17 in nsFrameManager::CaptureFrameState(nsIFrame*, nsILayoutHistoryState*) /gecko/layout/base/nsFrameManager.cpp:175:7
    #7 0x7f6c63d0ba17 in nsFrameManager::CaptureFrameState(nsIFrame*, nsILayoutHistoryState*) /gecko/layout/base/nsFrameManager.cpp:175:7
    #8 0x7f6c63d0ba17 in nsFrameManager::CaptureFrameState(nsIFrame*, nsILayoutHistoryState*) /gecko/layout/base/nsFrameManager.cpp:175:7
    #9 0x7f6c63d0609d in nsCSSFrameConstructor::CaptureStateForFramesOf(nsIContent*, nsILayoutHistoryState*) /gecko/layout/base/nsCSSFrameConstructor.cpp:8195:5
    #10 0x7f6c63d05159 in nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, nsCSSFrameConstructor::RemoveFlags) /gecko/layout/base/nsCSSFrameConstructor.cpp:7457:7
    #11 0x7f6c63cfa96a in nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind) /gecko/layout/base/nsCSSFrameConstructor.cpp:8569:7
    #12 0x7f6c63d07817 in nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame*) /gecko/layout/base/nsCSSFrameConstructor.cpp:8471:3
    #13 0x7f6c63cfa916 in nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind) /gecko/layout/base/nsCSSFrameConstructor.cpp:8558:16
    #14 0x7f6c63d021d5 in nsCSSFrameConstructor::WipeInsertionParent(nsContainerFrame*) /gecko/layout/base/nsCSSFrameConstructor.cpp
    #15 0x7f6c63d010be in nsCSSFrameConstructor::ContentAppended(nsIContent*, nsCSSFrameConstructor::InsertionKind) /gecko/layout/base/nsCSSFrameConstructor.cpp:6672:7
    #16 0x7f6c63c97220 in mozilla::RestyleManager::ProcessRestyledFrames(nsStyleChangeList&) /gecko/layout/base/RestyleManager.cpp:1380:27
    #17 0x7f6c63ca1a8c in mozilla::RestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /gecko/layout/base/RestyleManager.cpp:3037:9
    #18 0x7f6c63c627b1 in ProcessPendingRestyles /gecko/layout/base/RestyleManager.cpp:3116:3
    #19 0x7f6c63c627b1 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /gecko/layout/base/PresShell.cpp:4196:39
    #20 0x7f6c5f043a3d in FlushPendingNotifications /builds/worker/workspace/obj-build/dist/include/mozilla/PresShell.h:1412:5
    #21 0x7f6c5f043a3d in mozilla::dom::Document::FlushPendingNotifications(mozilla::ChangesToFlush) /gecko/dom/base/Document.cpp:10092:16
    #22 0x7f6c5da69a4e in nsDocLoader::DocLoaderIsEmpty(bool, mozilla::Maybe<nsresult> const&) /gecko/uriloader/base/nsDocLoader.cpp:702:14
    #23 0x7f6c5da6c71d in nsDocLoader::OnStopRequest(nsIRequest*, nsresult) /gecko/uriloader/base/nsDocLoader.cpp:640:5
    #24 0x7f6c5da6d7bc in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsresult) /gecko/uriloader/base/nsDocLoader.cpp
    #25 0x7f6c5b355f37 in mozilla::net::nsLoadGroup::NotifyRemovalObservers(nsIRequest*, nsresult) /gecko/netwerk/base/nsLoadGroup.cpp:615:22
    #26 0x7f6c5b359147 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /gecko/netwerk/base/nsLoadGroup.cpp:522:10
    #27 0x7f6c5f04a16f in mozilla::dom::Document::DoUnblockOnload() /gecko/dom/base/Document.cpp:10825:18
    #28 0x7f6c5f001a47 in mozilla::dom::Document::UnblockOnload(bool) /gecko/dom/base/Document.cpp:10755:9
    #29 0x7f6c5f024504 in mozilla::dom::Document::DispatchContentLoadedEvents() /gecko/dom/base/Document.cpp:7333:3
    #30 0x7f6c5f0f3694 in applyImpl<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1188:12
    #31 0x7f6c5f0f3694 in apply<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1194:12
    #32 0x7f6c5f0f3694 in mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1240:13
    #33 0x7f6c5b0565bd in mozilla::SchedulerGroup::Runnable::Run() /gecko/xpcom/threads/SchedulerGroup.cpp:146:20
    #34 0x7f6c5b060d29 in mozilla::RunnableTask::Run() /gecko/xpcom/threads/TaskController.cpp:242:16
    #35 0x7f6c5b05d215 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /gecko/xpcom/threads/TaskController.cpp:512:26
    #36 0x7f6c5b05b0d2 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /gecko/xpcom/threads/TaskController.cpp:371:15
    #37 0x7f6c5b05b50f in mozilla::TaskController::ProcessPendingMTTask(bool) /gecko/xpcom/threads/TaskController.cpp:168:36
    #38 0x7f6c5b06cb31 in operator() /gecko/xpcom/threads/TaskController.cpp:83:37
    #39 0x7f6c5b06cb31 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_4>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:577:5
    #40 0x7f6c5b091c0c in nsThread::ProcessNextEvent(bool, bool*) /gecko/xpcom/threads/nsThread.cpp:1242:14
    #41 0x7f6c5b09cafc in NS_ProcessNextEvent(nsIThread*, bool) /gecko/xpcom/threads/nsThreadUtils.cpp:513:10
    #42 0x7f6c5c493e0f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /gecko/ipc/glue/MessagePump.cpp:87:21
    #43 0x7f6c5c376287 in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #44 0x7f6c5c376287 in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #45 0x7f6c5c376287 in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #46 0x7f6c63714588 in nsBaseAppShell::Run() /gecko/widget/nsBaseAppShell.cpp:137:27
    #47 0x7f6c6731a856 in XRE_RunAppShell() /gecko/toolkit/xre/nsEmbedFunctions.cpp:913:20
    #48 0x7f6c5c376287 in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #49 0x7f6c5c376287 in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #50 0x7f6c5c376287 in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #51 0x7f6c67319e3f in XRE_InitChildProcess(int, char**, XREChildData const*) /gecko/toolkit/xre/nsEmbedFunctions.cpp:744:34
    #52 0x55b60d39d6f3 in content_process_main /gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #53 0x55b60d39d6f3 in main /gecko/browser/app/nsBrowserApp.cpp:303:18

Comment 1

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/JvHiXTgcVC0yL0VYWchfqw/index.html

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Details/summary and multicol, this sounds like TYLin's stuff :-)

Ting-Yu, do you have cycles to look at this? This means that we've removed a placeholder from the frame tree without removing its corresponding out-of-flow frame:

https://searchfox.org/mozilla-central/rev/0c97a6410ff018c22e65a0cbe4e5f2ca4581b22e/layout/base/nsFrameManager.cpp#168-175

Comment 3

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: rr-log-bug1663232.html

I captured a rr log by changing the following assertion [1] to MOZ_ASSERT in nsLayoutUtils::AssertTreeOnlyEmptyNextInFlows.

NS_ASSERTION(aSubtreeRoot->GetPrevInFlow(),
               "frame tree not empty, but caller reported complete status");

It looks like we delete a kid's next-in-flow, which contains a float element's first-in-flow in the subtree, because the kid is fully-complete. It results in the crash later (as reported by this bug) that we remove a placeholder without removing its out-of-flow frame because the out-of-flow frame is already gone.

I haven't figured out why this can happen.

[1] https://searchfox.org/mozilla-central/rev/d410917833190ee08a62b38f8f9de65cea1e661b/layout/base/nsLayoutUtils.cpp#8124-8125

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

It looks like we delete a kid's next-in-flow, which contains a float element's first-in-flow in the subtree.

OK. I found the cause. The kid is going to be reflow again due a clearance frame discovered in the subtree. In this case, it's reflow status shouldn't be used. We should add !aFrameRI.WillReflowAgainForClearance() to the if-statement that checking whether to delete the kid's next-in-flow in nsBlockReflowContext::ReflowBlock.

Comment 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1663232 - Don't delete a child's next-in-flow if it's going to be reflow again due to clearance.

After this patch, the testcase still triggers soft assertions like
"ASSERTION: Placeholder relationship should have been torn down already;
this might mean we have a stray placeholder in the tree." We have
multiple bugs filed on this assertions such as bug 856269.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4b357295aa3
Don't delete a child's next-in-flow if it's going to be reflow again due to clearance. r=emilio

Comment 7

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/26786 for changes under testing/web-platform/tests

Comment 8

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/f4b357295aa3

Comment 9

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by jgraham