Guard exp2f_ to never return a negative value
Certain inputs could end up returning -NaN. That would then be used as
input to eval later, and the sign-stripping wouldn't work correctly, so
we'd pass -NaN to powf_, triggering the assert.
Mathematically, exp2f_ should never return a negative value, so clamp
any underflow result to 0.
Bug: chromium:996795
Change-Id: I39072265eb2e3239590c9928df3b68cd0bfa83a3
Reviewed-on: https://skia-review.googlesource.com/c/skcms/+/290767
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
diff --git a/profiles/fuzz/inf_a.icc.txt b/profiles/fuzz/inf_a.icc.txt
index 27ac089..890b29a 100644
--- a/profiles/fuzz/inf_a.icc.txt
+++ b/profiles/fuzz/inf_a.icc.txt
@@ -12,14 +12,14 @@
'kTRC' : 'curv' : 544 : 388
rTRC : 16-bit table with 257 entries
- ~= : 0.07488938, 0.007860422, -3.070464e-05, 0.1250019, 0.00390625, -0.1817474, 0 (Max error: 1181.7) (D-gap: 5.7213e-05) (f(1) = 0.513702)
+ ~= : 0.1091044, 0.001314126, -5.133305e-06, 0.1250019, 0.00390625, 0.0004806519, 0 (Max error: 756.926) (D-gap: -7.63685e-06) (f(1) = 0.48514)
gTRC : 16-bit table with 257 entries
- ~= : 0.07488938, 0.007860422, -3.070464e-05, 0.1250019, 0.00390625, -0.1817474, 0 (Max error: 1181.7) (D-gap: 5.7213e-05) (f(1) = 0.513702)
+ ~= : 0.1091044, 0.001314126, -5.133305e-06, 0.1250019, 0.00390625, 0.0004806519, 0 (Max error: 756.926) (D-gap: -7.63685e-06) (f(1) = 0.48514)
bTRC : 16-bit table with 257 entries
- ~= : 0.07488938, 0.007860422, -3.070464e-05, 0.1250019, 0.00390625, -0.1817474, 0 (Max error: 1181.7) (D-gap: 5.7213e-05) (f(1) = 0.513702)
-Best : 0.07488938, 0.007860422, -3.070464e-05, 0.1250019, 0.00390625, -0.1817474, 0 (D-gap: 5.7213e-05) (f(1) = 0.513702)
-Inv : 13.35303, 1.437515, 0.2612647, 7.999878, 0.0004882887, 0.003726959, -0 (D-gap: -0.000179274) (f(1) = 1181.82)
-Best Error: | 1181.7 1181.7 1181.7 |
+ ~= : 0.1091044, 0.001314126, -5.133305e-06, 0.1250019, 0.00390625, 0.0004806519, 0 (Max error: 756.926) (D-gap: -7.63685e-06) (f(1) = 0.48514)
+Best : 0.1091044, 0.001314126, -5.133305e-06, 0.1250019, 0.00390625, 0.0004806519, 0 (D-gap: -7.63685e-06) (f(1) = 0.48514)
+Inv : 9.165529, 2.062317, -0.0009912564, 7.999878, 0.0004882887, 0.004421234, -0 (D-gap: 0.000514984) (f(1) = 757.043)
+Best Error: | 756.926 756.926 756.926 |
XYZ : | 0.9614257 0. 0. |
| 0. 1.0004882 0. |
| 0. 0. 0.82470703 |
diff --git a/profiles/fuzz/large_g.icc b/profiles/fuzz/large_g.icc
new file mode 100644
index 0000000..8f1c1f6
--- /dev/null
+++ b/profiles/fuzz/large_g.icc
Binary files differ
diff --git a/profiles/fuzz/large_g.icc.txt b/profiles/fuzz/large_g.icc.txt
new file mode 100644
index 0000000..8f10bd1
--- /dev/null
+++ b/profiles/fuzz/large_g.icc.txt
@@ -0,0 +1,42 @@
+ Size : 0x00000203 : 515
+ Data color space : 0x47524159 : 'GRAY'
+ PCS : 0x58595A20 : 'XYZ '
+ Tag count : 0x00000002 : 2
+
+ Tag : Type : Size : Offset
+ ------ : ------ : ------ : --------
+ 'kTRC' : 'para' : 110 : 276
+ 'A3B1' : 'mAB ' : 220 : 280
+
+rTRC : 1027, 1, 0, 0, 0, 0, 0 (f(1) = 1)
+gTRC : 1027, 1, 0, 0, 0, 0, 0 (f(1) = 1)
+bTRC : 1027, 1, 0, 0, 0, 0, 0 (f(1) = 1)
+Best : 1027, 1, 0, 0, 0, 0, 0 (f(1) = 1)
+Inv : 0.0009737099, 1, -0, 0, 0, 0, 0 (f(1) = 1)
+Best Error: | 0.917647 0.917647 0.917647 |
+ XYZ : | 0.9642028 0. 0. |
+ | 0. 1. 0. |
+ | 0. 0. 0.8249054 |
+252 random bytes transformed to linear XYZD50 bytes:
+ 000000 000000 000000 000000 000000 000000 000000
+ 000000 000000 000000 000000 000000 000000 000000
+ 000000 000000 000000 000000 000000 000000 000000
+ 000000 000000 00ff00 000000 000000 000000 000000
+ 000000 000000 000000 000000 000400 000000 000000
+ 000000 000000 000000 000000 000000 000000 000000
+ 000000 000000 000000 000000 000000 000000 000000
+ 000000 000000 000000 000000 000000 000000 000000
+ 000000 000000 000000 000000 000000 000000 000000
+ 000000 000000 000000 000000 000000 000000 000000
+ 000000 000000 000000 000000 000000 000000 000000
+ 000000 000000 000000 000000 000000 000000 000000
+81 edge-case pixels transformed to sRGB 8888 (unpremul):
+ 00000000 00000000 004b00ff 00000000 00000000 004b00ff 0000ff00 0000ff00 0000fcff
+ 00000000 00000000 004b00ff 00000000 00000000 004b00ff 0000ff00 0000ff00 0000fcff
+ 00ff2e00 00ff2e00 00ff00ff 00ff2e00 00ff2e00 00ff00ff 00f7ff00 00f7ff00 00ffffff
+ 7f000000 7f000000 7f4b00ff 7f000000 7f000000 7f4b00ff 7f00ff00 7f00ff00 7f00fcff
+ 7f000000 7f000000 7f4b00ff 7f000000 7f000000 7f4b00ff 7f00ff00 7f00ff00 7f00fcff
+ 7fff2e00 7fff2e00 7fff00ff 7fff2e00 7fff2e00 7fff00ff 7ff7ff00 7ff7ff00 7fffffff
+ ff000000 ff000000 ff4b00ff ff000000 ff000000 ff4b00ff ff00ff00 ff00ff00 ff00fcff
+ ff000000 ff000000 ff4b00ff ff000000 ff000000 ff4b00ff ff00ff00 ff00ff00 ff00fcff
+ ffff2e00 ffff2e00 ffff00ff ffff2e00 ffff2e00 ffff00ff fff7ff00 fff7ff00 ffffffff
diff --git a/skcms.cc b/skcms.cc
index ae5ba4f..5d6ae3d 100644
--- a/skcms.cc
+++ b/skcms.cc
@@ -84,11 +84,12 @@
// 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.
+ // Negative values are effectively underflow - we'll end up returning a (different) negative
+ // value, which makes no sense. So clamp to zero.
if (fbits >= (float)INT_MAX) {
return INFINITY_;
- } else if (fbits < (float)INT_MIN) {
- return -INFINITY_;
+ } else if (fbits < 0) {
+ return 0;
}
int32_t bits = (int32_t)fbits;
diff --git a/src/Transform_inl.h b/src/Transform_inl.h
index c4b3122..2dcf717 100644
--- a/src/Transform_inl.h
+++ b/src/Transform_inl.h
@@ -287,9 +287,11 @@
#else
F fract = x - floor_(x);
- I32 bits = cast<I32>((1.0f * (1<<23)) * (x + 121.274057500f
- - 1.490129070f*fract
- + 27.728023300f/(4.84252568f - fract)));
+ F fbits = (1.0f * (1<<23)) * (x + 121.274057500f
+ - 1.490129070f*fract
+ + 27.728023300f/(4.84252568f - fract));
+ I32 bits = cast<I32>(max_(fbits, F0));
+
return bit_pun<F>(bits);
#endif
}
diff --git a/tests.c b/tests.c
index 4bb2c46..aed3389 100644
--- a/tests.c
+++ b/tests.c
@@ -700,6 +700,9 @@
"profiles/fuzz/direct_fit_not_invertible.icc", // oss-fuzz:19341
"profiles/fuzz/direct_fit_negative_a.icc", // oss-fuzz:19467
+
+ // g = 1027 -> -nan from exp2f_, sign-strip doesn't work, leading to powf_ assert
+ "profiles/fuzz/large_g.icc", // chromium:996795
};
static void test_Parse(bool regen) {
