Bug 59098

Created attachment 72610 [details] [review]
check NULL pointer

I followed the while loop for "left" to add a NULL pointer check, and it fixed the crash.

Already fixed upstream.

(In reply to comment #2)
> Already fixed upstream.

Hmm, I ran into this bug using cairo 1.12.14. So this is not fixed yet, or am I missing something?

It was fixed... Can you please describe your test case?

(In reply to comment #4)
> It was fixed... Can you please describe your test case?

Same thing as in the original description, I am rendering a PDF and this comes from cairo_stroke. A null check is missing and there is an access violation in cairo-polygon-intersect.c, active_edges:

if unlikely ((right->deferred.other))

and right is null. Applying the attached patch fixes my case.

Since I can no longer reproduce this just by opening evince and watching it crash immediately, suggests that the symptoms are different and I need more information on how to reproduce it.

(Note the NULL check isn't missing - the problem is garbage in. If you simply add the NULL check, you get garbage out...)

Ok, I'll try to work out a way to easily reproduce this.

Created attachment 75575 [details]
CairoScript hopefully causing access violation

I extracted the problematic drawing from my program and recorded it using a cairo script surface. See the attached .cs. I have nothing to replay the script with here, so I can only hope it reproduces the crash :/. But if I just change that script surface to an image surface, the crash is reality.

(In reply to comment #8)
> I extracted the problematic drawing from my program and recorded it using a
> cairo script surface. See the attached .cs. I have nothing to replay the
> script with here, so I can only hope it reproduces the crash :/. But if I

Now i managed to run the script with cairo_script_interpreter and it does crash beautifully at cairo-polygon-intersect.c: active_edges(). So that's the way to reproduce.

Passes the usual smell tests, so it looks like a novel bug. Oh fun.

The input geometry after stroking looks quite odd. I think the next step is to reduce the script to the minimum path required to trigger the bug. Olli, do you have the time to tackle that?

Yep I'll try to minimize the path sooner or later.

Created attachment 77151 [details]
Minimized CairoScript still reproducing the bug

There's actually a second place in active_edges where a null check on 'right' is needed. It's the initial assignment of 'right' before the do/while loop. My test document required both NULL checks to render completely.

Guys...

Index: cairo-1.14.2/src/cairo-polygon-intersect.c
===================================================================
--- cairo-1.14.2.orig/src/cairo-polygon-intersect.c
+++ cairo-1.14.2/src/cairo-polygon-intersect.c
@@ -1236,11 +1236,10 @@ active_edges (cairo_bo_edge_t           *left,
                    edges_end (right, top, polygon);
 
                winding[right->a_or_b] += right->edge.dir;
-               if (is_zero (winding)) {
-                   if (right->next == NULL ||
-                       ! edges_colinear (right, right->next))
+               if (!right->next)
+                       break;
+               if (is_zero (winding) && ! edges_colinear (right, right->next))
                        break;
-               }
 
                right = right->next;
            } while (1);

(In reply to olaf from comment #14)
> +               if (!right->next)
> +                       break;


Should be return, either way, still broken since two years or more.

Created attachment 119444 [details] [review]
0001-polygon-intersection-fix-segfault-in-active_edges.patch

See bug 74779. This is a duplicate? It is mentioned there the two scripts attached to this bug are fixed (bug 74779 comment 9).

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/cairo/cairo/issues/15.