Commit f13cadd8 by Olli Etuaho Committed by Commit Bot

Fix checking negative index when indexing matrix/vector

It's important that the test against the maximum of the valid range is only done if the index is positive, so the sanitized index value is guaranteed to end up in the valid range. This fixes a regression from commit "Add GLSL support for runtime-sized arrays in SSBOs". BUG=chromium:789029 TEST=angle_unittests Change-Id: Ic7125e383a64e46994b072df6d7e642432c521af Reviewed-on: https://chromium-review.googlesource.com/792935Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
parent 57ea533f
...@@ -4038,25 +4038,29 @@ TIntermTyped *TParseContext::addIndexExpression(TIntermTyped *baseExpression, ...@@ -4038,25 +4038,29 @@ TIntermTyped *TParseContext::addIndexExpression(TIntermTyped *baseExpression,
safeIndex = 0; safeIndex = 0;
} }
} }
// Only do generic out-of-range check if similar error hasn't already been reported. }
if (safeIndex < 0) // Only do generic out-of-range check if similar error hasn't already been reported.
if (safeIndex < 0)
{
if (baseExpression->isArray())
{ {
safeIndex = checkIndexLessThan(outOfRangeIndexIsError, location, index, safeIndex = checkIndexLessThan(outOfRangeIndexIsError, location, index,
baseExpression->getOutermostArraySize(), baseExpression->getOutermostArraySize(),
"array index out of range"); "array index out of range");
} }
} else if (baseExpression->isMatrix())
else if (baseExpression->isMatrix()) {
{ safeIndex = checkIndexLessThan(outOfRangeIndexIsError, location, index,
safeIndex = checkIndexLessThan(outOfRangeIndexIsError, location, index, baseExpression->getType().getCols(),
baseExpression->getType().getCols(), "matrix field selection out of range");
"matrix field selection out of range"); }
} else
else if (baseExpression->isVector()) {
{ ASSERT(baseExpression->isVector());
safeIndex = checkIndexLessThan(outOfRangeIndexIsError, location, index, safeIndex = checkIndexLessThan(outOfRangeIndexIsError, location, index,
baseExpression->getType().getNominalSize(), baseExpression->getType().getNominalSize(),
"vector field selection out of range"); "vector field selection out of range");
}
} }
ASSERT(safeIndex >= 0); ASSERT(safeIndex >= 0);
...@@ -4092,6 +4096,8 @@ int TParseContext::checkIndexLessThan(bool outOfRangeIndexIsError, ...@@ -4092,6 +4096,8 @@ int TParseContext::checkIndexLessThan(bool outOfRangeIndexIsError,
{ {
// Should not reach here with an unsized / runtime-sized array. // Should not reach here with an unsized / runtime-sized array.
ASSERT(arraySize > 0); ASSERT(arraySize > 0);
// A negative index should already have been checked.
ASSERT(index >= 0);
if (index >= arraySize) if (index >= arraySize)
{ {
std::stringstream reasonStream; std::stringstream reasonStream;
......
...@@ -5554,3 +5554,21 @@ TEST_F(FragmentShaderValidationTest, AtomicAddWithNonStorageVariable) ...@@ -5554,3 +5554,21 @@ TEST_F(FragmentShaderValidationTest, AtomicAddWithNonStorageVariable)
FAIL() << "Shader compilation succeeded, expecting failure:\n" << mInfoLog; FAIL() << "Shader compilation succeeded, expecting failure:\n" << mInfoLog;
} }
} }
// Test that negative indexing of a matrix doesn't result in an assert.
TEST_F(FragmentShaderValidationTest, MatrixNegativeIndex)
{
const std::string &shaderString =
R"(
precision mediump float;
void main()
{
gl_FragColor = mat4(1.0)[-1];
})";
if (compile(shaderString))
{
FAIL() << "Shader compilation succeeded, expecting failure:\n" << mInfoLog;
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment