Fixes https://github.com/harfbuzz/harfbuzz/issues/4020
This is a hack.
To implement this properly we need to treat runs with different features
as independent runs for running the state machine, as the subtable flags
might be different. That would be a significant change to our internal
implementation.
Previously, some bad font data was accidentally being interpretted as
legit if it happened to not fall out of memory bounds. The intention
of the code was what this commit does. I'm surprised we weren't getting
a "arithmetic between signed and unsigned values" warning / error
before.
Say for USHORT, we were implementing casts from and to unsigned.
With this change, we cast from and to uint16_t only. This allows
compiler more opportunities to catch possible narrowing issues in
the code.
It needed a couple of fixes in the codebase though, because
previously, if a USHORT was participating in arithmetic with signed
numbers, eg. "u + 1", the result would have been unsigned. With
this change, it would be signed. The correct fix is to update the
code to read "u + 1u".
That said, I think about conditionally adding back the cast
out to signed/unsigned, to facilitate better type deduction.
But I couldn't think of a real situation where that would help
with anything. So I didn't add. Here's what it was:
template <typename Type2 = hb_conditional<hb_is_signed (Type), signed, unsigned>,
hb_enable_if (sizeof (Type) < sizeof (Type2))>
operator hb_type_identity_t<Type2> () const { return v; }
https://github.com/harfbuzz/harfbuzz/pull/2875
Some compilers don't like this:
../src/hb-aat-layout-common.hh:732:9: error: declaration of 'using StateTable = struct AAT::StateTable<Types, EntryData>' changes meaning of 'StateTable' [-fpermissive]
732 | using StateTable = StateTable<Types, EntryData>;
The state we are dealing with here is the previous state; so it should
cause unsafe_to_break before current glyph.
I'm surprised this wasn't caught by any tests. Guess we don't have any
fonts with fancy end-of-text forms.
Priority should be given to specific over dispatch. Broke sanitize before.
This fixes it, by moving prioritization to the context implementation, since
the correct priority cannot be done in the dispatch implementation. Done
for subset and sanitize only, which need it.
This allows partial-instantiating custom Null object for template Lookup<T>.
Before, this had to be handcoded per instantiation. Apparently I missed
adding one for AAT::ankr.lookupTable, so it was getting the wrong (generic)
null for Lookup object, which is wrong and unsafe.
Fixes https://bugs.chromium.org/p/chromium/issues/detail?id=944346
Behdad Esfahbod
Jonathan Kew
Ebrahim Byagowi
Behdad Esfahbod