Bug 688047

Summary: pdf embedded font-related problem on 64-bit platform (tru64)
Product: Ghostscript Reporter: Hin-Tak Leung <htl10>
Component: GeneralAssignee: Raph Levien <raph.levien>
Status: NOTIFIED FIXED    
Severity: normal CC: gsview, igor.melichev, manjiang
Priority: P2 Keywords: bountiable
Version: 8.50   
Hardware: DEC   
OS: other   
Customer: Word Size: ---
Attachments: wrong angstrom symbol in the middle
correct angstrom symbol
proposed fix for the embedded font problem on tru64.
generated obj/arch.h on Tru64.
generated obj/arch.h on x86 linux
the original pdf file with the symptom.
new/updated patch
diff for the win64 size_t change.
generated obj/arch.h on Win64

Description Hin-Tak Leung 2005-04-20 20:55:00 UTC
ghostscript 8.14/8.50/8.51 on tru64 won't process pdf file
with embedded fonts generated by Adobe distiller 6 on
windows, same version on ix86 linux would.

Tried esp ghostscript 8.15rc3 (to become 8.15.1), and it works - 
apparently somebody in Suse had made some changes between 
gpl 8.15 and esp 8.15rc3 to cope with 64-bit issues. Esp ghostscript
8.15rc3 on tru64 works alright, other than some autoconf/build issues:

http://www.cups.org/espgs/str.php?L1151+P0+S-2+C0+I0+E0+Q

Will try to extract the difference and see what works on 8.51.
Comment 1 Hin-Tak Leung 2005-04-20 21:40:30 UTC
As I suspected, the changes to 
   src/tttype.h
   src/tttypes.h
in http://www.cups.org/newsgroups.php?s521+gcups.commit+v529+T0

fixes the ghostscript-dying problem.

===================================================================
--- trunk/src/tttype.h 2005-03-18 15:53:37 UTC (rev 78)
+++ trunk/src/tttype.h 2005-03-29 18:52:36 UTC (rev 79)
@@ -62,15 +62,15 @@
   /*                                                                 */
   /*******************************************************************/

-  typedef signed long     TT_Fixed;   /* Signed Fixed 16.16 Float */
+  typedef signed int      TT_Fixed;   /* Signed Fixed 16.16 Float */

   typedef signed short    TT_FWord;   /* Distance in FUnits */
   typedef unsigned short  TT_UFWord;  /* Unsigned distance */

   typedef signed short    TT_Short;
   typedef unsigned short  TT_UShort;
-  typedef signed long     TT_Long;
-  typedef unsigned long   TT_ULong;
+  typedef signed int      TT_Long;
+  typedef unsigned int    TT_ULong;

   typedef signed short    TT_F2Dot14; /* Signed fixed float 2.14 used for */
                                       /* unary vectors, with layout:      */
@@ -85,10 +85,10 @@
                                       /*  added.                          */
                                       /*                                  */

-  typedef signed long     TT_F26Dot6; /* 26.6 fixed float, used for       */
+  typedef signed int      TT_F26Dot6; /* 26.6 fixed float, used for       */
                                       /* glyph points pixel coordinates.  */

-  typedef signed long     TT_Pos;     /* point position, expressed either */
+  typedef signed int      TT_Pos;     /* point position, expressed either */
                                       /* in fractional pixels or notional */
                                       /* units, depending on context. For */
                                       /* example, glyph coordinates       */

===================================================================
--- trunk/src/tttypes.h 2005-03-18 15:53:37 UTC (rev 78)
+++ trunk/src/tttypes.h 2005-03-29 18:52:36 UTC (rev 79)
@@ -62,16 +62,16 @@
   typedef unsigned short  UShort;
   typedef signed   short  Short;

-  typedef unsigned long   ULong;
-  typedef signed   long   Long;
+  typedef unsigned int    ULong;
+  typedef signed   int    Long;

 #if SIZEOF_INT == 4

-  typedef long int        Fixed;    /* signed fixed 16.16 float */
+  typedef int             Fixed;    /* signed fixed 16.16 float */

 #elif SIZEOF_LONG == 4

