Fix UBSan errors #1948
Fix UBSan errors #1948
Conversation
By using saturated casts for the int types (copied from vips_cast).
229d23a to
1a6854a
Compare
|
I think the above endnotes have been resolved.
This is ready for review now. |
|
This looks great Kleis. The flatten change is a shame -- the casts will be quite a bit slower. The overflows only happen if there is a mismatch between the expected range of the alpha (0-255, or 0-65535) and the available range of the numeric type, which will be a rare case, I think. For example, if the interpretation is srgb (0-255 alpha), but it's a ushort image (pixels can have much higher values). How about detecting mismatch, and if that happens, flattening to float and then calling vips_clip() to go back to the original type? It should fix the error, and we won't lose speed, except in these rare cases. |
I could no longer reproduce this with clang 12 locally.
|
Oh huh, I could no longer reproduce the flatten change locally with clang 12. But CI doesn't seem to pass when reverting this. |
|
OK, shall we merge? I could make another PR with a possible flatten fix after this, if you like. |
|
Feel free to merge, another PR to fix that error in flatten sounds good to me. FWIW, I'm testing locally with clang 12.0.1. CI seems to be testing with version 12.0.0. |
|
OK, let's do it. Well done for crunching through the fuzzer results. |
|
I made an issue with a reproducer for the flatten error. I'll have a go at a PR. |
As discussed in #1727 (comment). Commit 8113231 fixes this error1:
https://travis-ci.org/github/kleisauke/libvips/jobs/709752013#L2599
Commit 1dd6698 fixes this error:
https://travis-ci.org/github/kleisauke/libvips/jobs/709752013#L2315
Commit 1a6854a fixes this error2:
https://travis-ci.org/github/kleisauke/libvips/jobs/709752013#L2270
And the last commit (b8f18e1) fixes this error:
https://github.com/libvips/libvips/runs/3554490177#step:13:366
Marked this PR as draft due to these endnotes:
vips_mosaicis fuzzed (i.e. by separating the fuzzer parameters from the image), so perhaps we need to regenerate the existing corpus?vips_cast, I'm not sure if there's a better way to fix this?