[analyzer] Change the check::RegionChanges callback to include the regions explicitly requested for invalidation.

Also, allow CallOrObjCMessage to wrap a CXXConstructExpr as well.

Finally, this allows us to remove the clunky whitelisting system from CFRefCount/RetainReleaseChecker. Slight regression due to CXXNewExprs not yet being handled in post-statement callbacks (PR forthcoming).


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

7 files changed, 91 insertions, 108 deletions

diff --git a/lib/StaticAnalyzer/Core/CFRefCount.cpp b/lib/StaticAnalyzer/Core/CFRefCount.cpp
index 07487f14e1..5991e5325f 100644
--- a/lib/StaticAnalyzer/Core/CFRefCount.cpp
+++ b/lib/StaticAnalyzer/Core/CFRefCount.cpp
@@ -2439,23 +2439,6 @@ static QualType GetReturnType(const Expr *RetE, ASTContext &Ctx) {
   return RetTy;
 }
 
-
-// HACK: Symbols that have ref-count state that are referenced directly
-//  (not as structure or array elements, or via bindings) by an argument
-//  should not have their ref-count state stripped after we have
-//  done an invalidation pass.
-//
-// FIXME: This is a global to currently share between CFRefCount and
-// RetainReleaseChecker.  Eventually all functionality in CFRefCount should
-// be migrated to RetainReleaseChecker, and we can make this a non-global.
-llvm::DenseSet<SymbolRef> WhitelistedSymbols;
-namespace {
-struct ResetWhiteList {
-  ResetWhiteList() {}
-  ~ResetWhiteList() { WhitelistedSymbols.clear(); } 
-};
-}
-
 void CFRefCount::evalCallOrMessage(ExplodedNodeSet &Dst, ExprEngine &Eng,
                                    StmtNodeBuilder &Builder,
                                    const CallOrObjCMessage &callOrMsg,
@@ -2465,18 +2448,11 @@ void CFRefCount::evalCallOrMessage(ExplodedNodeSet &Dst, ExprEngine &Eng,
                                    const ProgramState *state) {
 
   SmallVector<const MemRegion*, 10> RegionsToInvalidate;
-  
-  // Use RAII to make sure the whitelist is properly cleared.
-  ResetWhiteList resetWhiteList;
 
   // Invalidate all instance variables of the receiver of a message.
   // FIXME: We should be able to do better with inter-procedural analysis.
   if (Receiver) {
     SVal V = Receiver.getSValAsScalarOrLoc(state);
-    if (SymbolRef Sym = V.getAsLocSymbol()) {
-      if (state->get<RefBindings>(Sym))
-        WhitelistedSymbols.insert(Sym);
-    }
     if (const MemRegion *region = V.getAsRegion())
       RegionsToInvalidate.push_back(region);
   }
@@ -2490,7 +2466,7 @@ void CFRefCount::evalCallOrMessage(ExplodedNodeSet &Dst, ExprEngine &Eng,
   }
   
   for (unsigned idx = 0, e = callOrMsg.getNumArgs(); idx != e; ++idx) {
-    SVal V = callOrMsg.getArgSValAsScalarOrLoc(idx);
+    SVal V = callOrMsg.getArgSVal(idx);
 
     // If we are passing a location wrapped as an integer, unwrap it and
     // invalidate the values referred by the location.
@@ -2499,10 +2475,6 @@ void CFRefCount::evalCallOrMessage(ExplodedNodeSet &Dst, ExprEngine &Eng,
     else if (!isa<Loc>(V))
       continue;
 
-    if (SymbolRef Sym = V.getAsLocSymbol())
-      if (state->get<RefBindings>(Sym))
-        WhitelistedSymbols.insert(Sym);
-
     if (const MemRegion *R = V.getAsRegion()) {
       // Invalidate the value of the variable passed by reference.
 
@@ -2562,9 +2534,7 @@ void CFRefCount::evalCallOrMessage(ExplodedNodeSet &Dst, ExprEngine &Eng,
   //  global variables.
   // NOTE: RetainReleaseChecker handles the actual invalidation of symbols.
   state =
-    state->invalidateRegions(RegionsToInvalidate.data(),
-                             RegionsToInvalidate.data() +
-                             RegionsToInvalidate.size(),
+    state->invalidateRegions(RegionsToInvalidate,
                              Ex, Count, &IS,
                              /* invalidateGlobals = */
                              Eng.doesInvalidateGlobals(callOrMsg));
@@ -2611,6 +2581,7 @@ class RetainReleaseChecker
                     check::PostStmt<BlockExpr>,
                     check::PostStmt<CastExpr>,
                     check::PostStmt<CallExpr>,
+                    check::PostStmt<CXXConstructExpr>,
                     check::PostObjCMessage,
                     check::PreStmt<ReturnStmt>,
                     check::RegionChanges,
@@ -2770,6 +2741,7 @@ public:
   void checkPostStmt(const CastExpr *CE, CheckerContext &C) const;
 
   void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
+  void checkPostStmt(const CXXConstructExpr *CE, CheckerContext &C) const;
   void checkPostObjCMessage(const ObjCMessage &Msg, CheckerContext &C) const;
   void checkSummary(const RetainSummary &Summ, const CallOrObjCMessage &Call,
                     InstanceReceiver Receiver, CheckerContext &C) const;
@@ -2779,10 +2751,11 @@ public:
   const ProgramState *evalAssume(const ProgramState *state, SVal Cond,
                                  bool Assumption) const;
 
-  const ProgramState *checkRegionChanges(const ProgramState *state,
-                          const StoreManager::InvalidatedSymbols *invalidated,
-                                         const MemRegion * const *begin,
-                                         const MemRegion * const *end) const;
+  const ProgramState *
+  checkRegionChanges(const ProgramState *state,
+                     const StoreManager::InvalidatedSymbols *invalidated,
+                     ArrayRef<const MemRegion *> ExplicitRegions,
+                     ArrayRef<const MemRegion *> Regions) const;
                                         
   bool wantsRegionChangeUpdate(const ProgramState *state) const {
     return true;
@@ -2912,11 +2885,18 @@ const ProgramState *RetainReleaseChecker::evalAssume(const ProgramState *state,
 const ProgramState *
 RetainReleaseChecker::checkRegionChanges(const ProgramState *state,
                             const StoreManager::InvalidatedSymbols *invalidated,
-                                         const MemRegion * const *begin,
-                                         const MemRegion * const *end) const {
+                                    ArrayRef<const MemRegion *> ExplicitRegions,
+                                    ArrayRef<const MemRegion *> Regions) const {
   if (!invalidated)
     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());
+  }
+
   for (StoreManager::InvalidatedSymbols::const_iterator I=invalidated->begin(),
        E = invalidated->end(); I!=E; ++I) {
     SymbolRef sym = *I;
@@ -3036,6 +3016,23 @@ void RetainReleaseChecker::checkPostStmt(const CallExpr *CE,
   checkSummary(*Summ, CallOrObjCMessage(CE, state), InstanceReceiver(), C);
 }
 
+void RetainReleaseChecker::checkPostStmt(const CXXConstructExpr *CE,
+                                         CheckerContext &C) const {
+  const CXXConstructorDecl *Ctor = CE->getConstructor();
+  if (!Ctor)
+    return;
+
+  RetainSummaryManager &Summaries = getSummaryManager(C.getASTContext());
+  RetainSummary *Summ = Summaries.getSummary(Ctor);
+
+  // If we didn't get a summary, this constructor doesn't affect retain counts.
+  if (!Summ)
+    return;
+
+  const ProgramState *state = C.getState();
+  checkSummary(*Summ, CallOrObjCMessage(CE, state), InstanceReceiver(), C);
+}
+
 void RetainReleaseChecker::checkPostObjCMessage(const ObjCMessage &Msg, 
                                                 CheckerContext &C) const {
   const ProgramState *state = C.getState();
@@ -3071,7 +3068,7 @@ void RetainReleaseChecker::checkSummary(const RetainSummary &Summ,
   SymbolRef ErrorSym = 0;
 
   for (unsigned idx = 0, e = CallOrMsg.getNumArgs(); idx != e; ++idx) {
-    SVal V = CallOrMsg.getArgSValAsScalarOrLoc(idx);
+    SVal V = CallOrMsg.getArgSVal(idx);
 
     if (SymbolRef Sym = V.getAsLocSymbol()) {
       if (RefBindings::data_type *T = state->get<RefBindings>(Sym)) {
@@ -3434,7 +3431,7 @@ bool RetainReleaseChecker::evalCall(const CallExpr *CE,
 
     // Invalidate the argument region.
     unsigned Count = C.getNodeBuilder().getCurrentBlockCount();
-    state = state->invalidateRegion(ArgRegion, CE, Count);
+    state = state->invalidateRegions(ArgRegion, CE, Count);
 
     // Restore the refcount status of the argument.
     if (Binding)
diff --git a/lib/StaticAnalyzer/Core/CheckerManager.cpp b/lib/StaticAnalyzer/Core/CheckerManager.cpp
index bcba98fb3a..196376c340 100644
--- a/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -346,14 +346,15 @@ bool CheckerManager::wantsRegionChangeUpdate(const ProgramState *state) {
 const ProgramState *
 CheckerManager::runCheckersForRegionChanges(const ProgramState *state,
                             const StoreManager::InvalidatedSymbols *invalidated,
-                                            const MemRegion * const *Begin,
-                                            const MemRegion * const *End) {
+                                    ArrayRef<const MemRegion *> ExplicitRegions,
+                                          ArrayRef<const MemRegion *> Regions) {
   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, Begin, End);
+    state = RegionChangesCheckers[i].CheckFn(state, invalidated, 
+                                             ExplicitRegions, Regions);
   }
   return state;
 }
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 6c318f756c..997e0789f0 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -192,10 +192,10 @@ bool ExprEngine::wantsRegionChangeUpdate(const ProgramState *state) {
 const ProgramState *
 ExprEngine::processRegionChanges(const ProgramState *state,
                             const StoreManager::InvalidatedSymbols *invalidated,
-                                 const MemRegion * const *Begin,
-                                 const MemRegion * const *End) {
+                                 ArrayRef<const MemRegion *> Explicits,
+                                 ArrayRef<const MemRegion *> Regions) {
   return getCheckerManager().runCheckersForRegionChanges(state, invalidated,
-                                                         Begin, End);
+                                                         Explicits, Regions);
 }
 
 void ExprEngine::processEndWorklist(bool hasWorkRemaining) {
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index d34afbad72..579dcb3f51 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -229,9 +229,7 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *E,
     }
     
     // Invalidate the regions.    
-    state = state->invalidateRegions(regionsToInvalidate.data(),
-                                     regionsToInvalidate.data() +
-                                     regionsToInvalidate.size(),
+    state = state->invalidateRegions(regionsToInvalidate,
                                      E, blockCount, 0,
                                      /* invalidateGlobals = */ true);
     
@@ -317,17 +315,13 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
     if (ObjTy->isRecordType()) {
       regionsToInvalidate.push_back(EleReg);
       // Invalidate the regions.
-      state = state->invalidateRegions(regionsToInvalidate.data(),
-                                       regionsToInvalidate.data() +
-                                       regionsToInvalidate.size(),
+      state = state->invalidateRegions(regionsToInvalidate,
                                        CNE, blockCount, 0,
                                        /* invalidateGlobals = */ true);
       
     } else {
       // Invalidate the regions.
-      state = state->invalidateRegions(regionsToInvalidate.data(),
-                                       regionsToInvalidate.data() +
-                                       regionsToInvalidate.size(),
+      state = state->invalidateRegions(regionsToInvalidate,
                                        CNE, blockCount, 0,
                                        /* invalidateGlobals = */ true);
 
diff --git a/lib/StaticAnalyzer/Core/ObjCMessage.cpp b/lib/StaticAnalyzer/Core/ObjCMessage.cpp
index 40ed83beb8..82b0e7c305 100644
--- a/lib/StaticAnalyzer/Core/ObjCMessage.cpp
+++ b/lib/StaticAnalyzer/Core/ObjCMessage.cpp
@@ -112,18 +112,22 @@ QualType CallOrObjCMessage::getResultType(ASTContext &ctx) const {
   QualType resultTy;
   bool isLVal = false;
 
-  if (CallE) {
-    isLVal = CallE->isLValue();
-    const Expr *Callee = CallE->getCallee();
-    if (const FunctionDecl *FD = State->getSVal(Callee).getAsFunctionDecl())
-      resultTy = FD->getResultType();
-    else
-      resultTy = CallE->getType();
-  }
-  else {
+  if (isObjCMessage()) {
     isLVal = isa<ObjCMessageExpr>(Msg.getOriginExpr()) &&
              Msg.getOriginExpr()->isLValue();
     resultTy = Msg.getResultType(ctx);
+  } else if (const CXXConstructExpr *Ctor =
+              CallE.dyn_cast<const CXXConstructExpr *>()) {
+    resultTy = Ctor->getType();
+  } else {
+    const CallExpr *FunctionCall = CallE.get<const CallExpr *>();
+
+    isLVal = FunctionCall->isLValue();
+    const Expr *Callee = FunctionCall->getCallee();
+    if (const FunctionDecl *FD = State->getSVal(Callee).getAsFunctionDecl())
+      resultTy = FD->getResultType();
+    else
+      resultTy = FunctionCall->getType();
   }
 
   if (isLVal)
@@ -132,25 +136,17 @@ QualType CallOrObjCMessage::getResultType(ASTContext &ctx) const {
   return resultTy;
 }
 
-SVal CallOrObjCMessage::getArgSValAsScalarOrLoc(unsigned i) const {
-  assert(i < getNumArgs());
-  if (CallE) return State->getSValAsScalarOrLoc(CallE->getArg(i));
-  QualType argT = Msg.getArgType(i);
-  if (Loc::isLocType(argT) || argT->isIntegerType())
-    return Msg.getArgSVal(i, State);
-  return UnknownVal();
-}
-
 SVal CallOrObjCMessage::getFunctionCallee() const {
   assert(isFunctionCall());
   assert(!isCXXCall());
-  const Expr *callee = CallE->getCallee()->IgnoreParens();
-  return State->getSVal(callee);
+  const Expr *Fun = CallE.get<const CallExpr *>()->getCallee()->IgnoreParens();
+  return State->getSVal(Fun);
 }
 
 SVal CallOrObjCMessage::getCXXCallee() const {
   assert(isCXXCall());
+  const CallExpr *ActualCall = CallE.get<const CallExpr *>();
   const Expr *callee =
-    cast<CXXMemberCallExpr>(CallE)->getImplicitObjectArgument();
+    cast<CXXMemberCallExpr>(ActualCall)->getImplicitObjectArgument();
   return State->getSVal(callee);  
 }
diff --git a/lib/StaticAnalyzer/Core/ProgramState.cpp b/lib/StaticAnalyzer/Core/ProgramState.cpp
index 046de0d0ff..54a626b676 100644
--- a/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ b/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -136,41 +136,38 @@ const ProgramState *ProgramState::bindDefault(SVal loc, SVal V) const {
            new_state;
 }
 
-const ProgramState *ProgramState::invalidateRegions(const MemRegion * const *Begin,
-                                          const MemRegion * const *End,
-                                          const Expr *E, unsigned Count,
-                                          StoreManager::InvalidatedSymbols *IS,
-                                          bool invalidateGlobals) const {
+const ProgramState *
+ProgramState::invalidateRegions(ArrayRef<const MemRegion *> Regions,
+                                const Expr *E, unsigned Count,
+                                StoreManager::InvalidatedSymbols *IS,
+                                bool invalidateGlobals) const {
   if (!IS) {
     StoreManager::InvalidatedSymbols invalidated;
-    return invalidateRegionsImpl(Begin, End, E, Count,
-                             invalidated, invalidateGlobals);
+    return invalidateRegionsImpl(Regions, E, Count,
+                                 invalidated, invalidateGlobals);
   }
-  return invalidateRegionsImpl(Begin, End, E, Count, *IS, invalidateGlobals);
+  return invalidateRegionsImpl(Regions, E, Count, *IS, invalidateGlobals);
 }
 
 const ProgramState *
-ProgramState::invalidateRegionsImpl(const MemRegion * const *Begin,
-                               const MemRegion * const *End,
-                               const Expr *E, unsigned Count,
-                               StoreManager::InvalidatedSymbols &IS,
-                               bool invalidateGlobals) const {
+ProgramState::invalidateRegionsImpl(ArrayRef<const MemRegion *> Regions,
+                                    const Expr *E, unsigned Count,
+                                    StoreManager::InvalidatedSymbols &IS,
+                                    bool invalidateGlobals) const {
   ProgramStateManager &Mgr = getStateManager();
   SubEngine* Eng = Mgr.getOwningEngine();
  
   if (Eng && Eng->wantsRegionChangeUpdate(this)) {
-    StoreManager::InvalidatedRegions Regions;
+    StoreManager::InvalidatedRegions Invalidated;
     const StoreRef &newStore
-      = Mgr.StoreMgr->invalidateRegions(getStore(), Begin, End, E, Count, IS,
-                                        invalidateGlobals, &Regions);
+      = Mgr.StoreMgr->invalidateRegions(getStore(), Regions, E, Count, IS,
+                                        invalidateGlobals, &Invalidated);
     const ProgramState *newState = makeWithStore(newStore);
-    return Eng->processRegionChanges(newState, &IS,
-                                     &Regions.front(),
-                                     &Regions.back()+1);
+    return Eng->processRegionChanges(newState, &IS, Regions, Invalidated);
   }
 
   const StoreRef &newStore =
-    Mgr.StoreMgr->invalidateRegions(getStore(), Begin, End, E, Count, IS,
+    Mgr.StoreMgr->invalidateRegions(getStore(), Regions, E, Count, IS,
                                     invalidateGlobals, NULL);
   return makeWithStore(newStore);
 }
diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp
index 30028c78a3..04c274d319 100644
--- a/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -236,13 +236,11 @@ public:
   // Binding values to regions.
   //===-------------------------------------------------------------------===//
 
-  StoreRef invalidateRegions(Store store,
-                             const MemRegion * const *Begin,
-                             const MemRegion * const *End,
+  StoreRef invalidateRegions(Store store, ArrayRef<const MemRegion *> Regions,
                              const Expr *E, unsigned Count,
                              InvalidatedSymbols &IS,
                              bool invalidateGlobals,
-                             InvalidatedRegions *Regions);
+                             InvalidatedRegions *Invalidated);
 
 public:   // Made public for helper classes.
 
@@ -721,21 +719,21 @@ void invalidateRegionsWorker::VisitBaseRegion(const MemRegion *baseR) {
 }
 
 StoreRef RegionStoreManager::invalidateRegions(Store store,
-                                               const MemRegion * const *I,
-                                               const MemRegion * const *E,
+                                            ArrayRef<const MemRegion *> Regions,
                                                const Expr *Ex, unsigned Count,
                                                InvalidatedSymbols &IS,
                                                bool invalidateGlobals,
-                                               InvalidatedRegions *Regions) {
+                                              InvalidatedRegions *Invalidated) {
   invalidateRegionsWorker W(*this, StateMgr,
                             RegionStoreManager::GetRegionBindings(store),
-                            Ex, Count, IS, Regions, invalidateGlobals);
+                            Ex, Count, IS, Invalidated, invalidateGlobals);
 
   // Scan the bindings and generate the clusters.
   W.GenerateClusters();
 
-  // Add I .. E to the worklist.
-  for ( ; I != E; ++I)
+  // Add the regions to the worklist.
+  for (ArrayRef<const MemRegion *>::iterator
+       I = Regions.begin(), E = Regions.end(); I != E; ++I)
     W.AddToWorkList(*I);
 
   W.RunWorkList();
@@ -755,8 +753,8 @@ StoreRef RegionStoreManager::invalidateRegions(Store store,
 
     // Even if there are no bindings in the global scope, we still need to
     // record that we touched it.
-    if (Regions)
-      Regions->push_back(GS);
+    if (Invalidated)
+      Invalidated->push_back(GS);
   }
 
   return StoreRef(B.getRootWithoutRetain(), *this);