-  typedef long            Fixed;    /* signed fixed 16.16 float */
+  typedef int             Fixed;    /* signed fixed 16.16 float */

 #else

@@ -81,7 +81,7 @@

   typedef int             Int;

-  typedef long            Integer;
+  typedef int             Integer;

   /* Simple access types: pointers and tables */

@@ -117,7 +117,7 @@
 #define NULL  (void*)0
 #endif

-  typedef long*  PStorage;
+  typedef int*  PStorage;


 /* Rounding mode constants */
=================================================================
Comment 2 Hin-Tak Leung 2005-04-20 21:42:44 UTC
However, after fixing the won't-load pdf/ps problem, there seems
to be some problem with displaying the glyph for the Angstrom symbol
(a small circle over capital A, for 0.1 nanometre in scientific terms).

Screen shots for wrong display on tru64 and right display on ix86 to follow. 
Comment 3 Hin-Tak Leung 2005-04-20 21:44:22 UTC
Created attachment 1325 [details]
wrong angstrom symbol in the middle

note the '8 angstrom' in the middle.
This is from tru64 ghostscript.
Comment 4 Hin-Tak Leung 2005-04-20 21:45:47 UTC
Created attachment 1326 [details]
correct angstrom symbol

note the '8 angstrom' is displayed correctly.

this is from ix86 linux ghostscript.
Comment 5 Hin-Tak Leung 2005-04-21 15:48:09 UTC
changed title to better reflect the nature of the problem. This is the crash
message:

gs 20050314.pdf
AFPL Ghostscript 8.14 (2004-02-20)
Copyright (C) 2004 artofcode LLC, Benicia, CA.  All rights reserved.
This software comes with NO WARRANTY: see the file PUBLIC for details.
Processing pages 1 through 20.
Page 1
   **** Warning: Fonts with Subtype = /TrueType should be embedded.
                 But TimesNewRomanPS-BoldMT is not embedded.
Substituting font Times-Roman for TimesNewRomanPS-BoldMT.
Loading NimbusRomNo9L-Regu font from
/usr/local/share/ghostscript/fonts/n021003l.pfb... 3352536 1631268 1667976
370954 3 done.
Error: /rangecheck in --awidthshow--
Operand stack:
   --dict:8/8(L)--
Execution stack:
   %interp_exit   .runexec2   --nostringval--   --nostringval--
--nostringval--   2   %stopped_push   --nostringval--   --nostringval--
--nostringval--   false   1   %stopped_push   1   3   %oparray_pop   1   3
%oparray_pop   1   3   %oparray_pop   --nostringval--   2   1   20
--nostringval--   %for_pos_int_continue   --nostringval--
--nostringval--   --nostringval--   --nostringval--   %array_continue
--nostringval--   false   1   %stopped_push   --nostringval--
%loop_continue   --nostringval--   --nostringval--   --nostringval--
%array_continue
Dictionary stack:
   --dict:1107/1123(ro)(G)--   --dict:0/20(G)--   --dict:75/200(L)--
--dict:75/200(L)--   --dict:104/127(ro)(G)--   --dict:238/347(ro)(G)--
--dict:20/24(L)--   --dict:4/6(L)--   --dict:27/32(L)--
Current allocation mode is local
AFPL Ghostscript 8.14: Unrecoverable error, exit code 1

Comment 6 Igor Melichev 2005-04-27 09:06:48 UTC
The suggested change to tttypes.h isn't good : the case "#elif SIZEOF_LONG == 4
" must not use 'int', since in this case it's size is not 4.

I suggest to seneralize tttype.h with inserting #if SIZEOF_INT == 4, etc., and 
complete tttypes.h with accurate checking of data type sizes. The patch must 
not break 32-bit platforms, right ?
Comment 7 Hin-Tak Leung 2005-04-27 09:28:39 UTC
Comment 6: Sorry - not really thinking when I copied and applied
the patch from esp ghostscript. Although the patch does the job of making 
it work on 64-bit and fixed the crash, (still have the symbol corruption 
to worry about) - and it doesn't break 32-bit platforms etc because
"int" is the same size as "long" (so the patch has no effect on 32-bit
platform), as Igor pointed out, it is obviously the wrong way to achieve 
the end result in the context of the location of change.

