Fix memory leak detected by ASan when

``IndependentSolver::computeInitialValues()`` was called where at least
one of the constraint sets computed by
``getAllIndependentConstraintsSets()`` is either unsatisfiable or
the solver failed.

To make things (a little) clearer I've made it so that no
``std::list<>*`` is passed to``getAllIndependentConstraintsSets()``.
Instead ``getAllIndependentConstraintsSets()`` now returns a
``std::list<>*`` that the caller is responsible for cleaning up. The
behaviour previously was really confusing because it was unclear if the
caller or callee was responsible for the clean up.

This fixes #322

1 files changed, 12 insertions, 6 deletions

diff --git a/lib/Solver/IndependentSolver.cpp b/lib/Solver/IndependentSolver.cpp
index 6b5a5586..78126ede 100644
--- a/lib/Solver/IndependentSolver.cpp
+++ b/lib/Solver/IndependentSolver.cpp
@@ -258,9 +258,11 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
 
 // Breaks down a constraint into all of it's individual pieces, returning a
 // list of IndependentElementSets or the independent factors.
-static void
-getAllIndependentConstraintsSets(const Query &query,
-                                 std::list<IndependentElementSet> *&factors) {
+//
+// Caller takes ownership of returned std::list.
+static std::list<IndependentElementSet>*
+getAllIndependentConstraintsSets(const Query &query) {
+  std::list<IndependentElementSet> *factors = new std::list<IndependentElementSet>();
   ConstantExpr *CE = dyn_cast<ConstantExpr>(query.expr);
   if (CE) {
     assert(CE && CE->isFalse() && "the expr should always be false and "
@@ -314,6 +316,8 @@ getAllIndependentConstraintsSets(const Query &query,
     delete factors;
     factors = done;
   } while (!doneLoop);
+
+  return factors;
 }
 
 static 
@@ -459,14 +463,14 @@ bool IndependentSolver::computeInitialValues(const Query& query,
                                              const std::vector<const Array*> &objects,
                                              std::vector< std::vector<unsigned char> > &values,
                                              bool &hasSolution){
-  std::list<IndependentElementSet> * factors = new std::list<IndependentElementSet>;
-
   // We assume the query has a solution except proven differently
   // This is important in case we don't have any constraints but
   // we need initial values for requested array objects.
   hasSolution = true;
+  // FIXME: When we switch to C++11 this should be a std::unique_ptr so we don't need
+  // to remember to manually call delete
+  std::list<IndependentElementSet> *factors = getAllIndependentConstraintsSets(query);
 
-  getAllIndependentConstraintsSets(query, factors);
   //Used to rearrange all of the answers into the correct order
   std::map<const Array*, std::vector<unsigned char> > retMap;
   for (std::list<IndependentElementSet>::iterator it = factors->begin();
@@ -483,9 +487,11 @@ bool IndependentSolver::computeInitialValues(const Query& query,
     if (!solver->impl->computeInitialValues(Query(tmp, ConstantExpr::alloc(0, Expr::Bool)),
                                             arraysInFactor, tempValues, hasSolution)){
       values.clear();
+      delete factors;
       return false;
     } else if (!hasSolution){
       values.clear();
+      delete factors;
       return true;
     } else {
       assert(tempValues.size() == arraysInFactor.size() &&