[gs-cvs] rev 8080 - trunk/gs/src

ray at ghostscript.com ray at ghostscript.com
Tue Jun 26 18:52:30 PDT 2007


Author: ray
Date: 2007-06-26 18:52:29 -0700 (Tue, 26 Jun 2007)
New Revision: 8080

Modified:
   trunk/gs/src/gdevwts.c
   trunk/gs/src/gswts.c
   trunk/gs/src/gswts.h
   trunk/gs/src/gxdhtserial.c
Log:
Fix an out of bounds buffer write in the wts_load_halftone logic and fine
tune the wts_resolve_one color conversion cache hashing for 4K (the default).
Bug 689255 for customer 951.

DETAILS:

Besides fixing the out of bounds logic, I added a 'bufsize' parameter to the
gs_wts_from_buf logic so that we can detect OOB conditions. Note that the
1+hdr_size+cell_size in gx_ht_read_component_wts is retained by this patch
even though it looks bogus (we want to commit the fix so we can send to a
customer, and will look at this later).

In refining the cache stats in the wtsimdi device, the 'fill_empty' case
and the 'collision' case were split to make sure that the hash is decent,
but even 1M cache hits (on ACDsee.prn) only save about 1.5 seconds (out
of 9 seconds). The hash algorithm for smaller collision rates won't really
matter much.


EXPECTED DIFFERENCES:

None. (what did you expect since we don't do regression on wtsimdi yet?)



Modified: trunk/gs/src/gdevwts.c
===================================================================
--- trunk/gs/src/gdevwts.c	2007-06-26 20:43:03 UTC (rev 8079)
+++ trunk/gs/src/gdevwts.c	2007-06-27 01:52:29 UTC (rev 8080)
@@ -97,15 +97,15 @@
 #if COLOR_CACHE_SIZE == 256
 #  define COLOR_TO_CACHE_INDEX(color) (((color + 0x000001) ^ (color >> 8) ^ (color >> 16)) & 0xff)
 #elif COLOR_CACHE_SIZE == 4096
-#  define COLOR_TO_CACHE_INDEX(color) (((color + 0x010101) ^ (color >> 12)) & 0xfff)
+#  define COLOR_TO_CACHE_INDEX(color) ((color ^ (color>>4) ^ (color>>8)) & 0xfff)
 #elif COLOR_CACHE_SIZE == 8192
-#  define COLOR_TO_CACHE_INDEX(color) (((color + 0x010101) ^ (((color << 24) | color) >> 11)) & 0x1fff)
+#  define COLOR_TO_CACHE_INDEX(color) (((color + 0x010204) ^ (((color << 24) | color) >> 11)) & 0x1fff)
 #elif COLOR_CACHE_SIZE == 16384
-#  define COLOR_TO_CACHE_INDEX(color) (((color + 0x010101) ^ (((color << 24) | color) >> 10)) & 0x3fff)
+#  define COLOR_TO_CACHE_INDEX(color) (((color + 0x010204) ^ (((color << 24) | color) >> 10)) & 0x3fff)
 #elif COLOR_CACHE_SIZE == 32768
-#  define COLOR_TO_CACHE_INDEX(color) (((color + 0x010101) ^ (((color << 24) | color) >> 9)) & 0x7fff)
+#  define COLOR_TO_CACHE_INDEX(color) (((color + 0x010204) ^ (((color << 24) | color) >> 9)) & 0x7fff)
 #elif COLOR_CACHE_SIZE == 65536
-#  define COLOR_TO_CACHE_INDEX(color) (((color + 0x010101) ^ (((color << 24) | color) >> 8)) & 0xffff)
+#  define COLOR_TO_CACHE_INDEX(color) (((color + 0x010204) ^ (((color << 24) | color) >> 8)) & 0xffff)
 #else
 #  define COLOR_TO_CACHE_INDEX(color) 0
 #endif
@@ -121,7 +121,9 @@
     imdi *mdo;
     cached_color *color_cache;
     cached_color current_color;
-    long color_cache_hit, color_cache_collision, color_is_current;
+#ifdef DEBUG
+    long color_cache_hit, color_cache_collision, cache_fill_empty, color_is_current;
+#endif
 } gx_device_wtsimdi;
 
 private const gx_device_procs wtsimdi_procs =
@@ -296,7 +298,7 @@
     }
     fread(buf, 1, size, f);
     fclose(f);
-    wts = gs_wts_from_buf(buf);
+    wts = gs_wts_from_buf(buf, size);
     gs_free(mem, buf, size, 1, "wts_load_halftone");
     wch->wts = wts;
     width_padded = wts->cell_width + 7;