I was hoping to find if there is any 64-bit related change in freetype 1, 
where this bit of code was derived, but couldn't. (and freetype is
having server problem and there is nothing beyond 31 dec 2004) I 
suppose a better way is to insert more #ifdef's as Igor suggested if these 
defines comes from the central autoconf code up one directory,
or try to insert some "#ifdef __osf__", which would make it
very ugly. 

OTOH, any thoughts on where I might be about to do something about
the symbol glyph corruption?
Comment 8 Jack Moffitt 2005-04-27 09:45:01 UTC
Hin-Tak has already done a lot of work for this, so the bountiable status is for
him.
Comment 9 Hin-Tak Leung 2005-04-27 10:27:59 UTC
Thanks for the bountiable status. My guess at the moment for the 
symbol corruption problem probably comes from other freetype 1 
derived code, so that's my next move.
Comment 10 Igor Melichev 2005-04-27 12:57:51 UTC
Hin-Tak Leung wrote :

> "int" is the same size as "long" 

Please do not rely on this assumption.
Ghostscript build is being controlled with configuration switches, which cover 
30+ platforms. The macro SIZEOF_INT is one of them. Please read gs/obj/arch.h, 
which is being generated automaticly depending on platform, and use 
definitions from there. The work on tru64 should start with providing a 
correct contents of this file for the platform.
Comment 11 Hin-Tak Leung 2005-04-27 14:55:02 UTC
"the size of int = size of long" comment only tried to explain 
the previous "it doesn't break 32-bit platforms because...".
I agree, on closer look, those changes in esp ghostscript is not 
the proper way to address 64-bit-related problems - they are just
the starting point.
Comment 12 Hin-Tak Leung 2005-04-27 22:31:34 UTC
Created attachment 1350 [details]
proposed fix for the embedded font problem on tru64.

Changes to src/tttypes.h, src/tttype.h, src/ttfoutl.h.

Type long is 64-bit on 64-bit platforms, but certain 
data structures in the truetype spec are of fixed 32-bit 
size; so some of the longs need to be change to INT
depending on obj/arch.h defines.

Though a bit tedious, I think this patch is formally
a correct solution to the problem. Tested on Tru64 and
it fixes the glyph correction problem (which seems
to be caused by src/ttfoutl.h), and also on x86 linux.
Comment 13 Hin-Tak Leung 2005-04-27 22:32:58 UTC
Created attachment 1351 [details]
generated obj/arch.h on Tru64.

generated obj/arch.h on Tru64, for reference.
Comment 14 Hin-Tak Leung 2005-04-27 22:34:59 UTC
Created attachment 1352 [details]
generated obj/arch.h on x86 linux

generated obj/arch.h on x86 linux, for reference and comparison.
Comment 15 Hin-Tak Leung 2005-04-27 22:44:42 UTC
Additional note: 

(1) Tried using bits32 (unsigned fixed 32bits) instead of 
the signed 32-bit the freetype code expects- won't work - seem to 
cause an infinite loop.

(2) The duplicate '#if   ARCH_LOG2_SIZEOF_LONG == 2' feels a bit tedious,
but the two files don't share common includes (in fact ttfoutl.h doesn't include
anything else). In the latest freetype 1 code, a similar test 
to this is done in ft_conf.h (ttconf.h in ghostscript source) and the test
removed from src/tttypes.h as in the patch; but doing it this way 
involves editing a totally unrelated file and refactoring, which I 
think I should leave for somebody else to do...
Comment 16 Igor Melichev 2005-04-27 23:21:20 UTC
Dear Hin-Tak Leung,

You'rew in the right direction, but please account that arch.h is being 
generated automaticly with genarch.c . You should build genarch.exe and run it 
to generate arch.h . I'm sorry for missing it at the first time. If the result 
of running genarch.exe isn't satisfactory, then patch genarch.c . But I guess 
it should work as it is.

