[analyzer] Add support for escape of const pointers and use it to allow “newed” pointers to escape

Add a new callback that notifies checkers when a const pointer escapes. Currently, this only works
for const pointers passed as a top level parameter into a function. We need to differentiate the const
pointers escape from regular escape since the content pointed by const pointer will not change;
if it’s a file handle, a file cannot be closed; but delete is allowed on const pointers.

This should suppress several false positives reported by the NewDelete checker on llvm codebase.

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

11 files changed, 195 insertions, 39 deletions

diff --git a/include/clang/StaticAnalyzer/Core/Checker.h b/include/clang/StaticAnalyzer/Core/Checker.h
index 305ae2579d..0dbaab033d 100644
--- a/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/include/clang/StaticAnalyzer/Core/Checker.h
@@ -324,11 +324,14 @@ class PointerEscape {
                      ProgramStateRef State,
                      const InvalidatedSymbols &Escaped,
                      const CallEvent *Call,
-                     PointerEscapeKind Kind) {
-    return ((const CHECKER *)checker)->checkPointerEscape(State, 
-                                                          Escaped, 
-                                                          Call,
-                                                          Kind);
+                     PointerEscapeKind Kind,
+                    bool IsConst) {
+    if (!IsConst)
+      return ((const CHECKER *)checker)->checkPointerEscape(State,
+                                                            Escaped,
+                                                            Call,
+                                                            Kind);
+    return State;
   }
 
 public:
@@ -340,6 +343,33 @@ public:
   }
 };
 
+class ConstPointerEscape {
+  template <typename CHECKER>
+  static ProgramStateRef
+  _checkConstPointerEscape(void *checker,
+                      ProgramStateRef State,
+                      const InvalidatedSymbols &Escaped,
+                      const CallEvent *Call,
+                      PointerEscapeKind Kind,
+                      bool IsConst) {
+    if (IsConst)
+      return ((const CHECKER *)checker)->checkConstPointerEscape(State,
+                                                                 Escaped,
+                                                                 Call,
+                                                                 Kind);
+    return State;
+  }
+
+public:
+  template <typename CHECKER>
+  static void _register(CHECKER *checker, CheckerManager &mgr) {
+    mgr._registerForPointerEscape(
+      CheckerManager::CheckPointerEscapeFunc(checker,
+                                            _checkConstPointerEscape<CHECKER>));
+  }
+};
+
+  
 template <typename EVENT>
 class Event {
   template <typename CHECKER>
diff --git a/include/clang/StaticAnalyzer/Core/CheckerManager.h b/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 4353ebf015..6f99fc1457 100644
--- a/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -346,12 +346,14 @@ public:
   /// \param Escaped The list of escaped symbols.
   /// \param Call The corresponding CallEvent, if the symbols escape as 
   ///        parameters to the given call.
+  /// \param IsConst Specifies if the pointer is const.
   /// \returns Checkers can modify the state by returning a new one.
   ProgramStateRef 
   runCheckersForPointerEscape(ProgramStateRef State,
                               const InvalidatedSymbols &Escaped,
                               const CallEvent *Call,
-                              PointerEscapeKind Kind);
+                              PointerEscapeKind Kind,
+                              bool IsConst = false);
 
   /// \brief Run checkers for handling assumptions on symbolic values.
   ProgramStateRef runCheckersForEvalAssume(ProgramStateRef state,
@@ -442,7 +444,8 @@ public:
   typedef CheckerFn<ProgramStateRef (ProgramStateRef,
                                      const InvalidatedSymbols &Escaped,
                                      const CallEvent *Call,
-                                     PointerEscapeKind Kind)>
+                                     PointerEscapeKind Kind,
+                                     bool IsConst)>
       CheckPointerEscapeFunc;
   
   typedef CheckerFn<ProgramStateRef (ProgramStateRef,
@@ -487,6 +490,8 @@ public:
 
   void _registerForPointerEscape(CheckPointerEscapeFunc checkfn);
 
+  void _registerForConstPointerEscape(CheckPointerEscapeFunc checkfn);
+
   void _registerForEvalAssume(EvalAssumeFunc checkfn);
 
   void _registerForEvalCall(EvalCallFunc checkfn);
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 3507533a0b..42c0c0488e 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -467,12 +467,14 @@ protected:
                                               SVal Loc, SVal Val);
   /// Call PointerEscape callback when a value escapes as a result of
   /// region invalidation.
