[clang] 5882e6f - [analyzer] Escape symbols conjured into specific regions during a conservative EvalCall

Gabor Horvath via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 11 11:51:08 PST 2019


Author: Gabor Horvath
Date: 2019-12-11T11:44:10-08:00
New Revision: 5882e6f36fd9bfc7382e6763c5591b3497428d83

URL: https://github.com/llvm/llvm-project/commit/5882e6f36fd9bfc7382e6763c5591b3497428d83
DIFF: https://github.com/llvm/llvm-project/commit/5882e6f36fd9bfc7382e6763c5591b3497428d83.diff

LOG: [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

Added: 
    clang/test/Analysis/pointer-escape-on-conservative-calls.c

Modified: 
    clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
    clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
    clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    clang/test/Analysis/analyzer-config.c
    clang/test/Analysis/expr-inspection.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index be455ae14f57..43632e801d2b 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1266,6 +1266,12 @@ def AnalysisOrderChecker : Checker<"AnalysisOrder">,
                   "false",
                   Released,
                   Hide>,
+    CmdLineOption<Boolean,
+                  "PointerEscape",
+                  "",
+                  "false",
+                  Released,
+                  Hide>,
     CmdLineOption<Boolean,
                   "*",
                   "Enables all callbacks.",

diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 45ab652d9e02..246ff8f90d35 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 2d0967616ff2..6e676c512b89 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -613,10 +613,16 @@ class ExprEngine : public SubEngine {
                 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 @@ class ExprEngine : public SubEngine {
                            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 7789b431c0a6..a7f3c28d4373 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 @@ class SubEngine {
     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 d0def6918932..2ef50a727ece 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 @@ class AnalysisOrderChecker
       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 efbc20f09250..2a23d1cae7b5 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 02a398c77ac8..b17f26aa9c53 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 345d4d817dea..01a371e664b2 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 6c6883d88d69..42e5269511fc 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 7c5d9e5c49d3..283fa9bdb724 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 000000000000..7ddd7a1ddb8f
--- /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


        


More information about the cfe-commits mailing list