Qualify stored constant union data with const
This prevents accidentally changing data that may be shared between
multiple TIntermConstantUnion nodes.
Besides making the code less prone to bugs in general, this will make it
easier to implement constant folding of array constructors.
BUG=541551
TEST=angle_unittests, WebGL conformance tests
Change-Id: I4f3059f70b841d9dd0cf20fea4d37684da9cd47e
Reviewed-on: https://chromium-review.googlesource.com/312440
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Tryjob-Request: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/IntermNode.cpp b/src/compiler/translator/IntermNode.cpp
index 0f7cc74..49546ff 100644
--- a/src/compiler/translator/IntermNode.cpp
+++ b/src/compiler/translator/IntermNode.cpp
@@ -169,7 +169,7 @@
}
}
-float VectorLength(TConstantUnion *paramArray, size_t paramArraySize)
+float VectorLength(const TConstantUnion *paramArray, size_t paramArraySize)
{
float result = 0.0f;
for (size_t i = 0; i < paramArraySize; i++)
@@ -180,7 +180,9 @@
return sqrtf(result);
}
-float VectorDotProduct(TConstantUnion *paramArray1, TConstantUnion *paramArray2, size_t paramArraySize)
+float VectorDotProduct(const TConstantUnion *paramArray1,
+ const TConstantUnion *paramArray2,
+ size_t paramArraySize)
{
float result = 0.0f;
for (size_t i = 0; i < paramArraySize; i++)
@@ -202,7 +204,9 @@
return folded;
}
-angle::Matrix<float> GetMatrix(TConstantUnion *paramArray, const unsigned int &rows, const unsigned int &cols)
+angle::Matrix<float> GetMatrix(const TConstantUnion *paramArray,
+ const unsigned int &rows,
+ const unsigned int &cols)
{
std::vector<float> elements;
for (size_t i = 0; i < rows * cols; i++)
@@ -212,7 +216,7 @@
return angle::Matrix<float>(elements, rows, cols).transpose();
}
-angle::Matrix<float> GetMatrix(TConstantUnion *paramArray, const unsigned int &size)
+angle::Matrix<float> GetMatrix(const TConstantUnion *paramArray, const unsigned int &size)
{
std::vector<float> elements;
for (size_t i = 0; i < size * size; i++)
@@ -416,12 +420,7 @@
TIntermConstantUnion::TIntermConstantUnion(const TIntermConstantUnion &node) : TIntermTyped(node)
{
- size_t arraySize = mType.getObjectSize();
- mUnionArrayPointer = new TConstantUnion[arraySize];
- for (size_t i = 0u; i < arraySize; ++i)
- {
- mUnionArrayPointer[i] = node.mUnionArrayPointer[i];
- }
+ mUnionArrayPointer = node.mUnionArrayPointer;
}
TIntermAggregate::TIntermAggregate(const TIntermAggregate &node)
@@ -969,8 +968,8 @@
//
TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUnion *rightNode, TInfoSink &infoSink)
{
- TConstantUnion *leftArray = getUnionArrayPointer();
- TConstantUnion *rightArray = rightNode->getUnionArrayPointer();
+ const TConstantUnion *leftArray = getUnionArrayPointer();
+ const TConstantUnion *rightArray = rightNode->getUnionArrayPointer();
if (!leftArray)
return nullptr;
@@ -1317,7 +1316,7 @@
// Do operations where the return type has a different number of components compared to the operand type.
//
- TConstantUnion *operandArray = getUnionArrayPointer();
+ const TConstantUnion *operandArray = getUnionArrayPointer();
if (!operandArray)
return nullptr;
@@ -1530,7 +1529,7 @@
// Do unary operations where the return type is the same as operand type.
//
- TConstantUnion *operandArray = getUnionArrayPointer();
+ const TConstantUnion *operandArray = getUnionArrayPointer();
if (!operandArray)
return nullptr;
@@ -2082,7 +2081,7 @@
TOperator op = aggregate->getOp();
TIntermSequence *sequence = aggregate->getSequence();
unsigned int paramsCount = static_cast<unsigned int>(sequence->size());
- std::vector<TConstantUnion *> unionArrays(paramsCount);
+ std::vector<const TConstantUnion *> unionArrays(paramsCount);
std::vector<size_t> objectSizes(paramsCount);
size_t maxObjectSize = 0;
TBasicType basicType = EbtVoid;
diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index 00baf67..ad500e2 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -302,16 +302,16 @@
class TIntermConstantUnion : public TIntermTyped
{
public:
- TIntermConstantUnion(TConstantUnion *unionPointer, const TType &type)
- : TIntermTyped(type),
- mUnionArrayPointer(unionPointer) { }
+ TIntermConstantUnion(const TConstantUnion *unionPointer, const TType &type)
+ : TIntermTyped(type), mUnionArrayPointer(unionPointer)
+ {
+ }
TIntermTyped *deepCopy() const override { return new TIntermConstantUnion(*this); }
bool hasSideEffects() const override { return false; }
const TConstantUnion *getUnionArrayPointer() const { return mUnionArrayPointer; }
- TConstantUnion *getUnionArrayPointer() { return mUnionArrayPointer; }
int getIConst(size_t index) const
{
@@ -330,7 +330,7 @@
return mUnionArrayPointer ? mUnionArrayPointer[index].getBConst() : false;
}
- void replaceConstantUnion(TConstantUnion *safeConstantUnion)
+ void replaceConstantUnion(const TConstantUnion *safeConstantUnion)
{
// Previous union pointer freed on pool deallocation.
mUnionArrayPointer = safeConstantUnion;
@@ -349,7 +349,8 @@
static TConstantUnion *FoldAggregateBuiltIn(TIntermAggregate *aggregate, TInfoSink &infoSink);
protected:
- TConstantUnion *mUnionArrayPointer;
+ // Same data may be shared between multiple constant unions, so it can't be modified.
+ const TConstantUnion *mUnionArrayPointer;
private:
typedef float(*FloatTypeUnaryFunc) (float);
diff --git a/src/compiler/translator/Intermediate.cpp b/src/compiler/translator/Intermediate.cpp
index c973535..0adb721 100644
--- a/src/compiler/translator/Intermediate.cpp
+++ b/src/compiler/translator/Intermediate.cpp
@@ -375,8 +375,9 @@
// Returns the constant union node created.
//
-TIntermConstantUnion *TIntermediate::addConstantUnion(
- TConstantUnion *constantUnion, const TType &type, const TSourceLoc &line)
+TIntermConstantUnion *TIntermediate::addConstantUnion(const TConstantUnion *constantUnion,
+ const TType &type,
+ const TSourceLoc &line)
{
TIntermConstantUnion *node = new TIntermConstantUnion(constantUnion, type);
node->setLine(line);
diff --git a/src/compiler/translator/Intermediate.h b/src/compiler/translator/Intermediate.h
index 07a0ecc..f723fc7 100644
--- a/src/compiler/translator/Intermediate.h
+++ b/src/compiler/translator/Intermediate.h
@@ -52,8 +52,9 @@
TIntermTyped *right,
const TSourceLoc &line,
int shaderVersion);
- TIntermConstantUnion *addConstantUnion(
- TConstantUnion *constantUnion, const TType &type, const TSourceLoc &line);
+ TIntermConstantUnion *addConstantUnion(const TConstantUnion *constantUnion,
+ const TType &type,
+ const TSourceLoc &line);
TIntermNode *addLoop(TLoopType, TIntermNode *, TIntermTyped *, TIntermTyped *,
TIntermNode *, const TSourceLoc &);
TIntermBranch *addBranch(TOperator, const TSourceLoc &);
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index fd9d2c0..8486ac0 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -1216,7 +1216,7 @@
if (variable->getConstPointer())
{
- TConstantUnion *constArray = variable->getConstPointer();
+ const TConstantUnion *constArray = variable->getConstPointer();
return intermediate.addConstantUnion(constArray, variable->getType(), location);
}
else
@@ -1353,7 +1353,7 @@
symbolTable.find(initializer->getAsSymbolNode()->getSymbol(), 0);
const TVariable *tVar = static_cast<const TVariable *>(symbol);
- TConstantUnion *constArray = tVar->getConstPointer();
+ const TConstantUnion *constArray = tVar->getConstPointer();
if (constArray)
{
variable->shareConstPointer(constArray);
@@ -2373,7 +2373,7 @@
index = node->getType().getCols() - 1;
}
- TConstantUnion *unionArray = node->getUnionArrayPointer();
+ const TConstantUnion *unionArray = node->getUnionArrayPointer();
int size = node->getType().getCols();
return intermediate.addConstantUnion(&unionArray[size * index], node->getType(), line);
}
@@ -2401,8 +2401,8 @@
outOfRangeError(outOfRangeIndexIsError, line, "", "[", extraInfo.c_str());
index = node->getType().getArraySize() - 1;
}
- size_t arrayElementSize = arrayElementType.getObjectSize();
- TConstantUnion *unionArray = node->getUnionArrayPointer();
+ size_t arrayElementSize = arrayElementType.getObjectSize();
+ const TConstantUnion *unionArray = node->getUnionArrayPointer();
return intermediate.addConstantUnion(&unionArray[arrayElementSize * index], node->getType(),
line);
}
@@ -2437,7 +2437,7 @@
TIntermConstantUnion *tempConstantNode = node->getAsConstantUnion();
if (tempConstantNode)
{
- TConstantUnion *constArray = tempConstantNode->getUnionArrayPointer();
+ const TConstantUnion *constArray = tempConstantNode->getUnionArrayPointer();
// type will be changed in the calling function
typedNode = intermediate.addConstantUnion(constArray + instanceSize,
@@ -2796,8 +2796,9 @@
safeIndex = baseExpression->getType().getNominalSize() - 1;
}
- // Don't modify the data of the previous constant union, because it can point
- // to builtins, like gl_MaxDrawBuffers. Instead use a new sanitized object.
+ // Data of constant unions can't be changed, because it may be shared with other
+ // constant unions or even builtins, like gl_MaxDrawBuffers. Instead use a new
+ // sanitized object.
if (safeIndex != -1)
{
TConstantUnion *safeConstantUnion = new TConstantUnion();
diff --git a/src/compiler/translator/SymbolTable.h b/src/compiler/translator/SymbolTable.h
index 66b9174..2f5eb36 100644
--- a/src/compiler/translator/SymbolTable.h
+++ b/src/compiler/translator/SymbolTable.h
@@ -128,34 +128,16 @@
type.setQualifier(qualifier);
}
- TConstantUnion *getConstPointer()
- {
- if (!unionArray)
- unionArray = new TConstantUnion[type.getObjectSize()];
+ const TConstantUnion *getConstPointer() const { return unionArray; }
- return unionArray;
- }
-
- TConstantUnion *getConstPointer() const
- {
- return unionArray;
- }
-
- void shareConstPointer(TConstantUnion *constArray)
- {
- if (unionArray == constArray)
- return;
-
- delete[] unionArray;
- unionArray = constArray;
- }
+ void shareConstPointer(const TConstantUnion *constArray) { unionArray = constArray; }
private:
TType type;
bool userType;
// we are assuming that Pool Allocator will free the memory
// allocated to unionArray when this object is destroyed.
- TConstantUnion *unionArray;
+ const TConstantUnion *unionArray;
};
// Immutable version of TParameter.
@@ -400,7 +382,9 @@
{
TVariable *constant = new TVariable(
NewPoolTString(name), TType(EbtInt, EbpUndefined, EvqConst, 1));
- constant->getConstPointer()->setIConst(value);
+ TConstantUnion *unionArray = new TConstantUnion[1];
+ unionArray[0].setIConst(value);
+ constant->shareConstPointer(unionArray);
return insert(level, constant);
}
@@ -408,7 +392,9 @@
{
TVariable *constant =
new TVariable(NewPoolTString(name), TType(EbtInt, EbpUndefined, EvqConst, 1));
- constant->getConstPointer()->setIConst(value);
+ TConstantUnion *unionArray = new TConstantUnion[1];
+ unionArray[0].setIConst(value);
+ constant->shareConstPointer(unionArray);
return insert(level, ext, constant);
}