[analyzer] Process state in checkEndFunction in RetainCountChecker

Modify the RetainCountChecker to perform state "adjustments" in
checkEndFunction, as performing work in PreStmt<ReturnStmt> does not
work with destructors.
The previous version made an implicit assumption that no code runs
after the return statement is executed.

rdar://43945028

Differential Revision: https://reviews.llvm.org/D52338

llvm-svn: 342770
diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index de113bb..e5d27f5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -814,12 +814,9 @@
   return true;
 }
 
-//===----------------------------------------------------------------------===//
-// Handle return statements.
-//===----------------------------------------------------------------------===//
-
-void RetainCountChecker::checkPreStmt(const ReturnStmt *S,
-                                      CheckerContext &C) const {
+ExplodedNode * RetainCountChecker::processReturn(const ReturnStmt *S,
+                                                 CheckerContext &C) const {
+  ExplodedNode *Pred = C.getPredecessor();
 
   // Only adjust the reference count if this is the top-level call frame,
   // and not the result of inlining.  In the future, we should do
@@ -827,22 +824,25 @@
   // with their expected semantics (e.g., the method should return a retained
   // object, etc.).
   if (!C.inTopFrame())
-    return;
+    return Pred;
+
+  if (!S)
+    return Pred;
 
   const Expr *RetE = S->getRetValue();
   if (!RetE)
-    return;
+    return Pred;
 
   ProgramStateRef state = C.getState();
   SymbolRef Sym =
     state->getSValAsScalarOrLoc(RetE, C.getLocationContext()).getAsLocSymbol();
   if (!Sym)
-    return;
+    return Pred;
 
   // Get the reference count binding (if any).
   const RefVal *T = getRefBinding(state, Sym);
   if (!T)
-    return;
+    return Pred;
 
   // Change the reference count.
   RefVal X = *T;
@@ -861,20 +861,19 @@
       if (cnt) {
         X.setCount(cnt - 1);
         X = X ^ RefVal::ReturnedOwned;
-      }
-      else {
+      } else {
         X = X ^ RefVal::ReturnedNotOwned;
       }
       break;
     }
 
     default:
-      return;
+      return Pred;
   }
 
   // Update the binding.
   state = setRefBinding(state, Sym, X);
-  ExplodedNode *Pred = C.addTransition(state);
+  Pred = C.addTransition(state);
 
   // At this point we have updated the state properly.
   // Everything after this is merely checking to see if the return value has
@@ -882,15 +881,15 @@
 
   // Did we cache out?
   if (!Pred)
-    return;
+    return nullptr;
 
   // Update the autorelease counts.
   static CheckerProgramPointTag AutoreleaseTag(this, "Autorelease");
-  state = handleAutoreleaseCounts(state, Pred, &AutoreleaseTag, C, Sym, X);
+  state = handleAutoreleaseCounts(state, Pred, &AutoreleaseTag, C, Sym, X, S);
 
-  // Did we cache out?
+  // Have we generated a sink node?
   if (!state)
-    return;
+    return nullptr;
 
   // Get the updated binding.
   T = getRefBinding(state, Sym);
@@ -913,10 +912,10 @@
     }
   }
 
-  checkReturnWithRetEffect(S, C, Pred, RE, X, Sym, state);
+  return checkReturnWithRetEffect(S, C, Pred, RE, X, Sym, state);
 }
 
-void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S,
+ExplodedNode * RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S,
                                                   CheckerContext &C,
                                                   ExplodedNode *Pred,
                                                   RetEffect RE, RefVal X,
@@ -929,20 +928,17 @@
   //   [self addSubview:_contentView]; // invalidates 'self'
   //   [_contentView release];
   if (X.getIvarAccessHistory() != RefVal::IvarAccessHistory::None)
