Avoid integer overflows with correct signing (tracking HB) and assertions

[alerque]

See discussion in issue #142, currently known to be incomplete and untested. The lack of test cases that cover the original code made a POC hard to work on.

Closes #142.

[alerque]

After some more fiddling I ruled out most of the other potential instances of subtractions from usize that could wrap (as happened #142) being reachable in actual use, but I wasn't able to easily rule out these cases being theoretically possible with a buggy font. It doesn't mean they are definitely reachable cases, just that it isn't obvious to me what keeps the rhs from being greater than the lhs.

It might be better to fix these with an assert() of some kind to make sure they do not saturate in practice, because clamping the result is potentially going to do something different than wrapping and potentially break worse than expected.

[behdad]

See #142 (comment)

[LaurenzV]

end is always guaranteed to be positive here?

[alerque]

Good question. At first blush it looks like on this point it is a bug-for-bug port of HB. The upstream function takes an unsigned integer argument:

bool
hb_buffer_t::move_to (unsigned int i)

But it is being passed a signed integer:

int end;
...
(void) buffer->move_to (end);

I don't know if/how C resolves this. @behdad?

At some point it looks like there is a guarantee that it will be at least as large as...

if (end < int (match_positions[idx]))
{
  end = match_positions[idx];

[behdad]

Yes, end is also always positive because there's at least one glyph in the match sequence... You can assert as you cast it.

Would you be able to send a PR to HB to add similar asserts?

[alerque]

I added an assertion for this case too (via Rust's TryInto trait since we aren't enforcing any context specific bounds, just making sure the cast can't loose information) and also opened a matching PR on HB.

[LaurenzV]

I didn't compare it with the HB code, but seems fine. 😄