Then, I'm not sure how do you build Ghostscript. Normally it is being build 
with the 'make' utility - see doc/Make.htm . In this case genarch is being 
build and run automaticly from makefiles. Please try diong so on your platform 
and please explain problems if any. Note that Ghostscript makefiles call 
several utilities, which customize Ghostscript for user's needs on the 
platform. For the beginning, the default configuration should be enough.
Comment 17 Hin-Tak Leung 2005-04-27 23:27:20 UTC
Additional note:

(3) There is a fixedfloat 16-bit type "TT_F2Dot14" which is
defined in tttype.h as signed short. This might be a related future 
problem if 'short' is ever larger or smaller than 16-bit. 
Comment 18 Hin-Tak Leung 2005-04-27 23:39:14 UTC
(Comment 16) genarch is the first thing that get run after ./configure ; make.
I don't have problem with it on tru64. The patch I just attached 
(comment 12) is the correct approach, although a few lines are duplicated and 
is a bit tedious.
Comment 19 Hin-Tak Leung 2005-04-27 23:46:00 UTC
Here is more explanation of the fix:

The changes in tttypes.h and tttype.h seems to be responsible for
parsing and hence the crash, while the one in ttfoutl.h seems to be
responsible for byte-code interpretation and hence the glyph
corruption. The relevant original files from freetype 1 and Igor were
very clearly written with comments to differentiate between "Fixed Floats"
and other (more flexible) types.


Comment 20 Igor Melichev 2005-04-28 00:00:36 UTC
Well, the logics looks good except one thing. I'm not sure where the 
comment "fixed float" comes from, but it definitely is incorrect. I guess one 
meant "fixed fractional". Since you change this line, lets do it in the right 
direction, rather than distributing the wrong term through code. Please don't 
define fixedfloat32_t since its name is confusing. Instead continue doing like 
this :

#if   ARCH_LOG2_SIZEOF_LONG == 2
    typedef signed long     TT_F26Dot6; /* 26.6 fixed float, used for       */
    typedef signed long     TT_Pos;     /* point position, expressed either */
#elif ARCH_LOG2_SIZEOF_INT  == 2
    typedef signed int      TT_F26Dot6; /* 26.6 fixed float, used for       */
    typedef signed int      TT_Pos;     /* point position, expressed either */
#else
#   error "No appropriate type for 32-bit fixed fractionals."
#endif
Comment 21 Igor Melichev 2005-04-28 00:02:39 UTC
Oops, more cleaning :

#if   ARCH_LOG2_SIZEOF_LONG == 2
    typedef signed long     TT_F26Dot6; /* 26.6 fixed fractional, used 
for       */
    typedef signed long     TT_Pos;     /* point position, expressed either */
#elif ARCH_LOG2_SIZEOF_INT  == 2
    typedef signed int      TT_F26Dot6; /* 26.6 fixed fractional, used 
for       */
    typedef signed int      TT_Pos;     /* point position, expressed either */
#else
#   error "No appropriate type for 32-bit fixed fractionals."
#endif
Comment 22 Hin-Tak Leung 2005-04-28 00:17:44 UTC
Those are the exact same terms in freetype-current/lib/freetype.h
(the latest freetype 1 source - ghostscript/src/tttype.h 
derived from an earlier version of that file), occuring quite a 
few times as "signed fixed 16.16 float", "Signed fixed float 2.14", 
"26.6 fixed float", and only once as "26.6 fractional pixels".
Those comments are as the freetype people wrote them.
Comment 23 Hin-Tak Leung 2005-04-28 00:32:26 UTC
The #if, #elif, #else, #endif in patch comment 12 occured twice (and two 
places uses the fixedfloat32_t directly, one later in tttype.h, aother 
in tttypes.h which includes tttype.h)- which I think is too much, but if doing 
it 4 times is the preferred way, I suppose 4 times it is. I did try 
using ghostscript's inline 'bits32' (in "doc/C-style.htm#Language_extensions")
without success. a ubits32 type would be appropriate.
Comment 24 Hin-Tak Leung 2005-05-19 19:11:04 UTC
Created attachment 1400 [details]
the original pdf file with the symptom.

