[analyzer] Escape symbols conjured into specific regions during a conservative EvalCall

This patch introduced additional PointerEscape callbacks after conservative
calls for output parameters. This should not really affect the current
checkers but the upcoming FuchsiaHandleChecker relies on this heavily.

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

11 files changed, 155 insertions, 50 deletions

diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index be455ae14f5..43632e801d2 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1267,6 +1267,12 @@ def AnalysisOrderChecker : Checker<"AnalysisOrder">,
                   Released,
                   Hide>,
     CmdLineOption<Boolean,
+                  "PointerEscape",
+                  "",
+                  "false",
+                  Released,
+                  Hide>,
+    CmdLineOption<Boolean,
                   "*",
                   "Enables all callbacks.",
                   "false",
diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 45ab652d9e0..246ff8f90d3 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -85,6 +85,11 @@ enum PointerEscapeKind {
   /// argument to a function.
   PSK_IndirectEscapeOnCall,
 
+
+  /// Escape for a new symbol that was generated into a region
+  /// that the analyzer cannot follow during a conservative call.
+  PSK_EscapeOutParameters,
+
   /// The reason for pointer escape is unknown. For example,
   /// a region containing this pointer is invalidated.
   PSK_EscapeOther
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 2d0967616ff..6e676c512b8 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -613,10 +613,16 @@ protected:
                 const ProgramPoint *PP = nullptr);
 
   /// Call PointerEscape callback when a value escapes as a result of bind.
-  ProgramStateRef processPointerEscapedOnBind(ProgramStateRef State,
-                                              SVal Loc,
-                                              SVal Val,
-                                              const LocationContext *LCtx) override;
+  ProgramStateRef processPointerEscapedOnBind(
+      ProgramStateRef State, ArrayRef<std::pair<SVal, SVal>> LocAndVals,
+      const LocationContext *LCtx, PointerEscapeKind Kind,
+      const CallEvent *Call) override;
+
+  ProgramStateRef
+  processPointerEscapedOnBind(ProgramStateRef State,
+                              SVal Loc, SVal Val,
+                              const LocationContext *LCtx);
+
   /// Call PointerEscape callback when a value escapes as a result of
   /// region invalidation.
   /// \param[in] ITraits Specifies invalidation traits for regions/symbols.
@@ -628,9 +634,10 @@ protected:
                            RegionAndSymbolInvalidationTraits &ITraits) override;
 
   /// A simple wrapper when you only need to notify checkers of pointer-escape
-  /// of a single value.
-  ProgramStateRef escapeValue(ProgramStateRef State, SVal V,
-                              PointerEscapeKind K) const;
+  /// of some values.
+  ProgramStateRef escapeValues(ProgramStateRef State, ArrayRef<SVal> Vs,
+                               PointerEscapeKind K,
+                               const CallEvent *Call = nullptr) const;
 
 public:
   // FIXME: 'tag' should be removed, and a LocationContext should be used
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
index 7789b431c0a..a7f3c28d437 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
@@ -15,6 +15,7 @@
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
 
 namespace clang {
 
@@ -148,8 +149,10 @@ public:
     return processRegionChanges(state, nullptr, MR, MR, LCtx, nullptr);
   }
 
