More work towards improving collect_glyphs() against bad input

The three "XXXXX"'s should be switched to false.  Doing that separately for ease
of bisecting...

5 files changed, 48 insertions, 61 deletions

diff --git a/src/hb-ot-layout-common-private.hh b/src/hb-ot-layout-common-private.hh
index 47a05b5e..55436485 100644
--- a/src/hb-ot-layout-common-private.hh
+++ b/src/hb-ot-layout-common-private.hh
@@ -156,8 +156,7 @@ struct RangeRecord
 
   template <typename set_t>
   inline bool add_coverage (set_t *glyphs) const {
-    glyphs->add_range (start, end);
-    return likely (start <= end);
+    return glyphs->add_range (start, end);
   }
 
   GlyphID	start;		/* First GlyphID in the range */
@@ -820,7 +819,7 @@ struct CoverageFormat2
     unsigned int count = rangeRecord.len;
     for (unsigned int i = 0; i < count; i++)
       if (unlikely (!rangeRecord[i].add_coverage (glyphs)))
-        return false;
+        return true;//XXXXXXXXXXXXfalse;
     return true;
   }
 
@@ -935,7 +934,7 @@ struct Coverage
     switch (u.format) {
     case 1: return u.format1.add_coverage (glyphs);
     case 2: return u.format2.add_coverage (glyphs);
-    default:return false;
+    default:return true;//XXXXXXXXXXXfalse;
     }
   }
 
diff --git a/src/hb-ot-layout-gpos-table.hh b/src/hb-ot-layout-gpos-table.hh
index 215e2d74..f1b61301 100644
--- a/src/hb-ot-layout-gpos-table.hh
+++ b/src/hb-ot-layout-gpos-table.hh
@@ -607,12 +607,7 @@ struct PairSet
     unsigned int record_size = UINT16::static_size * (1 + len1 + len2);
 
     const PairValueRecord *record = CastP<PairValueRecord> (arrayZ);
-    unsigned int count = len;
-    for (unsigned int i = 0; i < count; i++)
-    {
-      c->input->add (record->secondGlyph);
-      record = &StructAtOffset<PairValueRecord> (record, record_size);
-    }
+    c->input->add_array (&record->secondGlyph, len, record_size);
   }
 
   inline bool apply (hb_apply_context_t *c,
@@ -689,7 +684,7 @@ struct PairPosFormat1
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
-    (this+coverage).add_coverage (c->input);
+    if (unlikely (!(this+coverage).add_coverage (c->input))) return;
     unsigned int count = pairSet.len;
     for (unsigned int i = 0; i < count; i++)
       (this+pairSet[i]).collect_glyphs (c, valueFormat);
@@ -755,7 +750,7 @@ struct PairPosFormat2
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
-    (this+coverage).add_coverage (c->input);
+    if (unlikely (!(this+coverage).add_coverage (c->input))) return;
 
     unsigned int count1 = class1Count;
     if (count1)
@@ -904,7 +899,7 @@ struct CursivePosFormat1
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
-    (this+coverage).add_coverage (c->input);
+    if (unlikely (!(this+coverage).add_coverage (c->input))) return;
   }
 
   inline const Coverage &get_coverage (void) const
@@ -1062,8 +1057,8 @@ struct MarkBasePosFormat1
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
-    (this+markCoverage).add_coverage (c->input);
-    (this+baseCoverage).add_coverage (c->input);
+    if (unlikely (!(this+markCoverage).add_coverage (c->input))) return;
+    if (unlikely (!(this+baseCoverage).add_coverage (c->input))) return;
   }
 
   inline const Coverage &get_coverage (void) const
@@ -1165,8 +1160,8 @@ struct MarkLigPosFormat1
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
-    (this+markCoverage).add_coverage (c->input);
-    (this+ligatureCoverage).add_coverage (c->input);
+    if (unlikely (!(this+markCoverage).add_coverage (c->input))) return;
+    if (unlikely (!(this+ligatureCoverage).add_coverage (c->input))) return;
   }
 
   inline const Coverage &get_coverage (void) const
@@ -1278,8 +1273,8 @@ struct MarkMarkPosFormat1
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
-    (this+mark1Coverage).add_coverage (c->input);
-    (this+mark2Coverage).add_coverage (c->input);
+    if (unlikely (!(this+mark1Coverage).add_coverage (c->input))) return;
+    if (unlikely (!(this+mark2Coverage).add_coverage (c->input))) return;
   }
 
   inline const Coverage &get_coverage (void) const
