[HarfBuzz] Valid range of deltaGlyphID is too small

[Jonathan Kew]

On 27 Sep 2011, at 11:04, Dohyun Kim wrote:

> Hi,
> 
> While testing harfbuzz-ng with a huge font which has more than 64,000
> glyphs, I encountered a strange result of hb-shape:
> 
>  <uniAC00=0+970|gid-1389=0|uniB098=6+970|gid-1388=6>
> 
> As shown, minus GID numbers are printed on the screen; this of course
> is not valid.
> 
> The problem seems to occur because of the valid range of deltaGlyphID
> in hb-ot-layout-gsub-table.hh.
> After having changed 67th line of hb-ot-layout-gsub-table.hh from
> 
>  SHORT         deltaGlyphID;           /* Add to original GlyphID to get
> 
> to
> 
>  USHORT        deltaGlyphID;           /* Add to original GlyphID to get
> 
> the problem disappeared and correct glyph names were printed instead
> of minus GIDs.
> 
> I have no idea about whether USHORT typecast is good for this case.
> Anyway, changing to USHORT has solved an issue which has nagged me for months.

The OT spec shows deltaGlyphID as a (signed) 16-bit value, so SHORT (and not USHORT) is the correct type. I'd guess the real issue is that GlyphID arithmetic is implicitly supposed to happen in 16-bit unsigned space, with silent wraparound of over- or under-flow, but HBNG ends up doing the arithmetic with 32-bit values and doesn't implement the modulo-64K truncation appropriately everywhere. (Note that hb_codepoint_t, often used for glyph indexes, is a 32-bit type.)

Behdad can no doubt identify the proper place to fix this, either immediately in SingleSubstFormat1::apply where the deltaGlyphID addition is done, or somewhere more general under replace_glyph(). (Not sure offhand if there may be other potential sources of glyph-ID-arithmetic overflow?)

JK