-  virtual ProgramStateRef
-  processPointerEscapedOnBind(ProgramStateRef State, SVal Loc, SVal Val, const LocationContext *LCtx) = 0;
+  virtual ProgramStateRef processPointerEscapedOnBind(
+      ProgramStateRef State, ArrayRef<std::pair<SVal, SVal>> LocAndVals,
+      const LocationContext *LCtx, PointerEscapeKind Kind,
+      const CallEvent *Call) = 0;
 
   virtual ProgramStateRef
   notifyCheckersOfPointerEscape(ProgramStateRef State,
diff --git a/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
index d0def691893..2ef50a727ec 100644
--- a/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
@@ -40,6 +40,7 @@ class AnalysisOrderChecker
                      check::EndFunction,
                      check::NewAllocator,
                      check::Bind,
+                     check::PointerEscape,
                      check::RegionChanges,
                      check::LiveSymbols> {
 
@@ -165,6 +166,15 @@ public:
       llvm::errs() << "RegionChanges\n";
     return State;
   }
+
+  ProgramStateRef checkPointerEscape(ProgramStateRef State,
+                                     const InvalidatedSymbols &Escaped,
+                                     const CallEvent *Call,
+                                     PointerEscapeKind Kind) const {
+    if (isCallbackEnabled(State, "PointerEscape"))
+      llvm::errs() << "PointerEscape\n";
+    return State;
+  }
 };
 } // end anonymous namespace
 
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index efbc20f0925..2a23d1cae7b 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1172,13 +1172,16 @@ void ExprEngine::VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE,
   }
 }
 
-ProgramStateRef ExprEngine::escapeValue(ProgramStateRef State, SVal V,
-                                        PointerEscapeKind K) const {
+ProgramStateRef ExprEngine::escapeValues(ProgramStateRef State,
+                                         ArrayRef<SVal> Vs,
+                                         PointerEscapeKind K,
+                                         const CallEvent *Call) const {
   class CollectReachableSymbolsCallback final : public SymbolVisitor {
-    InvalidatedSymbols Symbols;
+    InvalidatedSymbols &Symbols;
 
   public:
-    explicit CollectReachableSymbolsCallback(ProgramStateRef) {}
+    explicit CollectReachableSymbolsCallback(InvalidatedSymbols &Symbols)
+        : Symbols(Symbols) {}
 
     const InvalidatedSymbols &getSymbols() const { return Symbols; }
 
@@ -1187,11 +1190,13 @@ ProgramStateRef ExprEngine::escapeValue(ProgramStateRef State, SVal V,
       return true;
     }
   };
+  InvalidatedSymbols Symbols;
+  CollectReachableSymbolsCallback CallBack(Symbols);
+  for (SVal V : Vs)
+    State->scanReachableSymbols(V, CallBack);
 
-  const CollectReachableSymbolsCallback &Scanner =
-      State->scanReachableSymbols<CollectReachableSymbolsCallback>(V);
   return getCheckerManager().runCheckersForPointerEscape(
-      State, Scanner.getSymbols(), /*CallEvent*/ nullptr, K, nullptr);
+      State, CallBack.getSymbols(), Call, K, nullptr);
 }
 
 void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
@@ -1484,7 +1489,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
           for (auto Child : Ex->children()) {
             assert(Child);
             SVal Val = State->getSVal(Child, LCtx);
-            State = escapeValue(State, Val, PSK_EscapeOther);
+            State = escapeValues(State, Val, PSK_EscapeOther);
           }
 
         Bldr2.generateNode(S, N, State);