The pdf file for which the problem was seen. Asked
the authors/owners for permission to include in the bug report
a long time ago; just thought it is a bit big to include;
had problem making up a small alternative pdf with such problem.

The angstrom symbol is on the 2nd page, 4th line from bottom; 
glyph corruption is also seen for the french name with e' 
on the 3rd page, 7th line from top.
Comment 25 Hin-Tak Leung 2005-05-19 19:15:25 UTC
Created attachment 1401 [details]
new/updated patch

re-work of the patch, with separate of ARCH_*.
Comment 26 Hin-Tak Leung 2005-05-19 19:33:19 UTC
Learned from Werner of freetype that the freetype cvs repository 
has since been resurrected from the crash, and relocated to savannah 
a few months ago, just that the official web site www.freetype.org
had not been updated yet:

http://savannah.nongnu.org/projects/freetype

I believe the "#if SIZEOF_INT == 4 ...typedef long int..."
in src/tttypes.h is a typo in ghostscript, as it is not in the 
freetype code *ever* - it is meant to be either 'typedef signed int'
or just 'typedef int' 
http://savannah.nongnu.org/cgi-bin/viewcvs/*checkout*/freetype/freetype/lib/tttypes.h?rev=1.1
http://savannah.nongnu.org/cgi-bin/viewcvs/freetype/freetype/lib/tttypes.h.diff?r1=1.12&r2=1.13

Also, somewhat curious at what point the fork occurred -
that bit of code in ghostscript was commited initially in 
Oct 2003 but only existed in freetype1 up to Jun 1998 (and 
removed/reorganized afterwards).
Comment 27 Igor Melichev 2005-05-19 22:42:31 UTC
> somewhat curious at what point the fork occurred

Ghostscript branched a code from FreeType 1, because Free Type 2 includes a 
big unrelated extra complexity.
Comment 28 Hin-Tak Leung 2005-05-20 22:27:17 UTC
freetype branching: yes, I know the ghostscript truetype code is branched
off freetype 1 rather than freetype 2; the curiosity is when/how/why - from
ghostscript's cvs's repository it seems the files concerned were
incoporated in Oct 2003, at which point freetype 1 had rearched v1.3.1 
already (freetype 1.3.1 was released in Mar 2000, 3 years earlier); 
and yet the freetype 1 code incoporated into ghostscript seems
to be of a much earlier version (freetype 1.1-ish around or before 1998).

gs-code-review: BTW, I was looking for my earlier patch at gs-code-review's
archive (which I e-mailed, around the same time I attached the patch) 
and somehow it isn't there. Is there anything I should know about sending 
patch in for reviewing, e.g. non-subscriber's patch silently get dropped? 
(the new patch has also been sent, but I don't expect it to appear 
in the mailing list archive yet.). 
Comment 29 Igor Melichev 2005-05-23 01:39:04 UTC
> code incoporated into ghostscript seems
> to be of a much earlier version (freetype 1.1-ish around or before 1998)

Probably you're right. There was some development for another project, and 
later it was branched again from that project to Ghostscript. I apologise that 
the first branch date is missed. If you help to reconstruct the branch date, 
I'd very appreciate that. With my old archieve I can restrict the branch date 
with January - May 1998.

> e.g. non-subscriber's patch silently get dropped? 

No, we do review them and reply. However I recognized your pathes as 
intermediate steps of a development, which isn't completed yet. Since I can't 
test incompleted code, I left them uncommitted. Please make a single complete 
patch, which passed a testing with your system, then we'll test it for 
regression with our testbase and if succeeded, it will be committed. Note that 
you can get some $s since the but is bountiable. For faster replying please cc 
your postings to me.
Comment 30 Hin-Tak Leung 2005-05-23 19:43:21 UTC
I think the latest patch does conceptually all what it should do -
and it is whether the style/how to do the same thing is considered
congruous with the rest of how ghostscript is written.