-  ProgramStateRef processPointerEscapedOnInvalidateRegions(
+  /// \param[in] IsConst Specifies that the pointer is const.
+  ProgramStateRef notifyCheckersOfPointerEscape(
                             ProgramStateRef State,
                             const InvalidatedSymbols *Invalidated,
                             ArrayRef<const MemRegion *> ExplicitRegions,
                             ArrayRef<const MemRegion *> Regions,
-                            const CallEvent *Call);
+                            const CallEvent *Call,
+                            bool IsConst);
 
 public:
   // FIXME: 'tag' should be removed, and a LocationContext should be used
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h b/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
index 8318fe0c26..9ae24c446e 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -187,6 +187,8 @@ public:
   ///   accessible. Pass \c NULL if this information will not be used.
   /// \param[in] Call The call expression which will be used to determine which
   ///   globals should get invalidated.
+  /// \param[in,out] ConstIS A set to fill with any symbols corresponding to
+  ///   the ConstRegions.
   /// \param[in,out] Invalidated A vector to fill with any regions being
   ///   invalidated. This should include any regions explicitly invalidated
   ///   even if they do not currently have bindings. Pass \c NULL if this
@@ -198,6 +200,7 @@ public:
                                      InvalidatedSymbols &IS,
                                      const CallEvent *Call,
                                      ArrayRef<const MemRegion *> ConstRegions,
+                                     InvalidatedSymbols &ConstIS,
                                      InvalidatedRegions *Invalidated) = 0;
 
   /// enterStackFrame - Let the StoreManager to do something when execution
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
index 0e9f25375d..4578f6f323 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
@@ -120,11 +120,12 @@ public:
   processPointerEscapedOnBind(ProgramStateRef State, SVal Loc, SVal Val) = 0;
 
   virtual ProgramStateRef
-  processPointerEscapedOnInvalidateRegions(ProgramStateRef State,
+  notifyCheckersOfPointerEscape(ProgramStateRef State,
                            const InvalidatedSymbols *Invalidated,
                            ArrayRef<const MemRegion *> ExplicitRegions,
                            ArrayRef<const MemRegion *> Regions,
-                           const CallEvent *Call) = 0;
+                           const CallEvent *Call,
+                           bool IsConst = false) = 0;
 
   /// printState - Called by ProgramStateManager to print checker-specific data.
   virtual void printState(raw_ostream &Out, ProgramStateRef State,
diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index b4e45a29d6..57666499c4 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -134,6 +134,7 @@ typedef std::pair<const ExplodedNode*, const MemRegion*> LeakInfo;
 
 class MallocChecker : public Checker<check::DeadSymbols,
                                      check::PointerEscape,
+                                     check::ConstPointerEscape,
                                      check::PreStmt<ReturnStmt>,
                                      check::PreStmt<CallExpr>,
                                      check::PostStmt<CallExpr>,
@@ -185,6 +186,10 @@ public:
                                     const InvalidatedSymbols &Escaped,
                                     const CallEvent *Call,
                                     PointerEscapeKind Kind) const;
+  ProgramStateRef checkConstPointerEscape(ProgramStateRef State,
+                                          const InvalidatedSymbols &Escaped,
+                                          const CallEvent *Call,
+                                          PointerEscapeKind Kind) const;
 
   void printState(raw_ostream &Out, ProgramStateRef State,
                   const char *NL, const char *Sep) const;
@@ -270,6 +275,13 @@ private:
   bool doesNotFreeMemOrInteresting(const CallEvent *Call,
                                    ProgramStateRef State) const;
 
+  // Implementation of the checkPointerEscape callabcks.
+  ProgramStateRef checkPointerEscapeAux(ProgramStateRef State,
+                                  const InvalidatedSymbols &Escaped,
+                                  const CallEvent *Call,
+                                  PointerEscapeKind Kind,
+                                  bool(*CheckRefState)(const RefState*)) 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, 
@@ -1865,10 +1877,35 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call,
   return true;
 }
 
