]> granicus.if.org Git - clang/commitdiff
[analyzer] Make Malloc Checker optimistic in presence of inlining.
authorAnna Zaks <ganna@apple.com>
Tue, 14 Feb 2012 21:55:24 +0000 (21:55 +0000)
committerAnna Zaks <ganna@apple.com>
Tue, 14 Feb 2012 21:55:24 +0000 (21:55 +0000)
(In response of Ted's review of r150112.)

This moves the logic which checked if a symbol escapes through a
parameter to invalidateRegionCallback (instead of post CallExpr visit.)

To accommodate the change, added a CallOrObjCMessage parameter to
checkRegionChanges callback.

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

include/clang/StaticAnalyzer/Core/Checker.h
include/clang/StaticAnalyzer/Core/CheckerManager.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
lib/StaticAnalyzer/Checkers/CStringChecker.cpp
lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
lib/StaticAnalyzer/Core/CheckerManager.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ProgramState.cpp

index 1e9055500a77f596d8d7f74ee4bb4005169604bf..76d8c15f75cb6273ec6f62364c663c7a178b1c07 100644 (file)
@@ -265,9 +265,10 @@ class RegionChanges {
                       ProgramStateRef state,
                       const StoreManager::InvalidatedSymbols *invalidated,
                       ArrayRef<const MemRegion *> Explicits,
-                      ArrayRef<const MemRegion *> Regions) {
+                      ArrayRef<const MemRegion *> Regions,
+                      const CallOrObjCMessage *Call) {
     return ((const CHECKER *)checker)->checkRegionChanges(state, invalidated,
-                                                          Explicits, Regions);
+                                                      Explicits, Regions, Call);
   }
   template <typename CHECKER>
   static bool _wantsRegionChangeUpdate(void *checker,
index a404657882cc421960bdcbcb5f6c41c657633ad2..fa22f53d5fd874e6158615c1a710392093d6da01 100644 (file)
@@ -52,6 +52,19 @@ public:
 
 template <typename T> class CheckerFn;
 
+template <typename RET, typename P1, typename P2, typename P3, typename P4,
+          typename P5>
+class CheckerFn<RET(P1, P2, P3, P4, P5)> {
+  typedef RET (*Func)(void *, P1, P2, P3, P4, P5);
+  Func Fn;
+public:
+  CheckerBase *Checker;
+  CheckerFn(CheckerBase *checker, Func fn) : Fn(fn), Checker(checker) { }
+  RET operator()(P1 p1, P2 p2, P3 p3, P4 p4, P5 p5) const {
+    return Fn(Checker, p1, p2, p3, p4, p5);
+  }
+};
+
 template <typename RET, typename P1, typename P2, typename P3, typename P4>
 class CheckerFn<RET(P1, P2, P3, P4)> {
   typedef RET (*Func)(void *, P1, P2, P3, P4);
@@ -269,11 +282,14 @@ public:
   ///   For example, in the case of a function call, these would be arguments.
   /// \param Regions The transitive closure of accessible regions,
   ///   i.e. all regions that may have been touched by this change.
+  /// \param The call expression wrapper if the regions are invalidated by a
+  ///   call.
   ProgramStateRef 
   runCheckersForRegionChanges(ProgramStateRef state,
                             const StoreManager::InvalidatedSymbols *invalidated,
                               ArrayRef<const MemRegion *> ExplicitRegions,
-                              ArrayRef<const MemRegion *> Regions);
+                              ArrayRef<const MemRegion *> Regions,
+                              const CallOrObjCMessage *Call);
 
   /// \brief Run checkers for handling assumptions on symbolic values.
   ProgramStateRef runCheckersForEvalAssume(ProgramStateRef state,
@@ -349,8 +365,9 @@ public:
   
   typedef CheckerFn<ProgramStateRef (ProgramStateRef,
                                 const StoreManager::InvalidatedSymbols *symbols,
-                                    ArrayRef<const MemRegion *> ExplicitRegions,
-                                          ArrayRef<const MemRegion *> Regions)>
+                                ArrayRef<const MemRegion *> ExplicitRegions,
+                                ArrayRef<const MemRegion *> Regions,
+                                const CallOrObjCMessage *Call)>
       CheckRegionChangesFunc;
   
   typedef CheckerFn<bool (ProgramStateRef)> WantsRegionChangeUpdateFunc;
index f1a5d88e79fc45736e6b862514cae81109b90764..b7c4958c510882e528ddbf60b42a34cba4335cc2 100644 (file)
@@ -212,7 +212,8 @@ public:
   processRegionChanges(ProgramStateRef state,
                        const StoreManager::InvalidatedSymbols *invalidated,
                        ArrayRef<const MemRegion *> ExplicitRegions,
-                       ArrayRef<const MemRegion *> Regions);
+                       ArrayRef<const MemRegion *> Regions,
+                       const CallOrObjCMessage *Call);
 
   /// printState - Called by ProgramStateManager to print checker-specific data.
   void printState(raw_ostream &Out, ProgramStateRef State,
index d252da13a2bf20f6c38102299830060f6cce7b7e..0fb04b83250be6f1f417e39e5575ac41c4afbd90 100644 (file)
@@ -97,19 +97,20 @@ public:
   ///  region change should trigger a processRegionChanges update.
   virtual bool wantsRegionChangeUpdate(ProgramStateRef state) = 0;
 
-  /// processRegionChanges - Called by ProgramStateManager whenever a change is made
-  ///  to the store. Used to update checkers that track region values.
+  /// processRegionChanges - Called by ProgramStateManager whenever a change is
+  /// made to the store. Used to update checkers that track region values.
   virtual ProgramStateRef 
   processRegionChanges(ProgramStateRef state,
                        const StoreManager::InvalidatedSymbols *invalidated,
                        ArrayRef<const MemRegion *> ExplicitRegions,
-                       ArrayRef<const MemRegion *> Regions) = 0;
+                       ArrayRef<const MemRegion *> Regions,
+                       const CallOrObjCMessage *Call) = 0;
 
 
   inline ProgramStateRef 
   processRegionChange(ProgramStateRef state,
                       const MemRegion* MR) {
-    return processRegionChanges(state, 0, MR, MR);
+    return processRegionChanges(state, 0, MR, MR, 0);
   }
 
   /// printState - Called by ProgramStateManager to print checker-specific data.
