Make ClipDstRect/ClipSrcRect cease on non-finite
Now that the GLES unit tests can cause the float calculations in
these functions to result in inf and NaN, some platforms are throwing
errors when those floats are converted to integers. The Clip*Rect
functions no longer continue processing if any operation results in a
non-finite value, and tell their callers that they have failed to clip
properly.
Make Context.cpp set the error state to GL_INVALID_OPERATION when it
fails to blit.
Bug chromium:979986
Change-Id: I8e11e77b1e25eab37dee97aa616a908737b16036
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/35269
Presubmit-Ready: Sean Risser <srisser@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Tested-by: Sean Risser <srisser@google.com>
diff --git a/src/OpenGL/libGLESv2/Context.cpp b/src/OpenGL/libGLESv2/Context.cpp
index 9047cb6..1954a34 100644
--- a/src/OpenGL/libGLESv2/Context.cpp
+++ b/src/OpenGL/libGLESv2/Context.cpp
@@ -4146,7 +4146,11 @@
if(mState.scissorTestEnabled) // Only write to parts of the destination framebuffer which pass the scissor test
{
sw::Rect scissorRect(mState.scissorX, mState.scissorY, mState.scissorX + mState.scissorWidth, mState.scissorY + mState.scissorHeight);
- Device::ClipDstRect(sourceScissoredRect, destScissoredRect, scissorRect, flipX, flipY);
+ if (!Device::ClipDstRect(sourceScissoredRect, destScissoredRect, scissorRect, flipX, flipY))
+ {
+ // Failed to clip, blitting can't happen.
+ return error(GL_INVALID_OPERATION);
+ }
}
sw::SliceRectF sourceTrimmedRect = sourceScissoredRect;
@@ -4155,10 +4159,18 @@
// The source & destination rectangles also may need to be trimmed if
// they fall out of the bounds of the actual draw and read surfaces.
sw::Rect sourceTrimRect(0, 0, readBufferWidth, readBufferHeight);
- Device::ClipSrcRect(sourceTrimmedRect, destTrimmedRect, sourceTrimRect, flipX, flipY);
+ if (!Device::ClipSrcRect(sourceTrimmedRect, destTrimmedRect, sourceTrimRect, flipX, flipY))
+ {
+ // Failed to clip, blitting can't happen.
+ return error(GL_INVALID_OPERATION);
+ }
sw::Rect destTrimRect(0, 0, drawBufferWidth, drawBufferHeight);
- Device::ClipDstRect(sourceTrimmedRect, destTrimmedRect, destTrimRect, flipX, flipY);
+ if (!Device::ClipDstRect(sourceTrimmedRect, destTrimmedRect, destTrimRect, flipX, flipY))
+ {
+ // Failed to clip, blitting can't happen.
+ return error(GL_INVALID_OPERATION);
+ }
bool partialBufferCopy = false;
diff --git a/src/OpenGL/libGLESv2/Device.cpp b/src/OpenGL/libGLESv2/Device.cpp
index da76467..c70da83 100644
--- a/src/OpenGL/libGLESv2/Device.cpp
+++ b/src/OpenGL/libGLESv2/Device.cpp
@@ -453,8 +453,8 @@
int dHeight = dest->getHeight();
if(sourceRect && destRect &&
- (sourceRect->width() == 0.0f || std::isnan(sourceRect->width()) ||
- sourceRect->height() == 0.0f || std::isnan(sourceRect->height()) ||
+ (sourceRect->width() == 0.0f || !std::isfinite(sourceRect->width()) ||
+ sourceRect->height() == 0.0f || !std::isfinite(sourceRect->height()) ||
destRect->width() == 0.0f || destRect->height() == 0.0f))
{
return true; // No work to do.
@@ -530,14 +530,20 @@
}
sw::Rect srcClipRect(0, 0, sWidth, sHeight);
- ClipSrcRect(sRect, dRect, srcClipRect, flipX, flipY);
+ if (!ClipSrcRect(sRect, dRect, srcClipRect, flipX, flipY))
+ {
+ return true;
+ }
sw::Rect dstClipRect(0, 0, dWidth, dHeight);
- ClipDstRect(sRect, dRect, dstClipRect, flipX, flipY);
+ if (!ClipDstRect(sRect, dRect, dstClipRect, flipX, flipY))
+ {
+ return true;
+ }
if((sRect.width() == 0) || (sRect.height() == 0) ||
(dRect.width() == 0) || (dRect.height() == 0) ||
- (std::isnan(sRect.width()) || std::isnan(sRect.height())))
+ !std::isfinite(sRect.width()) || !std::isfinite(sRect.height()))
{
return true; // no work to do
}
@@ -885,7 +891,8 @@
return false;
}
- if (std::isnan(rect->x0) || std::isnan(rect->x1) || std::isnan(rect->y0) || std::isnan(rect->y1))
+ if (!std::isfinite(rect->x0) || !std::isfinite(rect->x1) ||
+ !std::isfinite(rect->y0) || !std::isfinite(rect->y1))
{
return false;
}
@@ -893,11 +900,15 @@
return true;
}
- void Device::ClipDstRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX, bool flipY)
+ bool Device::ClipDstRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX, bool flipY)
{
if(dstRect.x0 < clipRect.x0)
{
float offset = (static_cast<float>(clipRect.x0 - dstRect.x0) / static_cast<float>(dstRect.width())) * srcRect.width();
+ if (!std::isfinite(offset))
+ {
+ return false;
+ }
if(flipX)
{
srcRect.x1 -= offset;
@@ -911,6 +922,10 @@
if(dstRect.x1 > clipRect.x1)
{
float offset = (static_cast<float>(dstRect.x1 - clipRect.x1) / static_cast<float>(dstRect.width())) * srcRect.width();
+ if (!std::isfinite(offset))
+ {
+ return false;
+ }
if(flipX)
{
srcRect.x0 += offset;
@@ -924,6 +939,10 @@
if(dstRect.y0 < clipRect.y0)
{
float offset = (static_cast<float>(clipRect.y0 - dstRect.y0) / static_cast<float>(dstRect.height())) * srcRect.height();
+ if (!std::isfinite(offset))
+ {
+ return false;
+ }
if(flipY)
{
srcRect.y1 -= offset;
@@ -937,6 +956,10 @@
if(dstRect.y1 > clipRect.y1)
{
float offset = (static_cast<float>(dstRect.y1 - clipRect.y1) / static_cast<float>(dstRect.height())) * srcRect.height();
+ if (!std::isfinite(offset))
+ {
+ return false;
+ }
if(flipY)
{
srcRect.y0 += offset;
@@ -947,14 +970,19 @@
}
dstRect.y1 = clipRect.y1;
}
+ return true;
}
- void Device::ClipSrcRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX, bool flipY)
+ bool Device::ClipSrcRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX, bool flipY)
{
if(srcRect.x0 < static_cast<float>(clipRect.x0))
{
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))
+ {
+ return false;
+ }
int offset = static_cast<int>(offsetf);
if(flipX)
{
@@ -970,6 +998,10 @@
{
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))
+ {
+ return false;
+ }
int offset = static_cast<int>(offsetf);
if(flipX)
{
@@ -985,6 +1017,10 @@
{
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))
+ {
+ return false;
+ }
int offset = static_cast<int>(offsetf);
if(flipY)
{
@@ -1000,6 +1036,10 @@
{
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))
+ {
+ return false;
+ }
int offset = static_cast<int>(offsetf);
if(flipY)
{
@@ -1011,6 +1051,7 @@
}
srcRect.y1 -= offsetf / ratio;
}
+ return true;
}
void Device::finish()
diff --git a/src/OpenGL/libGLESv2/Device.hpp b/src/OpenGL/libGLESv2/Device.hpp
index 1b0636b..9b34e29 100644
--- a/src/OpenGL/libGLESv2/Device.hpp
+++ b/src/OpenGL/libGLESv2/Device.hpp
@@ -75,8 +75,8 @@
bool stretchCube(sw::Surface *sourceSurface, sw::Surface *destSurface);
void finish();
- static void ClipDstRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX = false, bool flipY = false);
- static void ClipSrcRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX = false, bool flipY = false);
+ static bool ClipDstRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX = false, bool flipY = false);
+ static bool ClipSrcRect(sw::RectF &srcRect, sw::Rect &dstRect, sw::Rect &clipRect, bool flipX = false, bool flipY = false);
private:
sw::Context *const context;
diff --git a/tests/GLESUnitTests/unittests.cpp b/tests/GLESUnitTests/unittests.cpp
index 56a377e..97e5520 100644
--- a/tests/GLESUnitTests/unittests.cpp
+++ b/tests/GLESUnitTests/unittests.cpp
@@ -1760,9 +1760,6 @@
EXPECT_EQ(red[0][1], black[0][1]);
// Check that glBlitFramebuffer doesn't crash with ugly input.
- glBlitFramebuffer(-2, -2, 127, 2147483470, 10, 10, 200, 200, GL_COLOR_BUFFER_BIT, GL_NEAREST);
- EXPECT_NO_GL_ERROR();
-
const int big = (int) 2e9;
const int small = 200;
const int neg_small = -small;
@@ -1774,6 +1771,7 @@
{1, 1, 1, 1, 1, 1, 1, 1},
{-1, -1, 1, 1, -1, -1, 1, 1},
{0, 0, 127, (int) 2e9, 10, 10, 200, 200},
+ {-2, -2, 127, 2147483470, 10, 10, 200, 200},
{big, small, small, big, big, big, small, small},
{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},
@@ -1787,9 +1785,12 @@
data[i][0], data[i][1], data[i][2], data[i][3],
data[i][4], data[i][5], data[i][6], data[i][7],
GL_COLOR_BUFFER_BIT, GL_NEAREST);
- EXPECT_NO_GL_ERROR();
+ // Ignore error state, just make sure that we don't crash on these inputs.
}
+ // Clear the error state before uninitializing test.
+ glGetError();
+
glDeleteFramebuffers(2, fbos);
glDeleteTextures(2, textures);
Uninitialize();
