Bug 146690 – Crash when appending an SVG <use> element dynamically which has animated SVG <path> element

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Said Abou-Hallawa]

Created attachment 256314 [details]
crash test case

1. Open the attached test case.
2. Click the "Add" button"

Result: WebKit crashes with the follow call stack:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x0000000114523aa7 WTFCrash + 39
1   com.apple.WebCore             	0x0000000117b56ad6 WebCore::SVGAnimatedPathSegListPropertyTearOff::animValDidChange() + 70 (SVGAnimatedPathSegListPropertyTearOff.h:90)
2   com.apple.WebCore             	0x0000000117b568d7 void WebCore::SVGAnimatedTypeAnimator::executeAction<WebCore::SVGAnimatedPathSegListPropertyTearOff>(WebCore::SVGAnimatedTypeAnimator::AnimationAction, WTF::Vector<WebCore::SVGElementAnimatedProperties, 0ul, WTF::CrashOnOverflow, 16ul> const&, unsigned int, WebCore::SVGAnimatedPathSegListPropertyTearOff::ContentType*) + 599 (SVGAnimatedTypeAnimator.h:194)
3   com.apple.WebCore             	0x0000000117b54117 void WebCore::SVGAnimatedTypeAnimator::animValDidChangeForType<WebCore::SVGAnimatedPathSegListPropertyTearOff>(WTF::Vector<WebCore::SVGElementAnimatedProperties, 0ul, WTF::CrashOnOverflow, 16ul> const&) + 135 (SVGAnimatedTypeAnimator.h:101)
4   com.apple.WebCore             	0x0000000117b52cd8 WebCore::SVGAnimatedPathAnimator::animValDidChange(WTF::Vector<WebCore::SVGElementAnimatedProperties, 0ul, WTF::CrashOnOverflow, 16ul> const&) + 40 (SVGAnimatedPath.cpp:86)
5   com.apple.WebCore             	0x0000000117b6afbc WebCore::SVGAnimateElementBase::resetAnimatedType() + 1516 (SVGAnimateElementBase.cpp:217)
6   com.apple.WebCore             	0x0000000117c528cf WebCore::SVGSMILElement::progress(WebCore::SMILTime, WebCore::SVGSMILElement*, bool) + 1135 (SVGSMILElement.cpp:1100)
7   com.apple.WebCore             	0x00000001179e7b5b WebCore::SMILTimeContainer::updateAnimations(WebCore::SMILTime, bool) + 779 (SMILTimeContainer.cpp:288)
8   com.apple.WebCore             	0x00000001179e6b9b WebCore::SMILTimeContainer::timerFired() + 187 (SMILTimeContainer.cpp:218)
9   com.apple.WebCore             	0x00000001179eac08 void std::__1::__invoke_void_return_wrapper<void>::__call<std::__1::__bind<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*>&>(std::__1::__bind<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*>&&&) + 248 (__functional_base:441)
10  com.apple.WebCore             	0x00000001179eaadc std::__1::__function::__func<std::__1::__bind<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*>, std::__1::allocator<std::__1::__bind<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*> >, void ()>::operator()() + 44 (functional:1407)
11  com.apple.WebCore             	0x0000000115ce0ada std::__1::function<void ()>::operator()() const + 26 (functional:1793)
12  com.apple.WebCore             	0x0000000115ce09fc WebCore::Timer::fired() + 28 (Timer.h:134)
13  com.apple.WebCore             	0x0000000117d237ea WebCore::ThreadTimers::sharedTimerFiredInternal() + 394 (ThreadTimers.cpp:135)
14  com.apple.WebCore             	0x0000000117d234a9 WebCore::ThreadTimers::sharedTimerFired() + 25 (ThreadTimers.cpp:108)
15  com.apple.WebCore             	0x00000001179d2172 WebCore::timerFired(__CFRunLoopTimer*, void*) + 34 (SharedTimerCF.cpp:82)
16  com.apple.CoreFoundation      	0x00007fff8737e7c4 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
17  com.apple.CoreFoundation      	0x00007fff8737e453 __CFRunLoopDoTimer + 1075
18  com.apple.CoreFoundation      	0x00007fff873f9d9a __CFRunLoopDoTimers + 298
19  com.apple.CoreFoundation      	0x00007fff87339a71 __CFRunLoopRun + 1841
20  com.apple.CoreFoundation      	0x00007fff873390d8 CFRunLoopRunSpecific + 296
21  com.apple.HIToolbox           	0x00007fff8bb60ce9 RunCurrentEventLoopInMode + 235
22  com.apple.HIToolbox           	0x00007fff8bb60a7f ReceiveNextEventCommon + 432
23  com.apple.HIToolbox           	0x00007fff8bb608bf _BlockUntilNextEventMatchingListInModeWithFilter + 71
24  com.apple.AppKit              	0x00007fff90c66732 _DPSNextEvent + 927
25  com.apple.AppKit              	0x00007fff91033f74 -[NSApplication _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 324
26  com.apple.AppKit              	0x00007fff90c5c6c2 -[NSApplication run] + 682
27  com.apple.AppKit              	0x00007fff90bdec4f NSApplicationMain + 1176
28  libxpc.dylib                  	0x00007fff872b619c _xpc_objc_main + 793
29  libxpc.dylib                  	0x00007fff872b78eb xpc_main + 494
30  com.apple.WebKit.WebContent.Development	0x000000010a224197 main + 39
31  libdyld.dylib                 	0x00007fff907835ad start + 1

[Said Abou-Hallawa]

<rdar://problem/20790376>

[Said Abou-Hallawa]

I was wring about the above call stack. It is just an assertion call stack. But if we comment all the assertion in SVGAnimatedListPropertyTearOff::animValDidChange() and VGAnimatedPathSegListPropertyTearOff::animValDidChange() we get the following crashing call stack which we should hit in a release build:

#0	0x00000001072a2360 in WebCore::SVGListProperty<WebCore::SVGPathSegList>::values() at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/properties/SVGListProperty.h:434
#1	0x00000001072a43bd in WebCore::SVGAnimatedListPropertyTearOff<WebCore::SVGPathSegList>::synchronizeWrappersIfNeeded() at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h:134
#2	0x00000001072a4715 in WebCore::SVGAnimatedListPropertyTearOff<WebCore::SVGPathSegList>::animValDidChange() at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h:153
#3	0x00000001072a40c6 in WebCore::SVGAnimatedPathSegListPropertyTearOff::animValDidChange() at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/properties/SVGAnimatedPathSegListPropertyTearOff.h:101
#4	0x00000001072a3e87 in void WebCore::SVGAnimatedTypeAnimator::executeAction<WebCore::SVGAnimatedPathSegListPropertyTearOff>(WebCore::SVGAnimatedTypeAnimator::AnimationAction, WTF::Vector<WebCore::SVGElementAnimatedProperties, 0ul, WTF::CrashOnOverflow, 16ul> const&, unsigned int, WebCore::SVGAnimatedPathSegListPropertyTearOff::ContentType*) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/SVGAnimatedTypeAnimator.h:214
#5	0x00000001072a16c7 in void WebCore::SVGAnimatedTypeAnimator::animValDidChangeForType<WebCore::SVGAnimatedPathSegListPropertyTearOff>(WTF::Vector<WebCore::SVGElementAnimatedProperties, 0ul, WTF::CrashOnOverflow, 16ul> const&) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/SVGAnimatedTypeAnimator.h:100
#6	0x00000001072a0288 in WebCore::SVGAnimatedPathAnimator::animValDidChange(WTF::Vector<WebCore::SVGElementAnimatedProperties, 0ul, WTF::CrashOnOverflow, 16ul> const&) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/SVGAnimatedPath.cpp:85
#7	0x00000001072b828c in WebCore::SVGAnimateElementBase::resetAnimatedType() at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/SVGAnimateElementBase.cpp:215
#8	0x000000010739fb9f in WebCore::SVGSMILElement::progress(WebCore::SMILTime, WebCore::SVGSMILElement*, bool) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/animation/SVGSMILElement.cpp:1098
#9	0x00000001071352cb in WebCore::SMILTimeContainer::updateAnimations(WebCore::SMILTime, bool) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/animation/SMILTimeContainer.cpp:288
#10	0x000000010713430b in WebCore::SMILTimeContainer::timerFired() at /Volumes/Data/WebKit/OpenSource/Source/WebCore/svg/animation/SMILTimeContainer.cpp:217
#11	0x0000000107138378 in decltype(*(std::__1::forward<WebCore::SMILTimeContainer*&>(fp0)).*fp(std::__1::forward<>(fp1))) std::__1::__invoke<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*&, void>(void (WebCore::SMILTimeContainer::*&&&)(), WebCore::SMILTimeContainer*&&&) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.11.xctoolchain/usr/bin/../include/c++/v1/__functional_base:382
#12	0x00000001071382f2 in std::__1::__bind_return<void (WebCore::SMILTimeContainer::*)(), std::__1::tuple<WebCore::SMILTimeContainer*>, std::__1::tuple<>, _is_valid_bind_return<void (WebCore::SMILTimeContainer::*)(), std::__1::tuple<WebCore::SMILTimeContainer*>, std::__1::tuple<> >::value>::type std::__1::__apply_functor<void (WebCore::SMILTimeContainer::*)(), std::__1::tuple<WebCore::SMILTimeContainer*>, 0ul, std::__1::tuple<> >(void (WebCore::SMILTimeContainer::*&)(), std::__1::tuple<WebCore::SMILTimeContainer*>&, std::__1::__tuple_indices<0ul>, std::__1::tuple<>&&) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.11.xctoolchain/usr/bin/../include/c++/v1/functional:2060
#13	0x00000001071382ca in std::__1::__bind_return<void (WebCore::SMILTimeContainer::*)(), std::__1::tuple<WebCore::SMILTimeContainer*>, std::__1::tuple<>, _is_valid_bind_return<void (WebCore::SMILTimeContainer::*)(), std::__1::tuple<WebCore::SMILTimeContainer*>, std::__1::tuple<> >::value>::type std::__1::__bind<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*>::operator()<>() [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.11.xctoolchain/usr/bin/../include/c++/v1/functional:2123
#14	0x00000001071382ab in decltype(std::__1::forward<std::__1::__bind<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*>&>(fp)(std::__1::forward<>(fp0))) std::__1::__invoke<std::__1::__bind<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*>&>(std::__1::__bind<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*>&&&) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.11.xctoolchain/usr/bin/../include/c++/v1/__functional_base:415
#15	0x00000001071382a0 in void std::__1::__invoke_void_return_wrapper<void>::__call<std::__1::__bind<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*>&>(std::__1::__bind<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*>&&&) at /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.11.xctoolchain/usr/bin/../include/c++/v1/__functional_base:440
#16	0x000000010713824c in std::__1::__function::__func<std::__1::__bind<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*>, std::__1::allocator<std::__1::__bind<void (WebCore::SMILTimeContainer::*&)(), WebCore::SMILTimeContainer*> >, void ()>::operator()() at /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.11.xctoolchain/usr/bin/../include/c++/v1/functional:1407
#17	0x000000010542e24a in std::__1::function<void ()>::operator()() const at /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.11.xctoolchain/usr/bin/../include/c++/v1/functional:1793
#18	0x000000010542e16c in WebCore::Timer::fired() at /Volumes/Data/WebKit/OpenSource/Source/WebCore/platform/Timer.h:133
#19	0x0000000107470aba in WebCore::ThreadTimers::sharedTimerFiredInternal() at /Volumes/Data/WebKit/OpenSource/Source/WebCore/platform/ThreadTimers.cpp:132
#20	0x0000000107470779 in WebCore::ThreadTimers::sharedTimerFired() at /Volumes/Data/WebKit/OpenSource/Source/WebCore/platform/ThreadTimers.cpp:107
#21	0x000000010711f8e2 in WebCore::timerFired(__CFRunLoopTimer*, void*) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/platform/cf/SharedTimerCF.cpp:82

[Said Abou-Hallawa]

Created attachment 256343 [details]
Patch

[Said Abou-Hallawa]

Created attachment 256380 [details]
Patch

[Said Abou-Hallawa]

Created attachment 256381 [details]
Another test case

Other repro steps:

1. Open "Another test case".

Result: WebKit crashes.

[Said Abou-Hallawa]

Created attachment 256385 [details]
Patch

[Dean Jackson]

Comment on attachment 256385 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=256385&action=review

> Source/WebCore/ChangeLog:30
> +        The fix is to make SVGAnimatedPathAnimator::resetAnimValToBaseVal() ensures

Typo: ensure

[Dean Jackson]

Comment on attachment 256385 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=256385&action=review

> LayoutTests/svg/animations/insert-animate-use-path-while-animation.svg:17
> +    // wait until the only instance of 'animatedRect' starts animation

Minor nit: Start with an uppercase letter.

> LayoutTests/svg/animations/insert-animate-use-path-while-animation.svg:30
> +        // wait until the next animation cycle starts to make sure

Minor nit: Start with an uppercase letter.

[Said Abou-Hallawa]

Created attachment 256416 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 256416 [details]
Patch

Clearing flags on attachment: 256416

Committed r186541: <http://trac.webkit.org/changeset/186541>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.