@@ -2685,33 +2690,52 @@ void ExprEngine::VisitAtomicExpr(const AtomicExpr *AE, ExplodedNode *Pred,
 //     destructor. We won't see the destructor during analysis, but it's there.
 // (4) We are binding to a MemRegion with stack storage that the store
 //     does not understand.
+ProgramStateRef ExprEngine::processPointerEscapedOnBind(
+    ProgramStateRef State, ArrayRef<std::pair<SVal, SVal>> LocAndVals,
+    const LocationContext *LCtx, PointerEscapeKind Kind,
+    const CallEvent *Call) {
+  SmallVector<SVal, 8> Escaped;
+  for (const std::pair<SVal, SVal> &LocAndVal : LocAndVals) {
+    // Cases (1) and (2).
+    const MemRegion *MR = LocAndVal.first.getAsRegion();
+    if (!MR || !MR->hasStackStorage()) {
+      Escaped.push_back(LocAndVal.second);
+      continue;
+    }
+
+    // Case (3).
+    if (const auto *VR = dyn_cast<VarRegion>(MR->getBaseRegion()))
+      if (VR->hasStackParametersStorage() && VR->getStackFrame()->inTopFrame())
+        if (const auto *RD = VR->getValueType()->getAsCXXRecordDecl())
+          if (!RD->hasTrivialDestructor()) {
+            Escaped.push_back(LocAndVal.second);
+            continue;
+          }
+
+    // Case (4): in order to test that, generate a new state with the binding
+    // added. If it is the same state, then it escapes (since the store cannot
+    // represent the binding).
+    // Do this only if we know that the store is not supposed to generate the
+    // same state.
+    SVal StoredVal = State->getSVal(MR);
+    if (StoredVal != LocAndVal.second)
+      if (State ==
+          (State->bindLoc(loc::MemRegionVal(MR), LocAndVal.second, LCtx)))
+        Escaped.push_back(LocAndVal.second);
+  }
+
+  if (Escaped.empty())
+    return State;
+
+  return escapeValues(State, Escaped, Kind, Call);
+}
+
 ProgramStateRef
 ExprEngine::processPointerEscapedOnBind(ProgramStateRef State, SVal Loc,
                                         SVal Val, const LocationContext *LCtx) {
-
-  // Cases (1) and (2).
-  const MemRegion *MR = Loc.getAsRegion();
-  if (!MR || !MR->hasStackStorage())
-    return escapeValue(State, Val, PSK_EscapeOnBind);
-
-  // Case (3).
-  if (const auto *VR = dyn_cast<VarRegion>(MR->getBaseRegion()))
-    if (VR->hasStackParametersStorage() && VR->getStackFrame()->inTopFrame())
-      if (const auto *RD = VR->getValueType()->getAsCXXRecordDecl())
-        if (!RD->hasTrivialDestructor())
-          return escapeValue(State, Val, PSK_EscapeOnBind);
-
-  // Case (4): in order to test that, generate a new state with the binding
-  // added. If it is the same state, then it escapes (since the store cannot
-  // represent the binding).
-  // Do this only if we know that the store is not supposed to generate the
-  // same state.
-  SVal StoredVal = State->getSVal(MR);
-  if (StoredVal != Val)
-    if (State == (State->bindLoc(loc::MemRegionVal(MR), Val, LCtx)))
-      return escapeValue(State, Val, PSK_EscapeOnBind);
-
-  return State;
+  std::pair<SVal, SVal> LocAndVal(Loc, Val);
+  return processPointerEscapedOnBind(State, LocAndVal, LCtx, PSK_EscapeOnBind,
+                                     nullptr);
 }
 
 ProgramStateRef
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 02a398c77ac..b17f26aa9c5 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -102,8 +102,8 @@ void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
         state = state->BindExpr(B, LCtx, Result);
       } else {
         // If we cannot evaluate the operation escape the operands.
-        state = escapeValue(state, LeftV, PSK_EscapeOther);
-        state = escapeValue(state, RightV, PSK_EscapeOther);
+        state = escapeValues(state, LeftV, PSK_EscapeOther);
+        state = escapeValues(state, RightV, PSK_EscapeOther);
       }
 
       Bldr.generateNode(B, *it, state);