+static bool retTrue(const RefState *RS) {
+  return true;
+}
+
+static bool checkIfNewOrNewArrayFamily(const RefState *RS) {
+  return (RS->getAllocationFamily() == AF_CXXNewArray ||
+          RS->getAllocationFamily() == AF_CXXNew);
+}
+
 ProgramStateRef MallocChecker::checkPointerEscape(ProgramStateRef State,
                                              const InvalidatedSymbols &Escaped,
                                              const CallEvent *Call,
                                              PointerEscapeKind Kind) const {
+  return checkPointerEscapeAux(State, Escaped, Call, Kind, &retTrue);
+}
+
+ProgramStateRef MallocChecker::checkConstPointerEscape(ProgramStateRef State,
+                                              const InvalidatedSymbols &Escaped,
+                                              const CallEvent *Call,
+                                              PointerEscapeKind Kind) const {
+  return checkPointerEscapeAux(State, Escaped, Call, Kind,
+                               &checkIfNewOrNewArrayFamily);
+}
+
+ProgramStateRef MallocChecker::checkPointerEscapeAux(ProgramStateRef State,
+                                              const InvalidatedSymbols &Escaped,
+                                              const CallEvent *Call,
+                                              PointerEscapeKind Kind,
+                                  bool(*CheckRefState)(const RefState*)) const {
   // If we know that the call does not free memory, or we want to process the
   // call later, keep tracking the top level arguments.
   if ((Kind == PSK_DirectEscapeOnCall ||
@@ -1878,12 +1915,12 @@ ProgramStateRef MallocChecker::checkPointerEscape(ProgramStateRef State,
   }
 
   for (InvalidatedSymbols::const_iterator I = Escaped.begin(),
-                                          E = Escaped.end();
-                                          I != E; ++I) {
+       E = Escaped.end();
+       I != E; ++I) {
     SymbolRef sym = *I;
 
     if (const RefState *RS = State->get<RegionState>(sym)) {
-      if (RS->isAllocated())
+      if (RS->isAllocated() && CheckRefState(RS))
         State = State->remove<RegionState>(sym);
     }
   }
diff --git a/lib/StaticAnalyzer/Core/CheckerManager.cpp b/lib/StaticAnalyzer/Core/CheckerManager.cpp
index a383799788..8adf3262b3 100644
--- a/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -489,19 +489,19 @@ ProgramStateRef
 CheckerManager::runCheckersForPointerEscape(ProgramStateRef State,
                                            const InvalidatedSymbols &Escaped,
                                            const CallEvent *Call,
-                                           PointerEscapeKind Kind) {
+                                           PointerEscapeKind Kind,
+                                           bool IsConst) {
   assert((Call != NULL ||
           (Kind != PSK_DirectEscapeOnCall &&
            Kind != PSK_IndirectEscapeOnCall)) &&
          "Call must not be NULL when escaping on call");
-
-  for (unsigned i = 0, e = PointerEscapeCheckers.size(); i != e; ++i) {
-    // If any checker declares the state infeasible (or if it starts that way),
-    // bail out.
-    if (!State)
-      return NULL;
-    State = PointerEscapeCheckers[i](State, Escaped, Call, Kind);
-  }
+    for (unsigned i = 0, e = PointerEscapeCheckers.size(); i != e; ++i) {
+      // If any checker declares the state infeasible (or if it starts that
+      //  way), bail out.
+      if (!State)
+        return NULL;
+      State = PointerEscapeCheckers[i](State, Escaped, Call, Kind, IsConst);
+    }
   return State;
 }
 
@@ -666,6 +666,11 @@ void CheckerManager::_registerForPointerEscape(CheckPointerEscapeFunc checkfn){
   PointerEscapeCheckers.push_back(checkfn);
 }
 
+void CheckerManager::_registerForConstPointerEscape(
+                                          CheckPointerEscapeFunc checkfn) {
+  PointerEscapeCheckers.push_back(checkfn);
+}
+
 void CheckerManager::_registerForEvalAssume(EvalAssumeFunc checkfn) {
   EvalAssumeCheckers.push_back(checkfn);
 }
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index f245bbf49e..8ad5a3dbde 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1714,11 +1714,12 @@ ProgramStateRef ExprEngine::processPointerEscapedOnBind(ProgramStateRef State,
 }
 
 ProgramStateRef 
-ExprEngine::processPointerEscapedOnInvalidateRegions(ProgramStateRef State,
+ExprEngine::notifyCheckersOfPointerEscape(ProgramStateRef State,
     const InvalidatedSymbols *Invalidated,
     ArrayRef<const MemRegion *> ExplicitRegions,
     ArrayRef<const MemRegion *> Regions,
-    const CallEvent *Call) {
+    const CallEvent *Call,
+    bool IsConst) {
   
   if (!Invalidated || Invalidated->empty())
     return State;
@@ -1728,7 +1729,17 @@ ExprEngine::processPointerEscapedOnInvalidateRegions(ProgramStateRef State,
                                                            *Invalidated,
                                                            0,
                                                            PSK_EscapeOther);
-    
+
+  // Note: Due to current limitations of RegionStore, we only process the top
+  // level const pointers correctly. The lower level const pointers are
+  // currently treated as non-const.
+  if (IsConst)
+    return getCheckerManager().runCheckersForPointerEscape(State,
+                                                        *Invalidated,
+                                                        Call,
+                                                        PSK_DirectEscapeOnCall,
+                                                        true);
+
   // If the symbols were invalidated by a call, we want to find out which ones 
   // were invalidated directly due to being arguments to the call.
   InvalidatedSymbols SymbolsDirectlyInvalidated;
diff --git a/lib/StaticAnalyzer/Core/ProgramState.cpp b/lib/StaticAnalyzer/Core/ProgramState.cpp
index 3e47dcef2b..9aac8df0a2 100644
--- a/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ b/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -170,25 +170,34 @@ ProgramState::invalidateRegionsImpl(RegionList Regions,
                                     RegionList ConstRegions) const {
   ProgramStateManager &Mgr = getStateManager();
   SubEngine* Eng = Mgr.getOwningEngine();
- 
+  InvalidatedSymbols ConstIS;
+
   if (Eng) {
     StoreManager::InvalidatedRegions Invalidated;
     const StoreRef &newStore
       = Mgr.StoreMgr->invalidateRegions(getStore(), Regions, E, Count, LCtx, IS,
-                                        Call, ConstRegions, &Invalidated);
+                                        Call, ConstRegions, ConstIS,
+                                        &Invalidated);
 
     ProgramStateRef newState = makeWithStore(newStore);
 
-    if (CausedByPointerEscape)
-      newState = Eng->processPointerEscapedOnInvalidateRegions(newState,
+    if (CausedByPointerEscape) {
+      newState = Eng->notifyCheckersOfPointerEscape(newState,
                                                &IS, Regions, Invalidated, Call);
+      if (!ConstRegions.empty()) {
+        StoreManager::InvalidatedRegions Empty;
+        newState = Eng->notifyCheckersOfPointerEscape(newState, &ConstIS,
+                                                      ConstRegions, Empty, Call,
+                                                      true);
+      }
+    }
 
     return Eng->processRegionChanges(newState, &IS, Regions, Invalidated, Call);
   }
 
   const StoreRef &newStore =
     Mgr.StoreMgr->invalidateRegions(getStore(), Regions, E, Count, LCtx, IS,
-                                    Call, ConstRegions, NULL);
+                                    Call, ConstRegions, ConstIS, NULL);
   return makeWithStore(newStore);
 }
 
diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp
index b866a58d04..08110dd3b9 100644
--- a/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -371,6 +371,7 @@ public:
                              InvalidatedSymbols &IS,
                              const CallEvent *Call,
                              ArrayRef<const MemRegion *> ConstRegions,
+                             InvalidatedSymbols &ConstIS,
                              InvalidatedRegions *Invalidated);
 
   bool scanReachableSymbols(Store S, const MemRegion *R,
@@ -882,6 +883,7 @@ class invalidateRegionsWorker : public ClusterAnalysis<invalidateRegionsWorker>
   unsigned Count;
   const LocationContext *LCtx;
   InvalidatedSymbols &IS;
+  InvalidatedSymbols &ConstIS;
   StoreManager::InvalidatedRegions *Regions;
 public:
   invalidateRegionsWorker(RegionStoreManager &rm,
@@ -890,13 +892,16 @@ public:
                           const Expr *ex, unsigned count,
                           const LocationContext *lctx,
                           InvalidatedSymbols &is,
+                          InvalidatedSymbols &inConstIS,
                           StoreManager::InvalidatedRegions *r,
                           bool includeGlobals)
     : ClusterAnalysis<invalidateRegionsWorker>(rm, stateMgr, b, includeGlobals),
-      Ex(ex), Count(count), LCtx(lctx), IS(is), Regions(r) {}
+      Ex(ex), Count(count), LCtx(lctx), IS(is), ConstIS(inConstIS), Regions(r){}
 
+  /// \param IsConst Specifies if the region we are invalidating is constant.
+  /// If it is, we invalidate all subregions, but not the base region itself.
   void VisitCluster(const MemRegion *baseR, const ClusterBindings *C,
-                    bool Flag);
+                    bool IsConst);
   void VisitBinding(SVal V);
 };
 }
@@ -964,12 +969,19 @@ void invalidateRegionsWorker::VisitCluster(const MemRegion *baseR,
     return;
   }
 
-  if (IsConst)
-    return;
-
-  // Symbolic region?  Mark that symbol touched by the invalidation.
+  // Symbolic region?
+  SymbolRef RegionSym = 0;
   if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(baseR))
