clamp origin texel before narrowing to int in load_image_block#648
Open
sahvx655-wq wants to merge 1 commit into
Open
clamp origin texel before narrowing to int in load_image_block#648sahvx655-wq wants to merge 1 commit into
sahvx655-wq wants to merge 1 commit into
Conversation
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.
When compressing with the HDR-RGB/LDR-alpha profile, load_image_block builds the origin texel for each block and feeds blk.texel(0) straight into float_to_int() so it can be re-encoded through the LNS path. The RGB lanes have already passed through float_to_lns() and sit in [0, 65535], but the alpha lane takes the LDR unorm route and is just the input value scaled by 65535. A crafted HDR source (an .exr or .hdr carrying a large alpha) therefore presents a float well above INT_MAX to the scalar float_to_int(), which is undefined behaviour. UBSan flags it at astcenc_image.cpp:254 from an ordinary astcenc_compress_image() call: "3.9321e+09 is outside the range of representable values of type int".
The narrowed alpha lane is discarded by the later lns_mask select, so the value is never used for that channel, but the conversion still runs across all four lanes and trips the UB regardless. Clamping the vector into the valid LNS range [0, 65535] before the narrowing mirrors the clampzo guard this file already applies ahead of every other float_to_int/float_to_int_rtn, keeps the result bit-identical for in-range data, and folds NaN onto the low bound. Putting the clamp at the conversion site keeps the encoder sound whatever the image loaders hand it.