index 748fc43a6770678b991990f363226318946bf852..dcc62a60cb944d7b75347d8e311d05468985850e 100644 (file)
@@ -64,7 +64,8 @@ public:
     checkRegionChanges(ProgramStateRef state,
                        const StoreManager::InvalidatedSymbols *,
                        ArrayRef<const MemRegion *> ExplicitRegions,
-                       ArrayRef<const MemRegion *> Regions) const;
+                       ArrayRef<const MemRegion *> Regions,
+                       const CallOrObjCMessage *Call) const;
 
   typedef void (CStringChecker::*FnCheck)(CheckerContext &,
                                           const CallExpr *) const;
@@ -1807,7 +1808,8 @@ ProgramStateRef
 CStringChecker::checkRegionChanges(ProgramStateRef state,
                                    const StoreManager::InvalidatedSymbols *,
                                    ArrayRef<const MemRegion *> ExplicitRegions,
-                                   ArrayRef<const MemRegion *> Regions) const {
+                                   ArrayRef<const MemRegion *> Regions,
+                                   const CallOrObjCMessage *Call) const {
   CStringLength::EntryMap Entries = state->get<CStringLength>();
   if (Entries.isEmpty())
     return state;
index cd31ae38bea0d2c24cbdd5083e023435e2af8bd3..a5f9d300c91f89ca0b6558f91a45c3ccb6a82f35 100644 (file)
@@ -184,13 +184,27 @@ public:
   /// check::LiveSymbols
   void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const {}
 
-  /// check::RegionChanges
+
   bool wantsRegionChangeUpdate(ProgramStateRef St) const { return true; }
+  
+  /// check::RegionChanges
+  /// Allows tracking regions which get invalidated.
+  /// \param state The current program state.
+  /// \param invalidated A set of all symbols potentially touched by the change.
+  /// \param ExplicitRegions The regions explicitly requested for invalidation.
+  ///   For example, in the case of a function call, these would be arguments.
+  /// \param Regions The transitive closure of accessible regions,
+  ///   i.e. all regions that may have been touched by this change.
+  /// \param The call expression wrapper if the regions are invalidated by a
+  ///   call, 0 otherwise.
+  /// Note, in order to be notified, the checker should also implement 
+  /// wantsRegionChangeUpdate callback.
   ProgramStateRef 
     checkRegionChanges(ProgramStateRef State,
                        const StoreManager::InvalidatedSymbols *,
                        ArrayRef<const MemRegion *> ExplicitRegions,
-                       ArrayRef<const MemRegion *> Regions) const {
+                       ArrayRef<const MemRegion *> Regions,
+                       const CallOrObjCMessage *Call) const {
     return State;
   }
 
index 7cbb49e2d8f0b0e009e1a39d55d61317d31a6645..d57e8a9f621adb018fe0a27bddb5730bb6395fc0 100644 (file)
@@ -17,6 +17,7 @@
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
@@ -69,6 +70,7 @@ public:
 class MallocChecker : public Checker<check::DeadSymbols,
                                      check::EndPath,
                                      check::PreStmt<ReturnStmt>,
+                                     check::PreStmt<CallExpr>,
                                      check::PostStmt<CallExpr>,
                                      check::Location,
                                      check::Bind,
@@ -94,8 +96,7 @@ public:
 
   ChecksFilter Filter;
 
-  void initIdentifierInfo(CheckerContext &C) const;
-
+  void checkPreStmt(const CallExpr *S, CheckerContext &C) const;
   void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
   void checkEndPath(CheckerContext &C) const;
@@ -110,12 +111,19 @@ public:
   checkRegionChanges(ProgramStateRef state,
                      const StoreManager::InvalidatedSymbols *invalidated,
                      ArrayRef<const MemRegion *> ExplicitRegions,
-                     ArrayRef<const MemRegion *> Regions) const;
+                     ArrayRef<const MemRegion *> Regions,
+                     const CallOrObjCMessage *Call) const;
   bool wantsRegionChangeUpdate(ProgramStateRef state) const {
     return true;
   }
 
 private:
