From ede7b74584cfbbd7916dd78cf4280e79c35992dd Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Thu, 21 Jul 2022 21:45:04 +0000 Subject: [PATCH] [repacker] add sanitization for GSUB/LookupList/Lookup during extension promotion. --- src/graph/graph.hh | 1 - src/graph/gsubgpos-graph.cc | 16 ++++ src/graph/gsubgpos-graph.hh | 146 ++++++++++++++++++++---------------- src/graph/serialize.hh | 1 - src/hb-repacker.hh | 22 +++--- 5 files changed, 111 insertions(+), 75 deletions(-) diff --git a/src/graph/graph.hh b/src/graph/graph.hh index 0d09f337f..a23f79334 100644 --- a/src/graph/graph.hh +++ b/src/graph/graph.hh @@ -621,7 +621,6 @@ struct graph_t clone->obj.tail = tail; clone->distance = 0; clone->space = 0; - printf("Creating new node from %p to %p\n", head, tail); unsigned clone_idx = vertices_.length - 2; diff --git a/src/graph/gsubgpos-graph.cc b/src/graph/gsubgpos-graph.cc index fefb0da2f..12f44f83f 100644 --- a/src/graph/gsubgpos-graph.cc +++ b/src/graph/gsubgpos-graph.cc @@ -28,6 +28,22 @@ namespace graph { +make_extension_context_t::make_extension_context_t (hb_tag_t table_tag_, + graph_t& graph_, + hb_vector_t& buffer_) + : table_tag (table_tag_), + graph (graph_), + buffer (buffer_), + lookups () +{ + GSTAR* gstar = graph::GSTAR::graph_to_gstar (graph_); + if (gstar) + gstar->find_lookups (graph, lookups); + + unsigned extension_size = OT::ExtensionFormat1::static_size; + buffer.alloc (num_non_ext_subtables () * extension_size); +} + unsigned make_extension_context_t::num_non_ext_subtables () { unsigned count = 0; for (auto l : lookups.values ()) diff --git a/src/graph/gsubgpos-graph.hh b/src/graph/gsubgpos-graph.hh index 3ff9d3c0a..dd99e09d9 100644 --- a/src/graph/gsubgpos-graph.hh +++ b/src/graph/gsubgpos-graph.hh @@ -35,63 +35,16 @@ namespace graph { struct Lookup; -struct GSTAR : public OT::GSUBGPOS -{ - static GSTAR* graph_to_gstar (graph_t& graph) - { - return (GSTAR*) graph.root ().obj.head; - } - - const void* get_lookup_list_field_offset () const - { - switch (u.version.major) { - case 1: return &(u.version1.lookupList); -#ifndef HB_NO_BORING_EXPANSION - case 2: return &(u.version2.lookupList); -#endif - default: return 0; - } - } - - void find_lookups (graph_t& graph, - hb_hashmap_t& lookups /* OUT */) - { - // TODO: template on types, based on gstar version. - unsigned lookup_list_idx = graph.index_for_offset (graph.root_idx (), - get_lookup_list_field_offset()); - - const OT::LookupList* lookupList = - (const OT::LookupList*) graph.object (lookup_list_idx).head; - - for (unsigned i = 0; i < lookupList->len; i++) - { - unsigned lookup_idx = graph.index_for_offset (lookup_list_idx, &(lookupList->arrayZ[i])); - Lookup* lookup = (Lookup*) graph.object (lookup_idx).head; - lookups.set (lookup_idx, lookup); - } - } -}; - struct make_extension_context_t { hb_tag_t table_tag; graph_t& graph; hb_vector_t buffer; - GSTAR* gstar; hb_hashmap_t lookups; - make_extension_context_t (hb_tag_t table_tag_, - graph_t& graph_) - : table_tag (table_tag_), - graph (graph_), - buffer (), - gstar (graph::GSTAR::graph_to_gstar (graph_)), - lookups () - { - gstar->find_lookups (graph, lookups); - unsigned extension_size = OT::ExtensionFormat1::static_size; - buffer.alloc (num_non_ext_subtables () * extension_size); - } + HB_INTERNAL make_extension_context_t (hb_tag_t table_tag_, + graph_t& graph_, + hb_vector_t& buffer_); bool in_error () const { @@ -99,7 +52,7 @@ struct make_extension_context_t } private: - unsigned num_non_ext_subtables (); + HB_INTERNAL unsigned num_non_ext_subtables (); }; struct Lookup : public OT::Lookup @@ -109,6 +62,13 @@ struct Lookup : public OT::Lookup return subTable.len; } + bool sanitize (graph_t::vertex_t& vertex) const + { + int64_t vertex_len = vertex.obj.tail - vertex.obj.head; + if (vertex_len < OT::Lookup::min_size) return false; + return vertex_len >= this->get_size (); + } + bool is_extension (hb_tag_t table_tag) const { return lookupType == extension_type (table_tag); @@ -123,14 +83,13 @@ struct Lookup : public OT::Lookup if (!ext_type || is_extension (c.table_tag)) { // NOOP - printf("Already extension (obj %u).\n", this_index); return true; } - printf("Promoting lookup type %u (obj %u) to extension.\n", - type, - this_index); - + DEBUG_MSG (SUBSET_REPACK, nullptr, + "Promoting lookup type %u (obj %u) to extension.\n", + type, + this_index); for (unsigned i = 0; i < subTable.len; i++) { @@ -149,8 +108,6 @@ struct Lookup : public OT::Lookup unsigned lookup_index, unsigned subtable_index) { - printf(" Promoting subtable %u in lookup %u to extension.\n", subtable_index, lookup_index); - unsigned type = lookupType; unsigned extension_size = OT::ExtensionFormat1::static_size; unsigned start = c.buffer.length; @@ -165,9 +122,6 @@ struct Lookup : public OT::Lookup extension->extensionLookupType = type; extension->extensionOffset = 0; - unsigned type_prime = extension->extensionLookupType; - printf("Assigned type %d to extension\n", type_prime); - unsigned ext_index = c.graph.new_node (&c.buffer.arrayZ[start], &c.buffer.arrayZ[end]); if (ext_index == (unsigned) -1) return false; @@ -176,11 +130,8 @@ struct Lookup : public OT::Lookup for (auto& l : lookup_vertex.obj.real_links.writer ()) { if (l.objidx == subtable_index) - { // Change lookup to point at the extension. - printf(" Changing %d to %d\n", l.objidx, ext_index); l.objidx = ext_index; - } } // Make extension point at the subtable. @@ -213,6 +164,73 @@ struct Lookup : public OT::Lookup } }; +template +struct LookupList : public OT::LookupList +{ + bool sanitize (const graph_t::vertex_t& vertex) const + { + int64_t vertex_len = vertex.obj.tail - vertex.obj.head; + if (vertex_len < OT::LookupList::min_size) return false; + return vertex_len >= OT::LookupList::item_size * this->len; + } +}; + +struct GSTAR : public OT::GSUBGPOS +{ + static GSTAR* graph_to_gstar (graph_t& graph) + { + const auto& r = graph.root (); + + GSTAR* gstar = (GSTAR*) r.obj.head; + if (!gstar->sanitize (r)) + return nullptr; + + return gstar; + } + + const void* get_lookup_list_field_offset () const + { + switch (u.version.major) { + case 1: return &(u.version1.lookupList); +#ifndef HB_NO_BORING_EXPANSION + case 2: return &(u.version2.lookupList); +#endif + default: return 0; + } + } + + bool sanitize (const graph_t::vertex_t& vertex) + { + int64_t len = vertex.obj.tail - vertex.obj.head; + // Only need access to fields in min_size + return len >= OT::GSUBGPOS::min_size; + } + + void find_lookups (graph_t& graph, + hb_hashmap_t& lookups /* OUT */) + { + // TODO: template on types, based on gstar version. + unsigned lookup_list_idx = graph.index_for_offset (graph.root_idx (), + get_lookup_list_field_offset()); + + const LookupList* lookupList = + (const LookupList*) graph.object (lookup_list_idx).head; + if (!lookupList->sanitize (graph.vertices_[lookup_list_idx])) + return; + + for (unsigned i = 0; i < lookupList->len; i++) + { + unsigned lookup_idx = graph.index_for_offset (lookup_list_idx, &(lookupList->arrayZ[i])); + Lookup* lookup = (Lookup*) graph.object (lookup_idx).head; + if (!lookup->sanitize (graph.vertices_[lookup_idx])) continue; + lookups.set (lookup_idx, lookup); + } + } +}; + + + + } #endif /* GRAPH_GSUBGPOS_GRAPH_HH */ diff --git a/src/graph/serialize.hh b/src/graph/serialize.hh index 8797ed95a..ecc6cc5ae 100644 --- a/src/graph/serialize.hh +++ b/src/graph/serialize.hh @@ -205,7 +205,6 @@ inline hb_blob_t* serialize (const graph_t& graph) { hb_vector_t buffer; size_t size = graph.total_size_in_bytes (); - printf("Expected graph size: %lu.\n", size); if (!buffer.alloc (size)) { DEBUG_MSG (SUBSET_REPACK, nullptr, "Unable to allocate output buffer."); return nullptr; diff --git a/src/hb-repacker.hh b/src/hb-repacker.hh index 1d49c9039..c434c67ec 100644 --- a/src/hb-repacker.hh +++ b/src/hb-repacker.hh @@ -41,8 +41,12 @@ using graph::graph_t; * docs/repacker.md */ +/* + * Analyze the lookups in a GSUB/GPOS table and decide if any should be promoted + * to extension lookups. + */ static inline -bool _make_extensions (graph::make_extension_context_t& ext_context) +bool _promote_extensions_if_needed (graph::make_extension_context_t& ext_context) { // TODO: Move this into graph or gsubgpos graph? for (auto p : ext_context.lookups.iter ()) @@ -50,6 +54,7 @@ bool _make_extensions (graph::make_extension_context_t& ext_context) if (!p.second->make_extension (ext_context, p.first)) return false; } + return true; } @@ -169,11 +174,8 @@ inline hb_blob_t* hb_resolve_overflows (const T& packed, hb_tag_t table_tag, unsigned max_rounds = 20) { - printf("Resolving overflows!\n"); graph_t sorted_graph (packed); sorted_graph.sort_shortest_distance (); - printf("Initial graph size: %lu.\n", - sorted_graph.total_size_in_bytes ()); bool will_overflow = graph::will_overflow (sorted_graph); if (!will_overflow) @@ -181,16 +183,18 @@ hb_resolve_overflows (const T& packed, return graph::serialize (sorted_graph); } - graph::make_extension_context_t ext_context (table_tag, sorted_graph); - if (ext_context.in_error ()) - return nullptr; + hb_vector_t extension_buffer; // Needs to live until serialization is done. if ((table_tag == HB_OT_TAG_GPOS || table_tag == HB_OT_TAG_GSUB) && will_overflow) { - if (!_make_extensions (ext_context)) { - printf("make extensions failed.\n"); + graph::make_extension_context_t ext_context (table_tag, sorted_graph, extension_buffer); + if (ext_context.in_error ()) + return nullptr; + + if (1 && !_promote_extensions_if_needed (ext_context)) { + DEBUG_MSG (SUBSET_REPACK, nullptr, "Extensions promotion failed."); return nullptr; }