From 67449c39331babb88f7d29d737895d786cd5da33 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 14 Sep 2018 10:58:00 +0200 Subject: [PATCH 01/20] Don't dereference offset before check_struct() --- src/hb-open-file.hh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index a1f931d3c..38610a8ec 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -387,10 +387,9 @@ struct ResourceMap inline bool sanitize (hb_sanitize_context_t *c, const void *data_base) const { TRACE_SANITIZE (this); - const void *type_base = &(this+typeList); return_trace (c->check_struct (this) && typeList.sanitize (c, this, - type_base, + &(this+typeList), data_base)); } From 4653e6cf3c1ef5005886d901df30e952d57eed6c Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 14 Sep 2018 11:31:33 +0200 Subject: [PATCH 02/20] [aat] Add enums for pre-defined state and classes Not sure how I didn't add before... --- src/hb-aat-layout-common.hh | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index 7a5be84cc..c22ba5b28 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -439,10 +439,23 @@ struct Entry template struct StateTable { + enum State + { + STATE_START_OF_TEXT = 0, + STATE_START_OF_LINE = 1, + }; + enum Class + { + CLASS_END_OF_TEXT = 0, + CLASS_OUT_OF_BOUNDS = 1, + CLASS_DELETED_GLYPH = 2, + CLASS_END_OF_LINE = 3, + }; + inline unsigned int get_class (hb_codepoint_t glyph_id, unsigned int num_glyphs) const { const HBUINT16 *v = (this+classTable).get_value (glyph_id, num_glyphs); - return v ? *v : 1; + return v ? *v : CLASS_OUT_OF_BOUNDS; } inline const Entry *get_entries () const @@ -538,13 +551,13 @@ struct StateTableDriver if (!c->in_place) buffer->clear_output (); - unsigned int state = 0; + unsigned int state = StateTable::STATE_START_OF_TEXT; bool last_was_dont_advance = false; for (buffer->idx = 0;;) { unsigned int klass = buffer->idx < buffer->len ? machine.get_class (info[buffer->idx].codepoint, num_glyphs) : - 0 /* End of text */; + (unsigned) StateTable::CLASS_END_OF_TEXT; const Entry *entry = machine.get_entryZ (state, klass); if (unlikely (!entry)) break; From 957dbed388fc3214248f6aca65902ad277d070fb Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 14 Sep 2018 12:14:42 +0200 Subject: [PATCH 03/20] Fix builds --- src/hb-aat-layout-common.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index c22ba5b28..af71ebd38 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -455,7 +455,7 @@ struct StateTable inline unsigned int get_class (hb_codepoint_t glyph_id, unsigned int num_glyphs) const { const HBUINT16 *v = (this+classTable).get_value (glyph_id, num_glyphs); - return v ? *v : CLASS_OUT_OF_BOUNDS; + return v ? (unsigned) *v : (unsigned) CLASS_OUT_OF_BOUNDS; } inline const Entry *get_entries () const From 6ebbf514ac90712fe089b2b64f68d1cf681edd5d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 14 Sep 2018 12:15:53 +0200 Subject: [PATCH 04/20] Minor --- src/hb-ot-layout.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-ot-layout.hh b/src/hb-ot-layout.hh index 84981391c..94414d3f2 100644 --- a/src/hb-ot-layout.hh +++ b/src/hb-ot-layout.hh @@ -263,7 +263,7 @@ _hb_glyph_info_set_unicode_props (hb_glyph_info_t *info, hb_buffer_t *buffer) * Also, all Mn's that are Default_Ignorable, have ccc=0, hence * the "else if". */ - props |= unicode->modified_combining_class (info->codepoint)<<8; + props |= unicode->modified_combining_class (u)<<8; /* Recategorize emoji skin-tone modifiers as Unicode mark, so they * behave correctly in non-native directionality. They originally From 01b9148d9ae7d18228538774243e49840cfd2499 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 14 Sep 2018 14:23:09 +0200 Subject: [PATCH 05/20] [unicode] Move Fitzpatrick hack from ot-layout into unicode.hh --- src/hb-ot-layout.hh | 12 +----------- src/hb-unicode.hh | 36 +++++++++++++++++++++++++++--------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/hb-ot-layout.hh b/src/hb-ot-layout.hh index 94414d3f2..7a787b77e 100644 --- a/src/hb-ot-layout.hh +++ b/src/hb-ot-layout.hh @@ -245,7 +245,7 @@ _hb_glyph_info_set_unicode_props (hb_glyph_info_t *info, hb_buffer_t *buffer) props |= UPROPS_MASK_HIDDEN; } } - else if (unlikely (HB_UNICODE_GENERAL_CATEGORY_IS_NON_ENCLOSING_MARK_OR_MODIFIER_SYMBOL (gen_cat))) + else if (unlikely (HB_UNICODE_GENERAL_CATEGORY_IS_NON_ENCLOSING_MARK (gen_cat))) { /* The above check is just an optimization to let in only things we need further * processing on. */ @@ -264,16 +264,6 @@ _hb_glyph_info_set_unicode_props (hb_glyph_info_t *info, hb_buffer_t *buffer) * the "else if". */ props |= unicode->modified_combining_class (u)<<8; - - /* Recategorize emoji skin-tone modifiers as Unicode mark, so they - * behave correctly in non-native directionality. They originally - * are MODIFIER_SYMBOL. Fixes: - * https://github.com/harfbuzz/harfbuzz/issues/169 - */ - if (unlikely (hb_in_range (u, 0x1F3FBu, 0x1F3FFu))) - { - props = gen_cat = HB_UNICODE_GENERAL_CATEGORY_ENCLOSING_MARK; - } } } diff --git a/src/hb-unicode.hh b/src/hb-unicode.hh index e6bca5f08..6ee4a3aed 100644 --- a/src/hb-unicode.hh +++ b/src/hb-unicode.hh @@ -102,23 +102,42 @@ HB_UNICODE_FUNCS_IMPLEMENT_CALLBACKS_SIMPLE } + inline hb_unicode_general_category_t + modified_general_category (hb_codepoint_t u) + { + hb_unicode_general_category_t cat = general_category (u); + + if (unlikely (cat == HB_UNICODE_GENERAL_CATEGORY_MODIFIER_SYMBOL)) + { + /* Recategorize emoji skin-tone modifiers as Unicode mark, so they + * behave correctly in non-native directionality. They originally + * are MODIFIER_SYMBOL. Fixes: + * https://github.com/harfbuzz/harfbuzz/issues/169 + */ + if (unlikely (hb_in_range (u, 0x1F3FBu, 0x1F3FFu))) + cat = HB_UNICODE_GENERAL_CATEGORY_ENCLOSING_MARK; + } + + return cat; + } + inline unsigned int - modified_combining_class (hb_codepoint_t unicode) + modified_combining_class (hb_codepoint_t u) { /* XXX This hack belongs to the Myanmar shaper. */ - if (unlikely (unicode == 0x1037u)) unicode = 0x103Au; + if (unlikely (u == 0x1037u)) u = 0x103Au; /* XXX This hack belongs to the USE shaper (for Tai Tham): * Reorder SAKOT to ensure it comes after any tone marks. */ - if (unlikely (unicode == 0x1A60u)) return 254; + if (unlikely (u == 0x1A60u)) return 254; /* XXX This hack belongs to the Tibetan shaper: * Reorder PADMA to ensure it comes after any vowel marks. */ - if (unlikely (unicode == 0x0FC6u)) return 254; + if (unlikely (u == 0x0FC6u)) return 254; /* Reorder TSA -PHRU to reorder before U+0F74 */ - if (unlikely (unicode == 0x0F39u)) return 127; + if (unlikely (u == 0x0F39u)) return 127; - return _hb_modified_combining_class[combining_class (unicode)]; + return _hb_modified_combining_class[combining_class (u)]; } static inline hb_bool_t @@ -360,10 +379,9 @@ DECLARE_NULL_INSTANCE (hb_unicode_funcs_t); FLAG (HB_UNICODE_GENERAL_CATEGORY_ENCLOSING_MARK) | \ FLAG (HB_UNICODE_GENERAL_CATEGORY_NON_SPACING_MARK))) -#define HB_UNICODE_GENERAL_CATEGORY_IS_NON_ENCLOSING_MARK_OR_MODIFIER_SYMBOL(gen_cat) \ +#define HB_UNICODE_GENERAL_CATEGORY_IS_NON_ENCLOSING_MARK(gen_cat) \ (FLAG_UNSAFE (gen_cat) & \ (FLAG (HB_UNICODE_GENERAL_CATEGORY_SPACING_MARK) | \ - FLAG (HB_UNICODE_GENERAL_CATEGORY_NON_SPACING_MARK) | \ - FLAG (HB_UNICODE_GENERAL_CATEGORY_MODIFIER_SYMBOL))) + FLAG (HB_UNICODE_GENERAL_CATEGORY_NON_SPACING_MARK))) #endif /* HB_UNICODE_HH */ From f8ccb545c47abe8f0f4ed318ff7b5bf176913893 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 14 Sep 2018 18:59:53 +0200 Subject: [PATCH 06/20] [dfont] Disable null-processsing for offsets An offset to unsized arrayis not safe to be redirected to our fixed-sized null pool. Plus, we want to reject, not repair, bad-looking dfonts. --- src/hb-open-file.hh | 8 ++++---- src/hb-open-type.hh | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index 38610a8ec..8772c79fa 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -348,7 +348,7 @@ struct ResourceTypeRecord protected: Tag tag; /* Resource type. */ HBUINT16 resCountM1; /* Number of resources minus 1. */ - OffsetTo > + OffsetTo, HBUINT16, false> resourcesZ; /* Offset from beginning of resource type list * to reference item list for this type. */ public: @@ -404,7 +404,7 @@ struct ResourceMap HBUINT32 reserved1; /* Reserved for handle to next resource map */ HBUINT16 resreved2; /* Reserved for file reference number */ HBUINT16 attrs; /* Resource fork attribute */ - OffsetTo > + OffsetTo, HBUINT16, false> typeList; /* Offset from beginning of map to * resource type list */ Offset16 nameList; /* Offset from beginning of map to @@ -436,10 +436,10 @@ struct ResourceForkHeader } protected: - LOffsetTo > + LOffsetTo, false> data; /* Offset from beginning of resource fork * to resource data */ - LOffsetTo + LOffsetTo map; /* Offset from beginning of resource fork * to resource map */ HBUINT32 dataLen; /* Length of resource data */ diff --git a/src/hb-open-type.hh b/src/hb-open-type.hh index 4f16c7d34..2e1e240c6 100644 --- a/src/hb-open-type.hh +++ b/src/hb-open-type.hh @@ -311,7 +311,7 @@ struct OffsetTo : Offset } DEFINE_SIZE_STATIC (sizeof(OffsetType)); }; -template struct LOffsetTo : OffsetTo {}; +template struct LOffsetTo : OffsetTo {}; template static inline const Type& operator + (const Base &base, const OffsetTo &offset) { return offset (base); } template From 29c2bd1795b933a611512af50a14f25e25d43159 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sat, 15 Sep 2018 14:47:18 +0200 Subject: [PATCH 07/20] [morx] Add stub for InsertionChain --- src/hb-aat-layout-morx-table.hh | 209 ++++++++++++++++++++++++++++++-- 1 file changed, 200 insertions(+), 9 deletions(-) diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index e4774c4c6..82e1d13ff 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -50,7 +50,8 @@ struct RearrangementSubtable struct driver_context_t { static const bool in_place = true; - enum Flags { + enum Flags + { MarkFirst = 0x8000, /* If set, make the current glyph the first * glyph to be rearranged. */ DontAdvance = 0x4000, /* If set, don't advance to the next glyph @@ -196,7 +197,8 @@ struct ContextualSubtable struct driver_context_t { static const bool in_place = true; - enum Flags { + enum Flags + { SetMark = 0x8000, /* If set, make the current glyph the marked glyph. */ DontAdvance = 0x4000, /* If set, don't advance to the next glyph before * going to the new state. */ @@ -329,7 +331,8 @@ struct LigatureSubtable struct driver_context_t { static const bool in_place = false; - enum Flags { + enum Flags + { SetComponent = 0x8000, /* Push this glyph onto the component stack for * eventual processing. */ DontAdvance = 0x4000, /* Leave the glyph pointer at this glyph for the @@ -338,7 +341,8 @@ struct LigatureSubtable * group. */ Reserved = 0x1FFF, /* These bits are reserved and should be set to 0. */ }; - enum LigActionFlags { + enum LigActionFlags + { LigActionLast = 0x80000000, /* This is the last action in the list. This also * implies storage. */ LigActionStore = 0x40000000, /* Store the ligature at the current cumulated index @@ -517,19 +521,205 @@ struct NoncontextualSubtable struct InsertionSubtable { + struct EntryData + { + HBUINT16 currentInsertIndex; /* Zero-based index into the insertion glyph table. + * The number of glyphs to be inserted is contained + * in the currentInsertCount field in the flags. + * A value of 0xFFFF indicates no insertion is to + * be done. */ + HBUINT16 markedInsertIndex; /* Zero-based index into the insertion glyph table. + * The number of glyphs to be inserted is contained + * in the markedInsertCount field in the flags. + * A value of 0xFFFF indicates no insertion is to + * be done. */ + public: + DEFINE_SIZE_STATIC (4); + }; + + struct driver_context_t + { + static const bool in_place = false; + enum Flags + { + SetMark = 0x8000, /* If set, mark the current glyph. */ + DontAdvance = 0x4000, /* If set, don't advance to the next glyph before + * going to the new state. This does not mean + * that the glyph pointed to is the same one as + * before. If you've made insertions immediately + * downstream of the current glyph, the next glyph + * processed would in fact be the first one + * inserted. */ + CurrentIsKashidaLike= 0x2000, /* If set, and the currentInsertList is nonzero, + * then the specified glyph list will be inserted + * as a kashida-like insertion, either before or + * after the current glyph (depending on the state + * of the currentInsertBefore flag). If clear, and + * the currentInsertList is nonzero, then the + * specified glyph list will be inserted as a + * split-vowel-like insertion, either before or + * after the current glyph (depending on the state + * of the currentInsertBefore flag). */ + MarkedIsKashidaLike= 0x1000, /* If set, and the markedInsertList is nonzero, + * then the specified glyph list will be inserted + * as a kashida-like insertion, either before or + * after the marked glyph (depending on the state + * of the markedInsertBefore flag). If clear, and + * the markedInsertList is nonzero, then the + * specified glyph list will be inserted as a + * split-vowel-like insertion, either before or + * after the marked glyph (depending on the state + * of the markedInsertBefore flag). */ + CurrentInsertBefore= 0x0800, /* If set, specifies that insertions are to be made + * to the left of the current glyph. If clear, + * they're made to the right of the current glyph. */ + MarkedInsertBefore= 0x0400, /* If set, specifies that insertions are to be + * made to the left of the marked glyph. If clear, + * they're made to the right of the marked glyph. */ + CurrentInsertCount= 0x3E0, /* This 5-bit field is treated as a count of the + * number of glyphs to insert at the current + * position. Since zero means no insertions, the + * largest number of insertions at any given + * current location is 31 glyphs. */ + MarkedInsertCount= 0x001F, /* This 5-bit field is treated as a count of the + * number of glyphs to insert at the marked + * position. Since zero means no insertions, the + * largest number of insertions at any given + * marked location is 31 glyphs. */ + }; + + inline driver_context_t (const InsertionSubtable *table, + hb_aat_apply_context_t *c_) : + ret (false), + c (c_), + mark_set (false), + mark (0), + insertionAction (table+table->insertionAction) {} + + inline bool is_actionable (StateTableDriver *driver, + const Entry *entry) + { + return (entry->flags & (CurrentInsertCount | MarkedInsertCount)) && + (entry->data.currentInsertIndex != 0xFFFF ||entry->data.markedInsertIndex != 0xFFFF); + } + inline bool transition (StateTableDriver *driver, + const Entry *entry) + { + hb_buffer_t *buffer = driver->buffer; + unsigned int flags = entry->flags; + +#if 0 + if (flags & SetComponent) + { + if (unlikely (match_length >= ARRAY_LENGTH (match_positions))) + return false; + + /* Never mark same index twice, in case DontAdvance was used... */ + if (match_length && match_positions[match_length - 1] == buffer->out_len) + match_length--; + + match_positions[match_length++] = buffer->out_len; + } + + if (flags & PerformAction) + { + unsigned int end = buffer->out_len; + unsigned int action_idx = entry->data.ligActionIndex; + unsigned int action; + unsigned int ligature_idx = 0; + do + { + if (unlikely (!match_length)) + return false; + + buffer->move_to (match_positions[--match_length]); + + const HBUINT32 &actionData = ligAction[action_idx]; + if (unlikely (!actionData.sanitize (&c->sanitizer))) return false; + action = actionData; + + uint32_t uoffset = action & LigActionOffset; + if (uoffset & 0x20000000) + uoffset += 0xC0000000; + int32_t offset = (int32_t) uoffset; + unsigned int component_idx = buffer->cur().codepoint + offset; + + const HBUINT16 &componentData = component[component_idx]; + if (unlikely (!componentData.sanitize (&c->sanitizer))) return false; + ligature_idx += componentData; + + if (action & (LigActionStore | LigActionLast)) + { + const GlyphID &ligatureData = ligature[ligature_idx]; + if (unlikely (!ligatureData.sanitize (&c->sanitizer))) return false; + hb_codepoint_t lig = ligatureData; + + match_positions[match_length++] = buffer->out_len; + buffer->replace_glyph (lig); + + //ligature_idx = 0; // XXX Yes or no? + } + else + { + buffer->skip_glyph (); + end--; + } + /* TODO merge_clusters / unsafe_to_break */ + + action_idx++; + } + while (!(action & LigActionLast)); + buffer->move_to (end); + } +#endif + + if (flags & SetMark) + { + mark_set = true; + mark = buffer->idx; + } + + + return true; + } + + public: + bool ret; + private: + hb_aat_apply_context_t *c; + bool mark_set; + unsigned int mark; + const UnsizedArrayOf &insertionAction; + }; + inline bool apply (hb_aat_apply_context_t *c) const { TRACE_APPLY (this); - /* TODO */ - return_trace (false); + + driver_context_t dc (this, c); + + StateTableDriver driver (machine, c->buffer, c->face); + driver.drive (&dc); + + return_trace (dc.ret); } inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - /* TODO */ - return_trace (true); + /* The rest of array sanitizations are done at run-time. */ + return_trace (c->check_struct (this) && machine.sanitize (c) && + insertionAction); } + + protected: + StateTable + machine; + LOffsetTo > + insertionAction; /* Byte offset from stateHeader to the start of + * the insertion glyph table. */ + public: + DEFINE_SIZE_STATIC (20); }; @@ -561,7 +751,8 @@ struct ChainSubtable inline unsigned int get_size (void) const { return length; } inline unsigned int get_type (void) const { return coverage & 0xFF; } - enum Type { + enum Type + { Rearrangement = 0, Contextual = 1, Ligature = 2, From 2f97da6e2d6629e112789d399765d90f96952c0a Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sat, 15 Sep 2018 14:51:50 +0200 Subject: [PATCH 08/20] [aat] Change version field --- src/hb-aat-layout-morx-table.hh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 82e1d13ff..52de7bcd0 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -831,7 +831,7 @@ struct Chain inline unsigned int get_size (void) const { return length; } - inline bool sanitize (hb_sanitize_context_t *c, unsigned int major) const + inline bool sanitize (hb_sanitize_context_t *c, unsigned int version) const { TRACE_SANITIZE (this); if (!length.sanitize (c) || @@ -862,7 +862,7 @@ struct Chain UnsizedArrayOf featureZ; /* Features. */ /*ChainSubtable firstSubtable;*//* Subtables. */ -/*subtableGlyphCoverageArray*/ /* Only if major == 3. */ +/*subtableGlyphCoverageArray*/ /* Only if version >= 3. We don't use. */ public: DEFINE_SIZE_MIN (16); @@ -892,8 +892,7 @@ struct morx inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - if (!version.sanitize (c) || - (version.major >> (sizeof (HBUINT32) == 4 ? 1 : 0)) != 1 || + if (!version.sanitize (c) || version < 2 || !chainCount.sanitize (c)) return_trace (false); @@ -901,7 +900,7 @@ struct morx unsigned int count = chainCount; for (unsigned int i = 0; i < count; i++) { - if (!chain->sanitize (c, version.major)) + if (!chain->sanitize (c, version)) return_trace (false); chain = &StructAfter (*chain); } @@ -910,8 +909,9 @@ struct morx } protected: - FixedVersion<>version; /* Version number of the glyph metamorphosis table. - * 1 for mort, 2 or 3 for morx. */ + HBUINT16 version; /* Version number of the glyph metamorphosis table. + * 2 or 3. */ + HBUINT16 unused; /* Set to 0. */ HBUINT32 chainCount; /* Number of metamorphosis chains contained in this * table. */ Chain firstChain; /* Chains. */ From 9ff76c6025b55d184c96b193f23aa935ab32f1fc Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sat, 15 Sep 2018 18:31:14 +0200 Subject: [PATCH 09/20] [morx] Respect default feature settings Does NOT apply user-selected features. But at least now enables correct subtables. --- src/hb-aat-layout-morx-table.hh | 36 ++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 52de7bcd0..c544cb376 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -760,11 +760,6 @@ struct ChainSubtable Insertion = 5 }; - inline void apply (hb_aat_apply_context_t *c) const - { - dispatch (c); - } - template inline typename context_t::return_t dispatch (context_t *c) const { @@ -810,21 +805,38 @@ struct Chain { inline void apply (hb_aat_apply_context_t *c) const { + uint32_t flags = defaultFlags; + { + /* Compute applicable flags. TODO Should move this to planning + * stage and take user-requested features into account. */ + unsigned int count = featureCount; + for (unsigned i = 0; i < count; i++) + { + const Feature &feature = featureZ[i]; + if (false) /* XXX Check if feature enabled... */ + { + flags &= feature.disableFlags; + flags |= feature.enableFlags; + } + } + } + const ChainSubtable *subtable = &StructAtOffset (&featureZ, featureZ[0].static_size * featureCount); unsigned int count = subtableCount; for (unsigned int i = 0; i < count; i++) { - if (!c->buffer->message (c->font, "start chain subtable %d", c->lookup_index)) - { - c->set_lookup_index (c->lookup_index + 1); - continue; - } + if (!(subtable->subFeatureFlags & flags)) + goto skip; - subtable->apply (c); - subtable = &StructAfter (*subtable); + if (!c->buffer->message (c->font, "start chain subtable %d", c->lookup_index)) + goto skip; + + subtable->dispatch (c); (void) c->buffer->message (c->font, "end chain subtable %d", c->lookup_index); + skip: + subtable = &StructAfter (*subtable); c->set_lookup_index (c->lookup_index + 1); } } From 10642b3fbfbc1776e784b190c43a9e0693dd423a Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sat, 15 Sep 2018 19:43:33 +0200 Subject: [PATCH 10/20] Disallow null-enabled offsets to unsized structures... ...like UnsizedArrayOf<>. This fixes a class of crasher bugs, mostly with color and AAT tables. We cannot use nullable offsets to varsized data that does not declare min_size, because it's nost safe to use our fixed-size null pool for types that have their size external. So, use non_null'able offsets for these. A further enhancement would be to make use of min_size in Null<> itself. Will try that after. --- src/hb-aat-layout-common.hh | 8 ++++---- src/hb-aat-layout-feat-table.hh | 2 +- src/hb-aat-layout-morx-table.hh | 10 +++++----- src/hb-aat-layout-trak-table.hh | 4 ++-- src/hb-aat-ltag-table.hh | 2 +- src/hb-open-type.hh | 5 +++++ src/hb-ot-color-cbdt-table.hh | 4 +--- src/hb-ot-color-colr-table.hh | 4 ++-- src/hb-ot-color-cpal-table.hh | 8 ++++---- src/hb-ot-color-svg-table.hh | 2 +- src/hb-ot-layout-common.hh | 18 ++++++++++-------- src/hb-ot-layout-jstf-table.hh | 4 ++-- 12 files changed, 38 insertions(+), 33 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index af71ebd38..57374b0bc 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -243,7 +243,7 @@ struct LookupSegmentArray GlyphID last; /* Last GlyphID in this segment */ GlyphID first; /* First GlyphID in this segment */ - OffsetTo > + OffsetTo, HBUINT16, false> valuesZ; /* A 16-bit offset from the start of * the table to the data. */ public: @@ -522,11 +522,11 @@ struct StateTable protected: HBUINT32 nClasses; /* Number of classes, which is the number of indices * in a single line in the state array. */ - LOffsetTo > + LOffsetTo, false> classTable; /* Offset to the class table. */ - LOffsetTo > + LOffsetTo, false> stateArrayTable;/* Offset to the state array. */ - LOffsetTo > > + LOffsetTo >, false> entryTable; /* Offset to the entry array. */ public: diff --git a/src/hb-aat-layout-feat-table.hh b/src/hb-aat-layout-feat-table.hh index b70076316..b670caab8 100644 --- a/src/hb-aat-layout-feat-table.hh +++ b/src/hb-aat-layout-feat-table.hh @@ -78,7 +78,7 @@ struct FeatureName protected: HBUINT16 feature; /* Feature type. */ HBUINT16 nSettings; /* The number of records in the setting name array. */ - LOffsetTo > + LOffsetTo, false> settingTable; /* Offset in bytes from the beginning of this table to * this feature's setting name array. The actual type of * record this offset refers to will depend on the diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index c544cb376..3bad5517f 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -311,7 +311,7 @@ struct ContextualSubtable protected: StateTable machine; - LOffsetTo, HBUINT32> > + LOffsetTo, HBUINT32>, false> substitutionTables; public: DEFINE_SIZE_STATIC (20); @@ -473,11 +473,11 @@ struct LigatureSubtable protected: StateTable machine; - LOffsetTo > + LOffsetTo, false> ligAction; /* Offset to the ligature action table. */ - LOffsetTo > + LOffsetTo, false> component; /* Offset to the component table. */ - LOffsetTo > + LOffsetTo, false> ligature; /* Offset to the actual ligature lists. */ public: DEFINE_SIZE_STATIC (28); @@ -715,7 +715,7 @@ struct InsertionSubtable protected: StateTable machine; - LOffsetTo > + LOffsetTo, false> insertionAction; /* Byte offset from stateHeader to the start of * the insertion glyph table. */ public: diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index c4bec2cab..3b7d43881 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -68,7 +68,7 @@ struct TrackTableEntry protected: Fixed track; /* Track value for this record. */ NameID trackNameID; /* The 'name' table index for this track */ - OffsetTo > + OffsetTo, HBUINT16, false> valuesZ; /* Offset from start of tracking table to * per-size tracking values for this track. */ @@ -134,7 +134,7 @@ struct TrackData protected: HBUINT16 nTracks; /* Number of separate tracks included in this table. */ HBUINT16 nSizes; /* Number of point sizes included in this table. */ - LOffsetTo > + LOffsetTo, false> sizeTable; /* Offset to array[nSizes] of size values. */ UnsizedArrayOf trackTable; /* Array[nTracks] of TrackTableEntry records. */ diff --git a/src/hb-aat-ltag-table.hh b/src/hb-aat-ltag-table.hh index e308ab28c..08a1b51a9 100644 --- a/src/hb-aat-ltag-table.hh +++ b/src/hb-aat-ltag-table.hh @@ -46,7 +46,7 @@ struct FTStringRange } protected: - OffsetTo > + OffsetTo, HBUINT16, false> tag; /* Offset from the start of the table to * the beginning of the string */ HBUINT16 length; /* String length (in bytes) */ diff --git a/src/hb-open-type.hh b/src/hb-open-type.hh index 2e1e240c6..beded06ce 100644 --- a/src/hb-open-type.hh +++ b/src/hb-open-type.hh @@ -226,9 +226,14 @@ struct FixedVersion * Use: (base+offset) */ +template struct assert_has_min_size { static_assert (Type::min_size > 0); }; +template struct assert_has_min_size {}; + template struct OffsetTo : Offset { + static_assert (sizeof (assert_has_min_size) || true); + inline const Type& operator () (const void *base) const { if (unlikely (this->is_null ())) return Null(Type); diff --git a/src/hb-ot-color-cbdt-table.hh b/src/hb-ot-color-cbdt-table.hh index 7c172ce43..561c59900 100644 --- a/src/hb-ot-color-cbdt-table.hh +++ b/src/hb-ot-color-cbdt-table.hh @@ -264,8 +264,6 @@ struct IndexSubtableArray protected: UnsizedArrayOf indexSubtablesZ; - public: - DEFINE_SIZE_ARRAY(0, indexSubtablesZ); }; struct BitmapSizeTable @@ -289,7 +287,7 @@ struct BitmapSizeTable } protected: - LOffsetTo + LOffsetTo indexSubtableArrayOffset; HBUINT32 indexTablesSize; HBUINT32 numberOfIndexSubtables; diff --git a/src/hb-ot-color-colr-table.hh b/src/hb-ot-color-colr-table.hh index 070226f18..a59d2bfa9 100644 --- a/src/hb-ot-color-colr-table.hh +++ b/src/hb-ot-color-colr-table.hh @@ -129,9 +129,9 @@ struct COLR protected: HBUINT16 version; /* Table version number */ HBUINT16 numBaseGlyphs; /* Number of Base Glyph Records */ - LOffsetTo > + LOffsetTo, false> baseGlyphsZ; /* Offset to Base Glyph records. */ - LOffsetTo > + LOffsetTo, false> layersZ; /* Offset to Layer Records */ HBUINT16 numLayers; /* Number of Layer Records */ public: diff --git a/src/hb-ot-color-cpal-table.hh b/src/hb-ot-color-cpal-table.hh index 2c9fced09..e354ced5c 100644 --- a/src/hb-ot-color-cpal-table.hh +++ b/src/hb-ot-color-cpal-table.hh @@ -118,15 +118,15 @@ struct CPALV1Tail } protected: - LOffsetTo > + LOffsetTo, false> paletteFlagsZ; /* Offset from the beginning of CPAL table to * the Palette Type Array. Set to 0 if no array * is provided. */ - LOffsetTo > + LOffsetTo, false> paletteLabelZ; /* Offset from the beginning of CPAL table to * the Palette Labels Array. Set to 0 if no * array is provided. */ - LOffsetTo > + LOffsetTo, false> paletteEntryLabelZ; /* Offset from the beginning of CPAL table to * the Palette Entry Label Array. Set to 0 * if no array is provided. */ @@ -207,7 +207,7 @@ struct CPAL HBUINT16 numPalettes; /* Number of palettes in the table. */ HBUINT16 numColorRecords; /* Total number of color records, combined for * all palettes. */ - LOffsetTo > + LOffsetTo, false> colorRecordsZ; /* Offset from the beginning of CPAL table to * the first ColorRecord. */ UnsizedArrayOf diff --git a/src/hb-ot-color-svg-table.hh b/src/hb-ot-color-svg-table.hh index ad9162f7c..53d466846 100644 --- a/src/hb-ot-color-svg-table.hh +++ b/src/hb-ot-color-svg-table.hh @@ -54,7 +54,7 @@ struct SVGDocumentIndexEntry * this index entry. */ HBUINT16 endGlyphID; /* The last glyph ID in the range described by * this index entry. Must be >= startGlyphID. */ - LOffsetTo > + LOffsetTo, false> svgDoc; /* Offset from the beginning of the SVG Document Index * to an SVG document. Must be non-zero. */ HBUINT32 svgDocLength; /* Length of the SVG document. diff --git a/src/hb-ot-layout-common.hh b/src/hb-ot-layout-common.hh index 8a3a703b3..e5e996d4d 100644 --- a/src/hb-ot-layout-common.hh +++ b/src/hb-ot-layout-common.hh @@ -70,6 +70,11 @@ namespace OT { * Script, ScriptList, LangSys, Feature, FeatureList, Lookup, LookupList */ +struct Record_sanitize_closure_t { + hb_tag_t tag; + const void *list_base; +}; + template struct Record { @@ -77,14 +82,10 @@ struct Record return tag.cmp (a); } - struct sanitize_closure_t { - hb_tag_t tag; - const void *list_base; - }; inline bool sanitize (hb_sanitize_context_t *c, const void *base) const { TRACE_SANITIZE (this); - const sanitize_closure_t closure = {tag, base}; + const Record_sanitize_closure_t closure = {tag, base}; return_trace (c->check_struct (this) && offset.sanitize (c, base, &closure)); } @@ -240,7 +241,7 @@ struct LangSys } inline bool sanitize (hb_sanitize_context_t *c, - const Record::sanitize_closure_t * = nullptr) const + const Record_sanitize_closure_t * = nullptr) const { TRACE_SANITIZE (this); return_trace (c->check_struct (this) && featureIndex.sanitize (c)); @@ -291,7 +292,7 @@ struct Script } inline bool sanitize (hb_sanitize_context_t *c, - const Record