Bug 56574 - WebKit crash regression observed in Cairo commit cd1004ce1
WebKit crash regression observed in Cairo commit cd1004ce1
Status: RESOLVED FIXED
Product: cairo
Classification: Unclassified
Component: general
1.12.4
Other All
: medium critical
Assigned To: Chris Wilson
cairo-bugs mailing list
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-30 08:41 UTC by Dominik Röttsches (drott)
Modified: 2012-11-01 08:51 UTC (History)
1 user (show)

See Also:


Attachments
Trace file for borderRadiusDashed04 test case (4.89 KB, text/plain)
2012-10-30 08:50 UTC, Dominik Röttsches (drott)
Details
BorderRadiusDashed04.png (5.35 KB, image/png)
2012-10-30 08:51 UTC, Dominik Röttsches (drott)
Details
Trace file for borderRadiusDashed04 test case (new) (4.71 KB, text/plain)
2012-10-31 15:31 UTC, Dominik Röttsches (drott)
Details
Trace file for borderRadiusAllStylesAllCorners.html (141.52 KB, text/plain)
2012-10-31 15:32 UTC, Dominik Röttsches (drott)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dominik Röttsches (drott) 2012-10-30 08:41:46 UTC
Cairo commit cd1004ce1 "traps,spans-compositor: Avoid mistreating unaligned clips as aligned" breaks the following WebKit regression tests:

fast/borders/borderRadiusDashed04.html
fast/borders/borderRadiusAllStylesAllCorners.html
fast/backgrounds/background-leakage-transforms.html
fast/box-shadow/transform-fringing.html
fast/box-shadow/box-shadow-transformed.html

The regression test driver exits, looks like a crash.
Comment 1 Dominik Röttsches (drott) 2012-10-30 08:50:45 UTC
Created attachment 69294 [details]
Trace file for borderRadiusDashed04 test case

This trace file writes broderRadiusDashed04.png correctly before the offending patch. 

It causes a segmentation fault after the offending commit cd1004ce.
Comment 2 Dominik Röttsches (drott) 2012-10-30 08:51:54 UTC
Created attachment 69295 [details]
BorderRadiusDashed04.png

Correct output
Comment 3 Chris Wilson 2012-10-30 10:17:46 UTC
The trace doesn't crash here, but it does exhibit the unusual behaviour of the dash around the corners...
Comment 4 Chris Wilson 2012-10-30 11:42:42 UTC
Looks correct in epiphany (WebKit/gtk).

