Fix integer overflows in ClipSrcRect
While blitting, it's possible to generate a float that is outside the
representable values of an integer. Since converting such a float
results in undefined behavior, we instead throw an invalid operation
error and return without continuing the blit.
Bug chromium:1001874
Change-Id: Ic6938adf75176e34021d0ca1404176e4979a3ca6
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/36408
Reviewed-by: Alexis Hétu <sugoi@google.com>
Tested-by: Sean Risser <srisser@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
diff --git a/src/OpenGL/libGLESv2/Device.cpp b/src/OpenGL/libGLESv2/Device.cpp
index c70da83..0758428 100644
--- a/src/OpenGL/libGLESv2/Device.cpp
+++ b/src/OpenGL/libGLESv2/Device.cpp
@@ -979,7 +979,7 @@
{
float ratio = static_cast<float>(dstRect.width()) / srcRect.width();
float offsetf = roundf((static_cast<float>(clipRect.x0) - srcRect.x0) * ratio);
- if (!std::isfinite(offsetf) || !std::isfinite(ratio))
+ if (!FloatFitsInInt(offsetf) || !std::isfinite(ratio))
{
return false;
}
@@ -998,7 +998,7 @@
{
float ratio = static_cast<float>(dstRect.width()) / srcRect.width();
float offsetf = roundf((srcRect.x1 - static_cast<float>(clipRect.x1)) * ratio);
- if (!std::isfinite(offsetf) || !std::isfinite(ratio))
+ if (!FloatFitsInInt(offsetf) || !std::isfinite(ratio))
{
return false;
}
@@ -1017,7 +1017,7 @@
{
float ratio = static_cast<float>(dstRect.height()) / srcRect.height();
float offsetf = roundf((static_cast<float>(clipRect.y0) - srcRect.y0) * ratio);
- if (!std::isfinite(offsetf) || !std::isfinite(ratio))
+ if (!FloatFitsInInt(offsetf) || !std::isfinite(ratio))
{
return false;
}
@@ -1036,7 +1036,7 @@
{
float ratio = static_cast<float>(dstRect.height()) / srcRect.height();
float offsetf = roundf((srcRect.y1 - static_cast<float>(clipRect.y1)) * ratio);
- if (!std::isfinite(offsetf) || !std::isfinite(ratio))
+ if (!FloatFitsInInt(offsetf) || !std::isfinite(ratio))
{
return false;
}
diff --git a/src/OpenGL/libGLESv2/utilities.cpp b/src/OpenGL/libGLESv2/utilities.cpp
index 96510e7..b8c91be 100644
--- a/src/OpenGL/libGLESv2/utilities.cpp
+++ b/src/OpenGL/libGLESv2/utilities.cpp
@@ -2070,6 +2070,22 @@
return name.substr(0, open);
}
+
+ bool FloatFitsInInt(float f)
+ {
+ // We can't just do a raw comparison of "f > (float) INT32_MAX",
+ // because "(float) INT32_MAX" is unrepresentable as an integer.
+ //
+ // So instead I subtracted an ULP from "(float) INT32_MAX", cast that
+ // to an int, and do the comparison with that value. That value is
+ // 2147483520, and can be found with the following code:
+ // float f_max = static_cast<float>(INT32_MAX);
+ // int32_t f_bits = *static_cast<int32_t *>((void *)&f_max);
+ // f_bits -= 1;
+ // float f_next = *static_cast<float *>((void *)&f_bits);
+ // int32_t out = static_cast<int32_t>(f_next);
+ return std::isfinite(f) && (-2147483520.f < f) && (f < 2147483520.f);
+ }
}
namespace es2sw
@@ -2401,4 +2417,4 @@
return GL_DEPTH24_STENCIL8_OES;
}
-}
\ No newline at end of file
+}
diff --git a/src/OpenGL/libGLESv2/utilities.h b/src/OpenGL/libGLESv2/utilities.h
index 187ef13..d469d74 100644
--- a/src/OpenGL/libGLESv2/utilities.h
+++ b/src/OpenGL/libGLESv2/utilities.h
@@ -86,6 +86,8 @@
// Parse the base uniform name and array index. Returns the base name of the uniform. outSubscript is
// set to GL_INVALID_INDEX if the provided name is not an array or the array index is invalid.
std::string ParseUniformName(const std::string &name, unsigned int *outSubscript);
+
+ bool FloatFitsInInt(float f);
}
namespace es2sw
diff --git a/tests/GLESUnitTests/unittests.cpp b/tests/GLESUnitTests/unittests.cpp
index bfd2112..f4e1a86 100644
--- a/tests/GLESUnitTests/unittests.cpp
+++ b/tests/GLESUnitTests/unittests.cpp
@@ -1764,6 +1764,7 @@
const int small = 200;
const int neg_small = -small;
const int neg_big = -big;
+ int max = 0x7fffffff;
int data[][8] = {
// sx0, sy0, sx1, sy1, dx0, dy0, dx1, dy1
{0, 0, 0, 0, 0, 0, 0, 0},
@@ -1776,7 +1777,14 @@
{neg_small, small, neg_small, neg_small, neg_small, big, small},
{big, big-1, big-2, big-3, big-4, big-5, big-6, big-7},
{big, neg_big, neg_big, big, small, big, 0, neg_small},
- {323479648, 21931, 1769809195, 32733, 0, 0, -161640504, 32766}
+ {323479648, 21931, 1769809195, 32733, 0, 0, -161640504, 32766},
+ {0, 0, max, max, 0, 0, 8, 8},
+ {0, 0, 8, 8, 0, 0, max, max},
+ {0, 0, max, max, 0, 0, max, max},
+ {-1, -1, max, max, 0, 0, 8, 8},
+ {0, 0, 8, 8, -1, -1, max, max},
+ {-1, -1, max, max, -1, -1, max, max},
+ {-max-1, -max-1, max, max, -max-1, -max-1, max, max}
};
for (int i = 0; i < (int) (sizeof(data)/sizeof(data[0])); i++)
