From fa966bcc29e84a579fc32af7663a50bfe7814b1a Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Mon, 6 Dec 2021 12:54:19 -0800 Subject: [PATCH] [repacker] create repacker output buffer after final length is known. Don't rely on a buffer provided by the caller, as it may not be large enough. --- src/hb-repacker.hh | 60 ++++++++++++++++++++--------- src/hb-subset.cc | 20 ++++------ src/test-repacker.cc | 91 ++++++++++++++++---------------------------- 3 files changed, 83 insertions(+), 88 deletions(-) diff --git a/src/hb-repacker.hh b/src/hb-repacker.hh index 68438a93a..200f2565b 100644 --- a/src/hb-repacker.hh +++ b/src/hb-repacker.hh @@ -204,27 +204,46 @@ struct graph_t /* * serialize graph into the provided serialization buffer. */ - void serialize (hb_serialize_context_t* c) const + hb_blob_t* serialize () const { - c->start_serialize (); + hb_vector_t buffer; + size_t size = serialized_length (); + if (!buffer.alloc (size)) { + DEBUG_MSG (SUBSET_REPACK, nullptr, "Unable to allocate output buffer."); + return nullptr; + } + hb_serialize_context_t c((void *) buffer, size); + + c.start_serialize (); for (unsigned i = 0; i < vertices_.length; i++) { - c->push (); + c.push (); size_t size = vertices_[i].obj.tail - vertices_[i].obj.head; - char* start = c->allocate_size (size); - if (!start) return; + char* start = c.allocate_size (size); + if (!start) { + DEBUG_MSG (SUBSET_REPACK, nullptr, "Buffer out of space."); + return nullptr; + } memcpy (start, vertices_[i].obj.head, size); // Only real links needs to be serialized. for (const auto& link : vertices_[i].obj.real_links) - serialize_link (link, start, c); + serialize_link (link, start, &c); // All duplications are already encoded in the graph, so don't // enable sharing during packing. - c->pop_pack (false); + c.pop_pack (false); } - c->end_serialize (); + c.end_serialize (); + + if (c.in_error ()) { + DEBUG_MSG (SUBSET_REPACK, nullptr, "Error during serialization. Err flag: %d", + c.errors); + return nullptr; + } + + return c.copy_blob (); } /* @@ -725,6 +744,15 @@ struct graph_t private: + size_t serialized_length () const { + size_t total_size = 0; + for (unsigned i = 0; i < vertices_.length; i++) { + size_t size = vertices_[i].obj.tail - vertices_[i].obj.head; + total_size += size; + } + return total_size; + } + /* * Returns the numbers of incoming edges that are 32bits wide. */ @@ -1135,10 +1163,9 @@ static bool _process_overflows (const hb_vector_t& o * For a detailed writeup describing how the algorithm operates see: * docs/repacker.md */ -inline void +inline hb_blob_t* hb_resolve_overflows (const hb_vector_t& packed, hb_tag_t table_tag, - hb_serialize_context_t* c, unsigned max_rounds = 10) { // Kahn sort is ~twice as fast as shortest distance sort and works for many fonts // so try it first to save time. @@ -1146,8 +1173,7 @@ hb_resolve_overflows (const hb_vector_t& pac sorted_graph.sort_kahn (); if (!sorted_graph.will_overflow ()) { - sorted_graph.serialize (c); - return; + return sorted_graph.serialize (); } sorted_graph.sort_shortest_distance (); @@ -1186,17 +1212,17 @@ hb_resolve_overflows (const hb_vector_t& pac if (sorted_graph.in_error ()) { - c->err (HB_SERIALIZE_ERROR_OTHER); - return; + DEBUG_MSG (SUBSET_REPACK, nullptr, "Sorted graph in error state."); + return nullptr; } if (sorted_graph.will_overflow ()) { - c->err (HB_SERIALIZE_ERROR_OFFSET_OVERFLOW); DEBUG_MSG (SUBSET_REPACK, nullptr, "Offset overflow resolution failed."); - return; + return nullptr; } - sorted_graph.serialize (c); + + return sorted_graph.serialize (); } #endif /* HB_REPACKER_HH */ diff --git a/src/hb-subset.cc b/src/hb-subset.cc index 048bdf188..bb46e5b97 100644 --- a/src/hb-subset.cc +++ b/src/hb-subset.cc @@ -104,20 +104,16 @@ _repack (hb_tag_t tag, const hb_serialize_context_t& c) if (!c.offset_overflow ()) return c.copy_blob (); - hb_vector_t buf; - int buf_size = c.end - c.start; - if (unlikely (!buf.alloc (buf_size))) + hb_blob_t* result = hb_resolve_overflows (c.object_graph (), tag); + + if (unlikely (!result)) + { + DEBUG_MSG (SUBSET, nullptr, "OT::%c%c%c%c offset overflow resolution failed.", + HB_UNTAG (tag)); return nullptr; + } - hb_serialize_context_t repacked ((void *) buf, buf_size); - hb_resolve_overflows (c.object_graph (), tag, &repacked); - - if (unlikely (repacked.in_error ())) - // TODO(garretrieger): refactor so we can share the resize/retry logic with the subset - // portion. - return nullptr; - - return repacked.copy_blob (); + return result; } template diff --git a/src/test-repacker.cc b/src/test-repacker.cc index 86ab2ff19..7c18ba4c7 100644 --- a/src/test-repacker.cc +++ b/src/test-repacker.cc @@ -74,14 +74,13 @@ static void run_resolve_overflow_test (const char* name, graph_t graph (overflowing.object_graph ()); - unsigned buffer_size = overflowing.end - overflowing.start; - void* out_buffer = malloc (buffer_size); - hb_serialize_context_t out (out_buffer, buffer_size); assert (overflowing.offset_overflow ()); - hb_resolve_overflows (overflowing.object_graph (), HB_TAG ('G', 'S', 'U', 'B'), &out, num_iterations); - assert (!out.offset_overflow ()); - hb_bytes_t result = out.copy_bytes (); + hb_blob_t* out = hb_resolve_overflows (overflowing.object_graph (), + HB_TAG ('G', 'S', 'U', 'B'), num_iterations); + assert (out); + + hb_bytes_t result = out->as_bytes (); assert (!expected.offset_overflow ()); hb_bytes_t expected_result = expected.copy_bytes (); @@ -92,9 +91,8 @@ static void run_resolve_overflow_test (const char* name, assert (result[i] == expected_result[i]); } - result.fini (); expected_result.fini (); - free (out_buffer); + hb_blob_destroy (out); } static void add_virtual_offset (unsigned id, @@ -962,19 +960,14 @@ test_serialize () populate_serializer_simple (&c1); hb_bytes_t expected = c1.copy_bytes (); - void* buffer_2 = malloc (buffer_size); - hb_serialize_context_t c2 (buffer_2, buffer_size); - graph_t graph (c1.object_graph ()); - graph.serialize (&c2); - hb_bytes_t actual = c2.copy_bytes (); - - assert (actual == expected); - - actual.fini (); - expected.fini (); + hb_blob_t* out = graph.serialize (); free (buffer_1); - free (buffer_2); + + hb_bytes_t actual = out->as_bytes (); + assert (actual == expected); + expected.fini (); + hb_blob_destroy (out); } static void test_will_overflow_1 () @@ -1024,17 +1017,13 @@ static void test_resolve_overflows_via_sort () populate_serializer_with_overflow (&c); graph_t graph (c.object_graph ()); - void* out_buffer = malloc (buffer_size); - hb_serialize_context_t out (out_buffer, buffer_size); - - hb_resolve_overflows (c.object_graph (), HB_TAG_NONE, &out); - assert (!out.offset_overflow ()); - hb_bytes_t result = out.copy_bytes (); + hb_blob_t* out = hb_resolve_overflows (c.object_graph (), HB_TAG_NONE); + assert (out); + hb_bytes_t result = out->as_bytes (); assert (result.length == (80000 + 3 + 3 * 2)); - result.fini (); free (buffer); - free (out_buffer); + hb_blob_destroy (out); } static void test_resolve_overflows_via_duplication () @@ -1045,17 +1034,13 @@ static void test_resolve_overflows_via_duplication () populate_serializer_with_dedup_overflow (&c); graph_t graph (c.object_graph ()); - void* out_buffer = malloc (buffer_size); - hb_serialize_context_t out (out_buffer, buffer_size); - - hb_resolve_overflows (c.object_graph (), HB_TAG_NONE, &out); - assert (!out.offset_overflow ()); - hb_bytes_t result = out.copy_bytes (); + hb_blob_t* out = hb_resolve_overflows (c.object_graph (), HB_TAG_NONE); + assert (out); + hb_bytes_t result = out->as_bytes (); assert (result.length == (10000 + 2 * 2 + 60000 + 2 + 3 * 2)); - result.fini (); free (buffer); - free (out_buffer); + hb_blob_destroy (out); } static void test_resolve_overflows_via_space_assignment () @@ -1085,19 +1070,15 @@ static void test_resolve_overflows_via_isolation () populate_serializer_with_isolation_overflow (&c); graph_t graph (c.object_graph ()); - void* out_buffer = malloc (buffer_size); - hb_serialize_context_t out (out_buffer, buffer_size); - assert (c.offset_overflow ()); - hb_resolve_overflows (c.object_graph (), HB_TAG ('G', 'S', 'U', 'B'), &out, 0); - assert (!out.offset_overflow ()); - hb_bytes_t result = out.copy_bytes (); + hb_blob_t* out = hb_resolve_overflows (c.object_graph (), HB_TAG ('G', 'S', 'U', 'B'), 0); + assert (out); + hb_bytes_t result = out->as_bytes (); assert (result.length == (1 + 10000 + 60000 + 1 + 1 + 4 + 3 * 2)); - result.fini (); free (buffer); - free (out_buffer); + hb_blob_destroy (out); } static void test_resolve_overflows_via_isolation_with_recursive_duplication () @@ -1164,21 +1145,17 @@ static void test_resolve_overflows_via_isolation_spaces () populate_serializer_with_isolation_overflow_spaces (&c); graph_t graph (c.object_graph ()); - void* out_buffer = malloc (buffer_size); - hb_serialize_context_t out (out_buffer, buffer_size); - assert (c.offset_overflow ()); - hb_resolve_overflows (c.object_graph (), HB_TAG ('G', 'S', 'U', 'B'), &out, 0); - assert (!out.offset_overflow ()); - hb_bytes_t result = out.copy_bytes (); + hb_blob_t* out = hb_resolve_overflows (c.object_graph (), HB_TAG ('G', 'S', 'U', 'B'), 0); + assert (out); + hb_bytes_t result = out->as_bytes (); unsigned expected_length = 3 + 2 * 60000; // objects expected_length += 2 * 4 + 2 * 2; // links assert (result.length == expected_length); - result.fini (); free (buffer); - free (out_buffer); + hb_blob_destroy (out); } static void test_resolve_overflows_via_splitting_spaces () @@ -1228,13 +1205,10 @@ static void test_virtual_link () hb_serialize_context_t c (buffer, buffer_size); populate_serializer_virtual_link (&c); - void* out_buffer = malloc (buffer_size); - hb_serialize_context_t out (out_buffer, buffer_size); + hb_blob_t* out = hb_resolve_overflows (c.object_graph (), HB_TAG_NONE); + assert (out); - hb_resolve_overflows (c.object_graph (), HB_TAG_NONE, &out); - assert (!out.offset_overflow ()); - - hb_bytes_t result = out.copy_bytes (); + hb_bytes_t result = out->as_bytes (); assert (result.length == 5 + 4 * 2); assert (result[0] == 'a'); assert (result[5] == 'c'); @@ -1242,9 +1216,8 @@ static void test_virtual_link () assert (result[9] == 'b'); assert (result[12] == 'd'); - result.fini (); free (buffer); - free (out_buffer); + hb_blob_destroy (out); } static void