Skip to content

A
angle
Commit 950457b3 by Olli Etuaho
Options

Fix destructing TStrings through BuiltInFunctionEmulator after free

BuiltInFunctionEmulator gets destructed after the PoolAllocator has already freed memory. That's why BuiltInFunctionEmulator can't hold any objects that contain parts stored in the memory pool that would be accessed in its destructor. Use only pointers to TType objects inside BuiltInFunctionEmulator, so that the BuiltInFunctionEmulator destructor doesn't access TStrings which have data in the memory pool. Also fix style issues in BuiltInFunctionEmulator. BUG=angleproject:1010 TEST=dEQP-GLES3.functional.shaders.builtin_functions.* Change-Id: Ic35caf80bf125d0427c2ed2024e98657756103b6 Reviewed-on: https://chromium-review.googlesource.com/272738Tested-by: 's avatarOlli Etuaho <oetuaho@nvidia.com> Reviewed-by: 's avatarZhenyao Mo <zmo@chromium.org>
parent c378cd8a
Showing with 111 additions and 108 deletions
src/compiler/translator/BuiltInFunctionEmulator.cpp
...@@ -11,28 +11,30 @@ ...@@ -11,28 +11,30 @@
class BuiltInFunctionEmulator::BuiltInFunctionEmulationMarker : public TIntermTraverser class BuiltInFunctionEmulator::BuiltInFunctionEmulationMarker : public TIntermTraverser
{ {
public: public:
BuiltInFunctionEmulationMarker(BuiltInFunctionEmulator& emulator) BuiltInFunctionEmulationMarker(BuiltInFunctionEmulator &emulator)
: mEmulator(emulator) : mEmulator(emulator)
{ {
} }
virtual bool visitUnary(Visit visit, TIntermUnary* node) virtual bool visitUnary(Visit visit, TIntermUnary *node)
{ {
if (visit == PreVisit) { if (visit == PreVisit)
bool needToEmulate = mEmulator.SetFunctionCalled( {
node->getOp(), node->getOperand()->getType()); bool needToEmulate = mEmulator.SetFunctionCalled(node->getOp(), node->getOperand()->getType());
if (needToEmulate) if (needToEmulate)
node->setUseEmulatedFunction(); node->setUseEmulatedFunction();
} }
return true; return true;
} }
virtual bool visitAggregate(Visit visit, TIntermAggregate* node) virtual bool visitAggregate(Visit visit, TIntermAggregate *node)
{ {
if (visit == PreVisit) { if (visit == PreVisit)
{
// Here we handle all the built-in functions instead of the ones we // Here we handle all the built-in functions instead of the ones we
// currently identified as problematic. // currently identified as problematic.
switch (node->getOp()) { switch (node->getOp())
{
case EOpLessThan: case EOpLessThan:
case EOpGreaterThan: case EOpGreaterThan:
case EOpLessThanEqual: case EOpLessThanEqual:
...@@ -59,14 +61,14 @@ class BuiltInFunctionEmulator::BuiltInFunctionEmulationMarker : public TIntermTr ...@@ -59,14 +61,14 @@ class BuiltInFunctionEmulator::BuiltInFunctionEmulationMarker : public TIntermTr
break; break;
default: default:
return true; return true;
}; }
const TIntermSequence& sequence = *(node->getSequence()); const TIntermSequence &sequence = *(node->getSequence());
bool needToEmulate = false; bool needToEmulate = false;
// Right now we only handle built-in functions with two or three parameters. // Right now we only handle built-in functions with two or three parameters.
if (sequence.size() == 2) if (sequence.size() == 2)
{ {
TIntermTyped* param1 = sequence[0]->getAsTyped(); TIntermTyped *param1 = sequence[0]->getAsTyped();
TIntermTyped* param2 = sequence[1]->getAsTyped(); TIntermTyped *param2 = sequence[1]->getAsTyped();
if (!param1 || !param2) if (!param1 || !param2)
return true; return true;
needToEmulate = mEmulator.SetFunctionCalled( needToEmulate = mEmulator.SetFunctionCalled(
...@@ -74,9 +76,9 @@ class BuiltInFunctionEmulator::BuiltInFunctionEmulationMarker : public TIntermTr ...@@ -74,9 +76,9 @@ class BuiltInFunctionEmulator::BuiltInFunctionEmulationMarker : public TIntermTr
} }
else if (sequence.size() == 3) else if (sequence.size() == 3)
{ {
TIntermTyped* param1 = sequence[0]->getAsTyped(); TIntermTyped *param1 = sequence[0]->getAsTyped();
TIntermTyped* param2 = sequence[1]->getAsTyped(); TIntermTyped *param2 = sequence[1]->getAsTyped();
TIntermTyped* param3 = sequence[2]->getAsTyped(); TIntermTyped *param3 = sequence[2]->getAsTyped();
if (!param1 || !param2 || !param3) if (!param1 || !param2 || !param3)
return true; return true;
needToEmulate = mEmulator.SetFunctionCalled( needToEmulate = mEmulator.SetFunctionCalled(
...@@ -94,34 +96,28 @@ class BuiltInFunctionEmulator::BuiltInFunctionEmulationMarker : public TIntermTr ...@@ -94,34 +96,28 @@ class BuiltInFunctionEmulator::BuiltInFunctionEmulationMarker : public TIntermTr
} }
private: private:
BuiltInFunctionEmulator& mEmulator; BuiltInFunctionEmulator &mEmulator;
}; };
BuiltInFunctionEmulator::BuiltInFunctionEmulator() BuiltInFunctionEmulator::BuiltInFunctionEmulator()
{} {}
void BuiltInFunctionEmulator::addEmulatedFunction( void BuiltInFunctionEmulator::addEmulatedFunction(TOperator op, const TType *param,
TOperator op, const TType& param, const char *emulatedFunctionDefinition)
const char* emulatedFunctionDefinition)
{ {
mEmulatedFunctions[FunctionId(op, param)] = mEmulatedFunctions[FunctionId(op, param)] = std::string(emulatedFunctionDefinition);
std::string(emulatedFunctionDefinition);
} }
void BuiltInFunctionEmulator::addEmulatedFunction( void BuiltInFunctionEmulator::addEmulatedFunction(TOperator op, const TType *param1, const TType *param2,
TOperator op, const TType& param1, const TType& param2, const char *emulatedFunctionDefinition)
const char* emulatedFunctionDefinition)
{ {
mEmulatedFunctions[FunctionId(op, param1, param2)] = mEmulatedFunctions[FunctionId(op, param1, param2)] = std::string(emulatedFunctionDefinition);
std::string(emulatedFunctionDefinition);
} }
void BuiltInFunctionEmulator::addEmulatedFunction( void BuiltInFunctionEmulator::addEmulatedFunction(TOperator op, const TType *param1, const TType *param2,
TOperator op, const TType& param1, const TType& param2, const TType& param3, const TType *param3, const char *emulatedFunctionDefinition)
const char* emulatedFunctionDefinition)
{ {
mEmulatedFunctions[FunctionId(op, param1, param2, param3)] = mEmulatedFunctions[FunctionId(op, param1, param2, param3)] = std::string(emulatedFunctionDefinition);
std::string(emulatedFunctionDefinition);
} }
bool BuiltInFunctionEmulator::IsOutputEmpty() const bool BuiltInFunctionEmulator::IsOutputEmpty() const
...@@ -129,48 +125,48 @@ bool BuiltInFunctionEmulator::IsOutputEmpty() const ...@@ -129,48 +125,48 @@ bool BuiltInFunctionEmulator::IsOutputEmpty() const
return (mFunctions.size() == 0); return (mFunctions.size() == 0);
} }
void BuiltInFunctionEmulator::OutputEmulatedFunctions( void BuiltInFunctionEmulator::OutputEmulatedFunctions(TInfoSinkBase &out) const
TInfoSinkBase& out) const
{ {
for (size_t i = 0; i < mFunctions.size(); ++i) { for (size_t i = 0; i < mFunctions.size(); ++i)
{
out << mEmulatedFunctions.find(mFunctions[i])->second << "\n\n"; out << mEmulatedFunctions.find(mFunctions[i])->second << "\n\n";
} }
} }
bool BuiltInFunctionEmulator::SetFunctionCalled( bool BuiltInFunctionEmulator::SetFunctionCalled(TOperator op, const TType &param)
TOperator op, const TType& param)
{ {
return SetFunctionCalled(FunctionId(op, param)); return SetFunctionCalled(FunctionId(op, &param));
} }
bool BuiltInFunctionEmulator::SetFunctionCalled( bool BuiltInFunctionEmulator::SetFunctionCalled(TOperator op, const TType &param1, const TType &param2)
TOperator op, const TType& param1, const TType& param2)
{ {
return SetFunctionCalled(FunctionId(op, param1, param2)); return SetFunctionCalled(FunctionId(op, &param1, &param2));
} }
bool BuiltInFunctionEmulator::SetFunctionCalled( bool BuiltInFunctionEmulator::SetFunctionCalled(TOperator op,
TOperator op, const TType& param1, const TType& param2, const TType& param3) const TType &param1, const TType &param2, const TType &param3)
{ {
return SetFunctionCalled(FunctionId(op, param1, param2, param3)); return SetFunctionCalled(FunctionId(op, &param1, &param2, &param3));
} }
bool BuiltInFunctionEmulator::SetFunctionCalled( bool BuiltInFunctionEmulator::SetFunctionCalled(const FunctionId &functionId)
const FunctionId& functionId) { {
if (mEmulatedFunctions.find(functionId) != mEmulatedFunctions.end()) if (mEmulatedFunctions.find(functionId) != mEmulatedFunctions.end())
{ {
for (size_t i = 0; i < mFunctions.size(); ++i) { for (size_t i = 0; i < mFunctions.size(); ++i)
{
if (mFunctions[i] == functionId) if (mFunctions[i] == functionId)
return true; return true;
} }
mFunctions.push_back(functionId); // Copy the functionId if it needs to be stored, to make sure that the TType pointers inside
// remain valid and constant.
mFunctions.push_back(functionId.getCopy());
return true; return true;
} }
return false; return false;
} }
void BuiltInFunctionEmulator::MarkBuiltInFunctionsForEmulation( void BuiltInFunctionEmulator::MarkBuiltInFunctionsForEmulation(TIntermNode *root)
TIntermNode* root)
{ {
ASSERT(root); ASSERT(root);
...@@ -188,32 +184,30 @@ void BuiltInFunctionEmulator::Cleanup() ...@@ -188,32 +184,30 @@ void BuiltInFunctionEmulator::Cleanup()
//static //static
TString BuiltInFunctionEmulator::GetEmulatedFunctionName( TString BuiltInFunctionEmulator::GetEmulatedFunctionName(
const TString& name) const TString &name)
{ {
ASSERT(name[name.length() - 1] == '('); ASSERT(name[name.length() - 1] == '(');
return "webgl_" + name.substr(0, name.length() - 1) + "_emu("; return "webgl_" + name.substr(0, name.length() - 1) + "_emu(";
} }
BuiltInFunctionEmulator::FunctionId::FunctionId BuiltInFunctionEmulator::FunctionId::FunctionId(TOperator op, const TType *param)
(TOperator op, const TType& param)
: mOp(op), : mOp(op),
mParam1(param), mParam1(param),
mParam2(EbtVoid), mParam2(new TType(EbtVoid)),
mParam3(EbtVoid) mParam3(new TType(EbtVoid))
{ {
} }
BuiltInFunctionEmulator::FunctionId::FunctionId BuiltInFunctionEmulator::FunctionId::FunctionId(TOperator op, const TType *param1, const TType *param2)
(TOperator op, const TType& param1, const TType& param2)
: mOp(op), : mOp(op),
mParam1(param1), mParam1(param1),
mParam2(param2), mParam2(param2),
mParam3(EbtVoid) mParam3(new TType(EbtVoid))
{ {
} }
BuiltInFunctionEmulator::FunctionId::FunctionId BuiltInFunctionEmulator::FunctionId::FunctionId(TOperator op,
(TOperator op, const TType& param1, const TType& param2, const TType& param3) const TType *param1, const TType *param2, const TType *param3)
: mOp(op), : mOp(op),
mParam1(param1), mParam1(param1),
mParam2(param2), mParam2(param2),
...@@ -221,25 +215,28 @@ BuiltInFunctionEmulator::FunctionId::FunctionId ...@@ -221,25 +215,28 @@ BuiltInFunctionEmulator::FunctionId::FunctionId
{ {
} }
bool BuiltInFunctionEmulator::FunctionId::operator== bool BuiltInFunctionEmulator::FunctionId::operator==(const BuiltInFunctionEmulator::FunctionId &other) const
(const BuiltInFunctionEmulator::FunctionId& other) const
{ {
return (mOp == other.mOp && return (mOp == other.mOp &&
mParam1 == other.mParam1 && *mParam1 == *other.mParam1 &&
mParam2 == other.mParam2 && *mParam2 == *other.mParam2 &&
mParam3 == other.mParam3); *mParam3 == *other.mParam3);
} }
bool BuiltInFunctionEmulator::FunctionId::operator< bool BuiltInFunctionEmulator::FunctionId::operator<(const BuiltInFunctionEmulator::FunctionId &other) const
(const BuiltInFunctionEmulator::FunctionId& other) const
{ {
if (mOp != other.mOp) if (mOp != other.mOp)
return mOp < other.mOp; return mOp < other.mOp;
if (mParam1 != other.mParam1) if (*mParam1 != *other.mParam1)
return mParam1 < other.mParam1; return *mParam1 < *other.mParam1;
if (mParam2 != other.mParam2) if (*mParam2 != *other.mParam2)
return mParam2 < other.mParam2; return *mParam2 < *other.mParam2;
if (mParam3 != other.mParam3) if (*mParam3 != *other.mParam3)
return mParam3 < other.mParam3; return *mParam3 < *other.mParam3;
return false; // all fields are equal return false; // all fields are equal
} }
BuiltInFunctionEmulator::FunctionId BuiltInFunctionEmulator::FunctionId::getCopy() const
{
return FunctionId(mOp, new TType(*mParam1), new TType(*mParam2), new TType(*mParam3));
}
src/compiler/translator/BuiltInFunctionEmulator.h
...@@ -21,23 +21,25 @@ class BuiltInFunctionEmulator ...@@ -21,23 +21,25 @@ class BuiltInFunctionEmulator
public: public:
BuiltInFunctionEmulator(); BuiltInFunctionEmulator();
void MarkBuiltInFunctionsForEmulation(TIntermNode* root); void MarkBuiltInFunctionsForEmulation(TIntermNode *root);
void Cleanup(); void Cleanup();
// "name(" becomes "webgl_name_emu(". // "name(" becomes "webgl_name_emu(".
static TString GetEmulatedFunctionName(const TString& name); static TString GetEmulatedFunctionName(const TString &name);
bool IsOutputEmpty() const; bool IsOutputEmpty() const;
// Output function emulation definition. This should be before any other // Output function emulation definition. This should be before any other
// shader source. // shader source.
void OutputEmulatedFunctions(TInfoSinkBase& out) const; void OutputEmulatedFunctions(TInfoSinkBase &out) const;
// Add functions that need to be emulated. // Add functions that need to be emulated.
void addEmulatedFunction(TOperator op, const TType& param, const char* emulatedFunctionDefinition); void addEmulatedFunction(TOperator op, const TType *param, const char *emulatedFunctionDefinition);
void addEmulatedFunction(TOperator op, const TType& param1, const TType& param2, const char* emulatedFunctionDefinition); void addEmulatedFunction(TOperator op, const TType *param1, const TType *param2,
void addEmulatedFunction(TOperator op, const TType& param1, const TType& param2, const TType& param3, const char* emulatedFunctionDefinition); const char *emulatedFunctionDefinition);
void addEmulatedFunction(TOperator op, const TType *param1, const TType *param2, const TType *param3,
const char *emulatedFunctionDefinition);
private: private:
class BuiltInFunctionEmulationMarker; class BuiltInFunctionEmulationMarker;
...@@ -46,28 +48,32 @@ class BuiltInFunctionEmulator ...@@ -46,28 +48,32 @@ class BuiltInFunctionEmulator
// emulated. If the function is not in mEmulatedFunctions, this becomes a // emulated. If the function is not in mEmulatedFunctions, this becomes a
// no-op. Returns true if the function call needs to be replaced with an // no-op. Returns true if the function call needs to be replaced with an
// emulated one. // emulated one.
bool SetFunctionCalled(TOperator op, const TType& param); bool SetFunctionCalled(TOperator op, const TType &param);
bool SetFunctionCalled( bool SetFunctionCalled(TOperator op, const TType &param1, const TType &param2);
TOperator op, const TType& param1, const TType& param2); bool SetFunctionCalled(TOperator op, const TType &param1, const TType &param2, const TType &param3);
bool SetFunctionCalled(
TOperator op, const TType& param1, const TType& param2, const TType& param3);
class FunctionId { class FunctionId {
public: public:
FunctionId(TOperator op, const TType& param); FunctionId(TOperator op, const TType *param);
FunctionId(TOperator op, const TType& param1, const TType& param2); FunctionId(TOperator op, const TType *param1, const TType *param2);
FunctionId(TOperator op, const TType& param1, const TType& param2, const TType& param3); FunctionId(TOperator op, const TType *param1, const TType *param2, const TType *param3);
bool operator==(const FunctionId& other) const; bool operator==(const FunctionId &other) const;
bool operator<(const FunctionId& other) const; bool operator<(const FunctionId &other) const;
FunctionId getCopy() const;
private: private:
TOperator mOp; TOperator mOp;
TType mParam1;
TType mParam2; // The memory that these TType objects use is freed by PoolAllocator. The BuiltInFunctionEmulator's lifetime
TType mParam3; // can extend until after the memory pool is freed, but that's not an issue since this class never destructs
// these objects.
const TType *mParam1;
const TType *mParam2;
const TType *mParam3;
}; };
bool SetFunctionCalled(const FunctionId& functionId); bool SetFunctionCalled(const FunctionId &functionId);
// Map from function id to emulated function definition // Map from function id to emulated function definition
std::map<FunctionId, std::string> mEmulatedFunctions; std::map<FunctionId, std::string> mEmulatedFunctions;
......
src/compiler/translator/BuiltInFunctionEmulatorGLSL.cpp
...@@ -17,10 +17,10 @@ void InitBuiltInFunctionEmulatorForGLSL(BuiltInFunctionEmulator *emu, sh::GLenum ...@@ -17,10 +17,10 @@ void InitBuiltInFunctionEmulatorForGLSL(BuiltInFunctionEmulator *emu, sh::GLenum
// evaluated. This is unlikely to show up in real shaders, but is something to // evaluated. This is unlikely to show up in real shaders, but is something to
// consider. // consider.
TType float1(EbtFloat); TType *float1 = new TType(EbtFloat);
TType float2(EbtFloat, 2); TType *float2 = new TType(EbtFloat, 2);
TType float3(EbtFloat, 3); TType *float3 = new TType(EbtFloat, 3);
TType float4(EbtFloat, 4); TType *float4 = new TType(EbtFloat, 4);
if (shaderType == GL_FRAGMENT_SHADER) if (shaderType == GL_FRAGMENT_SHADER)
{ {
......
src/compiler/translator/BuiltInFunctionEmulatorHLSL.cpp
...@@ -11,10 +11,10 @@ ...@@ -11,10 +11,10 @@
void InitBuiltInFunctionEmulatorForHLSL(BuiltInFunctionEmulator *emu) void InitBuiltInFunctionEmulatorForHLSL(BuiltInFunctionEmulator *emu)
{ {
TType float1(EbtFloat); TType *float1 = new TType(EbtFloat);
TType float2(EbtFloat, 2); TType *float2 = new TType(EbtFloat, 2);
TType float3(EbtFloat, 3); TType *float3 = new TType(EbtFloat, 3);
TType float4(EbtFloat, 4); TType *float4 = new TType(EbtFloat, 4);
emu->addEmulatedFunction(EOpMod, float1, float1, emu->addEmulatedFunction(EOpMod, float1, float1,
"float webgl_mod_emu(float x, float y)\n" "float webgl_mod_emu(float x, float y)\n"
...@@ -250,7 +250,7 @@ void InitBuiltInFunctionEmulatorForHLSL(BuiltInFunctionEmulator *emu) ...@@ -250,7 +250,7 @@ void InitBuiltInFunctionEmulatorForHLSL(BuiltInFunctionEmulator *emu)
" return (y << 16) | x;\n" " return (y << 16) | x;\n"
"}\n"); "}\n");
TType uint1(EbtUInt); TType *uint1 = new TType(EbtUInt);
emu->addEmulatedFunction(EOpUnpackSnorm2x16, uint1, emu->addEmulatedFunction(EOpUnpackSnorm2x16, uint1,
"float webgl_fromSnorm(in uint x) {\n" "float webgl_fromSnorm(in uint x) {\n"
...@@ -327,9 +327,9 @@ void InitBuiltInFunctionEmulatorForHLSL(BuiltInFunctionEmulator *emu) ...@@ -327,9 +327,9 @@ void InitBuiltInFunctionEmulatorForHLSL(BuiltInFunctionEmulator *emu)
" return mul(float4x1(r), float1x3(c));\n" " return mul(float4x1(r), float1x3(c));\n"
"}\n"); "}\n");
TType mat2(EbtFloat, 2, 2); TType *mat2 = new TType(EbtFloat, 2, 2);
TType mat3(EbtFloat, 3, 3); TType *mat3 = new TType(EbtFloat, 3, 3);
TType mat4(EbtFloat, 4, 4); TType *mat4 = new TType(EbtFloat, 4, 4);
// Remember here that the parameter matrix is actually the transpose // Remember here that the parameter matrix is actually the transpose
// of the matrix that we're trying to invert, and the resulting matrix // of the matrix that we're trying to invert, and the resulting matrix
...@@ -408,10 +408,10 @@ void InitBuiltInFunctionEmulatorForHLSL(BuiltInFunctionEmulator *emu) ...@@ -408,10 +408,10 @@ void InitBuiltInFunctionEmulatorForHLSL(BuiltInFunctionEmulator *emu)
" return cof / determinant(transpose(m));\n" " return cof / determinant(transpose(m));\n"
"}\n"); "}\n");
TType bool1(EbtBool); TType *bool1 = new TType(EbtBool);
TType bool2(EbtBool, 2); TType *bool2 = new TType(EbtBool, 2);
TType bool3(EbtBool, 3); TType *bool3 = new TType(EbtBool, 3);
TType bool4(EbtBool, 4); TType *bool4 = new TType(EbtBool, 4);
// Emulate ESSL3 variant of mix that takes last argument as boolean vector. // Emulate ESSL3 variant of mix that takes last argument as boolean vector.
// genType mix (genType x, genType y, genBType a): Selects which vector each returned component comes from. // genType mix (genType x, genType y, genBType a): Selects which vector each returned component comes from.
......
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