Bug 53900 – REGRESSION(r77740): CSSStyleSelector accessing deleted memory for svg/dom/use-transform.svg

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Andreas Kling]

svg/dom/use-transform.svg is currently crashing intermittently on the Qt bot.
The CSSStyleSelector* stored in the StyleSelectorParentPusher is deleted below recalcStyle().

A valgrind trace for your convenience:

==8437== Invalid read of size 4
==8437==    at 0x47336F7: WebCore::CSSStyleSelector::popParent(WebCore::Element*) (Vector.h:530)
==8437==    by 0x47ADDF2: WebCore::Element::recalcStyle(WebCore::Node::StyleChange) (Element.cpp:89)
==8437==    by 0x478BB36: WebCore::Document::recalcStyle(WebCore::Node::StyleChange) (Document.cpp:1527)
==8437==    by 0x478017A: WebCore::Document::updateStyleIfNeeded() (Document.cpp:1569)
==8437==    by 0x47809D6: WebCore::Document::updateLayout() (Document.cpp:1596)
==8437==    by 0x4791678: WebCore::Document::updateLayoutIgnorePendingStylesheets() (Document.cpp:1632)
==8437==    by 0x4DAD667: WebCore::SVGElementInstance::invalidateAllInstancesOfElement(WebCore::SVGElement*) (SVGElementInstance.cpp:111)
==8437==    by 0x4E20C2B: WebCore::SVGStyledElement::svgAttributeChanged(WebCore::QualifiedName const&) (SVGStyledElement.cpp:256)
==8437==    by 0x4DE65DB: WebCore::SVGGElement::svgAttributeChanged(WebCore::QualifiedName const&) (SVGGElement.cpp:60)
==8437==    by 0x4DAAC8F: WebCore::SVGElement::attributeChanged(WebCore::Attribute*, bool) (SVGElement.cpp:358)
==8437==    by 0x47AAE23: WebCore::Element::setAttribute(WTF::AtomicString const&, WTF::AtomicString const&, int&) (Element.cpp:664)
==8437==    by 0x42AA202: WebCore::jsElementPrototypeFunctionSetAttribute(JSC::ExecState*) (in /home/kling/src/webkit/WebKitBuild/Release/lib/libQtWebKit.so.4.9.0)

==8437==  Address 0xe999e88 is 32 bytes inside a block of size 780 free'd
==8437==    at 0x40257ED: free (vg_replace_malloc.c:366)
==8437==    by 0x4EAC5AC: WTF::fastFree(void*) (in /home/kling/src/webkit/WebKitBuild/Release/lib/libQtWebKit.so.4.9.0)
==8437==    by 0x4790446: WebCore::Document::recalcStyleSelector() (CSSStyleSelector.h:87)
==8437==    by 0x47909AE: WebCore::Document::styleSelectorChanged(WebCore::StyleSelectorUpdateFlag) (Document.cpp:2866)
==8437==    by 0x48F1700: WebCore::HTMLLinkElement::removedFromDocument() (in /home/kling/src/webkit/WebKitBuild/Release/lib/libQtWebKit.so.4.9.0)
==8437==    by 0x477697A: WebCore::ContainerNode::removedFromDocument() (ContainerNode.cpp:743)
==8437==    by 0x47AE67A: WebCore::Element::removedFromDocument() (Element.cpp:918)
==8437==    by 0x477697A: WebCore::ContainerNode::removedFromDocument() (ContainerNode.cpp:743)
==8437==    by 0x47AE67A: WebCore::Element::removedFromDocument() (Element.cpp:918)
==8437==    by 0x477697A: WebCore::ContainerNode::removedFromDocument() (ContainerNode.cpp:743)
==8437==    by 0x47AE67A: WebCore::Element::removedFromDocument() (Element.cpp:918)
==8437==    by 0x4E20555: WebCore::SVGStyledElement::removedFromDocument() (SVGStyledElement.cpp:284)
==8437==    by 0x477697A: WebCore::ContainerNode::removedFromDocument() (ContainerNode.cpp:743)
==8437==    by 0x47AE67A: WebCore::Element::removedFromDocument() (Element.cpp:918)
==8437==    by 0x4E20555: WebCore::SVGStyledElement::removedFromDocument() (SVGStyledElement.cpp:284)
==8437==    by 0x4776010: void WebCore::Private::addChildNodesToDeletionQueue<WebCore::Node, WebCore::ContainerNode>(WebCore::Node*&, WebCore::Node*&, WebCore::ContainerNode*) (ContainerNodeAlgorithms.h:99)
==8437==    by 0x4776051: WebCore::ContainerNode::removeAllChildren() (ContainerNodeAlgorithms.h:47)
==8437==    by 0x4D6A08C: WebCore::RenderSVGShadowTreeRootContainer::updateFromElement() (in /home/kling/src/webkit/WebKitBuild/Release/lib/libQtWebKit.so.4.9.0)
==8437==    by 0x4E3A328: WebCore::SVGUseElement::recalcStyle(WebCore::Node::StyleChange) (SVGUseElement.cpp:354)
==8437==    by 0x47ADD1D: WebCore::Element::recalcStyle(WebCore::Node::StyleChange) (Element.cpp:1106)
==8437==    by 0x478BB36: WebCore::Document::recalcStyle(WebCore::Node::StyleChange) (Document.cpp:1527)
==8437==    by 0x478017A: WebCore::Document::updateStyleIfNeeded() (Document.cpp:1569)
==8437==    by 0x47809D6: WebCore::Document::updateLayout() (Document.cpp:1596)
==8437==    by 0x4791678: WebCore::Document::updateLayoutIgnorePendingStylesheets() (Document.cpp:1632)
==8437==    by 0x4DAD667: WebCore::SVGElementInstance::invalidateAllInstancesOfElement(WebCore::SVGElement*) (SVGElementInstance.cpp:111)
==8437==    by 0x4E20C2B: WebCore::SVGStyledElement::svgAttributeChanged(WebCore::QualifiedName const&) (SVGStyledElement.cpp:256)
==8437==    by 0x4DE65DB: WebCore::SVGGElement::svgAttributeChanged(WebCore::QualifiedName const&) (SVGGElement.cpp:60)
==8437==    by 0x4DAAC8F: WebCore::SVGElement::attributeChanged(WebCore::Attribute*, bool) (SVGElement.cpp:358)
==8437==    by 0x47AAE23: WebCore::Element::setAttribute(WTF::AtomicString const&, WTF::AtomicString const&, int&) (Element.cpp:664)
==8437==    by 0x42AA202: WebCore::jsElementPrototypeFunctionSetAttribute(JSC::ExecState*) (in /home/kling/src/webkit/WebKitBuild/Release/lib/libQtWebKit.so.4.9.0)

[Antti Koivisto]

Silly svg code manages to do a styleselector update in the middle of a style recalc. That's totally wrong.

[Simon Fraser (smfr)]

We should make that assert as well as fixing the crash.

[Antti Koivisto]

Created attachment 81482 [details]
possible fix

Kling, could you try if this silences valgrind?

[Andreas Kling]

(In reply to comment #3)
> Kling, could you try if this silences valgrind?

It does indeed!

[Andreas Kling]

Comment on attachment 81482 [details]
possible fix

r=me

[Antti Koivisto]

http://trac.webkit.org/changeset/77834

[WebKit Review Bot]

http://trac.webkit.org/changeset/77834 might have broken GTK Linux 64-bit Debug

[Darin Adler]

On my computer at least, an assertion is firing in this new code. See bug 54344 for details.