1377826 - stack exhaustion in [@ nsLineBreaker::AppendText]

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.html

Found in m-c 20170630-587daa4bdc4b

#0  0x00007fffe71a8f2a in nsLineBreaker::AppendText(nsIAtom*, char16_t const*, unsigned int, unsigned int, nsILineBreakSink*) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#1  0x00007fffe859d43c in BuildTextRunsScanner::SetupBreakSinksForTextRun(gfxTextRun*, void const*) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#2  0x00007fffe85a3efa in BuildTextRunsScanner::BuildTextRunForFrames(void*) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#3  0x00007fffe85a4890 in BuildTextRunsScanner::FlushFrames(bool, bool) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#4  0x00007fffe85a5525 in BuildTextRuns(mozilla::gfx::DrawTarget*, nsTextFrame*, nsIFrame*, nsLineList_iterator const*, nsTextFrame::TextRunType) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#5  0x00007fffe85a5a89 in nsTextFrame::EnsureTextRun(nsTextFrame::TextRunType, mozilla::gfx::DrawTarget*, nsIFrame*, nsLineList_iterator const*, unsigned int*) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#6  0x00007fffe85ac261 in nsTextFrame::AddInlineMinISizeForFlow(gfxContext*, nsIFrame::InlineMinISizeData*, nsTextFrame::TextRunType) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#7  0x00007fffe85ad0ed in nsTextFrame::AddInlineMinISize(gfxContext*, nsIFrame::InlineMinISizeData*) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#8  0x00007fffe8505797 in nsBlockFrame::GetMinISize(gfxContext*) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#9  0x00007fffe8483403 in nsLayoutUtils::IntrinsicForAxis(mozilla::PhysicalAxis, gfxContext*, nsIFrame*, nsLayoutUtils::IntrinsicISizeType, mozilla::Maybe<mozilla::LogicalSize> const&, unsigned int, int) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#10 0x00007fffe8483f35 in nsLayoutUtils::IntrinsicForContainer(gfxContext*, nsIFrame*, nsLayoutUtils::IntrinsicISizeType, unsigned int) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#11 0x00007fffe85056fc in nsBlockFrame::GetMinISize(gfxContext*) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#12 0x00007fffe85bbcec in nsListControlFrame::GetMinISize(gfxContext*) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#13 0x00007fffe85c4f48 in nsComboboxControlFrame::GetIntrinsicISize(gfxContext*, nsLayoutUtils::IntrinsicISizeType) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#14 0x00007fffe84e8f0c in nsFrame::ShrinkWidthToFit(gfxContext*, int, nsIFrame::ComputeSizeFlags) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#15 0x00007fffe84f2820 in nsContainerFrame::ComputeAutoSize(gfxContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::LogicalSize const&, nsIFrame::ComputeSizeFlags) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#16 0x00007fffe84f8281 in nsFrame::ComputeSize(gfxContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::LogicalSize const&, nsIFrame::ComputeSizeFlags) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#17 0x00007fffe84d82f5 in mozilla::ReflowInput::InitConstraints(nsPresContext*, mozilla::LogicalSize const&, nsMargin const*, nsMargin const*, mozilla::LayoutFrameType) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#18 0x00007fffe84d9a73 in mozilla::ReflowInput::Init(nsPresContext*, mozilla::LogicalSize const*, nsMargin const*, nsMargin const*) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#19 0x00007fffe856aaa2 in nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#20 0x00007fffe850ea3f in nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#21 0x00007fffe851485d in nsBlockFrame::DoReflowInlineFrames(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#22 0x00007fffe852766a in nsBlockFrame::ReflowInlineFrames(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#23 0x00007fffe852775f in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) () from m-c-1499054547-opt/libxul.so
No symbol table info available.
#24 0x00007fffe8527d0b in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) () from m-c-1499054547-opt/libxul.so

Comment 1

[Ryan VanderMeulen [:RyanVM]]

INFO: Last good revision: 8f21d63b310a (2015-05-09)
INFO: First bad revision: 77d92f6d7679 (2015-05-10)
INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8f21d63b310a&tochange=77d92f6d7679

Impressive find. Regression from bug 1103348 maybe?

Comment 2

[(no longer active)]

Andrew can you please find an owner for this?  I won't have time to look at it before going on vacation.  (although this seems like a stack overflow during reflow, perhaps a more suitable bug for the layout team...)

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

In a debug build, it asserts earlier in a different place, which can probably explain why bug 1103348 causes this:

