[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.

llvm-svn: 178310

4 files changed, 63 insertions, 25 deletions

diff --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
index a3837997886..8adf3262b37 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/clang/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/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index f245bbf49e1..8ad5a3dbdee 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/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/clang/lib/StaticAnalyzer/Core/ProgramState.cpp b/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
index 3e47dcef2bf..9aac8df0a22 100644
--- a/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ b/clang/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/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index b866a58d04e..08110dd3b93 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/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.