Bug 53900

Summary: REGRESSION(r77740): CSSStyleSelector accessing deleted memory for svg/dom/use-transform.svg
Product: WebKit Reporter: Andreas Kling <kling>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, eric, hyatt, koivisto, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
possible fix kling: review+

Andreas Kling
Reported 2011-02-06 19:40:56 PST
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)
Attachments
possible fix (5.10 KB, patch)
2011-02-07 09:30 PST, Antti Koivisto 		kling: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Antti Koivisto
Comment 1 2011-02-07 05:53:21 PST
Silly svg code manages to do a styleselector update in the middle of a style recalc. That's totally wrong.
Simon Fraser (smfr)
Comment 2 2011-02-07 08:35:04 PST
We should make that assert as well as fixing the crash.
Antti Koivisto
Comment 3 2011-02-07 09:30:53 PST
Created attachment 81482 [details] possible fix Kling, could you try if this silences valgrind?
Andreas Kling
Comment 4 2011-02-07 11:48:51 PST
(In reply to comment #3) > Kling, could you try if this silences valgrind? It does indeed!
Andreas Kling
Comment 5 2011-02-07 12:02:11 PST
Comment on attachment 81482 [details] possible fix r=me
Antti Koivisto
Comment 6 2011-02-07 12:09:12 PST
http://trac.webkit.org/changeset/77834
WebKit Review Bot
Comment 7 2011-02-07 13:09:29 PST
http://trac.webkit.org/changeset/77834 might have broken GTK Linux 64-bit Debug
Darin Adler
Comment 8 2011-02-12 15:04:23 PST
On my computer at least, an assertion is firing in this new code. See bug 54344 for details.
Note You need to log in before you can comment on or make changes to this bug.