I'm starting to lose hope that  ./Tools/Scripts/build-webkit --efl --update-efl
will ever succeed :-p
Comment 5 Dominik Röttsches (drott) 2012-10-30 12:20:22 UTC
(In reply to comment #4)
> Looks correct in epiphany (WebKit/gtk).

Against older system cairo? Or against 1.12.4?

> I'm starting to lose hope that  ./Tools/Scripts/build-webkit --efl
> --update-efl
> will ever succeed :-p

You can come to #webkit-efl on freenode, we're there to help.
Comment 6 Chris Wilson 2012-10-30 12:23:00 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Looks correct in epiphany (WebKit/gtk).
> 
> Against older system cairo? Or against 1.12.4?

With git.
Comment 7 Dominik Röttsches (drott) 2012-10-30 12:27:10 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > Looks correct in epiphany (WebKit/gtk).
> > 
> > Against older system cairo? Or against 1.12.4?
> 
> With git.

Are you sure Epiphany picked up the right cairo library? We do see those cases fail in WebKit EFL ToT against cairo 1.12.4. Maybe a later commit fixed the crash, but the rendering is still flawed there, as you mentioned in comment 3.
Comment 8 Chris Wilson 2012-10-31 09:42:28 UTC
epiphany is picking up the right version of cairo and appears to render correctly. The trace is bizarre in that the red dashes appear to be clipped, that the line width appears to be much larger than the blue. Even more bizarre is that the blue is first drawn much wider and looks like the red then is redrawn correctly.
Comment 9 Chris Wilson 2012-10-31 12:57:24 UTC
Is there a chance you can grab a bt from the crash?
Comment 10 Dominik Röttsches (drott) 2012-10-31 12:58:22 UTC
(In reply to comment #9)
> Is there a chance you can grab a bt from the crash?

I'll see what I can do, will also check the trace again.
Comment 11 Dominik Röttsches (drott) 2012-10-31 14:59:28 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > Is there a chance you can grab a bt from the crash?

http://pastebin.com/mRSBnkDh
Seems to be infinite recursion from  _cairo_surface_stroke.
Comment 12 Dominik Röttsches (drott) 2012-10-31 15:05:23 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > Is there a chance you can grab a bt from the crash?
> 
> http://pastebin.com/mRSBnkDh
> Seems to be infinite recursion from  _cairo_surface_stroke.

This was with cairo @cd1004ce19c7ea28c7fedb64.
Comment 13 Chris Wilson 2012-10-31 15:17:26 UTC
* scratches head.

That should be impossible... The next task is working out where exactly we decide we cannot use the span compositor to draw the stroke. The clue has to be cd1004ce.
Comment 14 Dominik Röttsches (drott) 2012-10-31 15:23:37 UTC
(In reply to comment #13)
> * scratches head.
> 
> That should be impossible... The next task is working out where exactly we
> decide we cannot use the span compositor to draw the stroke. The clue has to
> be cd1004ce.

The recursion crash is gone in 1.12.4 and 1.12.6, some later commit made it go away. 

What I also tried is applying your fix from bug 56432 and try the first two test cases again. Rendering of fast/borders/borderRadiusAllStylesAllCorners.html is improved (i.e. no round joins anymore), but still too thick stroking and some double/overlay/extra stroking in incorrect places.
Comment 15 Dominik Röttsches (drott) 2012-10-31 15:31:39 UTC
Created attachment 69353 [details]
Trace file for borderRadiusDashed04 test case (new)

Is this trace file better? When I csi-exec this one, it shows exactly the same failure as in the WebKit layout test.
Comment 16 Dominik Röttsches (drott) 2012-10-31 15:32:25 UTC
Created attachment 69354 [details]
Trace file for borderRadiusAllStylesAllCorners.html
Comment 17 Chris Wilson 2012-10-31 15:33:14 UTC
(In reply to comment #14)
> (In reply to comment #13)
> > * scratches head.
> > 
> > That should be impossible... The next task is working out where exactly we
> > decide we cannot use the span compositor to draw the stroke. The clue has to
> > be cd1004ce.
> 
> The recursion crash is gone in 1.12.4 and 1.12.6, some later commit made it
> go away. 

Not intentionally. If it is easy enough, can you do a reversed bisect to find the fix? I still think that recursion should be prevented by design, so indicates a real bug somewhere that may just be masked.

> What I also tried is applying your fix from bug 56432 and try the first two
> test cases again. Rendering of
> fast/borders/borderRadiusAllStylesAllCorners.html is improved (i.e. no round
> joins anymore), but still too thick stroking and some double/overlay/extra
> stroking in incorrect places.

Right, that's the bizarreness in the rendering I see in the attached trace.
Comment 18 Dominik Röttsches (drott) 2012-10-31 16:26:43 UTC
(In reply to comment #17)

> Not intentionally. If it is easy enough, can you do a reversed bisect to
> find the fix? I still think that recursion should be prevented by design, so
> indicates a real bug somewhere that may just be masked.

Next commit, 4ea3ace6c810ba090464e, actually makes the crash go away.
Comment 19 Chris Wilson 2012-11-01 08:51:46 UTC
Ok, that took a lot of digging to spot that the clip wasn't being performed at all..

commit b6daf47fa08c74d9672040b2b98ac6dd1f841429
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Nov 1 08:39:01 2012 +0000

    spans: Do not assume that we manage to perform the clip geometrically
    
    Even for bounded masks, we may fail to perform the clipping
    geometrically for a variety of reasons, the prime one being that the
    clip has a mixture of antialias settings. So when compositing the
    polygon, we need to check whether a clip path still remains and so
    requires a clipmask.
    
    Fixes regression from
    
    commit cd1004ce19c7ea28c7fedb6464562a08416586c0
    Author: Chris Wilson <chris@chris-wilson.co.uk>
    Date:   Fri May 11 21:20:35 2012 +0100
    
        traps,spans-compositor: Avoid mistreating unaligned clips as aligned
    
    and
    
    commit 4ea3ace6c810ba090464e48795fac5456f6cdc24
    Author: Chris Wilson <chris@chris-wilson.co.uk>
    Date:   Fri May 11 21:51:44 2012 +0100
    
        spans: Only fallback for a clipmask if unbounded
    
    Reported-by: Dominik Röttsches <dominik.rottsches@intel.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=56574
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>