Let constraint manager inform checkers that some assumption logic has happend.

Add new states for symbolic regions tracked by malloc checker. This enables us
to do malloc checking more accurately. See test case.

Based on Lei Zhang's patch and discussion.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@92342 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Analysis/PathSensitive/Checker.h b/include/clang/Analysis/PathSensitive/Checker.h
index c87889a267961a552b97c2033bb1a97a2406afb9..924a8b11b098de613afccd77d0e3b3cd38d69ae9 100644 (file)
--- a/include/clang/Analysis/PathSensitive/Checker.h
+++ b/include/clang/Analysis/PathSensitive/Checker.h
@@ -269,6 +269,11 @@ public:
   virtual bool EvalCallExpr(CheckerContext &C, const CallExpr *CE) {
     return false;
   }
+
+  virtual const GRState *EvalAssume(const GRState *state, SVal Cond, 
+                                    bool Assumption) {
+    return state;
+  }
 };
 } // end clang namespace
 

diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index 3f5e16532ed486946f7e754f5a3f405294be9526..e05c6243846da6f813f6cba94e1c7cb6273e4e69 100644 (file)
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -80,7 +80,7 @@ class GRExprEngine : public GRSubEngine {
   typedef llvm::DenseMap<void *, unsigned> CheckerMap;
   CheckerMap CheckerM;
   
-  typedef std::vector<std::pair<void *, Checker*> >CheckersOrdered;
+  typedef std::vector<std::pair<void *, Checker*> > CheckersOrdered;
   CheckersOrdered Checkers;
 
   /// BR - The BugReporter associated with this engine.  It is important that
@@ -111,10 +111,10 @@ public:
   GRStmtNodeBuilder &getBuilder() { assert(Builder); return *Builder; }
 
   /// setTransferFunctions
-  void setTransferFunctions(GRTransferFuncs* tf);
+  void setTransferFunctionsAndCheckers(GRTransferFuncs* tf);
 
   void setTransferFunctions(GRTransferFuncs& tf) {
-    setTransferFunctions(&tf);
+    setTransferFunctionsAndCheckers(&tf);
   }
 
   /// ViewGraph - Visualize the ExplodedGraph created by executing the

diff --git a/include/clang/Analysis/PathSensitive/GRState.h b/include/clang/Analysis/PathSensitive/GRState.h
index 421ebbf9bd5e29300e06f086ef08bdbbc0c332a1..424b0d77e84c049111c914a58dedf4ffa5a1fd00 100644 (file)
--- a/include/clang/Analysis/PathSensitive/GRState.h
+++ b/include/clang/Analysis/PathSensitive/GRState.h
@@ -41,6 +41,7 @@ namespace clang {
 
 class GRStateManager;
 class GRTransferFuncs;
+class Checker;
 
 typedef ConstraintManager* (*ConstraintManagerCreator)(GRStateManager&);
 typedef StoreManager* (*StoreManagerCreator)(GRStateManager&);
@@ -160,6 +161,9 @@ public:
   SymbolManager &getSymbolManager() const;
   GRTransferFuncs &getTransferFuncs() const;
 
+  std::vector<std::pair<void *, Checker *> >::iterator checker_begin() const;
+  std::vector<std::pair<void *, Checker *> >::iterator checker_end() const;
+
   //==---------------------------------------------------------------------==//
   // Constraints on values.
   //==---------------------------------------------------------------------==//
@@ -418,6 +422,9 @@ private:
   ///  for manipulating and creating SVals.
   GRTransferFuncs* TF;
 
+  /// Reference to all checkers in GRExprEngine.
+  std::vector<std::pair<void *, Checker*> > *Checkers;
+
 public:
 
   GRStateManager(ASTContext& Ctx,
@@ -441,6 +448,8 @@ public:
 
   GRTransferFuncs& getTransferFuncs() { return *TF; }
 
+  std::vector<std::pair<void *, Checker *> > &getCheckers() { return *Checkers;}
+
   BasicValueFactory &getBasicVals() {
     return ValueMgr.getBasicValueFactory();
   }
@@ -697,6 +706,16 @@ inline GRTransferFuncs &GRState::getTransferFuncs() const {
   return getStateManager().getTransferFuncs();
 }
 
+inline std::vector<std::pair<void *, Checker *> >::iterator 
+GRState::checker_begin() const {
+  return getStateManager().getCheckers().begin();
+}
+
+inline std::vector<std::pair<void *, Checker *> >::iterator 
+GRState::checker_end() const {
+  return getStateManager().getCheckers().end();
+}
+
 template<typename T>
 const GRState *GRState::add(typename GRStateTrait<T>::key_type K) const {
   return getStateManager().add<T>(this, K, get_context<T>());

diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index 2ed35fa4696c4b2482f11d6f0018c0d693a2c8cb..2ce8edd1cc466224b8c21d7d459c7b088fed9531 100644 (file)
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -330,8 +330,9 @@ GRExprEngine::~GRExprEngine() {
 // Utility methods.
 //===----------------------------------------------------------------------===//
 
-void GRExprEngine::setTransferFunctions(GRTransferFuncs* tf) {
+void GRExprEngine::setTransferFunctionsAndCheckers(GRTransferFuncs* tf) {
   StateMgr.TF = tf;
+  StateMgr.Checkers = &Checkers;
   tf->RegisterChecks(*this);
   tf->RegisterPrinters(getStateManager().Printers);
 }

diff --git a/lib/Analysis/MallocChecker.cpp b/lib/Analysis/MallocChecker.cpp
index 2ed070a170cd992ec1e5a61336e9b4e51e30414a..fab73ee7b104e444d8a4aae06afd40abefec5fa1 100644 (file)
--- a/lib/Analysis/MallocChecker.cpp
+++ b/lib/Analysis/MallocChecker.cpp
@@ -23,13 +23,13 @@ using namespace clang;
 namespace {
 
 class RefState {
-  enum Kind { Allocated, Released, Escaped } K;
+  enum Kind { AllocateUnchecked, AllocateFailed, Released, Escaped } K;
   const Stmt *S;
 
 public:
   RefState(Kind k, const Stmt *s) : K(k), S(s) {}
 
-  bool isAllocated() const { return K == Allocated; }
+  bool isAllocated() const { return K == AllocateUnchecked; }
   bool isReleased() const { return K == Released; }
   bool isEscaped() const { return K == Escaped; }
 
@@ -37,7 +37,12 @@ public:
     return K == X.K && S == X.S;
   }
 
-  static RefState getAllocated(const Stmt *s) { return RefState(Allocated, s); }
+  static RefState getAllocateUnchecked(const Stmt *s) { 
+    return RefState(AllocateUnchecked, s); 
+  }
+  static RefState getAllocateFailed() {
+    return RefState(AllocateFailed, 0);
+  }
   static RefState getReleased(const Stmt *s) { return RefState(Released, s); }
   static RefState getEscaped(const Stmt *s) { return RefState(Escaped, s); }
 
@@ -62,6 +67,8 @@ public:
   void EvalDeadSymbols(CheckerContext &C,const Stmt *S,SymbolReaper &SymReaper);
   void EvalEndPath(GREndPathNodeBuilder &B, void *tag, GRExprEngine &Eng);
   void PreVisitReturnStmt(CheckerContext &C, const ReturnStmt *S);
+  const GRState *EvalAssume(const GRState *state, SVal Cond, bool Assumption);
+
 private:
   void MallocMem(CheckerContext &C, const CallExpr *CE);
   const GRState *MallocMemAux(CheckerContext &C, const CallExpr *CE,
@@ -74,6 +81,8 @@ private:
 };
 } // end anonymous namespace
 
+typedef llvm::ImmutableMap<SymbolRef, RefState> RegionStateTy;
+
 namespace clang {
   template <>
   struct GRStateTrait<RegionState> 
@@ -144,7 +153,7 @@ const GRState *MallocChecker::MallocMemAux(CheckerContext &C,
   SymbolRef Sym = RetVal.getAsLocSymbol();
   assert(Sym);
   // Set the symbol's state to Allocated.
-  return state->set<RegionState>(Sym, RefState::getAllocated(CE));
+  return state->set<RegionState>(Sym, RefState::getAllocateUnchecked(CE));
 }
 
 void MallocChecker::FreeMem(CheckerContext &C, const CallExpr *CE) {
@@ -298,3 +307,18 @@ void MallocChecker::PreVisitReturnStmt(CheckerContext &C, const ReturnStmt *S) {
 
   C.addTransition(state);
 }
+
+const GRState *MallocChecker::EvalAssume(const GRState *state, SVal Cond, 
+                                         bool Assumption) {
+  // If a symblic region is assumed to NULL, set its state to AllocateFailed.
+  // FIXME: should also check symbols assumed to non-null.
+
+  RegionStateTy RS = state->get<RegionState>();
+
+  for (RegionStateTy::iterator I = RS.begin(), E = RS.end(); I != E; ++I) {
+    if (state->getSymVal(I.getKey()))
+      state = state->set<RegionState>(I.getKey(),RefState::getAllocateFailed());
+  }
+
+  return state;
+}

diff --git a/lib/Analysis/SimpleConstraintManager.cpp b/lib/Analysis/SimpleConstraintManager.cpp
index 015db76080dfbe3091f14aa007406cf251e6885e..23c3b4175835984289116d03973bcb60dfb01502 100644 (file)
--- a/lib/Analysis/SimpleConstraintManager.cpp
+++ b/lib/Analysis/SimpleConstraintManager.cpp
@@ -15,6 +15,7 @@
 #include "SimpleConstraintManager.h"
 #include "clang/Analysis/PathSensitive/GRExprEngine.h"
 #include "clang/Analysis/PathSensitive/GRState.h"
+#include "clang/Analysis/PathSensitive/Checker.h"
 
 namespace clang {
 
@@ -72,8 +73,17 @@ const GRState *SimpleConstraintManager::Assume(const GRState *state, Loc Cond,
   // EvalAssume is used to call into the GRTransferFunction object to perform
   // any checker-specific update of the state based on this assumption being
   // true or false.
-  return state ? state->getTransferFuncs().EvalAssume(state, Cond, Assumption)
-               : NULL;
+
+  if (!state)
+    return 0;
+
+  std::vector<std::pair<void *, Checker*> >::iterator 
+    I = state->checker_begin(), E = state->checker_end();
+
+  for (; I != E; ++I) {
+    state = I->second->EvalAssume(state, Cond, Assumption);
+  }
+  return state->getTransferFuncs().EvalAssume(state, Cond, Assumption);
 }
 
 const GRState *SimpleConstraintManager::AssumeAux(const GRState *state,
@@ -128,8 +138,18 @@ const GRState *SimpleConstraintManager::Assume(const GRState *state,
   // EvalAssume is used to call into the GRTransferFunction object to perform
   // any checker-specific update of the state based on this assumption being
   // true or false.
-  return state ? state->getTransferFuncs().EvalAssume(state, Cond, Assumption)
-               : NULL;
+
+  if (!state)
+    return 0;
+
+  std::vector<std::pair<void *, Checker*> >::iterator 
+    I = state->checker_begin(), E = state->checker_end();
+
+  for (; I != E; ++I) {
+    state = I->second->EvalAssume(state, Cond, Assumption);
+  }
+
+  return state->getTransferFuncs().EvalAssume(state, Cond, Assumption);
 }
 
 const GRState *SimpleConstraintManager::AssumeAux(const GRState *state,

diff --git a/lib/Frontend/AnalysisConsumer.cpp b/lib/Frontend/AnalysisConsumer.cpp
index dd5c6d3013f043c7c793a1d1468048d839bc2572..6824d8f4eb40b37267f9b4d4bf53d4e6a1d4b02a 100644 (file)
--- a/lib/Frontend/AnalysisConsumer.cpp
+++ b/lib/Frontend/AnalysisConsumer.cpp
@@ -373,7 +373,7 @@ static void ActionGRExprEngine(AnalysisConsumer &C, AnalysisManager& mgr,
   if (C.Opts.EnableExperimentalChecks)
     RegisterExperimentalChecks(Eng);
   
-  Eng.setTransferFunctions(tf);  
+  Eng.setTransferFunctionsAndCheckers(tf);  
 
   // Set the graph auditor.
   llvm::OwningPtr<ExplodedNode::Auditor> Auditor;
@@ -506,7 +506,7 @@ static void ActionInlineCall(AnalysisConsumer &C, AnalysisManager &mgr,
   
   // Make a fake transfer function. The GRTransferFunc interface will be 
   // removed.
-  Eng.setTransferFunctions(new GRTransferFuncs());  
+  Eng.setTransferFunctionsAndCheckers(new GRTransferFuncs());  
 
   // Register call inliner as the last checker.
   RegisterCallInliner(Eng);

diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c
index c885587df76b521d0822c566109b8c46ee5983df..4d771eeb4bd3d13c28b3a96eb6630671a63ad611 100644 (file)
--- a/test/Analysis/malloc.c
+++ b/test/Analysis/malloc.c
@@ -43,3 +43,11 @@ int *f5() {
   q = realloc(q, 20);
   return q; // no-warning
 }
+
+void f6() {
+  int *p = malloc(10);
+  if (!p)
+    return; // no-warning
+  else
+    free(p);
+}