more careful float -> int range checks
After upgrading my emscripten toolchain I got a warning that
INT_MAX can't be represented with a float, so the comparison
if (fbits > INT_MAX)
may not be doing the comparison we want. In fact, it was not...
we want to exclude any fbits value that's equal to (float)INT_MAX,
since that's actually greater than INT_MAX.
So add explicit casts here to let that new toolchain know we know
what we're doing, and count INT_MAX among the infinite. INT_MIN
is a power of two so it's exactly representable as a float and that
comparison works as we were intending.
We do seem to hit these cases outside fuzzers, but I think only
when trying to fit curves that we'll fail to fit anyway.
Change-Id: I427d7c8e1d5a6a13328859646dd8d178f0e8f647
Reviewed-on: https://skia-review.googlesource.com/c/skcms/+/237211
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/skcms.cc b/skcms.cc
index b52c53c..2a86753 100644
--- a/skcms.cc
+++ b/skcms.cc
@@ -1398,11 +1398,16 @@
float fbits = (1.0f * (1<<23)) * (x + 121.274057500f
- 1.490129070f*fract
+ 27.728023300f/(4.84252568f - fract));
- if (fbits > INT_MAX) {
+
+ // Before we cast fbits to int32_t, check for out of range values to pacify UBSAN.
+ // INT_MAX is not exactly representable as a float, so exclude it as effectively infinite.
+ // INT_MIN is a power of 2 and exactly representable as a float, so it's fine.
+ if (fbits >= (float)INT_MAX) {
return INFINITY_;
- } else if (fbits < INT_MIN) {
+ } else if (fbits < (float)INT_MIN) {
return -INFINITY_;
}
+
int32_t bits = (int32_t)fbits;
small_memcpy(&x, &bits, sizeof(x));
return x;