-    IS.insert(SR->getSymbol());
+    RegionSym = SR->getSymbol();
+
+  if (IsConst) {
+    // Mark that symbol touched by the invalidation.
+    ConstIS.insert(RegionSym);
+    return;
+  }
+  
+  // Mark that symbol touched by the invalidation.
+  IS.insert(RegionSym);
 
   // Otherwise, we have a normal data region. Record that we touched the region.
   if (Regions)
@@ -1058,9 +1070,10 @@ RegionStoreManager::invalidateRegions(Store store,
                                       InvalidatedSymbols &IS,
                                       const CallEvent *Call,
                                       ArrayRef<const MemRegion *> ConstRegions,
+                                      InvalidatedSymbols &ConstIS,
                                       InvalidatedRegions *Invalidated) {
   RegionBindingsRef B = RegionStoreManager::getRegionBindings(store);
-  invalidateRegionsWorker W(*this, StateMgr, B, Ex, Count, LCtx, IS,
+  invalidateRegionsWorker W(*this, StateMgr, B, Ex, Count, LCtx, IS, ConstIS,
                             Invalidated, false);
 
   // Scan the bindings and generate the clusters.
diff --git a/test/Analysis/NewDelete-checker-test.mm b/test/Analysis/NewDelete-checker-test.mm
index f14f924816..5311ff6fe7 100644
--- a/test/Analysis/NewDelete-checker-test.mm
+++ b/test/Analysis/NewDelete-checker-test.mm
@@ -158,3 +158,43 @@ void testStandardPlacementNewAfterDelete() {
   delete p;
   p = new(p) int; // expected-warning{{Use of memory after it is freed}}
 }
+
+//--------------------------------
+// Test escape of newed const pointer. Note, a const pointer can be deleted.
+//--------------------------------
+struct StWithConstPtr {
+  const int *memp;
+};
+void escape(const int &x);
+void escapeStruct(const StWithConstPtr &x);
+void escapePtr(const StWithConstPtr *x);
+void escapeVoidPtr(const void *x);
+
+void testConstEscape() {
+  int *p = new int(1);
+  escape(*p);
+} // no-warning
+
+void testConstEscapeStruct() {
+  StWithConstPtr *St = new StWithConstPtr();
+  escapeStruct(*St);
+} // no-warning
+
+void testConstEscapeStructPtr() {
+  StWithConstPtr *St = new StWithConstPtr();
+  escapePtr(St);
+} // no-warning
+
+void testConstEscapeMember() {
+  StWithConstPtr St;
+  St.memp = new int(2);
+  escapeVoidPtr(St.memp);
+} // no-warning
+
+void testConstEscapePlacementNew() {
+  int *x = (int *)malloc(sizeof(int));
+  void *y = new (x) int;
+  escapeVoidPtr(y);
+} // no-warning
+
+