Without it, the compiler was reordering and batching the read
of array length and array[0] if the 0'th member was accessed
constantly and function was inlined. This felt safe to the
compiler because HB_VAR_ARRAY is 1, but could be unsafe actually.
The memory barrier disallows that.
This was found by afl/honggfuzz address sanitizers.
Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=49187
- Rename enum type and enum members.
- in_errors() now returns true for any error having been set. hb-subset now looks for offset overflow only errors to divert to repacker.
- Added INT_OVERFLOW and ARRAY_OVERFLOW enum values.
Previous commit didn't fix the bots. Putting it back now that I
understand why I initially did the "Wide" casts. But only doing
it for out-cast this time. This causes "narrowing" warnings
whenever we are converting signed/unsigned to smaller HBUINT16
etc. But those are valuable warnings. We should address those
separately instead of ignoring.
Maybe we should start using uint16_t more liberally in the
internal subsetter function signatures then.
My local clang12 is fine, but many bots are not:
../src/hb-ot-cff1-table.hh: In instantiation of ‘bool CFF::Charset1_2<TYPE>::sanitize(hb_sanitize_context_t*, unsigned int) const [with TYPE = OT::IntType<unsigned char>]’:
../src/hb-ot-cff1-table.hh:554:13: required from here
../src/hb-ot-cff1-table.hh:377:60: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
if (unlikely (!ranges[i].sanitize (c) || (num_glyphs < ranges[i].nLeft + 1)))
~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
Enabling the extra cast operator mentioned in previous commit to see if
that fixes this case.
Again, I'd be happy to say "use 1u instead of 1" if this was universally
erred on. But since some compilers happily compile this while others
err, it would be a huge headache. Let's see...
https://github.com/harfbuzz/harfbuzz/pull/2875
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
Behdad Esfahbod
Thomas Devoogdt
Garret Rieger
Ebrahim Byagowi