1469354 - crash near null in [@ nsPlaceholderFrame::GetLayoutParentStyleForOutOfFlow]

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Reduced with m-c:
BuildID=20180618090238
SourceStamp=f804cc575bba9c6dcb4e3770f7dafe4d8934e73c

==92287==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000028 (pc 0x7f725ade28f1 bp 0x7ffc7a914450 sp 0x7ffc7a914440 T0)
==92287==The signal is caused by a READ memory access.
==92287==Hint: address points to the zero page.
    #0 0x7f725ade28f0 in nsPlaceholderFrame::GetLayoutParentStyleForOutOfFlow(nsIFrame**) const src/layout/generic/nsPlaceholderFrame.cpp:241:45
    #1 0x7f725a9ec199 in mozilla::RestyleManager::DoReparentComputedStyleForFirstLine(nsIFrame*, mozilla::ServoStyleSet&) src/layout/base/RestyleManager.cpp:3476:38
    #2 0x7f725a9ebdbf in mozilla::RestyleManager::DoReparentComputedStyleForFirstLine(nsIFrame*, mozilla::ServoStyleSet&) src/layout/base/RestyleManager.cpp:3400:7
    #3 0x7f725adc180c in PullOneFrame src/layout/generic/nsInlineFrame.cpp:1055:37
    #4 0x7f725adc180c in nsFirstLineFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsInlineFrame.cpp:1115
    #5 0x7f725adbcf48 in nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) src/layout/generic/nsLineLayout.cpp:924:13
    #6 0x7f725abe8a6d in nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) src/layout/generic/nsBlockFrame.cpp:4158:15
    #7 0x7f725abe73e7 in nsBlockFrame::DoReflowInlineFrames(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) src/layout/generic/nsBlockFrame.cpp:3958:5
    #8 0x7f725abde041 in nsBlockFrame::ReflowInlineFrames(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3832:9
    #9 0x7f725abd61d2 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2816:5
    #10 0x7f725abcb960 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2352:7
    #11 0x7f725abc3034 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1225:3
    #12 0x7f725abe49bb in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:306:11
    #13 0x7f725abd8596 in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3463:11
    #14 0x7f725abd6324 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2813:5
    #15 0x7f725abcdfdd in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2637:11
    #16 0x7f725abc3034 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1225:3
    #17 0x7f725abe49bb in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:306:11
    #18 0x7f725abd8596 in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3463:11
    #19 0x7f725abd6324 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2813:5
    #20 0x7f725abcb960 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2352:7
    #21 0x7f725abc3034 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1225:3
    #22 0x7f725ac25096 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14
    #23 0x7f725ac29b0f in nsColumnSetFrame::ReflowChildren(mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&, nsColumnSetFrame::ReflowConfig const&, bool, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) src/layout/generic/nsColumnSetFrame.cpp:766:7
    #24 0x7f725ac2e67b in ReflowColumns src/layout/generic/nsColumnSetFrame.cpp:463:19
    #25 0x7f725ac2e67b in nsColumnSetFrame::FindBestBalanceBSize(mozilla::ReflowInput const&, nsPresContext*, nsColumnSetFrame::ReflowConfig&, nsColumnSetFrame::ColumnBalanceData&, mozilla::ReflowOutput&, nsCollapsingMargin&, bool&, bool&, nsReflowStatus&) src/layout/generic/nsColumnSetFrame.cpp:1102
    #26 0x7f725ac2f7a2 in nsColumnSetFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsColumnSetFrame.cpp:1209:5
    #27 0x7f725ac25096 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14
    #28 0x7f725ac238de in nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsCanvasFrame.cpp:713:5
    #29 0x7f725ac25096 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14
    #30 0x7f725ad11ff5 in nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*, bool) src/layout/generic/nsGfxScrollFrame.cpp:580:3
    #31 0x7f725ad13534 in nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&) src/layout/generic/nsGfxScrollFrame.cpp:703:3
    #32 0x7f725ad174fc in nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsGfxScrollFrame.cpp:1080:3
    #33 0x7f725aba6dde in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:995:14
    #34 0x7f725aba5963 in mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/ViewportFrame.cpp:338:7
    #35 0x7f725a98d78e in mozilla::PresShell::DoReflow(nsIFrame*, bool) src/layout/base/PresShell.cpp:8940:11
    #36 0x7f725a9a3450 in mozilla::PresShell::ProcessReflowCommands(bool) src/layout/base/PresShell.cpp:9113:24
    #37 0x7f725a9a189d in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4335:11
    #38 0x7f725aa7cfe1 in FlushPendingNotifications src/layout/base/nsIPresShell.h:566:5
    #39 0x7f725aa7cfe1 in nsDocumentViewer::LoadComplete(nsresult) src/layout/base/nsDocumentViewer.cpp:1082
    #40 0x7f725dbf752f in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) src/docshell/base/nsDocShell.cpp:7161:21
    #41 0x7f725dbf3914 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp:6954:7
    #42 0x7f725dbfb0ff in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp
    #43 0x7f7254737af7 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) src/uriloader/base/nsDocLoader.cpp:1309:3
    #44 0x7f7254736b79 in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:852:14
    #45 0x7f7254733728 in nsDocLoader::DocLoaderIsEmpty(bool) src/uriloader/base/nsDocLoader.cpp:741:9
    #46 0x7f72547356fa in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp:627:5
    #47 0x7f725473673c in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp
    #48 0x7f7252ba6f95 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) src/netwerk/base/nsLoadGroup.cpp:629:28
    #49 0x7f7255b04e9e in DoUnblockOnload src/dom/base/nsDocument.cpp:8345:18
    #50 0x7f7255b04e9e in nsDocument::UnblockOnload(bool) src/dom/base/nsDocument.cpp:8267
    #51 0x7f7255ae539d in nsIDocument::DispatchContentLoadedEvents() src/dom/base/nsDocument.cpp:5166:3
    #52 0x7f7255be0e34 in applyImpl<nsIDocument, void (nsIDocument::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1165:12
    #53 0x7f7255be0e34 in apply<nsIDocument, void (nsIDocument::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1171
    #54 0x7f7255be0e34 in mozilla::detail::RunnableMethodImpl<nsIDocument*, void (nsIDocument::*)(), true, (mozilla::RunnableKind)0>::Run() src/obj-firefox/dist/include/nsThreadUtils.h:1216
    #55 0x7f72529abd95 in mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:337:32
    #56 0x7f72529cab4e in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1059:14
    #57 0x7f72529e65c4 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
    #58 0x7f72538c054a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
    #59 0x7f72538154ac in RunInternal src/ipc/chromium/src/base/message_loop.cc:325:10
    #60 0x7f72538154ac in RunHandler src/ipc/chromium/src/base/message_loop.cc:318
    #61 0x7f72538154ac in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298
    #62 0x7f725a3afc8a in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:157:27
    #63 0x7f725e68616f in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:896:22
    #64 0x7f72538154ac in RunInternal src/ipc/chromium/src/base/message_loop.cc:325:10
    #65 0x7f72538154ac in RunHandler src/ipc/chromium/src/base/message_loop.cc:318
    #66 0x7f72538154ac in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298
    #67 0x7f725e685b26 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:722:34
    #68 0x4f1ca4 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
    #69 0x4f1ca4 in main src/browser/app/nsBrowserApp.cpp:287
    #70 0x7f72723d482f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #71 0x4210e8 in _start (firefox+0x4210e8)

