From 53ff7a002a8213a5d5e778bef2a895998d9890e1 Mon Sep 17 00:00:00 2001
From: Dan Liew <daniel.liew@imperial.ac.uk>
Date: Wed, 16 Dec 2015 18:13:11 +0000
Subject: Fix memory leaks of ``Array`` objects detected by ASan.
Some of these leaks were introduced by the factory constructor for Array
objects (f049ff3bc04daead8c3bb9f06e89e71e2054c82a) but a few others have
been around for far longer.
This leak was fixed by introducing a ``ArrayCache`` object which has two
purposes
* Retains ownership of all created ``Array`` objects and destroys them when
the ``ArrayCache`` destructor is called.
* Mimic the caching behaviour for symbolic arrays that was introduced
by f049ff3bc04daead8c3bb9f06e89e71e2054c82a where arrays with the same
name and size get "uniqued".
The Executor now maintains a ``arrayCache`` member that it uses and
passes by pointer to objects that need to construct ``Array`` objects (i.e.
``ObjectState``). This way when the Executor is destroyed all the
``Array`` objects get freed which seems like the right time to do this.
For Kleaver the ``ParserImpl`` has a ``TheArrayCache`` member that is
used for building ``Array`` objects. This means that the Parser must
live as long as the built expressions will be used otherwise we will
have a use after free. I'm not sure this is the right design choice.
It might be better to transfer ownership of the ``Array`` objects to
the root ``Decl`` returned by the parser.
---
lib/Core/Executor.cpp | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
(limited to 'lib/Core/Executor.cpp')
diff --git a/lib/Core/Executor.cpp b/lib/Core/Executor.cpp
index 0b34c357..4a21be3e 100644
--- a/lib/Core/Executor.cpp
+++ b/lib/Core/Executor.cpp
@@ -337,7 +337,7 @@ Executor::Executor(const InterpreterOptions &opts,
this->solver = new TimingSolver(solver, EqualitySubstitution);
- memory = new MemoryManager();
+ memory = new MemoryManager(&arrayCache);
}
@@ -2918,8 +2918,9 @@ ref<Expr> Executor::replaceReadWithSymbolic(ExecutionState &state,
// and return it.
static unsigned id;
- const Array *array = Array::CreateArray("rrws_arr" + llvm::utostr(++id),
- Expr::getMinBytesForWidth(e->getWidth()));
+ const Array *array =
+ arrayCache.CreateArray("rrws_arr" + llvm::utostr(++id),
+ Expr::getMinBytesForWidth(e->getWidth()));
ref<Expr> res = Expr::createTempRead(array, e->getWidth());
ref<Expr> eq = NotOptimizedExpr::create(EqExpr::create(e, res));
llvm::errs() << "Making symbolic: " << eq << "\n";
@@ -3257,7 +3258,7 @@ void Executor::executeMakeSymbolic(ExecutionState &state,
while (!state.arrayNames.insert(uniqueName).second) {
uniqueName = name + "_" + llvm::utostr(++id);
}
- const Array *array = Array::CreateArray(uniqueName, mo->size);
+ const Array *array = arrayCache.CreateArray(uniqueName, mo->size);
bindObjectInState(state, mo, false, array);
state.addSymbolic(mo, array);
@@ -3415,8 +3416,8 @@ void Executor::runFunctionAsMain(Function *f,
// hack to clear memory objects
delete memory;
- memory = new MemoryManager();
-
+ memory = new MemoryManager(NULL);
+
globalObjects.clear();
globalAddresses.clear();
--
cgit 1.4.1