Repository navigation
[GPOS] Clamp anchor coordinates when converting them from float - #6587
Merged
Merged
Conversation
Anchor coordinates are scaled as float, and apply_cursive_attachment() and the mark attachment code converted them to hb_position_t with a plain cast or an implicit conversion. A value out of range for the target type makes that conversion undefined; on x86 it gives INT32_MIN. harfbuzz#6154 made the additions around these conversions saturate, but not the conversions themselves. With NotoNastaliqUrdu-Regular.ttf at a scale of INT32_MAX, a cursive anchor scales to 2.23e9, and the glyph ended up with an x_advance of -2147483648. Convert with hb_clamp_to(), and negate the cursive cross-direction offsets with hb_saturate_neg(), since they can now be INT32_MIN. Results that fit do not change. Tested with test-shape under UBSan: the new test fails on the base commit at CursivePosFormat1.hh:141 and passes here. No test reaches the two conversions in MarkArray.hh; they are changed to match. Assisted-by: Claude
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Anchor coordinates are scaled as float, and the cursive and mark attachment code converted them to hb_position_t with a plain cast or an implicit conversion. A value out of the target type's range makes that conversion undefined; on x86 it produces INT32_MIN. #6154 made the additions around these conversions saturate, but not the conversions themselves.
With NotoNastaliqUrdu-Regular.ttf at a scale of INT32_MAX, a cursive anchor scales to 2.23e9 and the glyph ends up with an x_advance of -2147483648.
This converts with hb_clamp_to(), and negates the cursive cross-direction offsets with hb_saturate_neg(), since they can now be INT32_MIN. Results that fit in range do not change.
Testing (Linux; the branch is already on top of current main):
meson test -C build: 270 ok, 0 fail, 7 skipped (subset+repack, the same skips as unrelated runs on main).Not covered: no test reaches the two conversions in MarkArray.hh (mark.x_offset, mark.y_offset); they are changed to match the cursive ones, as the commit message also notes.
AI disclosure: this change was prepared with AI assistance, as the commit's Assisted-by trailer says. I have read the full diff and run the tests above myself before submitting.