[Pixman] bug: pixman_region32_t overflow checks

[Benjamin Otte]

Hey,

I got notified about repaint regressions in GTK3 with large icon views
today. After investigating, I found that pixman_region32_translate()
does overflow checks against SHRT_MIN/MAX instead of INT_MIN/MAX. I did
a quick patch (attached), but I'm not sure it's the most beautiful way
to fix it.

Also, if Søren intends to do another 0.18 release, this sounds like a
good candidate.

Benjamin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test25.c
Type: text/x-csrc
Size: 679 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20100823/fa85f035/attachment.c>
-------------- next part --------------
commit da67d5dfbc8630fa58bd7709ac49c67d74d4d079
Author: Benjamin Otte <otte at redhat.com>
Date:   Mon Aug 23 18:20:09 2010 +0200

    region: Don't restrict pixman_region32_t to SHRT_MIN/MAX
    
    pixman_region32_translate() was testing overflow against SHRT_MIN and
    SHRT_MAX. This patch internally introduces PIXMAN_REGION_MIN and
    PIXMAN_REGION_MAX that get defined to INT_MIN/MAX for the 32bit region.
    Also, the checks are done using int64_t to catch overflows with 32bit
    code.

diff --git a/pixman/pixman-region.c b/pixman/pixman-region.c
index a6a4005..f819f1b 100644
--- a/pixman/pixman-region.c
+++ b/pixman/pixman-region.c
@@ -2194,7 +2194,7 @@ PIXMAN_EXPORT PREFIX (_contains_rectangle) (region_type_t *  region,
 PIXMAN_EXPORT void
 PREFIX (_translate) (region_type_t *region, int x, int y)
 {
-    int x1, x2, y1, y2;
+    int64_t x1, x2, y1, y2;
     int nbox;
     box_type_t * pbox;
 
@@ -2204,7 +2204,7 @@ PREFIX (_translate) (region_type_t *region, int x, int y)
     region->extents.x2 = x2 = region->extents.x2 + x;
     region->extents.y2 = y2 = region->extents.y2 + y;
     
-    if (((x1 - SHRT_MIN) | (y1 - SHRT_MIN) | (SHRT_MAX - x2) | (SHRT_MAX - y2)) >= 0)
+    if (((x1 - PIXMAN_REGION_MIN) | (y1 - PIXMAN_REGION_MIN) | (PIXMAN_REGION_MAX - x2) | (PIXMAN_REGION_MAX - y2)) >= 0)
     {
         if (region->data && (nbox = region->data->numRects))
         {
@@ -2219,7 +2219,7 @@ PREFIX (_translate) (region_type_t *region, int x, int y)
         return;
     }
 
-    if (((x2 - SHRT_MIN) | (y2 - SHRT_MIN) | (SHRT_MAX - x1) | (SHRT_MAX - y1)) <= 0)
+    if (((x2 - PIXMAN_REGION_MIN) | (y2 - PIXMAN_REGION_MIN) | (PIXMAN_REGION_MAX - x1) | (PIXMAN_REGION_MAX - y1)) <= 0)
     {
         region->extents.x2 = region->extents.x1;
         region->extents.y2 = region->extents.y1;
@@ -2228,15 +2228,15 @@ PREFIX (_translate) (region_type_t *region, int x, int y)
         return;
     }
 
-    if (x1 < SHRT_MIN)
-	region->extents.x1 = SHRT_MIN;
-    else if (x2 > SHRT_MAX)
-	region->extents.x2 = SHRT_MAX;
+    if (x1 < PIXMAN_REGION_MIN)
+	region->extents.x1 = PIXMAN_REGION_MIN;
+    else if (x2 > PIXMAN_REGION_MAX)
+	region->extents.x2 = PIXMAN_REGION_MAX;
 
-    if (y1 < SHRT_MIN)
-	region->extents.y1 = SHRT_MIN;
-    else if (y2 > SHRT_MAX)
-	region->extents.y2 = SHRT_MAX;
+    if (y1 < PIXMAN_REGION_MIN)
+	region->extents.y1 = PIXMAN_REGION_MIN;
+    else if (y2 > PIXMAN_REGION_MAX)
+	region->extents.y2 = PIXMAN_REGION_MAX;
 
     if (region->data && (nbox = region->data->numRects))
     {
@@ -2249,22 +2249,22 @@ PREFIX (_translate) (region_type_t *region, int x, int y)
             pbox_out->x2 = x2 = pbox->x2 + x;
             pbox_out->y2 = y2 = pbox->y2 + y;
 
-            if (((x2 - SHRT_MIN) | (y2 - SHRT_MIN) |
-                 (SHRT_MAX - x1) | (SHRT_MAX - y1)) <= 0)
+            if (((x2 - PIXMAN_REGION_MIN) | (y2 - PIXMAN_REGION_MIN) |
+                 (PIXMAN_REGION_MAX - x1) | (PIXMAN_REGION_MAX - y1)) <= 0)
             {
                 region->data->numRects--;
                 continue;
 	    }
 
-            if (x1 < SHRT_MIN)
-		pbox_out->x1 = SHRT_MIN;
-            else if (x2 > SHRT_MAX)
-		pbox_out->x2 = SHRT_MAX;
+            if (x1 < PIXMAN_REGION_MIN)
+		pbox_out->x1 = PIXMAN_REGION_MIN;
+            else if (x2 > PIXMAN_REGION_MAX)
+		pbox_out->x2 = PIXMAN_REGION_MAX;
 
-            if (y1 < SHRT_MIN)
-		pbox_out->y1 = SHRT_MIN;
-            else if (y2 > SHRT_MAX)
-		pbox_out->y2 = SHRT_MAX;
+            if (y1 < PIXMAN_REGION_MIN)
+		pbox_out->y1 = PIXMAN_REGION_MIN;
+            else if (y2 > PIXMAN_REGION_MAX)
+		pbox_out->y2 = PIXMAN_REGION_MAX;
 
             pbox_out++;
 	}
diff --git a/pixman/pixman-region16.c b/pixman/pixman-region16.c
index 46f5e26..9f17d90 100644
--- a/pixman/pixman-region16.c
+++ b/pixman/pixman-region16.c
@@ -42,6 +42,9 @@ typedef struct {
 
 #define PREFIX(x) pixman_region##x
 
+#define PIXMAN_REGION_MAX SHRT_MAX
+#define PIXMAN_REGION_MIN SHRT_MIN
+
 #include "pixman-region.c"
 
 /* This function exists only to make it possible to preserve the X ABI -
diff --git a/pixman/pixman-region32.c b/pixman/pixman-region32.c
index aeee86c..194b19c 100644
--- a/pixman/pixman-region32.c
+++ b/pixman/pixman-region32.c
@@ -40,4 +40,7 @@ typedef struct {
 
 #define PREFIX(x) pixman_region32##x
 
+#define PIXMAN_REGION_MAX INT_MAX
+#define PIXMAN_REGION_MIN INT_MIN
+
 #include "pixman-region.c"