-    return;
+    return Pred;
 
   // Any leaks or other errors?
   if (X.isReturnedOwned() && X.getCount() == 0) {
     if (RE.getKind() != RetEffect::NoRet) {
-      bool hasError = false;
       if (!RE.isOwned()) {
+
         // The returning type is a CF, we expect the enclosing method should
         // return ownership.
-        hasError = true;
         X = X ^ RefVal::ErrorLeakReturned;
-      }
 
-      if (hasError) {
         // Generate an error node.
         state = setRefBinding(state, Sym, X);
 
@@ -950,10 +946,12 @@
         ExplodedNode *N = C.addTransition(state, Pred, &ReturnOwnLeakTag);
         if (N) {
           const LangOptions &LOpts = C.getASTContext().getLangOpts();
-          C.emitReport(llvm::make_unique<CFRefLeakReport>(
+          auto R = llvm::make_unique<CFRefLeakReport>(
               *getLeakAtReturnBug(LOpts), LOpts, SummaryLog, N, Sym, C,
-              IncludeAllocationLine));
+              IncludeAllocationLine);
+          C.emitReport(std::move(R));
         }
+        return N;
       }
     }
   } else if (X.isReturnedNotOwned()) {
@@ -977,13 +975,16 @@
           if (!returnNotOwnedForOwned)
             returnNotOwnedForOwned.reset(new ReturnedNotOwnedForOwned(this));
 
-          C.emitReport(llvm::make_unique<CFRefReport>(
+          auto R = llvm::make_unique<CFRefReport>(
               *returnNotOwnedForOwned, C.getASTContext().getLangOpts(),
-              SummaryLog, N, Sym));
+              SummaryLog, N, Sym);
+          C.emitReport(std::move(R));
         }
+        return N;
       }
     }
   }
+  return Pred;
 }
 
 //===----------------------------------------------------------------------===//
@@ -1107,16 +1108,14 @@
   return state;
 }
 
-//===----------------------------------------------------------------------===//
-// Handle dead symbols and end-of-path.
-//===----------------------------------------------------------------------===//
-
 ProgramStateRef
 RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state,
                                             ExplodedNode *Pred,
                                             const ProgramPointTag *Tag,
                                             CheckerContext &Ctx,
-                                            SymbolRef Sym, RefVal V) const {
+                                            SymbolRef Sym,
+                                            RefVal V,
+                                            const ReturnStmt *S) const {
   unsigned ACnt = V.getAutoreleaseCount();
 
   // No autorelease counts?  Nothing to be done.
@@ -1141,10 +1140,11 @@
   if (ACnt <= Cnt) {
     if (ACnt == Cnt) {
       V.clearCounts();
-      if (V.getKind() == RefVal::ReturnedOwned)
+      if (V.getKind() == RefVal::ReturnedOwned) {
         V = V ^ RefVal::ReturnedNotOwned;
-      else
+      } else {
         V = V ^ RefVal::NotOwned;
+      }
     } else {
       V.setCount(V.getCount() - ACnt);
       V.setAutoreleaseCount(0);
@@ -1181,8 +1181,9 @@
       overAutorelease.reset(new OverAutorelease(this));
 
     const LangOptions &LOpts = Ctx.getASTContext().getLangOpts();
-    Ctx.emitReport(llvm::make_unique<CFRefReport>(
-        *overAutorelease, LOpts, SummaryLog, N, Sym, os.str()));
+    auto R = llvm::make_unique<CFRefReport>(*overAutorelease, LOpts, SummaryLog,
+                                            N, Sym, os.str());
+    Ctx.emitReport(std::move(R));
   }
 
   return nullptr;
@@ -1281,9 +1282,15 @@
 
 void RetainCountChecker::checkEndFunction(const ReturnStmt *RS,
                                           CheckerContext &Ctx) const {
-  ProgramStateRef state = Ctx.getState();
+  ExplodedNode *Pred = processReturn(RS, Ctx);
+
+  // Created state cached out.
+  if (!Pred) {
+    return;
+  }
+
+  ProgramStateRef state = Pred->getState();
   RefBindingsTy B = state->get<RefBindings>();
-  ExplodedNode *Pred = Ctx.getPredecessor();
 
   // Don't process anything within synthesized bodies.
   const LocationContext *LCtx = Pred->getLocationContext();