Translator: Fix assert in ScalarizeVecAndMatConstructorArgs
This transformation assumed that precision can be derived for constants
in every possible scenario, but that's not true. The fuzzer produced
the following code:
void main()
{
mat4 m;
mat2(0, bvec3(m));
}
In the above, the constant 0 legimitately has no precision. The assert
was in a function that attempted to make a temporary out of the
constant, which this commit changes to use the original constant as-is.
Bug: chromium:1246781
Change-Id: I6f247264e5213cfd9449fdfb1dc312d02b99f2f1
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3149191
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
diff --git a/src/compiler/translator/tree_ops/ScalarizeVecAndMatConstructorArgs.cpp b/src/compiler/translator/tree_ops/ScalarizeVecAndMatConstructorArgs.cpp
index c711fbc..195ca53 100644
--- a/src/compiler/translator/tree_ops/ScalarizeVecAndMatConstructorArgs.cpp
+++ b/src/compiler/translator/tree_ops/ScalarizeVecAndMatConstructorArgs.cpp
@@ -27,12 +27,12 @@
namespace
{
-TIntermBinary *ConstructVectorIndexBinaryNode(TIntermSymbol *symbolNode, int index)
+TIntermBinary *ConstructVectorIndexBinaryNode(TIntermTyped *symbolNode, int index)
{
return new TIntermBinary(EOpIndexDirect, symbolNode, CreateIndexNode(index));
}
-TIntermBinary *ConstructMatrixIndexBinaryNode(TIntermSymbol *symbolNode, int colIndex, int rowIndex)
+TIntermBinary *ConstructMatrixIndexBinaryNode(TIntermTyped *symbolNode, int colIndex, int rowIndex)
{
TIntermBinary *colVectorNode = ConstructVectorIndexBinaryNode(symbolNode, colIndex);
@@ -63,7 +63,7 @@
// vec4 v(1, s0[0][0], s0[0][1], s0[0][2]);
// This function is to create nodes for "mat4 s0 = m;" and insert it to the code sequence. This
// way the possible side effects of the constructor argument will only be evaluated once.
- TVariable *createTempVariable(TIntermTyped *original);
+ TIntermTyped *createTempVariable(TIntermTyped *original);
std::vector<TIntermSequence> mBlockStack;
@@ -123,10 +123,10 @@
ASSERT(size > 0);
TIntermTyped *originalArg = originalArgNode->getAsTyped();
ASSERT(originalArg);
- TVariable *argVariable = createTempVariable(originalArg);
+ TIntermTyped *argVariable = createTempVariable(originalArg);
if (originalArg->isScalar())
{
- sequence->push_back(CreateTempSymbolNode(argVariable));
+ sequence->push_back(argVariable);
size--;
}
else if (originalArg->isVector())
@@ -137,15 +137,14 @@
size -= repeat;
for (int index = 0; index < repeat; ++index)
{
- TIntermSymbol *symbolNode = CreateTempSymbolNode(argVariable);
- TIntermBinary *newNode = ConstructVectorIndexBinaryNode(symbolNode, index);
+ TIntermBinary *newNode =
+ ConstructVectorIndexBinaryNode(argVariable->deepCopy(), index);
sequence->push_back(newNode);
}
}
else
{
- TIntermSymbol *symbolNode = CreateTempSymbolNode(argVariable);
- sequence->push_back(symbolNode);
+ sequence->push_back(argVariable);
size -= originalArg->getNominalSize();
}
}
@@ -159,9 +158,8 @@
size -= repeat;
while (repeat > 0)
{
- TIntermSymbol *symbolNode = CreateTempSymbolNode(argVariable);
TIntermBinary *newNode =
- ConstructMatrixIndexBinaryNode(symbolNode, colIndex, rowIndex);
+ ConstructMatrixIndexBinaryNode(argVariable->deepCopy(), colIndex, rowIndex);
sequence->push_back(newNode);
rowIndex++;
if (rowIndex >= originalArg->getRows())
@@ -174,15 +172,14 @@
}
else
{
- TIntermSymbol *symbolNode = CreateTempSymbolNode(argVariable);
- sequence->push_back(symbolNode);
+ sequence->push_back(argVariable);
size -= originalArg->getCols() * originalArg->getRows();
}
}
}
}
-TVariable *ScalarizeArgsTraverser::createTempVariable(TIntermTyped *original)
+TIntermTyped *ScalarizeArgsTraverser::createTempVariable(TIntermTyped *original)
{
ASSERT(original);
@@ -190,9 +187,16 @@
type->setQualifier(EvqTemporary);
// The precision of the constant must have been retained (or derived), which will now apply to
- // the temp variable.
- ASSERT(!IsPrecisionApplicableToType(type->getBasicType()) ||
- type->getPrecision() != EbpUndefined);
+ // the temp variable. In some cases, the precision cannot be derived, so use the constant as
+ // is. For example, in the following standalone statement, the precision of the constant 0
+ // cannot be determined:
+ //
+ // mat2(0, bvec3(m));
+ //
+ if (IsPrecisionApplicableToType(type->getBasicType()) && type->getPrecision() == EbpUndefined)
+ {
+ return original;
+ }
TVariable *variable = CreateTempVariable(mSymbolTable, type);
@@ -201,7 +205,7 @@
TIntermDeclaration *declaration = CreateTempInitDeclarationNode(variable, original);
sequence.push_back(declaration);
- return variable;
+ return CreateTempSymbolNode(variable);
}
} // namespace
diff --git a/src/tests/compiler_tests/ScalarizeVecAndMatConstructorArgs_test.cpp b/src/tests/compiler_tests/ScalarizeVecAndMatConstructorArgs_test.cpp
index 6518e2a..80b0582 100644
--- a/src/tests/compiler_tests/ScalarizeVecAndMatConstructorArgs_test.cpp
+++ b/src/tests/compiler_tests/ScalarizeVecAndMatConstructorArgs_test.cpp
@@ -116,4 +116,21 @@
EXPECT_TRUE(foundInCodeInOrder(expectedStrings));
}
+// Verifies that constructors without precision don't cause issues.
+TEST_F(ScalarizeVecAndMatConstructorArgsTest, ConstructorWithoutPrecision)
+{
+ const std::string shaderString =
+ R"(
+ precision mediump float;
+
+ uniform float u;
+
+ void main()
+ {
+ mat4 m = mat4(u);
+ mat2(0, bvec3(m));
+ })";
+ compile(shaderString);
+}
+
} // anonymous namespace
