Bug 5336

Summary: SVG with animation crashes WebKit+SVG
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: SVGAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P4    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: http://www.dotuscomus.com/kde/gearobolo_colorshift.svgz
Attachments:
Description Flags
Reduced test case.
none
More reduced test case
none
Proposed patch
eric: review-
new patch eric: review-

Description Eric Seidel (no email) 2005-10-11 02:52:32 PDT
SVG with animation crashes WebKit+SVG

I have not had a chance to either reduce, or diagnose.  It seemed to be crashing in animation code, but 
looked like it might be smashing memory.  Possibly relating to the svgz changes I have in my tree.  (w/o 
the svgz changes attached to http://bugzilla.opendarwin.org/show_bug.cgi?id=5246 it's not possible to 
view this SVG w/o first downloading and decompressing it.
Comment 1 Eric Seidel (no email) 2005-10-13 14:30:24 PDT
This seems to be smashing the stack, or somehow confusing gdb.  Viewing this svg in DrawTest, with 
libgmalloc leads to this backtrace:

#0  0x0191b87c in typeinfo for KDOM::KDOMPart ()
#1  0x0137fe90 in KSVG::TimeScheduler::slotTimerNotify (this=0xe7c64000) at /Volumes/Stuff/
Projects/WKOpen2/WebCore/../SVGSupport/ksvg2/misc/KSVGTimeScheduler.cpp:494
#2  0x013800a0 in KSVG::TimeScheduler::connectIntervalTimer (this=0xbfffdfd0, element=0x108d634) 
at /Volumes/Stuff/Projects/WKOpen2/WebCore/../SVGSupport/ksvg2/misc/KSVGTimeScheduler.cpp:
422
#3  0x0126f0d8 in KWQSlot::call (this=0xe7c64fec, job=0xbfffe078, data=0x50100646 <Address 
0x50100646 out of bounds>, size=-519811140) at /Volumes/Stuff/Projects/WKOpen2/WebCore/
kwq/KWQSlot.mm:278
#4  0x0126e984 in KWQSignal::connect (this=0x41e00000, slot=@0x0) at /Volumes/Stuff/Projects/
WKOpen2/WebCore/kwq/KWQSignal.mm:59
#5  0x0108d690 in +[KWQSingleShotTimerTarget targetWithQObject:member:] (self=0x40300000, 
_cmd=0x0, object=0x40300000, member=0x0) at /Volumes/Stuff/Projects/WKOpen2/WebCore/kwq/
KWQTimer.mm:74
#6  0x0108d6d4 in +[KWQSingleShotTimerTarget targetWithQObject:member:] (self=0xff, 
_cmd=0xbfffe220, object=0x2, member=0x4 <Address 0x4 out of bounds>) at /Volumes/Stuff/
Projects/WKOpen2/WebCore/kwq/KWQTimer.mm:75
#7  0x928dd57c in __NSFireTimer ()
#8  0x90770ae0 in __CFRunLoopDoTimer ()
#9  0x9075d458 in __CFRunLoopRun ()
#10 0x9075ca0c in CFRunLoopRunSpecific ()
#11 0x93182260 in RunCurrentEventLoopInMode ()
#12 0x9318186c in ReceiveNextEventCommon ()
#13 0x93181760 in BlockUntilNextEventMatchingListInMode ()
#14 0x93680904 in _DPSNextEvent ()
#15 0x936805c8 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#16 0x9367cb0c in -[NSApplication run] ()
#17 0x9376d618 in NSApplicationMain ()
#18 0x000eafa4 in main (argc=1, argv=0xbffff85c) at /Volumes/Stuff/Projects/WKOpen2/
WebKitTools/DrawTest/main.m:28

Which is total nonsense.
Comment 2 Eric Seidel (no email) 2005-10-13 14:47:18 PDT
Created attachment 4351 [details]
Reduced test case.

This test is more reduced than before, but still could be smaller.
Comment 3 Julien Palmas 2005-10-14 05:06:11 PDT
Created attachment 4357 [details]
More reduced test case

This very simple SVG example causes the crash.
This example would not make sense though. More information is needed to create
a proper SVG.
Comment 4 Julien Palmas 2005-10-14 05:07:43 PDT
Needs to change the keyword to HasReduction
Comment 5 Julien Palmas 2005-10-14 08:15:43 PDT
Created attachment 4359 [details]
Proposed patch

Checks isSVGElement() and prints an error message if not the case.
Comment 6 Eric Seidel (no email) 2005-10-14 08:41:38 PDT
Comment on attachment 4359 [details]
Proposed patch

This is a wonderful isolation of the problem.  We now know *what* is going
wrong, but not yet *why*.  This fix notices a symtom of the problem (that
somehow we have an animation setup pointing at a KDOM element instead of a
KSVG2 element) but does not answer the question why? and is "too late" a place
to fix.  We need to find out why this gets set up this way in the first place,
and make this check earlier.  "tit.key()" is typed SVGElementImpl, according to
the map... so the fact that something other than an SVGElementImpl got in there
is the bug, not that when pulling it out we didn't check correctly.  This is a
good start, but not the right fix yet.
Comment 7 Julien Palmas 2005-10-14 10:44:15 PDT
Created attachment 4361 [details]
new patch

a static_cast in SVGAnimationElementImpl::targetElement() was converting
KDOM::XMLElementImpl to SVGElementImpl for not yet implemented elements.
Comment 8 Eric Seidel (no email) 2005-10-14 13:11:33 PDT
Comment on attachment 4361 [details]
new patch

This shoudl use the svg_dynamic_cast function instead.