@@ -375,7 +377,10 @@
 	/* allocate 1 more for sytems that return NULL if requested count is 0 */
     idev->color_cache = (cached_color *)gs_malloc(idev->memory, COLOR_CACHE_SIZE + 1,
 				sizeof(cached_color), "wtscmyk_print_page(color_cache)");
-    idev->color_cache_hit = idev->color_cache_collision = idev->color_is_current = 0;
+#ifdef DEBUG
+    idev->color_cache_hit = idev->color_cache_collision =
+	idev->color_is_current = idev->cache_fill_empty = 0;
+#endif
     for (i=0; i<COLOR_CACHE_SIZE; i++)		/* clear cache to empty */
 	idev->color_cache[i].color_index = gx_no_color_index;
     pbm_bytes = (pdev->width + 7) >> 3;
@@ -418,11 +423,15 @@
 		fwrite(dst + i * pbm_bytes, 1, pbm_bytes, ostream[i]);
     }
 out:
+#ifdef DEBUG
     for (i=0,unused_color_cache_slots=0; i<COLOR_CACHE_SIZE; i++)
 	if (idev->color_cache[i].color_index == gx_no_color_index)
 	    unused_color_cache_slots++;
-    dprintf4("wtscmyk_print_page: color cache stats: current=%ld, hits=%ld, collisions=%ld, unused_slots=%d\n",
-	idev->color_is_current, idev->color_cache_hit, idev->color_cache_collision, unused_color_cache_slots);
+    if_debug5(':',"wtscmyk_print_page color cache stats: current=%ld, hits=%ld,"
+	" collisions=%ld, fill=%ld, unused_slots=%d\n",
+	idev->color_is_current, idev->color_cache_hit, idev->color_cache_collision,
+	idev->cache_fill_empty, unused_color_cache_slots);
+#endif
     /* Clean up... */
     gs_free(pdev->memory, cmyk_line, cmyk_bytes, 1, "wtscmyk_print_page(in)");
     gs_free(pdev->memory, idev->color_cache, COLOR_CACHE_SIZE + 1, sizeof(cached_color),
@@ -558,7 +567,9 @@
 
 	if (idev->color_cache[hash].color_index == color) {
 	    /* cache hit */
+#ifdef DEBUG
 	    idev->color_cache_hit++;
+#endif
 	    idev->current_color = idev->color_cache[hash];
 	} else {
 	    /* cache collision or empty, fill it */
@@ -569,7 +580,12 @@
 	    double rgb[3];
 	    double cmyk[4];
 
-	    idev->color_cache_collision++;
+#ifdef DEBUG
+	    if (idev->color_cache[hash].color_index == gx_no_color_index)
+		idev->cache_fill_empty++;
+	    else
+		idev->color_cache_collision++;
+#endif
 	    rgb[0] = r / 255.0;
 	    rgb[1] = g / 255.0;
 	    rgb[2] = b / 255.0;
@@ -583,8 +599,11 @@
 	    idev->current_color.cmyk[3] = cmyk[3] * 255 + 0.5;
 	    idev->color_cache[hash] = idev->current_color;
 	}
-    } else
+    }
+#ifdef DEBUG
+    else
 	idev->color_is_current++;
+#endif
     return 0;
 }
 
@@ -1074,7 +1093,10 @@
 	/* allocate 1 more for sytems that return NULL if requested count is 0 */
     idev->color_cache = (cached_color *)gs_malloc(idev->memory, COLOR_CACHE_SIZE + 1,
 				sizeof(cached_color), "wtscmyk_print_page(color_cache)");
-    idev->color_cache_hit = idev->color_cache_collision = idev->color_is_current = 0;
+#ifdef DEBUG
+    idev->color_cache_hit = idev->color_cache_collision =
+	idev->color_is_current = idev->cache_fill_empty = 0;
+#endif
     for (i=0; i<COLOR_CACHE_SIZE; i++)		/* clear cache to empty */
 	idev->color_cache[i].color_index = gx_no_color_index;
 
@@ -1099,11 +1121,15 @@
 #endif
     }
 cleanup:
+#ifdef DEBUG
     for (i=0,unused_color_cache_slots=0; i<COLOR_CACHE_SIZE; i++)
 	if (idev->color_cache[i].color_index == gx_no_color_index)
 	    unused_color_cache_slots++;
