Bug 691665

Summary: Indeterminism in PCL code (shown by valgrind)
Product: GhostPCL Reporter: Robin Watts <robin.watts>
Component: PCL interpreterAssignee: Robin Watts <robin.watts>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P4    
Version: unspecified   
Hardware: PC   
OS: Linux   
Customer: Word Size: ---
Attachments: patch.txt

Description Robin Watts 2010-10-05 16:40:44 UTC 
While investigating Bug 691635, I've found that the following command:

./main/obj/pcl6 -sOutputFile=test.pcl -sDEVICE=ppmraw -dNOPAUSE -dBATCH
./mlc0495.pcl

shows a read of an uninitialised byte with valgrind.

This seems unlikely to be the cause of the SEGV, but it seems worthwhile dumping the results of my investigations here anyway as I think it may be responsible for some indeterminisms.

The uninitialised bytes that are read are those in the lookup table of an indexed gs colorspace. In the given file there seem to be 2 cases at fault, with similar root causes.

Firstly, when a colorspace is created with just 2 palette entries, only the first 6 values are initialised. This means 'hival' should be set to 1. As it is, it is left set to 255, meaning that later attempts to index higher colors will return uninitialised results. (The question of why such attempts might be made is another issue I leave to people with a clue about PCL!)

Secondly, when a colorspace is 'unshared', its space is recreated and data copied across. Currently hival is not copied and remains set to 255. This means that in this file, when a space with 8 colours in (hence having a hival of 7) is unshared, the newly created copy returned has a hival of 255, again allowing future reads to access the uninitialised data.
Comment 1 Robin Watts 2010-10-05 16:42:55 UTC 
Created attachment 6781 [details]
patch.txt

Simple patch that seems to solve the valgrind problems locally.

Requires review from someone with a better understanding of the code in question as I may be patching in the wrong place.
Comment 2 Robin Watts 2010-10-05 16:53:04 UTC 
> While investigating Bug 691635, ...

Bug 691638, sorry!
Comment 3 Robin Watts 2010-10-05 17:30:41 UTC 
Oddly, this does appear to solve (at least partially) the SEGV in 691638. I fear this may just be an indication that the underlying bug for 691638 is a random memory read and changing the source code a bit permutes memory enough for it to avoid crashing.
Comment 4 Robin Watts 2010-10-05 21:00:17 UTC 
A bmpcmp of this shows some things that are definite progressions, together with some changes that could be progressions or regressions.
Comment 5 Henry Stiles 2010-11-15 02:36:37 UTC 
Please commit.  I have a large project to change the color space implementation in pcl that will also fix this but I'm not sure when I'll get to it.
Comment 6 Robin Watts 2010-11-23 12:15:39 UTC 
Committed as bug 11909, after Ken reported that this was causing pdfwrite to output larger colour tables than expected, containing uninitialised entries. This was causing diffs in the pdfwrite tests.