Skip to content

A
angle
Commit 2cacb778 by Olli Etuaho Committed by Commit Bot
Options

Fix folding shifts when operands have different signedness

The code used to incorrectly assert that the right-hand side of shift should have the same signedness as the left-hand side. Instead simply assert that both the lhs and rhs are integer typed, and also don't rely on aliasing via union when accessing bit shift operands. Also disallow constant folded bit shifts where the right hand side is greater than 31. Shifting with values greater than the width of the type has undefined results in both ESSL and C++. BUG=chromium:648135 TEST=angle_unittests Change-Id: I84a99abc55f0eeda549b4781e954d17ba7b87552 Reviewed-on: https://chromium-review.googlesource.com/389351Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
parent 054f7ed0
Showing with 131 additions and 23 deletions
src/compiler/translator/ConstantUnion.cpp
...@@ -376,29 +376,60 @@ TConstantUnion TConstantUnion::rshift(const TConstantUnion &lhs, ...@@ -376,29 +376,60 @@ TConstantUnion TConstantUnion::rshift(const TConstantUnion &lhs,
const TSourceLoc &line) const TSourceLoc &line)
{ {
TConstantUnion returnValue; TConstantUnion returnValue;
ASSERT((lhs.type == rhs.type) && (lhs.type == EbtInt || lhs.type == EbtUInt)); ASSERT(lhs.type == EbtInt || lhs.type == EbtUInt);
ASSERT(rhs.type == EbtInt || rhs.type == EbtUInt);
if ((lhs.type == EbtInt && lhs.iConst < 0) ||
(rhs.type == EbtInt && (rhs.iConst < 0 || rhs.iConst > 31)) ||
(rhs.type == EbtUInt && rhs.uConst > 31))
{
diag->error(line, "Undefined shift (operand out of range)", ">>", "");
switch (lhs.type)
{
case EbtInt:
returnValue.setIConst(0);
break;
case EbtUInt:
returnValue.setUConst(0u);
break;
default:
UNREACHABLE();
}
return returnValue;
}
switch (lhs.type) switch (lhs.type)
{ {
case EbtInt: case EbtInt:
if (lhs.iConst < 0 || rhs.iConst < 0) switch (rhs.type)
{
diag->error(line, "Undefined shift", ">>", "");
returnValue.setIConst(0);
}
else
{ {
returnValue.setIConst(lhs.iConst >> rhs.iConst); case EbtInt:
returnValue.setIConst(lhs.iConst >> rhs.iConst);
break;
case EbtUInt:
returnValue.setIConst(lhs.iConst >> rhs.uConst);
break;
default:
UNREACHABLE();
} }
break; break;
case EbtUInt: case EbtUInt:
returnValue.setUConst(lhs.uConst >> rhs.uConst); switch (rhs.type)
{
case EbtInt:
returnValue.setUConst(lhs.uConst >> rhs.iConst);
break;
case EbtUInt:
returnValue.setUConst(lhs.uConst >> rhs.uConst);
break;
default:
UNREACHABLE();
}
break; break;
default: default:
UNREACHABLE(); UNREACHABLE();
} }
return returnValue; return returnValue;
} }
...@@ -409,32 +440,60 @@ TConstantUnion TConstantUnion::lshift(const TConstantUnion &lhs, ...@@ -409,32 +440,60 @@ TConstantUnion TConstantUnion::lshift(const TConstantUnion &lhs,
const TSourceLoc &line) const TSourceLoc &line)
{ {
TConstantUnion returnValue; TConstantUnion returnValue;
// The signedness of the second parameter might be different, but we ASSERT(lhs.type == EbtInt || lhs.type == EbtUInt);
// don't care, since the result is undefined if the second parameter is ASSERT(rhs.type == EbtInt || rhs.type == EbtUInt);
// negative, and aliasing should not be a problem with unions. if ((lhs.type == EbtInt && lhs.iConst < 0) ||
ASSERT((lhs.type == rhs.type) && (lhs.type == EbtInt || lhs.type == EbtUInt)); (rhs.type == EbtInt && (rhs.iConst < 0 || rhs.iConst > 31)) ||
(rhs.type == EbtUInt && rhs.uConst > 31))
{
diag->error(line, "Undefined shift (operand out of range)", "<<", "");
switch (lhs.type)
{
case EbtInt:
returnValue.setIConst(0);
break;
case EbtUInt:
returnValue.setUConst(0u);
break;
default:
UNREACHABLE();
}
return returnValue;
}
switch (lhs.type) switch (lhs.type)
{ {
case EbtInt: case EbtInt:
if (lhs.iConst < 0 || rhs.iConst < 0) switch (rhs.type)
{
diag->error(line, "Undefined shift", "<<", "");
returnValue.setIConst(0);
}
else
{ {
returnValue.setIConst(lhs.iConst << rhs.iConst); case EbtInt:
returnValue.setIConst(lhs.iConst << rhs.iConst);
break;
case EbtUInt:
returnValue.setIConst(lhs.iConst << rhs.uConst);
break;
default:
UNREACHABLE();
} }
break; break;
case EbtUInt: case EbtUInt:
returnValue.setUConst(lhs.uConst << rhs.uConst); switch (rhs.type)
{
case EbtInt:
returnValue.setUConst(lhs.uConst << rhs.iConst);
break;
case EbtUInt:
returnValue.setUConst(lhs.uConst << rhs.uConst);
break;
default:
UNREACHABLE();
}
break; break;
default: default:
UNREACHABLE(); UNREACHABLE();
} }
return returnValue; return returnValue;
} }
......
src/tests/compiler_tests/ConstantFolding_test.cpp
...@@ -792,3 +792,35 @@ TEST_F(ConstantFoldingTest, FoldNonSquareOuterProduct) ...@@ -792,3 +792,35 @@ TEST_F(ConstantFoldingTest, FoldNonSquareOuterProduct)
std::vector<float> result(outputElements, outputElements + 6); std::vector<float> result(outputElements, outputElements + 6);
ASSERT_TRUE(constantColumnMajorMatrixFoundInAST(result)); ASSERT_TRUE(constantColumnMajorMatrixFoundInAST(result));
} }
// Test that folding bit shift left with non-matching signedness works.
TEST_F(ConstantFoldingTest, FoldBitShiftLeftDifferentSignedness)
{
const std::string &shaderString =
"#version 300 es\n"
"precision mediump float;\n"
"out vec4 my_FragColor;\n"
"void main()\n"
"{\n"
" uint u = 0xffffffffu << 31;\n"
" my_FragColor = vec4(u);\n"
"}\n";
compile(shaderString);
ASSERT_TRUE(constantFoundInAST(0x80000000u));
}
// Test that folding bit shift right with non-matching signedness works.
TEST_F(ConstantFoldingTest, FoldBitShiftRightDifferentSignedness)
{
const std::string &shaderString =
"#version 300 es\n"
"precision mediump float;\n"
"out vec4 my_FragColor;\n"
"void main()\n"
"{\n"
" uint u = 0xffffffffu >> 30;\n"
" my_FragColor = vec4(u);\n"
"}\n";
compile(shaderString);
ASSERT_TRUE(constantFoundInAST(0x3u));
}
src/tests/compiler_tests/MalformedShader_test.cpp
...@@ -2553,3 +2553,19 @@ TEST_F(MalformedShaderTest, VariableDeclarationWithInvariantAndLayoutQualifierES ...@@ -2553,3 +2553,19 @@ TEST_F(MalformedShaderTest, VariableDeclarationWithInvariantAndLayoutQualifierES
FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog; FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
} }
} }
// Bit shift with a rhs value > 31 has an undefined result in the GLSL spec. We disallow it.
// ESSL 3.00.6 section 5.9.
TEST_F(MalformedShaderTest, ShiftBy32)
{
const std::string &shaderString =
"#version 300 es\n"
"precision mediump float;\n"
"void main() {\n"
" uint u = 1u << 32u;\n"
"}\n";
if (compile(shaderString))
{
FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
}
}
\ No newline at end of file
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