From 73ed59f7a68fc5363ef444b6de131c92cc5ca836 Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Wed, 17 Mar 2021 15:53:10 -0700 Subject: [PATCH] [subset] store errors in the serializer as a flag set. Make check_assign/check_equal specify the type of error to set. --- src/hb-cff-interp-common.hh | 2 +- src/hb-open-type.hh | 10 +++--- src/hb-ot-cmap-table.hh | 17 ++++++--- src/hb-ot-hdmx-table.hh | 2 +- src/hb-ot-hmtx-table.hh | 2 +- src/hb-ot-layout-gpos-table.hh | 8 ++--- src/hb-ot-layout-gsub-table.hh | 4 +-- src/hb-ot-name-table.hh | 3 +- src/hb-repacker.hh | 4 +-- src/hb-serialize.hh | 64 +++++++++++++++++++++------------- src/hb-subset.cc | 8 ++--- src/test-repacker.cc | 4 +-- 12 files changed, 76 insertions(+), 52 deletions(-) diff --git a/src/hb-cff-interp-common.hh b/src/hb-cff-interp-common.hh index 91a9b7d0d..b53b544b0 100644 --- a/src/hb-cff-interp-common.hh +++ b/src/hb-cff-interp-common.hh @@ -263,7 +263,7 @@ struct UnsizedByteStr : UnsizedArrayOf T *ip = c->allocate_size (T::static_size); if (unlikely (!ip)) return_trace (false); - return_trace (c->check_assign (*ip, value)); + return_trace (c->check_assign (*ip, value, HB_SERIALIZE_ERR_OTHER)); } template diff --git a/src/hb-open-type.hh b/src/hb-open-type.hh index dc0ae1d98..f412d88dc 100644 --- a/src/hb-open-type.hh +++ b/src/hb-open-type.hh @@ -209,7 +209,9 @@ struct Offset : Type void *serialize (hb_serialize_context_t *c, const void *base) { void *t = c->start_embed (); - c->check_assign (*this, (unsigned) ((char *) t - (char *) base)); + c->check_assign (*this, + (unsigned) ((char *) t - (char *) base), + HB_SERIALIZE_ERR_OTHER); return t; } @@ -621,7 +623,7 @@ struct ArrayOf { TRACE_SERIALIZE (this); if (unlikely (!c->extend_min (*this))) return_trace (false); - c->check_assign (len, items_len); + c->check_assign (len, items_len, HB_SERIALIZE_ERR_OTHER); if (unlikely (!c->extend (*this))) return_trace (false); return_trace (true); } @@ -656,7 +658,7 @@ struct ArrayOf TRACE_SERIALIZE (this); auto *out = c->start_embed (this); if (unlikely (!c->extend_min (out))) return_trace (nullptr); - c->check_assign (out->len, len); + c->check_assign (out->len, len, HB_SERIALIZE_ERR_OTHER); if (unlikely (!as_array ().copy (c))) return_trace (nullptr); return_trace (out); } @@ -787,7 +789,7 @@ struct HeadlessArrayOf { TRACE_SERIALIZE (this); if (unlikely (!c->extend_min (*this))) return_trace (false); - c->check_assign (lenP1, items_len + 1); + c->check_assign (lenP1, items_len + 1, HB_SERIALIZE_ERR_OTHER); if (unlikely (!c->extend (*this))) return_trace (false); return_trace (true); } diff --git a/src/hb-ot-cmap-table.hh b/src/hb-ot-cmap-table.hh index 878e02ff1..650be0270 100644 --- a/src/hb-ot-cmap-table.hh +++ b/src/hb-ot-cmap-table.hh @@ -276,7 +276,9 @@ struct CmapSubtableFormat4 HBUINT16 *idRangeOffset = serialize_rangeoffset_glyid (c, format4_iter, endCode, startCode, idDelta, segcount); if (unlikely (!c->check_success (idRangeOffset))) return; - if (unlikely (!c->check_assign(this->length, c->length () - table_initpos))) return; + if (unlikely (!c->check_assign(this->length, + c->length () - table_initpos, + HB_SERIALIZE_ERR_OTHER))) return; this->segCountX2 = segcount * 2; this->entrySelector = hb_max (1u, hb_bit_storage (segcount)) - 1; this->searchRange = 2 * (1u << this->entrySelector); @@ -850,7 +852,9 @@ struct DefaultUVS : SortedArrayOf } else { - if (unlikely (!c->check_assign (out->len, (c->length () - init_len) / UnicodeValueRange::static_size))) return nullptr; + if (unlikely (!c->check_assign (out->len, + (c->length () - init_len) / UnicodeValueRange::static_size, + HB_SERIALIZE_ERR_OTHER))) return nullptr; return out; } } @@ -1112,10 +1116,11 @@ struct CmapSubtableFormat14 return; int tail_len = init_tail - c->tail; - c->check_assign (this->length, c->length () - table_initpos + tail_len); + c->check_assign (this->length, c->length () - table_initpos + tail_len, HB_SERIALIZE_ERR_OTHER); c->check_assign (this->record.len, (c->length () - table_initpos - CmapSubtableFormat14::min_size) / - VariationSelectorRecord::static_size); + VariationSelectorRecord::static_size, + HB_SERIALIZE_ERR_OTHER); /* Correct the incorrect write order by reversing the order of the variation records array. */ @@ -1401,7 +1406,9 @@ struct cmap } } - c->check_assign(this->encodingRecord.len, (c->length () - cmap::min_size)/EncodingRecord::static_size); + c->check_assign(this->encodingRecord.len, + (c->length () - cmap::min_size)/EncodingRecord::static_size, + HB_SERIALIZE_ERR_OTHER); } void closure_glyphs (const hb_set_t *unicodes, diff --git a/src/hb-ot-hdmx-table.hh b/src/hb-ot-hdmx-table.hh index c9c391bad..590fa154b 100644 --- a/src/hb-ot-hdmx-table.hh +++ b/src/hb-ot-hdmx-table.hh @@ -110,7 +110,7 @@ struct hdmx for (const hb_item_type& _ : +it) c->start_embed ()->serialize (c, _.first, _.second); - return_trace (c->successful); + return_trace (c->successful ()); } diff --git a/src/hb-ot-hmtx-table.hh b/src/hb-ot-hmtx-table.hh index d06c0fa4a..822c53176 100644 --- a/src/hb-ot-hmtx-table.hh +++ b/src/hb-ot-hmtx-table.hh @@ -146,7 +146,7 @@ struct hmtxvmtx _mtx.fini (); - if (unlikely (c->serializer->ran_out_of_room || c->serializer->in_error ())) + if (unlikely (c->serializer->ran_out_of_room () || c->serializer->in_error ())) return_trace (false); // Amend header num hmetrics diff --git a/src/hb-ot-layout-gpos-table.hh b/src/hb-ot-layout-gpos-table.hh index 72a44bf69..5c16786b2 100644 --- a/src/hb-ot-layout-gpos-table.hh +++ b/src/hb-ot-layout-gpos-table.hh @@ -694,7 +694,7 @@ struct MarkArray : ArrayOf /* Array of MarkRecords--in Coverage orde { TRACE_SERIALIZE (this); if (unlikely (!c->extend_min (*this))) return_trace (false); - if (unlikely (!c->check_assign (len, it.len ()))) return_trace (false); + if (unlikely (!c->check_assign (len, it.len (), HB_SERIALIZE_ERR_OTHER))) return_trace (false); c->copy_all (it, base, c->to_bias (this), klass_mapping, layout_variation_idx_map); return_trace (true); } @@ -756,7 +756,7 @@ struct SinglePosFormat1 { auto out = c->extend_min (*this); if (unlikely (!out)) return; - if (unlikely (!c->check_assign (valueFormat, valFormat))) return; + if (unlikely (!c->check_assign (valueFormat, valFormat, HB_SERIALIZE_ERR_OTHER))) return; + it | hb_map (hb_second) @@ -870,8 +870,8 @@ struct SinglePosFormat2 { auto out = c->extend_min (*this); if (unlikely (!out)) return; - if (unlikely (!c->check_assign (valueFormat, valFormat))) return; - if (unlikely (!c->check_assign (valueCount, it.len ()))) return; + if (unlikely (!c->check_assign (valueFormat, valFormat, HB_SERIALIZE_ERR_OTHER))) return; + if (unlikely (!c->check_assign (valueCount, it.len (), HB_SERIALIZE_ERR_OTHER))) return; + it | hb_map (hb_second) diff --git a/src/hb-ot-layout-gsub-table.hh b/src/hb-ot-layout-gsub-table.hh index ee95bbc00..132d008e9 100644 --- a/src/hb-ot-layout-gsub-table.hh +++ b/src/hb-ot-layout-gsub-table.hh @@ -102,7 +102,7 @@ struct SingleSubstFormat1 TRACE_SERIALIZE (this); if (unlikely (!c->extend_min (*this))) return_trace (false); if (unlikely (!coverage.serialize (c, this).serialize (c, glyphs))) return_trace (false); - c->check_assign (deltaGlyphID, delta); + c->check_assign (deltaGlyphID, delta, HB_SERIALIZE_ERR_OTHER); return_trace (true); } @@ -1551,7 +1551,7 @@ struct SubstLookup : Lookup template static inline typename context_t::return_t dispatch_recurse_func (context_t *c, unsigned int lookup_index); - + static inline typename hb_closure_context_t::return_t closure_glyphs_recurse_func (hb_closure_context_t *c, unsigned lookup_index, hb_set_t *covered_seq_indices, unsigned seq_index, unsigned end_index); static inline hb_closure_context_t::return_t dispatch_closure_recurse_func (hb_closure_context_t *c, unsigned lookup_index, hb_set_t *covered_seq_indices, unsigned seq_index, unsigned end_index) diff --git a/src/hb-ot-name-table.hh b/src/hb-ot-name-table.hh index ece3c2846..794a7cdb6 100644 --- a/src/hb-ot-name-table.hh +++ b/src/hb-ot-name-table.hh @@ -230,7 +230,8 @@ struct name c->copy_all (records, src_string_pool); free (records.arrayZ); - if (unlikely (c->ran_out_of_room)) return_trace (false); + + if (unlikely (c->ran_out_of_room ())) return_trace (false); this->stringOffset = c->length (); diff --git a/src/hb-repacker.hh b/src/hb-repacker.hh index 68165d4ce..15ed61bbe 100644 --- a/src/hb-repacker.hh +++ b/src/hb-repacker.hh @@ -738,13 +738,13 @@ hb_resolve_overflows (const hb_vector_t& pac } DEBUG_MSG (SUBSET_REPACK, nullptr, "No resolution available :("); - c->err_offset_overflow (); + c->set_error (HB_SERIALIZE_ERR_OFFSET_OVERFLOW); return; } if (sorted_graph.in_error ()) { - c->err_other_error (); + c->set_error (HB_SERIALIZE_ERR_OTHER); return; } sorted_graph.serialize (c); diff --git a/src/hb-serialize.hh b/src/hb-serialize.hh index 77ed74731..f9895e1fd 100644 --- a/src/hb-serialize.hh +++ b/src/hb-serialize.hh @@ -41,6 +41,14 @@ * Serialize */ +enum hb_serialize_error_type_t { + HB_SERIALIZE_ERR_NONE = 0x00000000u, + HB_SERIALIZE_ERR_OTHER = 0x00000001u, + HB_SERIALIZE_ERR_OFFSET_OVERFLOW = 0x00000002u, + HB_SERIALIZE_ERR_OUT_OF_ROOM = 0x00000004u, +}; +HB_MARK_AS_FLAG_T (hb_serialize_error_type_t); + struct hb_serialize_context_t { typedef unsigned objidx_t; @@ -51,6 +59,8 @@ struct hb_serialize_context_t Absolute /* Absolute: from the start of the serialize buffer. */ }; + + struct object_t { void fini () { links.fini (); } @@ -117,7 +127,12 @@ struct hb_serialize_context_t object_pool.fini (); } - bool in_error () const { return !this->successful; } + bool in_error () const { return errors & HB_SERIALIZE_ERR_OTHER; } + + bool successful () const { return !in_error (); } + + bool ran_out_of_room () const { return errors & HB_SERIALIZE_ERR_OUT_OF_ROOM; } + bool offset_overflow () const { return errors & HB_SERIALIZE_ERR_OFFSET_OVERFLOW; } void reset (void *start_, unsigned int size) { @@ -129,9 +144,7 @@ struct hb_serialize_context_t void reset () { - this->successful = true; - this->ran_out_of_room = false; - this->offset_overflow = false; + this->errors = HB_SERIALIZE_ERR_NONE; this->head = this->start; this->tail = this->end; this->debug_depth = 0; @@ -141,23 +154,27 @@ struct hb_serialize_context_t this->packed_map.init (); } - bool check_success (bool success) - { return this->successful && (success || (err_other_error (), false)); } + bool check_success (bool success, + hb_serialize_error_type_t err_type = HB_SERIALIZE_ERR_OTHER) + { + return successful () + && (success || (set_error (err_type), false)); + } template - bool check_equal (T1 &&v1, T2 &&v2) + bool check_equal (T1 &&v1, T2 &&v2, hb_serialize_error_type_t err_type) { if ((long long) v1 != (long long) v2) { - err_offset_overflow (); + set_error (err_type); return false; } return true; } template - bool check_assign (T1 &v1, T2 &&v2) - { return check_equal (v1 = v2, v2); } + bool check_assign (T1 &v1, T2 &&v2, hb_serialize_error_type_t err_type) + { return check_equal (v1 = v2, v2, err_type); } template bool propagate_error (T &&obj) { return check_success (!hb_deref (obj).in_error ()); } @@ -184,7 +201,7 @@ struct hb_serialize_context_t "end [%p..%p] serialized %u bytes; %s", this->start, this->end, (unsigned) (this->head - this->start), - this->successful ? "successful" : "UNSUCCESSFUL"); + successful () ? "successful" : "UNSUCCESSFUL"); propagate_error (packed, packed_map); @@ -368,7 +385,7 @@ struct hb_serialize_context_t for (const object_t::link_t &link : parent->links) { const object_t* child = packed[link.objidx]; - if (unlikely (!child)) { err_other_error(); return; } + if (unlikely (!child)) { set_error (HB_SERIALIZE_ERR_OTHER); return; } unsigned offset = 0; switch ((whence_t) link.whence) { case Head: offset = child->head - parent->head; break; @@ -415,20 +432,20 @@ struct hb_serialize_context_t Type *start_embed (const Type &obj) const { return start_embed (hb_addressof (obj)); } - /* Following two functions exist to allow setting breakpoint on. */ - void err_ran_out_of_room () { this->ran_out_of_room = true; } - void err_offset_overflow () { this->offset_overflow = true; } - void err_other_error () { this->successful = false; } + void set_error (hb_serialize_error_type_t err_type) + { + errors = errors | err_type; + } template Type *allocate_size (unsigned int size) { - if (unlikely (!this->successful)) return nullptr; + if (unlikely (in_error ())) return nullptr; if (this->tail - this->head < ptrdiff_t (size)) { - err_ran_out_of_room (); - this->successful = false; + set_error (HB_SERIALIZE_ERR_OUT_OF_ROOM); + set_error (HB_SERIALIZE_ERR_OTHER); return nullptr; } memset (this->head, 0, size); @@ -515,7 +532,7 @@ struct hb_serialize_context_t /* Output routines. */ hb_bytes_t copy_bytes () const { - assert (this->successful); + assert (successful ()); /* Copy both items from head side and tail side... */ unsigned int len = (this->head - this->start) + (this->end - this->tail); @@ -547,15 +564,13 @@ struct hb_serialize_context_t { auto &off = * ((BEInt *) (parent->head + link.position)); assert (0 == off); - check_assign (off, offset); + check_assign (off, offset, HB_SERIALIZE_ERR_OFFSET_OVERFLOW); } public: /* TODO Make private. */ char *start, *head, *tail, *end; unsigned int debug_depth; - bool successful; - bool ran_out_of_room; - bool offset_overflow; + hb_serialize_error_type_t errors; private: @@ -572,5 +587,4 @@ struct hb_serialize_context_t hb_hashmap_t packed_map; }; - #endif /* HB_SERIALIZE_HH */ diff --git a/src/hb-subset.cc b/src/hb-subset.cc index 0d64de1c8..0ba77b439 100644 --- a/src/hb-subset.cc +++ b/src/hb-subset.cc @@ -75,7 +75,7 @@ _repack (hb_tag_t tag, const hb_serialize_context_t& c) && tag != HB_OT_TAG_GSUB) return c.copy_blob (); - if (!c.offset_overflow) + if (!c.offset_overflow ()) return c.copy_blob (); hb_vector_t buf; @@ -86,7 +86,7 @@ _repack (hb_tag_t tag, const hb_serialize_context_t& c) hb_serialize_context_t repacked ((void *) buf, buf_size); hb_resolve_overflows (c.object_graph (), &repacked); - if (unlikely (repacked.ran_out_of_room || repacked.in_error () || repacked.offset_overflow)) + if (unlikely (repacked.ran_out_of_room () || repacked.in_error () || repacked.offset_overflow ())) // TODO(garretrieger): refactor so we can share the resize/retry logic with the subset // portion. return nullptr; @@ -105,7 +105,7 @@ _try_subset (const TableType *table, c->serializer->start_serialize (); bool needed = table->subset (c); - if (!c->serializer->ran_out_of_room) + if (!c->serializer->ran_out_of_room ()) { c->serializer->end_serialize (); return needed; @@ -163,7 +163,7 @@ _subset (hb_subset_plan_t *plan) } hb_blob_destroy (source_blob); - if (serializer.ran_out_of_room || serializer.in_error ()) + if (serializer.ran_out_of_room () || serializer.in_error ()) { DEBUG_MSG (SUBSET, nullptr, "OT::%c%c%c%c::subset FAILED!", HB_UNTAG (tag)); return false; diff --git a/src/test-repacker.cc b/src/test-repacker.cc index 1b4860394..a8cc6395f 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -434,7 +434,7 @@ static void test_resolve_overflows_via_sort () hb_serialize_context_t out (out_buffer, buffer_size); hb_resolve_overflows (c.object_graph (), &out); - assert (!out.offset_overflow); + assert (!out.offset_overflow ()); hb_bytes_t result = out.copy_bytes (); assert (result.length == (80000 + 3 + 3 * 2)); @@ -455,7 +455,7 @@ static void test_resolve_overflows_via_duplication () hb_serialize_context_t out (out_buffer, buffer_size); hb_resolve_overflows (c.object_graph (), &out); - assert (!out.offset_overflow); + assert (!out.offset_overflow ()); hb_bytes_t result = out.copy_bytes (); assert (result.length == (10000 + 2 * 2 + 60000 + 2 + 3 * 2));