Commit 244be01a by Olli Etuaho Committed by Commit Bot

Fix struct compound assignment being allowed in GLSL parsing

The shader translator used to accept some invalid struct operations, like struct += struct and struct == struct with a different type. Fix this. BUG=angleproject:1476 TEST=angle_unittests Change-Id: Ia2303fc1f740da4d78242e094ee6004b07364973 Reviewed-on: https://chromium-review.googlesource.com/372718Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
parent bccc65d3
...@@ -73,14 +73,6 @@ TIntermTyped *TIntermediate::addBinaryMath( ...@@ -73,14 +73,6 @@ TIntermTyped *TIntermediate::addBinaryMath(
TIntermTyped *TIntermediate::addAssign( TIntermTyped *TIntermediate::addAssign(
TOperator op, TIntermTyped *left, TIntermTyped *right, const TSourceLoc &line) TOperator op, TIntermTyped *left, TIntermTyped *right, const TSourceLoc &line)
{ {
if (left->getType().getStruct() || right->getType().getStruct())
{
if (left->getType() != right->getType())
{
return NULL;
}
}
TIntermBinary *node = new TIntermBinary(op); TIntermBinary *node = new TIntermBinary(op);
node->setLine(line); node->setLine(line);
......
...@@ -3441,6 +3441,28 @@ bool TParseContext::binaryOpCommonCheck(TOperator op, ...@@ -3441,6 +3441,28 @@ bool TParseContext::binaryOpCommonCheck(TOperator op,
TIntermTyped *right, TIntermTyped *right,
const TSourceLoc &loc) const TSourceLoc &loc)
{ {
if (left->getType().getStruct() || right->getType().getStruct())
{
switch (op)
{
case EOpIndexDirectStruct:
ASSERT(left->getType().getStruct());
break;
case EOpEqual:
case EOpNotEqual:
case EOpAssign:
case EOpInitialize:
if (left->getType() != right->getType())
{
return false;
}
break;
default:
error(loc, "Invalid operation for structs", GetOperatorString(op));
return false;
}
}
if (left->isArray() || right->isArray()) if (left->isArray() || right->isArray())
{ {
if (mShaderVersion < 300) if (mShaderVersion < 300)
...@@ -3572,8 +3594,9 @@ TIntermTyped *TParseContext::addBinaryMathInternal(TOperator op, ...@@ -3572,8 +3594,9 @@ TIntermTyped *TParseContext::addBinaryMathInternal(TOperator op,
case EOpGreaterThan: case EOpGreaterThan:
case EOpLessThanEqual: case EOpLessThanEqual:
case EOpGreaterThanEqual: case EOpGreaterThanEqual:
ASSERT(!left->isArray() && !right->isArray()); ASSERT(!left->isArray() && !right->isArray() && !left->getType().getStruct() &&
if (left->isMatrix() || left->isVector() || left->getBasicType() == EbtStruct) !right->getType().getStruct());
if (left->isMatrix() || left->isVector())
{ {
return nullptr; return nullptr;
} }
...@@ -3581,7 +3604,8 @@ TIntermTyped *TParseContext::addBinaryMathInternal(TOperator op, ...@@ -3581,7 +3604,8 @@ TIntermTyped *TParseContext::addBinaryMathInternal(TOperator op,
case EOpLogicalOr: case EOpLogicalOr:
case EOpLogicalXor: case EOpLogicalXor:
case EOpLogicalAnd: case EOpLogicalAnd:
ASSERT(!left->isArray() && !right->isArray()); ASSERT(!left->isArray() && !right->isArray() && !left->getType().getStruct() &&
!right->getType().getStruct());
if (left->getBasicType() != EbtBool || left->isMatrix() || left->isVector()) if (left->getBasicType() != EbtBool || left->isMatrix() || left->isVector())
{ {
return nullptr; return nullptr;
...@@ -3591,17 +3615,18 @@ TIntermTyped *TParseContext::addBinaryMathInternal(TOperator op, ...@@ -3591,17 +3615,18 @@ TIntermTyped *TParseContext::addBinaryMathInternal(TOperator op,
case EOpSub: case EOpSub:
case EOpDiv: case EOpDiv:
case EOpMul: case EOpMul:
ASSERT(!left->isArray() && !right->isArray()); ASSERT(!left->isArray() && !right->isArray() && !left->getType().getStruct() &&
if (left->getBasicType() == EbtStruct || left->getBasicType() == EbtBool) !right->getType().getStruct());
if (left->getBasicType() == EbtBool)
{ {
return nullptr; return nullptr;
} }
break; break;
case EOpIMod: case EOpIMod:
ASSERT(!left->isArray() && !right->isArray()); ASSERT(!left->isArray() && !right->isArray() && !left->getType().getStruct() &&
!right->getType().getStruct());
// Note that this is only for the % operator, not for mod() // Note that this is only for the % operator, not for mod()
if (left->getBasicType() == EbtStruct || left->getBasicType() == EbtBool || if (left->getBasicType() == EbtBool || left->getBasicType() == EbtFloat)
left->getBasicType() == EbtFloat)
{ {
return nullptr; return nullptr;
} }
......
...@@ -1718,6 +1718,46 @@ TEST_F(MalformedShaderTest, MaxImageUnitsInES3Shader) ...@@ -1718,6 +1718,46 @@ TEST_F(MalformedShaderTest, MaxImageUnitsInES3Shader)
} }
} }
// struct += struct is an invalid operation.
TEST_F(MalformedShaderTest, StructCompoundAssignStruct)
{
const std::string &shaderString =
"#version 300 es\n"
"precision mediump float;\n"
"out vec4 myOutput;\n"
"struct S { float foo; };\n"
"void main() {\n"
" S a, b;\n"
" a += b;\n"
" myOutput = vec4(0);\n"
"}\n";
if (compile(shaderString))
{
FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
}
}
// struct == different struct is an invalid operation.
TEST_F(MalformedShaderTest, StructEqDifferentStruct)
{
const std::string &shaderString =
"#version 300 es\n"
"precision mediump float;\n"
"out vec4 myOutput;\n"
"struct S { float foo; };\n"
"struct S2 { float foobar; };\n"
"void main() {\n"
" S a;\n"
" S2 b;\n"
" a == b;\n"
" myOutput = vec4(0);\n"
"}\n";
if (compile(shaderString))
{
FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
}
}
// Compute shaders are not supported in versions lower than 310. // Compute shaders are not supported in versions lower than 310.
TEST_F(MalformedComputeShaderTest, Version100) TEST_F(MalformedComputeShaderTest, Version100)
{ {
......
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