[gs-bugs] [Bug 691328] non deterministic behavior in comparefiles/Bug688807

bugzilla-daemon at ghostscript.com bugzilla-daemon at ghostscript.com
Sun Jul 25 06:57:58 UTC 2010


http://bugs.ghostscript.com/show_bug.cgi?id=691328

Ray Johnston <ray.johnston at artifex.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
                 CC|                            |chris.liddell at artifex.com
         Resolution|                            |FIXED
         AssignedTo|chris.liddell at artifex.com   |ray.johnston at artifex.com

--- Comment #4 from Ray Johnston <ray.johnston at artifex.com> 2010-07-25 06:57:54 UTC ---
I had a look at this. Since this is in the clist code, I'll take it.

I agree with SaGS that the 11362 patch is not the correct fix, but I also would
like Chris and SaGS to review my analysis...

I think the problem is that when we are calling bytes_copy_rectangle with
short_raster that is 'full_raster', but the 'short_size' is not large
enough for the (height * full_raster) number of bytes.

This is because the 'short_size' calculation from the clist_bitmap_bytes call
that sets 'short_raster' may be expecting that the last line will be truncated:

cmd_put_bits calls:
    uint short_size = 
    clist_bitmap_bytes(width_bits, height,
               compression_mask & ~cmd_mask_compress_any,
               &short_raster, &full_raster);
-----
clist_bitmap_bytes sets:
    uint full_raster = *raster = bitmap_raster(width_bits);
    uint short_raster = (width_bits + 7) >> 3;
-----
the clist_bitmap_bytes _may_ take the path:
   else
    *width_bytes = full_raster, width_bytes_last = short_raster;
    return
    (height == 0 ? 0 : *width_bytes * (height - 1) + width_bytes_last);
-----

This will set short_raster to full_raster, but the short_size will expect
the smaller width_bytes_last to truncate the last line's padding.

When cmd_put_bits calls:

    bytes_copy_rectangle(dp + op_size, short_raster, data, raster,
             short_raster, height);

This will write short_raster == full_raster for the last line instead of
what the clist_bitmap_bytes calculated as 'width_bytes_last'.
----------

For a maximum savings for 3 bytes in an uncommon case, this seems rather
pointless, but this could be fixed in cmd_put_bits (but not in the
bytes_copy_rectangle as SaGS points out, and as r11532 attempts).
----------

Since all of the valgrind issues were from 'cmd_put_bits' this is the
first place to fix, but I will look at all of the other clients of
bytes_copy_rectangle to see if any of them have problems and come up
with a patch that fixes these (and reverts r11352).

-- 
Configure bugmail: http://bugs.ghostscript.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.


More information about the gs-bugs mailing list