@@ -275,7 +275,7 @@ ProgramStateRef ExprEngine::handleLValueBitCast(
     V = evalMinus(V);
   state = state->BindExpr(CastE, LCtx, V);
   if (V.isUnknown() && !OrigV.isUnknown()) {
-    state = escapeValue(state, OrigV, PSK_EscapeOther);
+    state = escapeValues(state, OrigV, PSK_EscapeOther);
   }
   Bldr.generateNode(CastE, Pred, state);
 
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 345d4d817de..01a371e664b 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "clang/AST/Decl.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "PrettyStackTraceLocationContext.h"
 #include "clang/AST/CXXInheritance.h"
@@ -592,9 +593,45 @@ void ExprEngine::evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
   for (auto I : dstCallEvaluated)
     finishArgumentConstruction(dstArgumentCleanup, I, Call);
 
-  // Finally, run any post-call checks.
-  getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup,
+  ExplodedNodeSet dstPostCall;
+  getCheckerManager().runCheckersForPostCall(dstPostCall, dstArgumentCleanup,
                                              Call, *this);
+
+  // Escaping symbols conjured during invalidating the regions above.
+  // Note that, for inlined calls the nodes were put back into the worklist,
+  // so we can assume that every node belongs to a conservative call at this
+  // point.
+
+  // Run pointerEscape callback with the newly conjured symbols.
+  SmallVector<std::pair<SVal, SVal>, 8> Escaped;
+  for (auto I : dstPostCall) {
+    NodeBuilder B(I, Dst, *currBldrCtx);
+    ProgramStateRef State = I->getState();
+    Escaped.clear();
+    {
+      unsigned Arg = -1;
+      for (const ParmVarDecl *PVD : Call.parameters()) {
+        ++Arg;
+        QualType ParamTy = PVD->getType();
+        if (ParamTy.isNull() ||
+            (!ParamTy->isPointerType() && !ParamTy->isReferenceType()))
+          continue;
+        QualType Pointee = ParamTy->getPointeeType();
+        if (Pointee.isConstQualified() || Pointee->isVoidType())
+          continue;
+        if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion())
+          Escaped.emplace_back(loc::MemRegionVal(MR), State->getSVal(MR, Pointee));
+      }
+    }
+
+    State = processPointerEscapedOnBind(State, Escaped, I->getLocationContext(),
+                                        PSK_EscapeOutParameters, &Call);
+
+    if (State == I->getState())
+      Dst.insert(I);
+    else
+      B.generateNode(I->getLocation(), State, I);
+  }
 }
 
 ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
@@ -670,8 +707,7 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
 // Conservatively evaluate call by invalidating regions and binding
 // a conjured return value.
 void ExprEngine::conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr,
-                                      ExplodedNode *Pred,
-                                      ProgramStateRef State) {
+                                      ExplodedNode *Pred, ProgramStateRef State) {
   State = Call.invalidateRegions(currBldrCtx->blockCount(), State);
   State = bindReturnValue(Call, Pred->getLocationContext(), State);
 
diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 6c6883d88d6..42e5269511f 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -37,6 +37,7 @@
 // CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
 // CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
 // CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
+// CHECK-NEXT: debug.AnalysisOrder:PointerEscape = false
 // CHECK-NEXT: debug.AnalysisOrder:PostCall = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
@@ -97,4 +98,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 94
+// CHECK-NEXT: num-entries = 95
diff --git a/clang/test/Analysis/expr-inspection.c b/clang/test/Analysis/expr-inspection.c
index 7c5d9e5c49d..283fa9bdb72 100644
--- a/clang/test/Analysis/expr-inspection.c
+++ b/clang/test/Analysis/expr-inspection.c
@@ -50,5 +50,5 @@ struct S {
 
 void test_field_dumps(struct S s, struct S *p) {
   clang_analyzer_dump_pointer(&s.x); // expected-warning{{&s.x}}
-  clang_analyzer_dump_pointer(&p->x); // expected-warning{{&SymRegion{reg_$0<struct S * p>}.x}}
+  clang_analyzer_dump_pointer(&p->x); // expected-warning{{&SymRegion{reg_$1<struct S * p>}.x}}
 }
diff --git a/clang/test/Analysis/pointer-escape-on-conservative-calls.c b/clang/test/Analysis/pointer-escape-on-conservative-calls.c
new file mode 100644
index 00000000000..7ddd7a1ddb8
--- /dev/null
+++ b/clang/test/Analysis/pointer-escape-on-conservative-calls.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.AnalysisOrder -analyzer-config debug.AnalysisOrder:PointerEscape=true -analyzer-config debug.AnalysisOrder:PostCall=true %s 2>&1 | FileCheck %s
+
+
+void f(int *);
+int *getMem();
+
+int main() {
+    f(getMem());
+    return 0;
+}
+
+// CHECK: PostCall (f)
+// CHECK-NEXT: PointerEscape