Closed
Bug 1416544
Opened 7 years ago
Closed 7 years ago
Crash in nsPlaceholderFrame::RenumberFrameAndDescendants
Categories
(Core :: Layout: Floats, defect)
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox56 | --- | wontfix |
firefox57 | --- | wontfix |
firefox58 | --- | fixed |
firefox59 | --- | fixed |
People
(Reporter: bc, Assigned: MatsPalmgren_bugz)
References
(Blocks 1 open bug, )
Details
(Keywords: crash, csectype-nullptr, testcase)
Crash Data
Attachments
(3 files)
323 bytes,
text/html
|
Details | |
11.52 KB,
text/html
|
Details | |
3.78 KB,
patch
|
dholbert
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
1. http://www.mellowship.nu/menu 2. Crash Beta/57, Nightly/58 This and related crashes involving nsFontFaceUtils::MarkDirtyForFontChange occur on Windows and Linux. This bug was filed from the Socorro interface and is report bp-7fbf4d13-ebe2-4b13-812a-02b6e0171112. ============================================================= Top 10 frames of crashing thread: 0 xul.dll nsPlaceholderFrame::RenumberFrameAndDescendants layout/generic/nsPlaceholderFrame.h:159 1 xul.dll nsBlockFrame::RenumberChildFrames layout/generic/nsBlockFrame.cpp:7167 2 xul.dll nsContainerFrame::RenumberFrameAndDescendants layout/generic/nsContainerFrame.cpp:1921 3 xul.dll nsBlockFrame::RenumberChildFrames layout/generic/nsBlockFrame.cpp:7167 4 xul.dll nsContainerFrame::RenumberFrameAndDescendants layout/generic/nsContainerFrame.cpp:1906 5 xul.dll nsBlockFrame::RenumberChildFrames layout/generic/nsBlockFrame.cpp:7167 6 xul.dll nsContainerFrame::RenumberList layout/generic/nsContainerFrame.cpp:1829 7 xul.dll nsBlockFrame::Reflow layout/generic/nsBlockFrame.cpp:1191 8 xul.dll nsContainerFrame::ReflowChild layout/generic/nsContainerFrame.cpp:934 9 xul.dll nsColumnSetFrame::ReflowChildren layout/generic/nsColumnSetFrame.cpp:810 ============================================================= Nightly/58 on Linux produced bp-9aec4739-87d9-4bf1-87b5-dcd750171112 [@ nsFontFaceUtils::MarkDirtyForFontChange ] 0 libxul.so nsFontFaceUtils::MarkDirtyForFontChange mfbt/RefPtr.h:287 1 libxul.so nsFontFaceLoader::OnStreamComplete layout/style/nsFontFaceLoader.cpp:283 2 libxul.so mozilla::net::nsStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) 3 libxul.so nsCORSListenerProxy::OnStopRequest(nsIRequest*, nsISupports*, nsresult) 4 libxul.so mozilla::net::HttpChannelChild::DoOnStopRequest netwerk/protocol/http/HttpChannelChild.cpp:1206 5 libxul.so mozilla::net::HttpChannelChild::OnStopRequest netwerk/protocol/http/HttpChannelChild.cpp:1099 6 libxul.so mozilla::net::ChannelEventQueue::FlushQueue netwerk/ipc/ChannelEventQueue.cpp:93 7 libxul.so mozilla::net::ChannelEventQueue::ResumeInternal()::CompleteResumeRunnable::Run() 8 libxul.so mozilla::SchedulerGroup::Runnable::Run xpcom/threads/SchedulerGroup.cpp:396 9 libxul.so nsThread::ProcessNextEvent(bool, bool*) 10 libxul.so NS_ProcessNextEvent(nsIThread*, bool) debug asan gives: ================================================================= ==10898==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x7f67e8aba86c bp 0x7ffdd43c4d90 sp 0x7ffdd43c4a60 T0) ==10898==The signal is caused by a READ memory access. ==10898==Hint: address points to the zero page. #0 0x7f67e8aba86b in get /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:287:27 #1 0x7f67e8aba86b in operator nsStyleContext * /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:300 #2 0x7f67e8aba86b in StyleContext /builds/worker/workspace/build/src/obj-firefox/dist/include/nsIFrame.h:786 #3 0x7f67e8aba86b in PresContext /builds/worker/workspace/build/src/obj-firefox/dist/include/nsIFrame.h:641 #4 0x7f67e8aba86b in FrameUsesFont /builds/worker/workspace/build/src/layout/style/nsFontFaceUtils.cpp:52 #5 0x7f67e8aba86b in nsFontFaceUtils::MarkDirtyForFontChange(nsIFrame*, gfxUserFontEntry const*) /builds/worker/workspace/build/src/layout/style/nsFontFaceUtils.cpp:131 #6 0x7f67e8a0e991 in nsFontFaceLoader::OnStreamComplete(nsIStreamLoader*, nsISupports*, nsresult, unsigned int, unsigned char const*) /builds/worker/workspace/build/src/layout/style/nsFontFaceLoader.cpp:283:14 #7 0x7f67e1f794fd in mozilla::net::nsStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/workspace/build/src/netwerk/base/nsStreamLoader.cpp:108:30 #8 0x7f67e274051c in nsCORSListenerProxy::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/workspace/build/src/netwerk/protocol/http/nsCORSListenerProxy.cpp:638:27 #9 0x7f67e26d3bbc in mozilla::net::HttpChannelChild::DoOnStopRequest(nsIRequest*, nsresult, nsISupports*) /builds/worker/workspace/build/src/netwerk/protocol/http/HttpChannelChild.cpp:1206:16 #10 0x7f67e26dff3a in mozilla::net::HttpChannelChild::OnStopRequest(nsresult const&, mozilla::net::ResourceTimingStruct const&) /builds/worker/workspace/build/src/netwerk/protocol/http/HttpChannelChild.cpp:1099:5
Assignee | ||
Comment 1•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 2•7 years ago
|
||
Also occurs in 56.0.2: bp-e14fddfc-bcc5-426b-9e05-445911171112
status-firefox56:
--- → affected
Assignee | ||
Comment 3•7 years ago
|
||
When we first reflow the "Block(div)(1)@7ff86927f018" (lime) it pushes a float (red) and the frame that follows (blue). The problem occurs on the next reflow we pull up the in-flow frame (blue) and signal that the reflow is complete. When deleting the continuations we also delete the pushed float, which triggers various assertions: ###!!! ASSERTION: frame tree not empty, but caller reported complete status: 'aSubtreeRoot->GetPrevInFlow()', file layout/base/nsLayoutUtils.cpp, line 7840 ###!!! ASSERTION: frame tree not empty, but caller reported complete status: 'aSubtreeRoot->GetPrevInFlow()', file layout/base/nsLayoutUtils.cpp, line 7840 ###!!! ASSERTION: frame tree not empty, but caller reported complete status: 'start == end || IsInLetterFrame(aSubtreeRoot)', file layout/base/nsLayoutUtils.cpp, line 7854 ###!!! ASSERTION: Placeholder relationship should have been torn down already; this might mean we have a stray placeholder in the tree.: '!placeholder || nsLayoutUtils::IsProperAncestorFrame(aDestructRoot, placeholder)', file layout/generic/nsFrame.cpp, line 760 ###!!! ASSERTION: Null out-of-flow for placeholder?: 'outOfFlow', file layout/generic/nsPlaceholderFrame.h, line 183
Assignee: nobody → mats
Assignee | ||
Comment 4•7 years ago
|
||
It appears to always be a null-pointer crash for me. I'm guessing the worst case is we try to use the deleted float frame, which is covered by frame-poisoning so it should be non-exploitable.
Assignee | ||
Comment 5•7 years ago
|
||
This patch amends an existing workaround, but as the NOTE there says, we should have pulled up those floats and reflowed them somewhere (and pushed them again potentially, and then we wouldn't be COMPLETE). It's unclear to me where that pull-up is supposed to happen though.
Attachment #8927995 -
Flags: review?(dholbert)
Assignee | ||
Comment 6•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6e03c29079595bbac9aaec0a196558c48324f97
Assignee | ||
Updated•7 years ago
|
Keywords: csectype-nullptr
Comment 7•7 years ago
|
||
Comment on attachment 8927995 [details] [diff] [review] Don't report COMPLETE reflow status when we still have pushed floats on some next-in-flow frame Review of attachment 8927995 [details] [diff] [review]: ----------------------------------------------------------------- r=me
Attachment #8927995 -
Flags: review?(dholbert) → review+
Pushed by mpalmgren@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9ff1b6740768 Don't report COMPLETE reflow status when we still have pushed floats on some next-in-flow frame. r=dholbert
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9ff1b6740768
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 10•7 years ago
|
||
Hi :mats, Since this bug also affects 58, do you think it's worth uplifting to 58 if this patch is not too risky?
Flags: needinfo?(mats)
Assignee | ||
Comment 12•7 years ago
|
||
Comment on attachment 8927995 [details] [diff] [review] Don't report COMPLETE reflow status when we still have pushed floats on some next-in-flow frame Approval Request Comment [Feature/Bug causing the regression]:no [User impact if declined]:crash in rare multicolumn layouts containing floats [Is this code covered by automated tests?]:yes [Has the fix been verified in Nightly?]:yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: [Is the change risky?]:no [Why is the change risky/not risky?]:simple change to amend an already present wallpaper with one more case [String changes made/needed]:none
Attachment #8927995 -
Flags: approval-mozilla-beta?
Updated•7 years ago
|
Comment 13•7 years ago
|
||
Comment on attachment 8927995 [details] [diff] [review] Don't report COMPLETE reflow status when we still have pushed floats on some next-in-flow frame Fix a crash. Beta58+.
Attachment #8927995 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 14•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/4c9e90fc3912
Updated•7 years ago
|
status-firefox-esr52:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•