Assertion failure: !mInStyleRefresh, at c:/mozilla-source/central/layout/base/GeckoRestyleManager.cpp:206
#01: mozilla::PresShell::ContentStateChanged (c:\mozilla-source\central\layout\base\presshell.cpp:4303)
#02: nsDocument::ContentStateChanged (c:\mozilla-source\central\dom\base\nsdocument.cpp:5411)
#03: mozilla::dom::Element::UpdateState (c:\mozilla-source\central\dom\base\element.cpp:272)
#04: mozilla::dom::Element::SetDirectionality (c:\mozilla-source\central\obj-firefox-opt\dist\include\mozilla\dom\element.h:464)
#05: mozilla::SetAncestorDirectionIfAuto (c:\mozilla-source\central\dom\base\directionalityutils.cpp:850)
#06: mozilla::SetDirectionFromNewTextNode (c:\mozilla-source\central\dom\base\directionalityutils.cpp:923)
#07: nsTextNode::BindToTree (c:\mozilla-source\central\dom\base\nstextnode.cpp:148)
#08: nsCSSFrameConstructor::GetAnonymousContent (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:4342)
#09: nsCSSFrameConstructor::ConstructSelectFrame (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:3229)
#10: nsCSSFrameConstructor::ConstructFrameFromItemInternal (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:3969)
#11: nsCSSFrameConstructor::ConstructFramesFromItem (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:6344)
#12: nsCSSFrameConstructor::ConstructFramesFromItemList (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:10973)
#13: nsCSSFrameConstructor::ProcessChildren (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:11286)
#14: nsCSSFrameConstructor::ConstructBlock (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:12301)
#15: nsCSSFrameConstructor::ConstructNonScrollableBlockWithConstructor (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:5062)
#16: nsCSSFrameConstructor::ConstructNonScrollableBlock (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:5023)
#17: nsCSSFrameConstructor::ConstructFrameFromItemInternal (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:3969)
#18: nsCSSFrameConstructor::ConstructFramesFromItem (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:6344)
#19: nsCSSFrameConstructor::ConstructFramesFromItemList (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:10973)
#20: nsCSSFrameConstructor::ContentRangeInserted (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:8373)
#21: nsCSSFrameConstructor::RecreateFramesForContent (c:\mozilla-source\central\layout\base\nscssframeconstructor.cpp:10047)
#22: mozilla::RestyleManager::ProcessRestyledFrames (c:\mozilla-source\central\layout\base\restylemanager.cpp:1548)
#23: mozilla::GeckoRestyleManager::ComputeAndProcessStyleChange (c:\mozilla-source\central\layout\base\geckorestylemanager.cpp:3479)
#24: mozilla::GeckoRestyleManager::RestyleElement (c:\mozilla-source\central\layout\base\geckorestylemanager.cpp:153)
#25: mozilla::RestyleTracker::ProcessOneRestyle (c:\mozilla-source\central\layout\base\restyletracker.cpp:94)
#26: mozilla::RestyleTracker::DoProcessRestyles (c:\mozilla-source\central\layout\base\restyletracker.cpp:257)
#27: mozilla::GeckoRestyleManager::ProcessPendingRestyles (c:\mozilla-source\central\layout\base\geckorestylemanager.cpp:502)
#28: mozilla::PresShell::DoFlushPendingNotifications (c:\mozilla-source\central\layout\base\presshell.cpp:4214)

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

So the problem here is that the frame constructor creates an anonymous text node, and bind it to the tree. The text node tries to update the direction of its ancestors because their direction is auto, and the update sends notification for content state change.

I guess we can fix this via having aNotify to be false somehow in the path, probably when the text node passed to SetDirectionFromNewTextNode is anonymous. Not sure what that means for everything else, though.

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1377826 - Don't allow anonymous text node to affect ancestor direction.

Given that elements in anonymous subtree have already been excluded from
participating in auto direction, it shouldn't make anything worse to
also exclude anonymous text node from that.

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

Comment 6

[(no longer active)]

Comment on attachment 8884135 [details]
Bug 1377826 - Don't allow anonymous text node to affect ancestor direction.

https://reviewboard.mozilla.org/r/155050/#review160120

Thank you!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/435668e4e83c
Don't allow anonymous text node to affect ancestor direction. r=Ehsan

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/435668e4e83c

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Long-running crash, but seemingly pretty low frequency. What are your thoughts on nominating for backport vs. riding the trains?

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

I think it has low risk, and given its history, it doesn't seem to be something critical either. I guess I'm fine either way.

We can probably nominate it for backport and let release manager to decide.

Comment 12

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8884135 [details]
Bug 1377826 - Don't allow anonymous text node to affect ancestor direction.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1103348
[User impact if declined]: may crash in certain cases
[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]: no
[Is the change risky?]: no
[Why is the change risky/not risky?]: trivial change, just change the specific case that we were crashing
[String changes made/needed]: n/a

Comment 13

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8884135 [details]
Bug 1377826 - Don't allow anonymous text node to affect ancestor direction.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: crash in some case
Fix Landed on Version: 56
Risk to taking this patch (and alternatives if risky): not risky
String or UUID changes made by this patch: n/a

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 14

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

Comment on attachment 8884135 [details]
Bug 1377826 - Don't allow anonymous text node to affect ancestor direction.

Crash fix, let's uplift this for beta. 

The volume isn't very high on ESR and we try to limit uplifts to major security issues, past the first couple of ESR releases.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d25975fc2946

Comment 16

[Andrei Vaida [:avaida]]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #12)
> [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

Setting qe-verify- based on Xidorn Quan's assessment on manual testing needs and the fact that this fix has automated coverage.