diff --git a/src/hb-ot-layout-gsub-table.hh b/src/hb-ot-layout-gsub-table.hh
index 28e0790e..0b09c4e4 100644
--- a/src/hb-ot-layout-gsub-table.hh
+++ b/src/hb-ot-layout-gsub-table.hh
@@ -54,13 +54,13 @@ struct SingleSubstFormat1
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
+    if (unlikely (!(this+coverage).add_coverage (c->input))) return;
     Coverage::Iter iter;
     for (iter.init (this+coverage); iter.more (); iter.next ())
     {
       /* TODO Switch to range-based API to work around malicious fonts.
        * https://github.com/harfbuzz/harfbuzz/issues/363 */
       hb_codepoint_t glyph_id = iter.get_glyph ();
-      c->input->add (glyph_id);
       c->output->add ((glyph_id + deltaGlyphID) & 0xFFFFu);
     }
   }
@@ -139,13 +139,13 @@ struct SingleSubstFormat2
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
+    if (unlikely (!(this+coverage).add_coverage (c->input))) return;
     Coverage::Iter iter;
     unsigned int count = substitute.len;
     for (iter.init (this+coverage); iter.more (); iter.next ())
     {
       if (unlikely (iter.get_coverage () >= count))
         break; /* Work around malicious fonts. https://github.com/harfbuzz/harfbuzz/issues/363 */
-      c->input->add (iter.get_glyph ());
       c->output->add (substitute[iter.get_coverage ()]);
     }
   }
@@ -269,9 +269,7 @@ struct Sequence
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
-    unsigned int count = substitute.len;
-    for (unsigned int i = 0; i < count; i++)
-      c->output->add (substitute[i]);
+    c->output->add_array (substitute.array, substitute.len);
   }
 
   inline bool apply (hb_apply_context_t *c) const
@@ -348,7 +346,7 @@ struct MultipleSubstFormat1
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
-    (this+coverage).add_coverage (c->input);
+    if (unlikely (!(this+coverage).add_coverage (c->input))) return;
     unsigned int count = sequence.len;
     for (unsigned int i = 0; i < count; i++)
 	(this+sequence[i]).collect_glyphs (c);
@@ -474,17 +472,15 @@ struct AlternateSubstFormat1
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
+    if (unlikely (!(this+coverage).add_coverage (c->input))) return;
     Coverage::Iter iter;
     unsigned int count = alternateSet.len;
     for (iter.init (this+coverage); iter.more (); iter.next ())
     {
       if (unlikely (iter.get_coverage () >= count))
         break; /* Work around malicious fonts. https://github.com/harfbuzz/harfbuzz/issues/363 */
-      c->input->add (iter.get_glyph ());
       const AlternateSet &alt_set = this+alternateSet[iter.get_coverage ()];
-      unsigned int count = alt_set.len;
-      for (unsigned int i = 0; i < count; i++)
-	c->output->add (alt_set[i]);
+      c->output->add_array (alt_set.array, alt_set.len);
     }
   }
 
@@ -615,9 +611,7 @@ struct Ligature
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
-    unsigned int count = component.len;
-    for (unsigned int i = 1; i < count; i++)
-      c->input->add (component[i]);
+    c->input->add_array (component.array, component.len ? component.len - 1 : 0);
     c->output->add (ligGlyph);
   }
 
@@ -801,13 +795,13 @@ struct LigatureSubstFormat1
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
+    if (unlikely (!(this+coverage).add_coverage (c->input))) return;
     Coverage::Iter iter;
     unsigned int count = ligatureSet.len;
     for (iter.init (this+coverage); iter.more (); iter.next ())
     {
       if (unlikely (iter.get_coverage () >= count))
         break; /* Work around malicious fonts. https://github.com/harfbuzz/harfbuzz/issues/363 */
-      c->input->add (iter.get_glyph ());
       (this+ligatureSet[iter.get_coverage ()]).collect_glyphs (c);
     }
   }
@@ -970,25 +964,22 @@ struct ReverseChainSingleSubstFormat1
   inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
   {
     TRACE_COLLECT_GLYPHS (this);
-
-    const OffsetArrayOf<Coverage> &lookahead = StructAfter<OffsetArrayOf<Coverage> > (backtrack);
+    if (unlikely (!(this+coverage).add_coverage (c->input))) return;
 
     unsigned int count;
 
-    (this+coverage).add_coverage (c->input);
-
     count = backtrack.len;
     for (unsigned int i = 0; i < count; i++)
-      (this+backtrack[i]).add_coverage (c->before);
+      if (unlikely (!(this+backtrack[i]).add_coverage (c->before))) return;
 
+    const OffsetArrayOf<Coverage> &lookahead = StructAfter<OffsetArrayOf<Coverage> > (backtrack);
     count = lookahead.len;
     for (unsigned int i = 0; i < count; i++)
-      (this+lookahead[i]).add_coverage (c->after);
+      if (unlikely (!(this+lookahead[i]).add_coverage (c->after))) return;
 
     const ArrayOf<GlyphID> &substitute = StructAfter<ArrayOf<GlyphID> > (lookahead);
     count = substitute.len;
-    for (unsigned int i = 0; i < count; i++)
-      c->output->add (substitute[i]);
+    c->output->add_array (substitute.array, substitute.len);
   }
 
   inline const Coverage &get_coverage (void) const
diff --git a/src/hb-set-digest-private.hh b/src/hb-set-digest-private.hh
index 0f798ab6..e099a826 100644
--- a/src/hb-set-digest-private.hh
+++ b/src/hb-set-digest-private.hh
@@ -71,7 +71,7 @@ struct hb_set_digest_lowest_bits_t
     mask |= mask_for (g);
   }
 
-  inline void add_range (hb_codepoint_t a, hb_codepoint_t b) {
+  inline bool add_range (hb_codepoint_t a, hb_codepoint_t b) {
     if ((b >> shift) - (a >> shift) >= mask_bits - 1)
       mask = (mask_t) -1;
     else {
@@ -79,6 +79,7 @@ struct hb_set_digest_lowest_bits_t
       mask_t mb = mask_for (b);
       mask |= mb + (mb - ma) - (mb < ma);
     }
+    return true;
   }
 
   template <typename T>
@@ -128,9 +129,10 @@ struct hb_set_digest_combiner_t
     tail.add (g);
   }
 
-  inline void add_range (hb_codepoint_t a, hb_codepoint_t b) {
+  inline bool add_range (hb_codepoint_t a, hb_codepoint_t b) {
     head.add_range (a, b);
     tail.add_range (a, b);
+    return true;
   }
   template <typename T>
   inline void add_array (const T *array, unsigned int count, unsigned int stride=sizeof(T))
diff --git a/src/hb-set-private.hh b/src/hb-set-private.hh
index edeca6d7..4b6f7fb3 100644
--- a/src/hb-set-private.hh
+++ b/src/hb-set-private.hh
@@ -70,20 +70,19 @@ struct hb_set_t
 
     inline void add_range (hb_codepoint_t a, hb_codepoint_t b)
     {
-     elt_t *la = &elt (a);
-     elt_t *lb = &elt (b);
-     if (la == lb)
-       *la |= (mask (b) << 1) - mask(a);
-     else
-     {
-       *la |= ~(mask (a) - 1);
-       la++;
-
-       memset (la, 0xff, (char *) lb - (char *) la);
+      elt_t *la = &elt (a);
+      elt_t *lb = &elt (b);
+      if (la == lb)
+        *la |= (mask (b) << 1) - mask(a);
+      else
+      {
+	*la |= ~(mask (a) - 1);
+	la++;
 
-       *lb |= ((mask (b) << 1) - 1);
+	memset (la, 0xff, (char *) lb - (char *) la);
 
-     }
+	*lb |= ((mask (b) << 1) - 1);
+      }
     }
 
     inline bool is_equal (const page_t *other) const
@@ -230,34 +229,35 @@ struct hb_set_t
     if (unlikely (!page)) return;
     page->add (g);
   }
-  inline void add_range (hb_codepoint_t a, hb_codepoint_t b)
+  inline bool add_range (hb_codepoint_t a, hb_codepoint_t b)
   {
-    if (unlikely (in_error || a > b || a == INVALID || b == INVALID)) return;
+    if (unlikely (in_error || a > b || a == INVALID || b == INVALID)) return true;//XXXXXXXfalse;
     unsigned int ma = get_major (a);
     unsigned int mb = get_major (b);
     if (ma == mb)
     {
       page_t *page = page_for_insert (a);
-      if (unlikely (!page)) return;
+      if (unlikely (!page)) return false;
       page->add_range (a, b);
     }
     else
     {
       page_t *page = page_for_insert (a);
-      if (unlikely (!page)) return;
+      if (unlikely (!page)) return false;
       page->add_range (a, major_start (ma + 1) - 1);
 
       for (unsigned int m = ma + 1; m < mb; m++)
       {
 	page = page_for_insert (major_start (m));
-	if (unlikely (!page)) return;
+	if (unlikely (!page)) return false;
 	page->init1 ();
       }
 
       page = page_for_insert (b);
-      if (unlikely (!page)) return;
+      if (unlikely (!page)) return false;
       page->add_range (major_start (mb), b);
     }
+    return true;
   }
 
   template <typename T>