Commit 438dbcf1 by Jamie Madill Committed by Commit Bot

translator: Fix builtin function emulator use-after-free.

We were calling the global pool allocator in the builtin function emulator, which would lead to us freeing TTypes that were still referenced. Fix this by using the TCache which was designed for such a purpose, and locking the allocator around the builtin function emulator to try and prevent similar bugs from creeping in. Eventually we would like to get rid of the global allocator and replace it with different pools in different contexts, which are managed more safely. BUG=620937 Change-Id: If501ff6ea4d9bf8a2b8f89f2c94a01386f79ee3a Reviewed-on: https://chromium-review.googlesource.com/353671Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
parent 8bcba147
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "angle_gl.h" #include "angle_gl.h"
#include "compiler/translator/BuiltInFunctionEmulator.h" #include "compiler/translator/BuiltInFunctionEmulator.h"
#include "compiler/translator/SymbolTable.h" #include "compiler/translator/SymbolTable.h"
#include "compiler/translator/Cache.h"
class BuiltInFunctionEmulator::BuiltInFunctionEmulationMarker : public TIntermTraverser class BuiltInFunctionEmulator::BuiltInFunctionEmulationMarker : public TIntermTraverser
{ {
...@@ -194,8 +195,8 @@ TString BuiltInFunctionEmulator::GetEmulatedFunctionName( ...@@ -194,8 +195,8 @@ TString BuiltInFunctionEmulator::GetEmulatedFunctionName(
BuiltInFunctionEmulator::FunctionId::FunctionId(TOperator op, const TType *param) BuiltInFunctionEmulator::FunctionId::FunctionId(TOperator op, const TType *param)
: mOp(op), : mOp(op),
mParam1(param), mParam1(param),
mParam2(new TType(EbtVoid)), mParam2(TCache::getType(EbtVoid)),
mParam3(new TType(EbtVoid)) mParam3(TCache::getType(EbtVoid))
{ {
} }
...@@ -203,7 +204,7 @@ BuiltInFunctionEmulator::FunctionId::FunctionId(TOperator op, const TType *param ...@@ -203,7 +204,7 @@ BuiltInFunctionEmulator::FunctionId::FunctionId(TOperator op, const TType *param
: mOp(op), : mOp(op),
mParam1(param1), mParam1(param1),
mParam2(param2), mParam2(param2),
mParam3(new TType(EbtVoid)) mParam3(TCache::getType(EbtVoid))
{ {
} }
......
...@@ -318,7 +318,10 @@ TIntermNode *TCompiler::compileTreeImpl(const char *const shaderStrings[], ...@@ -318,7 +318,10 @@ TIntermNode *TCompiler::compileTreeImpl(const char *const shaderStrings[],
// Built-in function emulation needs to happen after validateLimitations pass. // Built-in function emulation needs to happen after validateLimitations pass.
if (success) if (success)
{ {
// TODO(jmadill): Remove global pool allocator.
GetGlobalPoolAllocator()->lock();
initBuiltInFunctionEmulator(&builtInFunctionEmulator, compileOptions); initBuiltInFunctionEmulator(&builtInFunctionEmulator, compileOptions);
GetGlobalPoolAllocator()->unlock();
builtInFunctionEmulator.MarkBuiltInFunctionsForEmulation(root); builtInFunctionEmulator.MarkBuiltInFunctionsForEmulation(root);
} }
......
...@@ -6,16 +6,16 @@ ...@@ -6,16 +6,16 @@
#include "compiler/translator/PoolAlloc.h" #include "compiler/translator/PoolAlloc.h"
#include "compiler/translator/InitializeGlobals.h"
#include "common/platform.h"
#include "common/angleutils.h"
#include "common/tls.h"
#include <stdint.h> #include <stdint.h>
#include <stdio.h> #include <stdio.h>
#include <assert.h> #include <assert.h>
#include "common/angleutils.h"
#include "common/debug.h"
#include "common/platform.h"
#include "common/tls.h"
#include "compiler/translator/InitializeGlobals.h"
TLSIndex PoolIndex = TLS_INVALID_INDEX; TLSIndex PoolIndex = TLS_INVALID_INDEX;
bool InitializePoolIndex() bool InitializePoolIndex()
...@@ -56,7 +56,8 @@ TPoolAllocator::TPoolAllocator(int growthIncrement, int allocationAlignment) : ...@@ -56,7 +56,8 @@ TPoolAllocator::TPoolAllocator(int growthIncrement, int allocationAlignment) :
freeList(0), freeList(0),
inUseList(0), inUseList(0),
numCalls(0), numCalls(0),
totalBytes(0) totalBytes(0),
mLocked(false)
{ {
// //
// Don't allow page sizes we know are smaller than all common // Don't allow page sizes we know are smaller than all common
...@@ -206,6 +207,8 @@ void TPoolAllocator::popAll() ...@@ -206,6 +207,8 @@ void TPoolAllocator::popAll()
void* TPoolAllocator::allocate(size_t numBytes) void* TPoolAllocator::allocate(size_t numBytes)
{ {
ASSERT(!mLocked);
// //
// Just keep some interesting statistics. // Just keep some interesting statistics.
// //
...@@ -284,6 +287,17 @@ void* TPoolAllocator::allocate(size_t numBytes) ...@@ -284,6 +287,17 @@ void* TPoolAllocator::allocate(size_t numBytes)
return initializeAllocation(inUseList, ret, numBytes); return initializeAllocation(inUseList, ret, numBytes);
} }
void TPoolAllocator::lock()
{
ASSERT(!mLocked);
mLocked = true;
}
void TPoolAllocator::unlock()
{
ASSERT(mLocked);
mLocked = false;
}
// //
// Check all allocations in a list for damage by calling check on each. // Check all allocations in a list for damage by calling check on each.
......
...@@ -152,6 +152,11 @@ public: ...@@ -152,6 +152,11 @@ public:
// by calling pop(), and to not have to solve memory leak problems. // by calling pop(), and to not have to solve memory leak problems.
// //
// Catch unwanted allocations.
// TODO(jmadill): Remove this when we remove the global allocator.
void lock();
void unlock();
protected: protected:
friend struct tHeader; friend struct tHeader;
...@@ -211,6 +216,7 @@ protected: ...@@ -211,6 +216,7 @@ protected:
private: private:
TPoolAllocator& operator=(const TPoolAllocator&); // dont allow assignment operator TPoolAllocator& operator=(const TPoolAllocator&); // dont allow assignment operator
TPoolAllocator(const TPoolAllocator&); // dont allow default copy constructor TPoolAllocator(const TPoolAllocator&); // dont allow default copy constructor
bool mLocked;
}; };
......
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