+  void initIdentifierInfo(ASTContext &C) const;
+
+  /// Check if this is one of the functions which can allocate/reallocate memory 
+  /// pointed to by one of its arguments.
+  bool isMemFunction(const FunctionDecl *FD, ASTContext &C) const;
+
   static void MallocMem(CheckerContext &C, const CallExpr *CE);
   static void MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE,
                                    const OwnershipAttr* Att);
@@ -144,6 +152,10 @@ private:
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
                          const Stmt *S = 0) const;
 
+  /// Check if the function is not known to us. So, for example, we could
+  /// conservatively assume it can free/reallocate it's pointer arguments.
+  bool hasUnknownBehavior(const FunctionDecl *FD, ProgramStateRef State) const;
+
   static bool SummarizeValue(raw_ostream &os, SVal V);
   static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR);
   void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange range) const;
@@ -220,8 +232,7 @@ public:
 };
 } // end anonymous namespace
 
-void MallocChecker::initIdentifierInfo(CheckerContext &C) const {
-  ASTContext &Ctx = C.getASTContext();
+void MallocChecker::initIdentifierInfo(ASTContext &Ctx) const {
   if (!II_malloc)
     II_malloc = &Ctx.Idents.get("malloc");
   if (!II_free)
@@ -232,11 +243,31 @@ void MallocChecker::initIdentifierInfo(CheckerContext &C) const {
     II_calloc = &Ctx.Idents.get("calloc");
 }
 
+bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) const {
+  initIdentifierInfo(C);
+  IdentifierInfo *FunI = FD->getIdentifier();
+  if (!FunI)
+    return false;
+
+  // TODO: Add more here : ex: reallocf!
+  if (FunI == II_malloc || FunI == II_free ||
+      FunI == II_realloc || FunI == II_calloc)
+    return true;
+
+  if (Filter.CMallocOptimistic && FD->hasAttrs() &&
+      FD->specific_attr_begin<OwnershipAttr>() !=
+          FD->specific_attr_end<OwnershipAttr>())
+    return true;
+
+
+  return false;
+}
+
 void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
   const FunctionDecl *FD = C.getCalleeDecl(CE);
   if (!FD)
     return;
-  initIdentifierInfo(C);
+  initIdentifierInfo(C.getASTContext());
 
   if (FD->getIdentifier() == II_malloc) {
     MallocMem(C, CE);
@@ -278,42 +309,6 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
       }
     }
   }