Comment 1

[Emilio Cobos Álvarez (:emilio)]

::first-line, my favorite kind of brokenness!

Comment 2

[Emilio Cobos Álvarez (:emilio)]

(Ugh, of course I tried with the patch in bug 1465474 applied... Now I need to get an actual build without it, so may not get to it today...)

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1469354: Use the first continuation to get the layout parent style.

Continuations may not have placeholders. There's a bunch of code that already
deals with that in other places in the tree.

Review commit: https://reviewboard.mozilla.org/r/252280/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/252280/

Comment 4

[Mats Palmgren (inactive)]

Comment on attachment 8987036 [details]
Bug 1469354: Use the first continuation to get the layout parent style.

https://reviewboard.mozilla.org/r/252280/#review259036

::: commit-message-2f021:3
(Diff revision 1)
> +Continuations may not have placeholders. There's a bunch of code that already
> +deals with that in other places in the tree.

As far as I know, we don't have placeholder continuations at all anymore, so s/may not/do not/

Comment 5

[Emilio Cobos Álvarez (:emilio)]

(In reply to Mats Palmgren (:mats) from comment #4)
> Comment on attachment 8987036 [details]
> Bug 1469354: Use the first continuation to get the layout parent style.
> 
> https://reviewboard.mozilla.org/r/252280/#review259036
> 
> ::: commit-message-2f021:3
> (Diff revision 1)
> > +Continuations may not have placeholders. There's a bunch of code that already
> > +deals with that in other places in the tree.
> 
> As far as I know, we don't have placeholder continuations at all anymore, so
> s/may not/do not/

Should we remove [1] in a followup then?

[1]: https://searchfox.org/mozilla-central/rev/39b790b29543a4718d876d8ca3fd179d82fc24f7/layout/base/nsCSSFrameConstructor.cpp#8601

Comment 6

[Mats Palmgren (inactive)]

Yeah, that looks like dead code to me.

Comment 7

[Mats Palmgren (inactive)]

Hmm, unless perhaps ReplicateFixedFrames does something like that?

Comment 8

[Mats Palmgren (inactive)]

It first says "we want to move over the placeholders when processing
the child of the pageContentFrame" and then "Iterate across fixed
frames and replicate each whose placeholder" so I'm guessing it
just creates new independent placeholders for the new fixed pos
frames on the new page, and that they aren't continuations of
the placeholder on the last page.

The easiest way to check is probably just to remove said code and
see if you trigger the MOZ_CRASH("unexpected frame type")...
(I'm pretty sure we have crashtests that trigger replicated fixed
pos frames.)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e6723170dbf
Use the first continuation to get the layout parent style. r=mats

Comment 10

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/5e6723170dbf

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Worth nominating for Beta uplift since we just started the cycle?

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8987036 [details]
Bug 1469354: Use the first continuation to get the layout parent style.

Approval Request Comment
[Feature/Bug causing the regression]: stylo
[User impact if declined]: crash
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes (just did)
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not risky
[Why is the change risky/not risky?]: one-liner crash fix to avoid looking to continuing frames.
[String changes made/needed]:

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8987036 [details]
Bug 1469354: Use the first continuation to get the layout parent style.

Crash fix, let's take this for beta 4.

Comment 14

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

https://hg.mozilla.org/releases/mozilla-beta/rev/73e476e6f9d2