Bug 20400 – Infinite recursion crash in WebCore::RenderSVGRoot::absoluteClippedOverflowRect on a <stop> element outside of a gradient block

• Component: SVG
• Status: RESOLVED FIXED

[Filipe Almeida]

Webkit runs out of stack if it encounters a <stop> element outside a gradient block.

[Filipe Almeida]

Created attachment 22816 [details]
test case (crashes safari)

[Mark Rowe (bdash)]

[... snip ...]
#149625 0x02765064 in WebCore::RenderObject::absoluteClippedOverflowRect (this=0x19b0f7dc) at WebCore/rendering/RenderObject.cpp:1991
#149626 0x0277f83f in WebCore::RenderSVGRoot::absoluteClippedOverflowRect (this=0x19b0f69c) at WebCore/rendering/RenderSVGRoot.cpp:232
#149627 0x02765064 in WebCore::RenderObject::absoluteClippedOverflowRect (this=0x19b0f7dc) at WebCore/rendering/RenderObject.cpp:1991
#149628 0x0277f83f in WebCore::RenderSVGRoot::absoluteClippedOverflowRect (this=0x19b0f69c) at WebCore/rendering/RenderSVGRoot.cpp:232
#149629 0x02765064 in WebCore::RenderObject::absoluteClippedOverflowRect (this=0x19b0f7dc) at WebCore/rendering/RenderObject.cpp:1991
#149630 0x0277f83f in WebCore::RenderSVGRoot::absoluteClippedOverflowRect (this=0x19b0f69c) at WebCore/rendering/RenderSVGRoot.cpp:232
#149631 0x02780417 in WebCore::RenderSVGRoot::layout (this=0x19b0f69c) at WebCore/rendering/RenderSVGRoot.cpp:96
#149632 0x02715897 in WebCore::RenderBlock::layoutBlockChildren (this=0x19b0f0bc, relayoutChildren=true, maxFloatBottom=@0xbfffde1c) at WebCore/rendering/RenderBlock.cpp:1281
#149633 0x02717625 in WebCore::RenderBlock::layoutBlock (this=0x19b0f0bc, relayoutChildren=true) at WebCore/rendering/RenderBlock.cpp:627
#149634 0x02705a3c in WebCore::RenderBlock::layout (this=0x19b0f0bc) at WebCore/rendering/RenderBlock.cpp:536
#149635 0x027b1db8 in WebCore::RenderView::layout (this=0x19b0f0bc) at WebCore/rendering/RenderView.cpp:118
#149636 0x024ea52b in WebCore::FrameView::layout (this=0x19b455b0, allowSubtree=true) at WebCore/page/FrameView.cpp:486
#149637 0x02439c1e in WebCore::Document::implicitClose (this=0x4978400) at WebCore/dom/Document.cpp:1591


Confirmed with TOT.

[Mark Rowe (bdash)]

<rdar://problem/6152395>

[Eric Seidel (no email)]

This seems like a fundamental misunderstanding in SVG's absoluteClippedOverflowRect implementation:

IntRect RenderSVGRoot::absoluteClippedOverflowRect()
{
    IntRect repaintRect;

    for (RenderObject* current = firstChild(); current != 0; current = current->nextSibling())
        repaintRect.unite(current->absoluteClippedOverflowRect());

#if ENABLE(SVG_FILTERS)
    // Filters can expand the bounding box
    SVGResourceFilter* filter = getFilterById(document(), style()->svgStyle()->filter());
    if (filter)
        repaintRect.unite(enclosingIntRect(filter->filterBBoxForItemBBox(repaintRect)));
#endif

    return repaintRect;
}

IntRect RenderObject::absoluteClippedOverflowRect()
{
    if (parent())
        return parent()->absoluteClippedOverflowRect();
    return IntRect();
}

These two recursively call each other. :(

One fix would be to add a absoluteClippedOverflowRect() implementation to RenderSVGGradientStop.  I'll do that  for now, but I think we may need more fixes to SVG here.

[Eric Seidel (no email)]

Created attachment 24545 [details]
Fix the crash

 LayoutTests/ChangeLog                          |   10 ++++++++++
 LayoutTests/svg/custom/stop-crash-expected.txt |    1 +
 LayoutTests/svg/custom/stop-crash.svg          |    8 ++++++++
 WebCore/ChangeLog                              |   13 +++++++++++++
 WebCore/rendering/RenderSVGGradientStop.h      |   10 ++++++++--
 5 files changed, 40 insertions(+), 2 deletions(-)

[Eric Seidel (no email)]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/svg/custom/stop-crash-expected.txt
	A	LayoutTests/svg/custom/stop-crash.svg
	M	WebCore/ChangeLog
	M	WebCore/rendering/RenderSVGGradientStop.h
Committed r37899

[Darin Adler]

I wish the comment explained things in a more positive way. It talks about "preventing a crash", but I'd like to understand more positively what the design is.

[Eric Seidel (no email)]

(In reply to comment #7)
> I wish the comment explained things in a more positive way. It talks about
> "preventing a crash", but I'd like to understand more positively what the
> design is.

Me too.  I'm not sure the design is right.  I don't understand how absoluteClippedOverflowRect is supposed to work.  Hyatt may be the only one who does.