-
-  // Check use after free, when a freed pointer is passed to a call.
-  ProgramStateRef State = C.getState();
-  for (CallExpr::const_arg_iterator I = CE->arg_begin(),
-                                    E = CE->arg_end(); I != E; ++I) {
-    const Expr *A = *I;
-    if (A->getType().getTypePtr()->isAnyPointerType()) {
-      SymbolRef Sym = State->getSVal(A, C.getLocationContext()).getAsSymbol();
-      if (!Sym)
-        continue;
-      if (checkUseAfterFree(Sym, C, A))
-        return;
-    }
-  }
-
-  // The pointer might escape through a function call.
-  // TODO: This should be rewritten to take into account inlining.
-  if (Filter.CMallocPessimistic) {
-    SourceLocation FLoc = FD->getLocation();
-    // We assume that the pointers cannot escape through calls to system
-    // functions.
-    if (C.getSourceManager().isInSystemHeader(FLoc))
-      return;
-
-    ProgramStateRef State = C.getState();
-    for (CallExpr::const_arg_iterator I = CE->arg_begin(),
-                                      E = CE->arg_end(); I != E; ++I) {
-      const Expr *A = *I;
-      if (A->getType().getTypePtr()->isAnyPointerType()) {
-        SymbolRef Sym = State->getSVal(A, C.getLocationContext()).getAsSymbol();
-        if (!Sym)
-          continue;
-        checkEscape(Sym, A, C);
-      }
-    }
-  }
 }
 
 void MallocChecker::MallocMem(CheckerContext &C, const CallExpr *CE) {
@@ -802,6 +797,25 @@ bool MallocChecker::checkEscape(SymbolRef Sym, const Stmt *S,
   return false;
 }
 
+void MallocChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const {
+  if (isMemFunction(C.getCalleeDecl(CE), C.getASTContext()))
+    return;
+
+  // Check use after free, when a freed pointer is passed to a call.
+  ProgramStateRef State = C.getState();
+  for (CallExpr::const_arg_iterator I = CE->arg_begin(),
+                                    E = CE->arg_end(); I != E; ++I) {
+    const Expr *A = *I;
+    if (A->getType().getTypePtr()->isAnyPointerType()) {
+      SymbolRef Sym = State->getSVal(A, C.getLocationContext()).getAsSymbol();
+      if (!Sym)
+        continue;
+      if (checkUseAfterFree(Sym, C, A))
+        return;
+    }
+  }
+}
+
 void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const {
   const Expr *E = S->getRetValue();
   if (!E)
@@ -926,22 +940,54 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state,
   return state;
 }
 
+// Check if the function is not known to us. So, for example, we could
+// conservatively assume it can free/reallocate it's pointer arguments.
+// (We assume that the pointers cannot escape through calls to system
+// functions not handled by this checker.)
+bool MallocChecker::hasUnknownBehavior(const FunctionDecl *FD,
+                                       ProgramStateRef State) const {
+  ASTContext &ASTC = State->getStateManager().getContext();
+
+  // If it's one of the allocation functions we can reason about, we model it's
+  // behavior explicitly.
+  if (isMemFunction(FD, ASTC)) {
+    return false;
+  }
+
+  // If it's a system call, we know it does not free the memory.
+  SourceManager &SM = ASTC.getSourceManager();
+  if (SM.isInSystemHeader(FD->getLocation())) {
+    return false;
+  }
+
+  // Otherwise, assume that the function can free memory.
+  return true;
+}
+
 // If the symbol we are tracking is invalidated, but not explicitly (ex: the &p
 // escapes, when we are tracking p), do not track the symbol as we cannot reason
 // about it anymore.
 ProgramStateRef
-MallocChecker::checkRegionChanges(ProgramStateRef state,
+MallocChecker::checkRegionChanges(ProgramStateRef State,
                             const StoreManager::InvalidatedSymbols *invalidated,
                                     ArrayRef<const MemRegion *> ExplicitRegions,
-                                    ArrayRef<const MemRegion *> Regions) const {
+                                    ArrayRef<const MemRegion *> Regions,
+                                    const CallOrObjCMessage *Call) const {
   if (!invalidated)
-    return state;
-
+    return State;
   llvm::SmallPtrSet<SymbolRef, 8> WhitelistedSymbols;
-  for (ArrayRef<const MemRegion *>::iterator I = ExplicitRegions.begin(),
-       E = ExplicitRegions.end(); I != E; ++I) {
-    if (const SymbolicRegion *SR = (*I)->StripCasts()->getAs<SymbolicRegion>())
-      WhitelistedSymbols.insert(SR->getSymbol());
+
+  const FunctionDecl *FD = (Call ? dyn_cast<FunctionDecl>(Call->getDecl()) : 0);
+
+  // If it's a call which might free or reallocate memory, we assume that all
+  // regions (explicit and implicit) escaped. Otherwise, whitelist explicit
+  // pointers; we still can track them.
+  if (!(FD && hasUnknownBehavior(FD, State))) {
+    for (ArrayRef<const MemRegion *>::iterator I = ExplicitRegions.begin(),
+        E = ExplicitRegions.end(); I != E; ++I) {
+      if (const SymbolicRegion *R = (*I)->StripCasts()->getAs<SymbolicRegion>())
+        WhitelistedSymbols.insert(R->getSymbol());
+    }
   }
 
   for (StoreManager::InvalidatedSymbols::const_iterator I=invalidated->begin(),
@@ -949,10 +995,11 @@ MallocChecker::checkRegionChanges(ProgramStateRef state,
     SymbolRef sym = *I;
     if (WhitelistedSymbols.count(sym))
       continue;
-    // Don't track the symbol.
-    state = state->remove<RegionState>(sym);
+    // The symbol escaped.
+    if (const RefState *RS = State->get<RegionState>(sym))
+      State = State->set<RegionState>(sym, RefState::getEscaped(RS->getStmt()));
   }
-  return state;
+  return State;
 }
 
 PathDiagnosticPiece *
index 6678a0b5eb52d8e1258cd20775e5dd06f9a1a7db..96cc2f4d2a9ead8dfc7a26884007bd2e685f517c 100644 (file)
@@ -2439,7 +2439,8 @@ public:
   checkRegionChanges(ProgramStateRef state,
                      const StoreManager::InvalidatedSymbols *invalidated,
                      ArrayRef<const MemRegion *> ExplicitRegions,
-                     ArrayRef<const MemRegion *> Regions) const;
+                     ArrayRef<const MemRegion *> Regions,
+                     const CallOrObjCMessage *Call) const;
                                         
   bool wantsRegionChangeUpdate(ProgramStateRef state) const {
     return true;
@@ -3303,7 +3304,8 @@ ProgramStateRef
 RetainCountChecker::checkRegionChanges(ProgramStateRef state,
                             const StoreManager::InvalidatedSymbols *invalidated,
                                     ArrayRef<const MemRegion *> ExplicitRegions,
-                                    ArrayRef<const MemRegion *> Regions) const {
+                                    ArrayRef<const MemRegion *> Regions,
+                                    const CallOrObjCMessage *Call) const {
   if (!invalidated)
     return state;
 
index 732a821876d5b0e4707e05c05ed3b8f0cdfd22fe..cb4da7cf4c0eee185af96136eb12baa45f022f2c 100644 (file)
@@ -413,14 +413,15 @@ ProgramStateRef
 CheckerManager::runCheckersForRegionChanges(ProgramStateRef state,
                             const StoreManager::InvalidatedSymbols *invalidated,
                                     ArrayRef<const MemRegion *> ExplicitRegions,
-                                          ArrayRef<const MemRegion *> Regions) {
+                                          ArrayRef<const MemRegion *> Regions,
+                                          const CallOrObjCMessage *Call) {
   for (unsigned i = 0, e = RegionChangesCheckers.size(); i != e; ++i) {
     // If any checker declares the state infeasible (or if it starts that way),
     // bail out.
     if (!state)
       return NULL;
     state = RegionChangesCheckers[i].CheckFn(state, invalidated, 
-                                             ExplicitRegions, Regions);
+                                             ExplicitRegions, Regions, Call);
   }
   return state;
 }
index 9e5d6a90af9aa383ba22e6f36db8833a79d6723c..b0ed1815f9a410dc5431bf9fa24215291b1f80af 100644 (file)
@@ -176,9 +176,10 @@ ProgramStateRef
 ExprEngine::processRegionChanges(ProgramStateRef state,
                             const StoreManager::InvalidatedSymbols *invalidated,
                                  ArrayRef<const MemRegion *> Explicits,
-                                 ArrayRef<const MemRegion *> Regions) {
+                                 ArrayRef<const MemRegion *> Regions,
+                                 const CallOrObjCMessage *Call) {
   return getCheckerManager().runCheckersForRegionChanges(state, invalidated,
-                                                         Explicits, Regions);
+                                                      Explicits, Regions, Call);
 }
 
 void ExprEngine::printState(raw_ostream &Out, ProgramStateRef State,
index 459bf83ce5ef7fb69799bfe9a0d566d22c41a919..f52369ef4a764dec745ecb72950ff95cdccbe234 100644 (file)
@@ -179,7 +179,7 @@ ProgramState::invalidateRegionsImpl(ArrayRef<const MemRegion *> Regions,
       = Mgr.StoreMgr->invalidateRegions(getStore(), Regions, E, Count, IS,
                                         Call, &Invalidated);
     ProgramStateRef newState = makeWithStore(newStore);
-    return Eng->processRegionChanges(newState, &IS, Regions, Invalidated);
+    return Eng->processRegionChanges(newState, &IS, Regions, Invalidated, Call);
   }
 
   const StoreRef &newStore =