Tested it on both Alpha tru64 and x86 linux
with the test file (now attached - I was a bit reluctant at
first about its size, but seeing other bugs have 4MB pdf attachments, 
1.5MB isn't so big any more). As it isn't my system - nor
the document mine - I can't make new accounts for testing,
but the HP test drive program does include Tru64 Alpha. 
Comment 31 Igor Melichev 2005-05-24 11:09:29 UTC
> I think the latest patch does conceptually all what it should do

Just to be sure : do you mean the patch in Comment #25 ?
I'll try it when I'll have a time. Please give me few days.

BTW, sometimes it checks ARCH_LOG2_SIZEOF_LONG == 2, and in other places it 
checks SIZEOF_LONG = 4. Rather they are equivalent, would you mind to use a 
single thing in all places (just for a better clarity) ? Thnx. Same with 'INT'.
Comment 32 Hin-Tak Leung 2005-05-24 16:10:28 UTC
>> I think the latest patch does conceptually all what it should do
>
> Just to be sure : do you mean the patch in Comment #25 ?
> I'll try it when I'll have a time. Please give me few days.

Yes, that's the one I meant.

> BTW, sometimes it checks ARCH_LOG2_SIZEOF_LONG == 2, and in other places it 
> checks SIZEOF_LONG = 4. Rather they are equivalent, would you mind to use a 
> single thing in all places (just for a better clarity) ? Thnx. Same 
> with 'INT'.

I did think about replacing the SIZEOF_LONG/SIZEOF_INT with the ARCH_* ;
in fact that's how I did it when I started reworking the patch, but
then decided against it, because the SIZE_OF_* defines were freetype's
and were in freetype1 in 1998, the ARCH_* are ghostscript's. So comment #25's
patch is for minimal fork from 1998's freetype1. Later freetype 1 than 
1998 don't even have those, but have rewritten some of that with a #define
TT_Int32 from freetype.h or tttype.h (i.e. a bit similiar to how I did 
it in comment #12). So there are really 3 ways to do the SIZEOF_*:

(1) leve them as is, as comment #25, minimal fork from 1998's freetype1.

(2) change the SIZEOF_* to ARCH_* - more like how the rest of 
ghostscript does it; not back-portable to freetype1 as they don't 
use the ARCH_* defines.

(3) bring the old freetype1 code in ghostscript up in sync with 
current freetype1.

I started with (2), then tried asking Werner Lemberg of freetype
about the status of freetype1, to see if it is worth going (3). 
Then went for (1) instead. Ideally, my personal preference
would be to make the 64-bit change in such a way that it is 
applicable to both freetype1 and ghostscript. In the absence of
that option (it is a fair amount of work and may be risky, 
and since freetype 1 is no longer in active development, it is 
probably not worth doing), 

I don't have any strong preference over (1) versus (2); (2) is more
uniform across all of ghostscript's code, but has 
a slight disadvantage as the change would then be 
ghostscript-specific and not trackable/diff'able against 
any possible further development in freetype 1, if it ever 
goes forward.
Comment 33 Igor Melichev 2005-05-24 17:15:55 UTC
Thanks for explanation. Lets leave (1).
Comment 34 Igor Melichev 2005-05-31 04:12:06 UTC
Changing the bug assignment to myself for further processing.
Comment 36 Igor Melichev 2005-06-01 09:19:40 UTC
The patch to HEAD caused no regresion. Closing the bug now.
Comment 37 Alex Cherepanov 2005-07-21 03:43:39 UTC
*** Bug 688217 has been marked as a duplicate of this bug. ***
Comment 38 Russell Lang 2005-10-10 18:03:31 UTC
This bug fix causes the Win64 compile to fail, including the 8.52 release.

"c:\winddk\3790\bin\win64\x86\amd64\cl" /c /DGX_COLOR_INDEX_TYPE="unsigned
__int64"  -I"c:\winddk\3790\inc\wnet" -I"c:\winddk\3790\inc\crt" @.\obj\ccf32.tr
 /GF /O2 /Obc:\data\src\gs8.52\src\tttypes.h(125) : 
fatal error C1189: #error :  "Size of pointer type is not equal to either long
or int"
NMAKE : fatal error U1077: '"c:\winddk\3790\bin\win64\x86\amd64\cl"' : return
code '0x2'

On Win64, sizeof(int) = 2, sizeof(long) = 4, sizeof(pointer) = 8.
I'm guessing that the TT bytecode interpreter works with 32-bit
integers, so first suggested fix is to make PStorage a pointer
to a 32-bit integer on every platform.  Long might need to get
the same treatment.

If it really does need an integer the same size as a pointer
(which I initially doubt), then try using size_t.

Comment 39 Hin-Tak Leung 2005-10-11 01:56:12 UTC
The freetype-derived code is 32-bit all over the place; I'll have
a look and see - most probably size_t is the right answer.
Comment 40 Russell Lang 2005-10-11 03:41:35 UTC
I doubt that using size_t is the right answer, but it might be 
a kludge that allows it to compile.  I would expect (without
having reading any documents) that the TT bytecode interpreter 
would work exclusively with 32-bit integers.  So the fix is
likely to be to find the 32-bit integer type for each platform
and to use that.  This would be long on Win32, long on Win64,
long on Unix 32-bit, and int on Unix 64-bit.

size_t is 32-bit on Win32, 64-bit on Win64.
We would only need to use this if you really do need to
store a pointer in an integer type.
Comment 41 Hin-Tak Leung 2005-10-12 14:26:28 UTC
Here are the instances:
======================
c> grep 'PStorage' *
ttinterp.c:#define INS_ARG         EXEC_OPS PStorage args  /* see ttexec.h */
ttobjs.h:    PStorage        storage;    /* storage area            */
ttobjs.h:    PStorage        stack;      /* current exec. stack */
tttables.h:    PStorage  Table;
tttypes.h:  typedef long*  PStorage;
tttypes.h:  typedef int*   PStorage;
======================

The type is really used in three places: 
struct TLoca in tttables.h,
struct TExecution_Context in ttobjs.h, 
and all over the place in ttinterp.c

TLoca is not used anywhere in ghostscript HEAD,
The usage inside ttinterp.c is a bit complicated, but
the two in TExecution_Context are passed as arguments to 
FREE() and ALLOC_ARRAY() (and the usage inside ttinterp.c is 
probably related to the ->stack member), so things are going 
to be very bad if they are smaller than a pointer's size...
Curiously enough, earliest freetype uses long, and 
(after the gs fork) then changed to signed long. 

I am inclined to think the "correct" answer is the integer
type corresponding to pointer size. i.e. that section of code
should be, on win64:

===================
#if   ARCH_SIZEOF_PTR == SIZEOF_LONG
  typedef long*  PStorage;
#elif ARCH_SIZEOF_PTR == SIZEOF_INT
  typedef int*   PStorage;
#else
#warning "Size of pointer type is not equal to either long or int"
  typedef size_t  Storage_t;
  typedef Storage_t* PStorage;
#endif
===================
Comment 42 Hin-Tak Leung 2005-10-12 14:42:38 UTC
Created attachment 1717 [details]
diff for the win64 size_t change.

A suggested diff corresponding to the last comment. I also tried 
compiling the files under linux and the compiler complains about
the PInstance.storage, which is alloc'ed and 
copied to Execution_Context.storage. So PInstance.storage should 
also be of type PStorage, rather than PLong.
Hence the change.
Comment 43 Hin-Tak Leung 2005-10-18 16:01:14 UTC
Sorry, please ignore comment 42 and 2nd half of comment 41 (not making sense
there). Had another look at how storage and stack works, and I think Russell 
in comment 40 was right, so the correct fix should be:

===================
#if   4 == SIZEOF_LONG
  typedef long*  PStorage;
#elif 4 == SIZEOF_INT
  typedef int*   PStorage;
#else
#warning "Cannot find 32-bit pointer type"
  typedef size_t  Storage_t;
  typedef Storage_t* PStorage;
#endif
===================
Comment 44 Raph Levien 2005-10-20 11:59:40 UTC
After going over these comments, I've simply changed the type of PStorage to:

  typedef Long*    PStorage;
  
I've committed this into the 1.3 version of tttypes.h. On Tru64, the patch retains existing behavior 
(because Long is 64 bits, and version 1.2 also defined it to be a 64 bit quantity). On amd64, it defines 
PStorage to be a 32 bit type, which is consistent with comments #40 and #43.

Obviously, this patch should be tested on both Tru64 and amd64 platforms. Perhaps PStorage really 
does need to be 32 bits on all platforms including Tru64. However, the ALLOC_ARRAY at ttobjs.c:604 
hardcodes type of Long, so this discrepancy would need to be resolved.
Comment 45 Hin-Tak Leung 2005-10-20 16:55:10 UTC
I'll try the patch out on tru64 and possibly also amd64 as well.

I changed my mind earlier between I realise that although the pointer itself
is 64-bit at large machines (for indexing > 4GB of memory), but that's 
unrelated to long/int - rather, the quality appearing in front is the 
steping unit in the array. So "Long *PStorage" really means 
"Long PStorage[x]", etc (which it how that pointer is used later, as 
PSorage[i], etc), and the important issue is how the code steps and 
navigates through the array. If the code only uses the lower 32-bit 
of each member of the array, 32-bit-wide gaps would be between every 
32-bit of meaningful data.

Will report back whichever the outcome in the next few days.
Comment 46 Russell Lang 2005-10-20 17:16:08 UTC
I get the impression that an 8.53 release is imminent,
and so quick testing would be appreciated.
Comment 47 Ray Johnston 2005-10-20 22:53:46 UTC
Russell is correct that an 8.53 release is happening.

The 'tru64' platform is fairly uncommon, and it is much more serious for
us to work on AMD64 and Win64. If we still have a problem with the tru64
platform we will fix it in a later release (after testing on AMD64 with
Win64 and Linux).

We had not realized that our user/developer community did not include
those that regularly built on these platforms. Artifex will be adding
AMD64 platforms to our build/test machines. We probably will NOT be
adding a tru64 machine (I'm still not clear who makes that).
Comment 48 Hin-Tak Leung 2005-10-21 01:44:18 UTC
I agree AMD64 are quite popular, and win64 will be important soon. Will
test tru64/amd64 sometimes this weekend.

Tru64 - which runs on the Dec Aplha, formerly Digital/Compaq - is now 
sold by HP (http://h30097.www3.hp.com/unix/v51b3.html), but is end of
life for 2006 and end of support at 2011. Probably not worth buying
now.
Comment 49 Hin-Tak Leung 2005-10-24 03:56:15 UTC
Russell: can you please attach the obj/arch.h from win64 for reference?

Tested result of ps2ps and ps2pdf okay on both AMD64 linux and Tru64, 
of the pdf file attached to the bug report, and also the 4 files from 
(similar bugs for AMD64 for ESP ghostscript):

http://www.cups.org/espgs/str.php?L1168+P0+S0+C0+I0+E0+Qamd
http://qa.mandriva.com/show_bug.cgi?id=16694

i.e. the 5 files:
http://bugs.ghostscript.com/attachment.cgi?id=1400&action=view
http://www.cups.org/espgs/strfiles/1168/oootest.ps
http://www.cups.org/espgs/strfiles/1168/roman.ps
http://www.cups.org/espgs/strfiles/1168/symbol.ps
http://qa.mandriva.com/attachment.cgi?id=3158

Incidentally, obj/arch.h's are identical for tru64 and Amd64 linux and 
Tru64 and the ps and pdf files generated are byte-wise identical also,
other than the timestamp field within the files. (So the rumours about
the Dec/Alpha engineers getting hired by AMD is probably true...)

Please close when Win64 tested ok?
Comment 50 Russell Lang 2005-10-26 05:44:36 UTC
Created attachment 1730 [details]
generated obj/arch.h on Win64
Comment 51 Hin-Tak Leung 2006-06-13 12:47:00 UTC
The regression problem with win64 seems to have been fixed, since w64 
build of 853/854 are out there...