-    dprintf4("wtsimdi_print_page: color cache stats: current=%ld, hits=%ld, collisions=%ld, unused_slots=%d\n",
-	idev->color_is_current, idev->color_cache_hit, idev->color_cache_collision, unused_color_cache_slots);
+    if_debug5(':',"wtsimdi_print_page color cache stats: current=%ld, hits=%ld,"
+	" collisions=%ld, fill=%ld, unused_slots=%d\n",
+	idev->color_is_current, idev->color_cache_hit, idev->color_cache_collision,
+	idev->cache_fill_empty, unused_color_cache_slots);
+#endif
     gs_free(pdev->memory, idev->color_cache, COLOR_CACHE_SIZE, sizeof(cached_color),
 	    "wtscmyk_print_page(color_cache)");
     if (halftoned_buffer != NULL)

Modified: trunk/gs/src/gswts.c
===================================================================
--- trunk/gs/src/gswts.c	2007-06-26 20:43:03 UTC (rev 8079)
+++ trunk/gs/src/gswts.c	2007-06-27 01:52:29 UTC (rev 8080)
@@ -1440,21 +1440,32 @@
 }
 
 wts_screen_t *
-gs_wts_from_buf(const byte *buf)
+gs_wts_from_buf(const byte *buf, int bufsize)
 {
     const wts_screen_t *ws = (const wts_screen_t *)buf;
     wts_screen_t *result;
     int size = wts_size(ws);
+    int hdr_size;
     int cell_size; /* size of cell in bytes */
 
     result = (wts_screen_t *)malloc(size);
     if (result == NULL)
 	return NULL;
-    memcpy(result, ws, size);
+    
+#ifdef WTS_SCREEN_J_SIZE_NOCACHE	/* ??? isn't this always defined ??? */
+    if (ws->type == WTS_SCREEN_J) {
+	hdr_size = WTS_SCREEN_J_SIZE_NOCACHE;
+	memcpy(result, ws, hdr_size);
+    } else
+#endif
+    {
+	hdr_size = sizeof(wts_screen_t);
+        memcpy(result, ws, hdr_size);
+    }
     cell_size = ws->cell_width * ws->cell_height * sizeof(wts_screen_sample_t);
 
-    result->samples = (wts_screen_sample_t *)malloc(cell_size);
-    if (result->samples == NULL) {
+    if (bufsize < (cell_size + hdr_size) ||
+	(result->samples = (wts_screen_sample_t *)malloc(cell_size)) == NULL) {
 	free(result);
 	return NULL;
     }
@@ -1467,10 +1478,9 @@
 	    wsj->xcache[i].tag = -1;
 	for (i = 0; i < WTS_CACHE_SIZE_Y; i++)
 	    wsj->ycache[i].tag = -1;
-	size = WTS_SCREEN_J_SIZE_NOCACHE;
     }
 #endif
-    memcpy(result->samples, buf + size, cell_size);
+    memcpy(result->samples, buf + hdr_size, cell_size);
 
     return result;
 }

Modified: trunk/gs/src/gswts.h
===================================================================
--- trunk/gs/src/gswts.h	2007-06-26 20:43:03 UTC (rev 8079)
+++ trunk/gs/src/gswts.h	2007-06-27 01:52:29 UTC (rev 8080)
@@ -60,6 +60,6 @@
 wts_size(const wts_screen_t *ws);
 
 wts_screen_t *
-gs_wts_from_buf(const byte *buf);
+gs_wts_from_buf(const byte *buf, int bufsize);
 
 #endif

Modified: trunk/gs/src/gxdhtserial.c
===================================================================
--- trunk/gs/src/gxdhtserial.c	2007-06-26 20:43:03 UTC (rev 8079)
+++ trunk/gs/src/gxdhtserial.c	2007-06-27 01:52:29 UTC (rev 8080)
@@ -302,15 +302,18 @@
     int hdr_size = wts_size(ws);
     int cell_size = ws->cell_width * ws->cell_height *
 	sizeof(wts_screen_sample_t);
+    int bufsize = 1+hdr_size+cell_size;
 
     memset(&pcomp->corder, 0, sizeof(pcomp->corder));
 
-    pcomp->corder.wts = gs_wts_from_buf(data);
+    if (size < bufsize)
+	return -1;
+    pcomp->corder.wts = gs_wts_from_buf(data, bufsize);
     pcomp->cname = 0;
     if (pcomp->corder.wts == NULL)
 	return -1;
 
-    return 1 + hdr_size + cell_size;
+    return bufsize;
 }
 
 /*



More information about the gs-cvs mailing list