620270 - PairPosFormat1 relies on alignment to emulate an array when PairPosFormat1.apply() calls PairSetPairSet.apply()

Status: RESOLVED INVALID

(Core :: Graphics, defect)

Description

[timeless]

616 struct PairPosFormat1
617 {

621   inline bool apply (hb_apply_context_t *c) const
622   {

640     return (this+pairSet[index]).apply (c, &valueFormat1, j);
641   }

665   ValueFormat   valueFormat1;           /* Defines the types of data in
666                                          * ValueRecord1--for the first glyph
667                                          * in the pair--may be zero (0) */
668   ValueFormat   valueFormat2;           /* Defines the types of data in
669                                          * ValueRecord2--for the second glyph

551  	struct PairSet
552  	{
553  	  friend struct PairPosFormat1;
554  	
555  	  inline bool apply (hb_apply_context_t *c,
556  			     const ValueFormat *valueFormats,
557  			     unsigned int pos) const
558  	  {
560  	    unsigned int len1 = valueFormats[0].get_len ();
561  	    unsigned int len2 = valueFormats[1].get_len ();

This pointer arithmetic is scary and not a great idea, it's generally prefered to actually use a real array instead of relying on the compiler not deciding that it wants to pad between members.

Comment 1

[Jonathan Kew [:jfkthame]]

I don't see any particular need to be concerned here. If the compiler were to add unexpected padding between members in structs like this, a great deal of the harfbuzz code would break, as it relies heavily on mapping structs onto externally-defined binary data in font tables. Happily, we'll know if this happens as there are lots of compile-time assertions checking that the types harfbuzz defines for the various records, etc., do in fact have the expected size.

If there's a harfbuzz struct like this corresponding to some part of an OpenType table that *doesn't* have a compile-time assertion checking its size, that would most likely represent a bug we should ask Behdad to fix.

cc'ing Behdad in case he has any further comments.

Comment 2

[Behdad Esfahbod]

That's actually not quite pointer arithmetic as that + operator is overloaded.  However, it's hard not to do pointer arithmetic when data you have is a base pointer and an offset in bytes...

Nothing interesting here.