From 90a98dd62a3b8e9eb416b6777f36951c7f5a56a4 Mon Sep 17 00:00:00 2001 From: Qunxin Liu Date: Wed, 25 Jan 2023 10:06:23 -0800 Subject: [PATCH] [instancer] fix potential memory leak for compiled glyph bytes Also calculate max_offsets after glyph bytes are compiled, cause byte length of a glyph might change after compile --- src/OT/glyf/SubsetGlyph.hh | 10 +---- src/OT/glyf/glyf.hh | 81 +++++++++++++++++++++++--------------- 2 files changed, 51 insertions(+), 40 deletions(-) diff --git a/src/OT/glyf/SubsetGlyph.hh b/src/OT/glyf/SubsetGlyph.hh index c27504833..795925bba 100644 --- a/src/OT/glyf/SubsetGlyph.hh +++ b/src/OT/glyf/SubsetGlyph.hh @@ -21,18 +21,10 @@ struct SubsetGlyph bool serialize (hb_serialize_context_t *c, bool use_short_loca, - const hb_subset_plan_t *plan, - hb_font_t *font) + const hb_subset_plan_t *plan) { TRACE_SERIALIZE (this); - if (font) - { - const OT::glyf_accelerator_t &glyf = *font->face->table.glyf; - if (!this->compile_bytes_with_deltas (plan, font, glyf)) - return_trace (false); - } - hb_bytes_t dest_glyph = dest_start.copy (c); dest_glyph = hb_bytes_t (&dest_glyph, dest_glyph.length + dest_end.copy (c).length); unsigned int pad_length = use_short_loca ? padding () : 0; diff --git a/src/OT/glyf/glyf.hh b/src/OT/glyf/glyf.hh index 60a253b3c..d3e6ed404 100644 --- a/src/OT/glyf/glyf.hh +++ b/src/OT/glyf/glyf.hh @@ -43,14 +43,13 @@ struct glyf bool serialize (hb_serialize_context_t *c, Iterator it, bool use_short_loca, - const hb_subset_plan_t *plan, - hb_font_t *font) + const hb_subset_plan_t *plan) { TRACE_SERIALIZE (this); unsigned init_len = c->length (); for (auto &_ : it) - if (unlikely (!_.serialize (c, use_short_loca, plan, font))) + if (unlikely (!_.serialize (c, use_short_loca, plan))) return false; /* As a special case when all glyph in the font are empty, add a zero byte @@ -76,9 +75,6 @@ struct glyf glyf *glyf_prime = c->serializer->start_embed (); if (unlikely (!c->serializer->check_success (glyf_prime))) return_trace (false); - hb_vector_t glyphs; - _populate_subset_glyphs (c->plan, glyphs); - hb_font_t *font = nullptr; if (c->plan->normalized_coords) { @@ -86,52 +82,60 @@ struct glyf if (unlikely (!font)) return false; } - auto padded_offsets = - + hb_iter (glyphs) - | hb_map (&glyf_impl::SubsetGlyph::padded_size) - ; + hb_vector_t padded_offsets; + unsigned num_glyphs = c->plan->num_output_glyphs (); + if (unlikely (!padded_offsets.resize (num_glyphs))) + return false; + + hb_vector_t glyphs; + if (!_populate_subset_glyphs (c->plan, font, glyphs)) + return false; + + if (font) + hb_font_destroy (font); + + unsigned max_offset = 0; + for (unsigned i = 0; i < num_glyphs; i++) + { + padded_offsets[i] = glyphs[i].padded_size (); + max_offset += padded_offsets[i]; + } bool use_short_loca = false; if (likely (!c->plan->force_long_loca)) - { - unsigned max_offset = + padded_offsets | hb_reduce (hb_add, 0); use_short_loca = max_offset < 0x1FFFF; - } - - if (!glyf_prime->serialize (c->serializer, glyphs.writer (), use_short_loca, c->plan, font)) - return false; if (!use_short_loca) { - padded_offsets = - + hb_iter (glyphs) - | hb_map (&glyf_impl::SubsetGlyph::length) - ; + for (unsigned i = 0; i < num_glyphs; i++) + padded_offsets[i] = glyphs[i].length (); } - if (font) + if (!glyf_prime->serialize (c->serializer, glyphs.writer (), use_short_loca, c->plan)) { - if (!c->plan->pinned_at_default) - _free_compiled_subset_glyphs (&glyphs); - hb_font_destroy (font); + if (c->plan->normalized_coords && !c->plan->pinned_at_default) + _free_compiled_subset_glyphs (glyphs, glyphs.length - 1); + return false; } if (unlikely (c->serializer->in_error ())) return_trace (false); + return_trace (c->serializer->check_success (glyf_impl::_add_loca_and_head (c->plan, - padded_offsets, + padded_offsets.iter (), use_short_loca))); } - void + bool _populate_subset_glyphs (const hb_subset_plan_t *plan, + hb_font_t *font, hb_vector_t &glyphs /* OUT */) const; hb_font_t * _create_font_for_instancing (const hb_subset_plan_t *plan) const; - void _free_compiled_subset_glyphs (hb_vector_t *glyphs) const + void _free_compiled_subset_glyphs (hb_vector_t &glyphs, unsigned index) const { - for (auto _ : *glyphs) - _.free_compiled_bytes (); + for (unsigned i = 0; i <= index && i < glyphs.length; i++) + glyphs[i].free_compiled_bytes (); } protected: @@ -393,14 +397,16 @@ struct glyf_accelerator_t }; -inline void +inline bool glyf::_populate_subset_glyphs (const hb_subset_plan_t *plan, + hb_font_t *font, hb_vector_t& glyphs /* OUT */) const { OT::glyf_accelerator_t glyf (plan->source); unsigned num_glyphs = plan->num_output_glyphs (); - if (!glyphs.resize (num_glyphs)) return; + if (!glyphs.resize (num_glyphs)) return false; + unsigned idx = 0; for (auto p : plan->glyph_map->iter ()) { unsigned new_gid = p.second; @@ -422,7 +428,20 @@ glyf::_populate_subset_glyphs (const hb_subset_plan_t *plan, subset_glyph.drop_hints_bytes (); else subset_glyph.dest_start = subset_glyph.source_glyph.get_bytes (); + + if (font) + { + if (unlikely (!subset_glyph.compile_bytes_with_deltas (plan, font, glyf))) + { + // when pinned at default, only bounds are updated, thus no need to free + if (!plan->pinned_at_default && idx > 0) + _free_compiled_subset_glyphs (glyphs, idx - 1); + return false; + } + idx++; + } } + return true; } inline hb_font_t *