From 006386be3a069199ebaf22bcc55fa7233c62e0d5 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 18:04:53 -0500 Subject: [PATCH] [kern] Implement negative state numbers Let the fuzzing bots rip this code apart... --- src/hb-aat-layout-common.hh | 90 ++++++++++++++++++++++++--------- src/hb-aat-layout-kerx-table.hh | 10 ---- 2 files changed, 66 insertions(+), 34 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index 6efc99aa4..6a16d70c4 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -430,8 +430,8 @@ struct StateTable CLASS_END_OF_LINE = 3, }; - inline unsigned int new_state (unsigned int newState) const - { return Types::extended ? newState : (newState - stateArrayTable) / nClasses; } + inline int new_state (unsigned int newState) const + { return Types::extended ? newState : ((int) newState - (int) stateArrayTable) / nClasses; } inline unsigned int get_class (hb_codepoint_t glyph_id, unsigned int num_glyphs) const { @@ -444,7 +444,7 @@ struct StateTable return (this+entryTable).arrayZ; } - inline const Entry *get_entryZ (unsigned int state, unsigned int klass) const + inline const Entry *get_entryZ (int state, unsigned int klass) const { if (unlikely (klass >= nClasses)) return nullptr; @@ -452,7 +452,7 @@ struct StateTable const Entry *entries = (this+entryTable).arrayZ; unsigned int entry = states[state * nClasses + klass]; - DEBUG_MSG (APPLY, nullptr, "e%d", entry); + DEBUG_MSG (APPLY, nullptr, "e%u", entry); return &entries[entry]; } @@ -468,28 +468,69 @@ struct StateTable const Entry *entries = (this+entryTable).arrayZ; unsigned int num_classes = nClasses; + if (unlikely (hb_unsigned_mul_overflows (num_classes, states[0].static_size))) + return_trace (false); + unsigned int row_stride = num_classes * states[0].static_size; - unsigned int num_states = 1; + /* Apple 'kern' table has this peculiarity: + * + * "Because the stateTableOffset in the state table header is (strictly + * speaking) redundant, some 'kern' tables use it to record an initial + * state where that should not be StartOfText. To determine if this is + * done, calculate what the stateTableOffset should be. If it's different + * from the actual stateTableOffset, use it as the initial state." + * + * We implement this by calling the initial state zero, but allow *negative* + * states if the start state indeed was not the first state. Since the code + * is shared, this will also apply to 'mort' table. The 'kerx' / 'morx' + * tables are not affected since those address states by index, not offset. + */ + + int min_state = 0; + int max_state = 0; unsigned int num_entries = 0; - unsigned int state = 0; + int state_pos = 0; + int state_neg = 0; unsigned int entry = 0; - while (state < num_states) + while (min_state < state_neg || state_pos <= max_state) { - if (unlikely (hb_unsigned_mul_overflows (num_classes, states[0].static_size))) - return_trace (false); + if (min_state < state_neg) + { + /* Negative states. */ + if (unlikely (hb_unsigned_mul_overflows (min_state, num_classes))) + return_trace (false); + if (unlikely (!c->check_array (&states[min_state * num_classes], -min_state, row_stride))) + return_trace (false); + if ((c->max_ops -= state_neg - min_state) < 0) + return_trace (false); + { /* Sweep new states. */ + const HBUSHORT *stop = &states[min_state * num_classes]; + if (unlikely (stop > states)) + return_trace (false); + for (const HBUSHORT *p = states; stop < p; p--) + num_entries = MAX (num_entries, *(p - 1) + 1); + state_neg = min_state; + } + } - if (unlikely (!c->check_array (states, - num_states, - num_classes * states[0].static_size))) - return_trace (false); - if ((c->max_ops -= num_states - state) < 0) - return_trace (false); - { /* Sweep new states. */ - const HBUSHORT *stop = &states[num_states * num_classes]; - for (const HBUSHORT *p = &states[state * num_classes]; p < stop; p++) - num_entries = MAX (num_entries, *p + 1); - state = num_states; + if (state_pos <= max_state) + { + /* Positive states. */ + if (unlikely (!c->check_array (states, max_state + 1, row_stride))) + return_trace (false); + if ((c->max_ops -= max_state - state_pos + 1) < 0) + return_trace (false); + { /* Sweep new states. */ + if (unlikely (hb_unsigned_mul_overflows ((max_state + 1), num_classes))) + return_trace (false); + const HBUSHORT *stop = &states[(max_state + 1) * num_classes]; + if (unlikely (stop < states)) + return_trace (false); + for (const HBUSHORT *p = &states[state_pos * num_classes]; p < stop; p++) + num_entries = MAX (num_entries, *p + 1); + state_pos = max_state + 1; + } } if (unlikely (!c->check_array (entries, num_entries))) @@ -500,8 +541,9 @@ struct StateTable const Entry *stop = &entries[num_entries]; for (const Entry *p = &entries[entry]; p < stop; p++) { - unsigned int newState = new_state (p->newState); - num_states = MAX (num_states, newState + 1); + int newState = new_state (p->newState); + min_state = MIN (min_state, newState); + max_state = MAX (max_state, newState); } entry = num_entries; } @@ -623,14 +665,14 @@ struct StateTableDriver if (!c->in_place) buffer->clear_output (); - unsigned int state = StateTable::STATE_START_OF_TEXT; + int state = StateTable::STATE_START_OF_TEXT; bool last_was_dont_advance = false; for (buffer->idx = 0; buffer->successful;) { unsigned int klass = buffer->idx < buffer->len ? machine.get_class (buffer->info[buffer->idx].codepoint, num_glyphs) : (unsigned) StateTable::CLASS_END_OF_TEXT; - DEBUG_MSG (APPLY, nullptr, "c%d at %d", klass, buffer->idx); + DEBUG_MSG (APPLY, nullptr, "c%u at %u", klass, buffer->idx); const Entry *entry = machine.get_entryZ (state, klass); if (unlikely (!entry)) break; diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 272ebfd95..cc8281491 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -238,16 +238,6 @@ struct KerxSubTableFormat1 depth (0), crossStream (table->header.coverage & table->header.CrossStream) {} - /* TODO - * 'kern' table has this pecularity, we don't currently implement. - * - * "Because the stateTableOffset in the state table header is (strictly - * speaking) redundant, some 'kern' tables use it to record an initial - * state where that should not be StartOfText. To determine if this is - * done, calculate what the stateTableOffset should be. If it's different - * from the actual stateTableOffset, use it as the initial state." - */ - inline bool is_actionable (StateTableDriver *driver HB_UNUSED, const Entry *entry) {