[analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic.
authorArtem Dergachev <artem.dergachev@gmail.com>
Wed, 11 Sep 2019 20:54:21 +0000 (20:54 +0000)
committerArtem Dergachev <artem.dergachev@gmail.com>
Wed, 11 Sep 2019 20:54:21 +0000 (20:54 +0000)
These static functions deal with ExplodedNodes which is something we don't want
the PathDiagnostic interface to know anything about, as it's planned to be
moved out of libStaticAnalyzerCore.

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

llvm-svn: 371659

19 files changed:
clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
clang/lib/StaticAnalyzer/Checkers/Taint.cpp
clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

index dd65af4..aaeeb48 100644 (file)
@@ -52,11 +52,6 @@ class SourceManager;
 
 namespace ento {
 
-class ExplodedNode;
-class SymExpr;
-
-using SymbolRef = const SymExpr *;
-
 //===----------------------------------------------------------------------===//
 // High-level interface for handlers of path-sensitive diagnostics.
 //===----------------------------------------------------------------------===//
@@ -276,18 +271,21 @@ public:
   static PathDiagnosticLocation createDeclEnd(const LocationContext *LC,
                                                    const SourceManager &SM);
 
-  /// Create a location corresponding to the given valid ExplodedNode.
+  /// Create a location corresponding to the given valid ProgramPoint.
   static PathDiagnosticLocation create(const ProgramPoint &P,
                                        const SourceManager &SMng);
 
-  /// Create a location corresponding to the next valid ExplodedNode as end
-  /// of path location.
-  static PathDiagnosticLocation createEndOfPath(const ExplodedNode* N);
-
   /// Convert the given location into a single kind location.
   static PathDiagnosticLocation createSingleLocation(
                                              const PathDiagnosticLocation &PDL);
 
+  /// Construct a source location that corresponds to either the beginning
+  /// or the end of the given statement, or a nearby valid source location
+  /// if the statement does not have a valid source location of its own.
+  static SourceLocation
+  getValidSourceLocation(const Stmt *S, LocationOrAnalysisDeclContext LAC,
+                         bool UseEndOfStatement = false);
+
   bool operator==(const PathDiagnosticLocation &X) const {
     return K == X.K && Loc == X.Loc && Range == X.Range;
   }
@@ -332,13 +330,6 @@ public:
   void Profile(llvm::FoldingSetNodeID &ID) const;
 
   void dump() const;
-
-  /// Given an exploded node, retrieve the statement that should be used
-  /// for the diagnostic location.
-  static const Stmt *getStmt(const ExplodedNode *N);
-
-  /// Retrieve the statement corresponding to the successor node.
-  static const Stmt *getNextStmt(const ExplodedNode *N);
 };
 
 class PathDiagnosticLocationPair {
index 598e2f6..9773a55 100644 (file)
@@ -267,6 +267,30 @@ public:
   /// Trivial nodes may be skipped while printing exploded graph.
   bool isTrivial() const;
 
+  /// If the node's program point corresponds to a statement, retrieve that
+  /// statement. Useful for figuring out where to put a warning or a note.
+  /// If the statement belongs to a body-farmed definition,
+  /// retrieve the call site for that definition.
+  const Stmt *getStmtForDiagnostics() const;
+
+  /// Find the next statement that was executed on this node's execution path.
+  /// Useful for explaining control flow that follows the current node.
+  /// If the statement belongs to a body-farmed definition, retrieve the
+  /// call site for that definition.
+  const Stmt *getNextStmtForDiagnostics() const;
+
+  /// Find the statement that was executed immediately before this node.
+  /// Useful when the node corresponds to a CFG block entrance.
+  /// If the statement belongs to a body-farmed definition, retrieve the
+  /// call site for that definition.
+  const Stmt *getPreviousStmtForDiagnostics() const;
+
+  /// Find the statement that was executed at or immediately before this node.
+  /// Useful when any nearby statement will do.
+  /// If the statement belongs to a body-farmed definition, retrieve the
+  /// call site for that definition.
+  const Stmt *getCurrentOrPreviousStmtForDiagnostics() const;
+
 private:
   void replaceSuccessor(ExplodedNode *node) { Succs.replaceNode(node); }
   void replacePredecessor(ExplodedNode *node) { Preds.replaceNode(node); }
index 882df3e..8010c85 100644 (file)
@@ -108,7 +108,7 @@ DeleteWithNonVirtualDtorChecker::DeleteBugVisitor::VisitNode(
   if (Satisfied)
     return nullptr;
 
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
 
index d7a725c..8cc38f9 100644 (file)
@@ -103,7 +103,7 @@ PathDiagnosticPieceRef DynamicTypeChecker::DynamicTypeBugVisitor::VisitNode(
     return nullptr;
 
   // Retrieve the associated statement.
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
 
index ca2283d..cce3449 100644 (file)
@@ -937,7 +937,7 @@ PathDiagnosticPieceRef DynamicTypePropagation::GenericsBugVisitor::VisitNode(
     return nullptr;
 
   // Retrieve the associated statement.
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
 
index 9cb77c0..b0d101c 100644 (file)
@@ -284,7 +284,7 @@ PathDiagnosticPieceRef InnerPointerChecker::InnerPointerBRVisitor::VisitNode(
       isSymbolTracked(N->getFirstPred()->getState(), PtrToBuf))
     return nullptr;
 
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
 
index 9947b57..6be6762 100644 (file)
@@ -482,7 +482,7 @@ MacOSKeychainAPIChecker::generateAllocatedDataNotReleasedReport(
   // allocated, and only report a single path.
   PathDiagnosticLocation LocUsedForUniqueing;
   const ExplodedNode *AllocNode = getAllocationNode(N, AP.first, C);
-  const Stmt *AllocStmt = PathDiagnosticLocation::getStmt(AllocNode);
+  const Stmt *AllocStmt = AllocNode->getStmtForDiagnostics();
 
   if (AllocStmt)
     LocUsedForUniqueing = PathDiagnosticLocation::createBegin(AllocStmt,
index 7763c53..260e61e 100644 (file)
@@ -532,8 +532,7 @@ private:
       if (!IsLeak)
         return nullptr;
 
-      PathDiagnosticLocation L =
-          PathDiagnosticLocation::createEndOfPath(EndPathNode);
+      PathDiagnosticLocation L = BR.getLocation();
       // Do not add the statement itself as a range in case of leak.
       return std::make_shared<PathDiagnosticEventPiece>(L, BR.getDescription(),
                                                          false);
@@ -2332,7 +2331,7 @@ void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N,
   const MemRegion *Region = nullptr;
   std::tie(AllocNode, Region) = getAllocationSite(N, Sym, C);
 
-  const Stmt *AllocationStmt = PathDiagnosticLocation::getStmt(AllocNode);
+  const Stmt *AllocationStmt = AllocNode->getStmtForDiagnostics();
   if (AllocationStmt)
     LocUsedForUniqueing = PathDiagnosticLocation::createBegin(AllocationStmt,
                                               C.getSourceManager(),
@@ -2920,7 +2919,7 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
   const RefState *RS = state->get<RegionState>(Sym);
   const RefState *RSPrev = statePrev->get<RegionState>(Sym);
 
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   // When dealing with containers, we sometimes want to give a note
   // even if the statement is missing.
   if (!S && (!RS || RS->getAllocationFamily() != AF_InnerBuffer))
index a429123..1473c05 100644 (file)
@@ -289,7 +289,7 @@ MoveChecker::MovedBugVisitor::VisitNode(const ExplodedNode *N,
     return nullptr;
 
   // Retrieve the associated statement.
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
   Found = true;
@@ -401,7 +401,7 @@ ExplodedNode *MoveChecker::reportBug(const MemRegion *Region,
     PathDiagnosticLocation LocUsedForUniqueing;
     const ExplodedNode *MoveNode = getMoveLocation(N, Region, C);
 
-    if (const Stmt *MoveStmt = PathDiagnosticLocation::getStmt(MoveNode))
+    if (const Stmt *MoveStmt = MoveNode->getStmtForDiagnostics())
       LocUsedForUniqueing = PathDiagnosticLocation::createBegin(
           MoveStmt, C.getSourceManager(), MoveNode->getLocationContext());
 
index 4b68a4a..2f46e81 100644 (file)
@@ -309,7 +309,7 @@ PathDiagnosticPieceRef NullabilityChecker::NullabilityBugVisitor::VisitNode(
   // Retrieve the associated statement.
   const Stmt *S = TrackedNullab->getNullabilitySource();
   if (!S || S->getBeginLoc().isInvalid()) {
-    S = PathDiagnosticLocation::getStmt(N);
+    S = N->getStmtForDiagnostics();
   }
 
   if (!S)
index d3c6011..9853758 100644 (file)
@@ -738,11 +738,7 @@ RefLeakReportVisitor::getEndPath(BugReporterContext &BRC,
   const MemRegion* FirstBinding = AllocI.R;
   BR.markInteresting(AllocI.InterestingMethodContext);
 
-  // Compute an actual location for the leak.  Sometimes a leak doesn't
-  // occur at an actual statement (e.g., transition between blocks; end
-  // of function) so we need to walk the graph and compute a real location.
-  const ExplodedNode *LeakN = EndN;
-  PathDiagnosticLocation L = PathDiagnosticLocation::createEndOfPath(LeakN);
+  PathDiagnosticLocation L = cast<RefLeakReport>(BR).getEndOfPath();
 
   std::string sbuf;
   llvm::raw_string_ostream os(sbuf);
@@ -872,7 +868,7 @@ void RefLeakReport::deriveAllocLocation(CheckerContext &Ctx,
   // FIXME: This will crash the analyzer if an allocation comes from an
   // implicit call (ex: a destructor call).
   // (Currently there are no such allocations in Cocoa, though.)
-  AllocStmt = PathDiagnosticLocation::getStmt(AllocNode);
+  AllocStmt = AllocNode->getStmtForDiagnostics();
 
   if (!AllocStmt) {
     AllocBinding = nullptr;
index 323b50a..3b21212 100644 (file)
@@ -90,11 +90,14 @@ class RefLeakReport : public RefCountReport {
 public:
   RefLeakReport(const RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n,
                 SymbolRef sym, CheckerContext &Ctx);
-
   PathDiagnosticLocation getLocation() const override {
     assert(Location.isValid());
     return Location;
   }
+
+  PathDiagnosticLocation getEndOfPath() const {
+    return PathSensitiveBugReport::getLocation();
+  }
 };
 
 } // end namespace retaincountchecker
index 2a9456d..574d4ed 100644 (file)
@@ -213,7 +213,7 @@ PathDiagnosticPieceRef TaintBugVisitor::VisitNode(const ExplodedNode *N,
       isTainted(N->getFirstPred()->getState(), V))
     return nullptr;
 
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
 
index ca00912..0c83af1 100644 (file)
@@ -83,8 +83,7 @@ private:
       if (!IsLeak)
         return nullptr;
 
-      PathDiagnosticLocation L =
-          PathDiagnosticLocation::createEndOfPath(EndPathNode);
+      PathDiagnosticLocation L = BR.getLocation();
       // Do not add the statement itself as a range in case of leak.
       return std::make_shared<PathDiagnosticEventPiece>(L, BR.getDescription(),
                                                         false);
@@ -285,7 +284,7 @@ void ValistChecker::reportLeakedVALists(const RegionVector &LeakedVALists,
     const ExplodedNode *StartNode = getStartCallSite(N, Reg);
     PathDiagnosticLocation LocUsedForUniqueing;
 
-    if (const Stmt *StartCallStmt = PathDiagnosticLocation::getStmt(StartNode))
+    if (const Stmt *StartCallStmt = StartNode->getStmtForDiagnostics())
       LocUsedForUniqueing = PathDiagnosticLocation::createBegin(
           StartCallStmt, C.getSourceManager(), StartNode->getLocationContext());
 
@@ -381,7 +380,7 @@ PathDiagnosticPieceRef ValistChecker::ValistBugVisitor::VisitNode(
   ProgramStateRef State = N->getState();
   ProgramStateRef StatePrev = N->getFirstPred()->getState();
 
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
 
index c3b76b4..57f639a 100644 (file)
@@ -335,26 +335,6 @@ std::string StackHintGeneratorForSymbol::getMessageForArg(const Expr *ArgE,
 }
 
 //===----------------------------------------------------------------------===//
-// Helper routines for walking the ExplodedGraph and fetching statements.
-//===----------------------------------------------------------------------===//
-
-static const Stmt *GetPreviousStmt(const ExplodedNode *N) {
-  for (N = N->getFirstPred(); N; N = N->getFirstPred())
-    if (const Stmt *S = PathDiagnosticLocation::getStmt(N))
-      return S;
-
-  return nullptr;
-}
-
-static inline const Stmt*
-GetCurrentOrPreviousStmt(const ExplodedNode *N) {
-  if (const Stmt *S = PathDiagnosticLocation::getStmt(N))
-    return S;
-
-  return GetPreviousStmt(N);
-}
-
-//===----------------------------------------------------------------------===//
 // Diagnostic cleanup.
 //===----------------------------------------------------------------------===//
 
@@ -593,7 +573,7 @@ static void removePiecesWithInvalidLocations(PathPieces &Pieces) {
 
 PathDiagnosticLocation PathDiagnosticBuilder::ExecutionContinues(
     const PathDiagnosticConstruct &C) const {
-  if (const Stmt *S = PathDiagnosticLocation::getNextStmt(C.getCurrentNode()))
+  if (const Stmt *S = C.getCurrentNode()->getNextStmtForDiagnostics())
     return PathDiagnosticLocation(S, getSourceManager(),
                                   C.getCurrLocationContext());
 
@@ -888,7 +868,7 @@ void PathDiagnosticBuilder::generateMinimalDiagForBlockEdge(
 
   case Stmt::GotoStmtClass:
   case Stmt::IndirectGotoStmtClass: {
-    if (const Stmt *S = PathDiagnosticLocation::getNextStmt(C.getCurrentNode()))
+    if (const Stmt *S = C.getCurrentNode()->getNextStmtForDiagnostics())
       C.getActivePath().push_front(generateDiagForGotoOP(C, S, Start));
     break;
   }
@@ -2177,8 +2157,11 @@ void PathSensitiveBugReport::Profile(llvm::FoldingSetNodeID &hash) const {
   if (UL.isValid()) {
     UL.Profile(hash);
   } else {
-    assert(ErrorNode);
-    hash.AddPointer(GetCurrentOrPreviousStmt(ErrorNode));
+    // TODO: The statement may be null if the report was emitted before any
+    // statements were executed. In particular, some checkers by design
+    // occasionally emit their reports in empty functions (that have no
+    // statements in their body). Do we profile correctly in this case?
+    hash.AddPointer(ErrorNode->getCurrentOrPreviousStmtForDiagnostics());
   }
 
   for (SourceRange range : Ranges) {
@@ -2333,10 +2316,10 @@ const Stmt *PathSensitiveBugReport::getStmt() const {
   if (Optional<BlockEntrance> BE = ProgP.getAs<BlockEntrance>()) {
     CFGBlock &Exit = ProgP.getLocationContext()->getCFG()->getExit();
     if (BE->getBlock() == &Exit)
-      S = GetPreviousStmt(ErrorNode);
+      S = ErrorNode->getPreviousStmtForDiagnostics();
   }
   if (!S)
-    S = PathDiagnosticLocation::getStmt(ErrorNode);
+    S = ErrorNode->getStmtForDiagnostics();
 
   return S;
 }
@@ -2353,7 +2336,45 @@ PathSensitiveBugReport::getRanges() const {
 
 PathDiagnosticLocation
 PathSensitiveBugReport::getLocation() const {
-  return PathDiagnosticLocation::createEndOfPath(ErrorNode);
+  assert(ErrorNode && "Cannot create a location with a null node.");
+  const Stmt *S = ErrorNode->getStmtForDiagnostics();
+    ProgramPoint P = ErrorNode->getLocation();
+  const LocationContext *LC = P.getLocationContext();
+  SourceManager &SM =
+      ErrorNode->getState()->getStateManager().getContext().getSourceManager();
+
+  if (!S) {
+    // If this is an implicit call, return the implicit call point location.
+    if (Optional<PreImplicitCall> PIE = P.getAs<PreImplicitCall>())
+      return PathDiagnosticLocation(PIE->getLocation(), SM);
+    if (auto FE = P.getAs<FunctionExitPoint>()) {
+      if (const ReturnStmt *RS = FE->getStmt())
+        return PathDiagnosticLocation::createBegin(RS, SM, LC);
+    }
+    S = ErrorNode->getNextStmtForDiagnostics();
+  }
+
+  if (S) {
+    // For member expressions, return the location of the '.' or '->'.
+    if (const auto *ME = dyn_cast<MemberExpr>(S))
+      return PathDiagnosticLocation::createMemberLoc(ME, SM);
+
+    // For binary operators, return the location of the operator.
+    if (const auto *B = dyn_cast<BinaryOperator>(S))
+      return PathDiagnosticLocation::createOperatorLoc(B, SM);
+
+    if (P.getAs<PostStmtPurgeDeadSymbols>())
+      return PathDiagnosticLocation::createEnd(S, SM, LC);
+
+    if (S->getBeginLoc().isValid())
+      return PathDiagnosticLocation(S, SM, LC);
+
+    return PathDiagnosticLocation(
+        PathDiagnosticLocation::getValidSourceLocation(S, LC), SM);
+  }
+
+  return PathDiagnosticLocation::createDeclEnd(ErrorNode->getLocationContext(),
+                                               SM);
 }
 
 //===----------------------------------------------------------------------===//
@@ -3070,7 +3091,7 @@ findExecutedLines(const SourceManager &SM, const ExplodedNode *N) {
       // Inlined function: show signature.
       const Decl* D = CE->getCalleeContext()->getDecl();
       populateExecutedLinesWithFunctionSignature(D, SM, *ExecutedLines);
-    } else if (const Stmt *S = PathDiagnosticLocation::getStmt(N)) {
+    } else if (const Stmt *S = N->getStmtForDiagnostics()) {
       populateExecutedLinesWithStmt(S, SM, *ExecutedLines);
 
       // Show extra context for some parent kinds.
index 7b26b8f..bf923da 100644 (file)
@@ -308,9 +308,7 @@ PathDiagnosticPieceRef
 BugReporterVisitor::getDefaultEndPath(const BugReporterContext &BRC,
                                       const ExplodedNode *EndPathNode,
                                       const PathSensitiveBugReport &BR) {
-  PathDiagnosticLocation L =
-      PathDiagnosticLocation::createEndOfPath(EndPathNode);
-
+  PathDiagnosticLocation L = BR.getLocation();
   const auto &Ranges = BR.getRanges();
 
   // Only add the statement itself as a range if we didn't specify any
@@ -852,7 +850,7 @@ private:
   /// \return Source location of right hand side of an assignment
   /// into \c RegionOfInterest, empty optional if none found.
   Optional<SourceLocation> matchAssignment(const ExplodedNode *N) {
-    const Stmt *S = PathDiagnosticLocation::getStmt(N);
+    const Stmt *S = N->getStmtForDiagnostics();
     ProgramStateRef State = N->getState();
     auto *LCtx = N->getLocationContext();
     if (!S)
@@ -1919,7 +1917,7 @@ static const Expr *peelOffOuterExpr(const Expr *Ex,
 static const ExplodedNode* findNodeForExpression(const ExplodedNode *N,
                                                  const Expr *Inner) {
   while (N) {
-    if (PathDiagnosticLocation::getStmt(N) == Inner)
+    if (N->getStmtForDiagnostics() == Inner)
       return N;
     N = N->getFirstPred();
   }
index 03e813e..a8fa7ad 100644 (file)
@@ -299,7 +299,9 @@ const CFGBlock *ExplodedNode::getCFGBlock() const {
     return BEP->getBlock();
 
   // Find the node's current statement in the CFG.
-  if (const Stmt *S = PathDiagnosticLocation::getStmt(this))
+  // FIXME: getStmtForDiagnostics() does nasty things in order to provide
+  // a valid statement for body farms, do we need this behavior here?
+  if (const Stmt *S = getStmtForDiagnostics())
     return getLocationContext()
         ->getAnalysisDeclContext()
         ->getCFGStmtMap()
@@ -308,6 +310,92 @@ const CFGBlock *ExplodedNode::getCFGBlock() const {
   return nullptr;
 }
 
+static const LocationContext *
+findTopAutosynthesizedParentContext(const LocationContext *LC) {
+  assert(LC->getAnalysisDeclContext()->isBodyAutosynthesized());
+  const LocationContext *ParentLC = LC->getParent();
+  assert(ParentLC && "We don't start analysis from autosynthesized code");
+  while (ParentLC->getAnalysisDeclContext()->isBodyAutosynthesized()) {
+    LC = ParentLC;
+    ParentLC = LC->getParent();
+    assert(ParentLC && "We don't start analysis from autosynthesized code");
+  }
+  return LC;
+}
+
+const Stmt *ExplodedNode::getStmtForDiagnostics() const {
+  // We cannot place diagnostics on autosynthesized code.
+  // Put them onto the call site through which we jumped into autosynthesized
+  // code for the first time.
+  const LocationContext *LC = getLocationContext();
+  if (LC->getAnalysisDeclContext()->isBodyAutosynthesized()) {
+    // It must be a stack frame because we only autosynthesize functions.
+    return cast<StackFrameContext>(findTopAutosynthesizedParentContext(LC))
+        ->getCallSite();
+  }
+  // Otherwise, see if the node's program point directly points to a statement.
+  // FIXME: Refactor into a ProgramPoint method?
+  ProgramPoint P = getLocation();
+  if (auto SP = P.getAs<StmtPoint>())
+    return SP->getStmt();
+  if (auto BE = P.getAs<BlockEdge>())
+    return BE->getSrc()->getTerminatorStmt();
+  if (auto CE = P.getAs<CallEnter>())
+    return CE->getCallExpr();
+  if (auto CEE = P.getAs<CallExitEnd>())
+    return CEE->getCalleeContext()->getCallSite();
+  if (auto PIPP = P.getAs<PostInitializer>())
+    return PIPP->getInitializer()->getInit();
+  if (auto CEB = P.getAs<CallExitBegin>())
+    return CEB->getReturnStmt();
+  if (auto FEP = P.getAs<FunctionExitPoint>())
+    return FEP->getStmt();
+
+  return nullptr;
+}
+
+const Stmt *ExplodedNode::getNextStmtForDiagnostics() const {
+  for (const ExplodedNode *N = getFirstSucc(); N; N = N->getFirstSucc()) {
+    if (const Stmt *S = N->getStmtForDiagnostics()) {
+      // Check if the statement is '?' or '&&'/'||'.  These are "merges",
+      // not actual statement points.
+      switch (S->getStmtClass()) {
+        case Stmt::ChooseExprClass:
+        case Stmt::BinaryConditionalOperatorClass:
+        case Stmt::ConditionalOperatorClass:
+          continue;
+        case Stmt::BinaryOperatorClass: {
+          BinaryOperatorKind Op = cast<BinaryOperator>(S)->getOpcode();
+          if (Op == BO_LAnd || Op == BO_LOr)
+            continue;
+          break;
+        }
+        default:
+          break;
+      }
+      // We found the statement, so return it.
+      return S;
+    }
+  }
+
+  return nullptr;
+}
+
+const Stmt *ExplodedNode::getPreviousStmtForDiagnostics() const {
+  for (const ExplodedNode *N = getFirstPred(); N; N = N->getFirstPred())
+    if (const Stmt *S = N->getStmtForDiagnostics())
+      return S;
+
+  return nullptr;
+}
+
+const Stmt *ExplodedNode::getCurrentOrPreviousStmtForDiagnostics() const {
+  if (const Stmt *S = getStmtForDiagnostics())
+    return S;
+
+  return getPreviousStmtForDiagnostics();
+}
+
 ExplodedNode *ExplodedGraph::getNode(const ProgramPoint &L,
                                      ProgramStateRef State,
                                      bool IsSink,
index 9838249..1a09a52 100644 (file)
@@ -165,7 +165,9 @@ static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
     return true;
 
   while (!N->pred_empty()) {
-    const Stmt *S = PathDiagnosticLocation::getStmt(N);
+    // FIXME: getStmtForDiagnostics() does nasty things in order to provide
+    // a valid statement for body farms, do we need this behavior here?
+    const Stmt *S = N->getStmtForDiagnostics();
     if (!S) {
       N = N->getFirstPred();
       continue;
index 94d1c09..853029a 100644 (file)
@@ -30,7 +30,6 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/None.h"
@@ -524,12 +523,12 @@ PathDiagnosticConsumer::FilesMade::getFiles(const PathDiagnostic &PD) {
 // PathDiagnosticLocation methods.
 //===----------------------------------------------------------------------===//
 
-static SourceLocation getValidSourceLocation(const Stmt* S,
-                                             LocationOrAnalysisDeclContext LAC,
-                                             bool UseEnd = false) {
-  SourceLocation L = UseEnd ? S->getEndLoc() : S->getBeginLoc();
-  assert(!LAC.isNull() && "A valid LocationContext or AnalysisDeclContext should "
-                          "be passed to PathDiagnosticLocation upon creation.");
+SourceLocation PathDiagnosticLocation::getValidSourceLocation(
+    const Stmt *S, LocationOrAnalysisDeclContext LAC, bool UseEndOfStatement) {
+  SourceLocation L = UseEndOfStatement ? S->getEndLoc() : S->getBeginLoc();
+  assert(!LAC.isNull() &&
+         "A valid LocationContext or AnalysisDeclContext should be passed to "
+         "PathDiagnosticLocation upon creation.");
 
   // S might be a temporary statement that does not have a location in the
   // source code, so find an enclosing statement and use its location.
@@ -559,7 +558,7 @@ static SourceLocation getValidSourceLocation(const Stmt* S,
         break;
       }
 
-      L = UseEnd ? Parent->getEndLoc() : Parent->getBeginLoc();
+      L = UseEndOfStatement ? Parent->getEndLoc() : Parent->getBeginLoc();
     } while (!L.isValid());
   }
 
@@ -778,117 +777,6 @@ PathDiagnosticLocation::create(const ProgramPoint& P,
   return PathDiagnosticLocation(S, SMng, P.getLocationContext());
 }
 
-static const LocationContext *
-findTopAutosynthesizedParentContext(const LocationContext *LC) {
-  assert(LC->getAnalysisDeclContext()->isBodyAutosynthesized());
-  const LocationContext *ParentLC = LC->getParent();
-  assert(ParentLC && "We don't start analysis from autosynthesized code");
-  while (ParentLC->getAnalysisDeclContext()->isBodyAutosynthesized()) {
-    LC = ParentLC;
-    ParentLC = LC->getParent();
-    assert(ParentLC && "We don't start analysis from autosynthesized code");
-  }
-  return LC;
-}
-
-const Stmt *PathDiagnosticLocation::getStmt(const ExplodedNode *N) {
-  // We cannot place diagnostics on autosynthesized code.
-  // Put them onto the call site through which we jumped into autosynthesized
-  // code for the first time.
-  const LocationContext *LC = N->getLocationContext();
-  if (LC->getAnalysisDeclContext()->isBodyAutosynthesized()) {
-    // It must be a stack frame because we only autosynthesize functions.
-    return cast<StackFrameContext>(findTopAutosynthesizedParentContext(LC))
-        ->getCallSite();
-  }
-  // Otherwise, see if the node's program point directly points to a statement.
-  ProgramPoint P = N->getLocation();
-  if (auto SP = P.getAs<StmtPoint>())
-    return SP->getStmt();
-  if (auto BE = P.getAs<BlockEdge>())
-    return BE->getSrc()->getTerminatorStmt();
-  if (auto CE = P.getAs<CallEnter>())
-    return CE->getCallExpr();
-  if (auto CEE = P.getAs<CallExitEnd>())
-    return CEE->getCalleeContext()->getCallSite();
-  if (auto PIPP = P.getAs<PostInitializer>())
-    return PIPP->getInitializer()->getInit();
-  if (auto CEB = P.getAs<CallExitBegin>())
-    return CEB->getReturnStmt();
-  if (auto FEP = P.getAs<FunctionExitPoint>())
-    return FEP->getStmt();
-
-  return nullptr;
-}
-
-const Stmt *PathDiagnosticLocation::getNextStmt(const ExplodedNode *N) {
-  for (N = N->getFirstSucc(); N; N = N->getFirstSucc()) {
-    if (const Stmt *S = getStmt(N)) {
-      // Check if the statement is '?' or '&&'/'||'.  These are "merges",
-      // not actual statement points.
-      switch (S->getStmtClass()) {
-        case Stmt::ChooseExprClass:
-        case Stmt::BinaryConditionalOperatorClass:
-        case Stmt::ConditionalOperatorClass:
-          continue;
-        case Stmt::BinaryOperatorClass: {
-          BinaryOperatorKind Op = cast<BinaryOperator>(S)->getOpcode();
-          if (Op == BO_LAnd || Op == BO_LOr)
-            continue;
-          break;
-        }
-        default:
-          break;
-      }
-      // We found the statement, so return it.
-      return S;
-    }
-  }
-
-  return nullptr;
-}
-
-PathDiagnosticLocation
-PathDiagnosticLocation::createEndOfPath(const ExplodedNode *N) {
-  assert(N && "Cannot create a location with a null node.");
-  const Stmt *S = getStmt(N);
-  const LocationContext *LC = N->getLocationContext();
-  SourceManager &SM =
-      N->getState()->getStateManager().getContext().getSourceManager();
-
-  if (!S) {
-    // If this is an implicit call, return the implicit call point location.
-    if (Optional<PreImplicitCall> PIE = N->getLocationAs<PreImplicitCall>())
-      return PathDiagnosticLocation(PIE->getLocation(), SM);
-    if (auto FE = N->getLocationAs<FunctionExitPoint>()) {
-      if (const ReturnStmt *RS = FE->getStmt())
-        return PathDiagnosticLocation::createBegin(RS, SM, LC);
-    }
-    S = getNextStmt(N);
-  }
-
-  if (S) {
-    ProgramPoint P = N->getLocation();
-
-    // For member expressions, return the location of the '.' or '->'.
-    if (const auto *ME = dyn_cast<MemberExpr>(S))
-      return PathDiagnosticLocation::createMemberLoc(ME, SM);
-
-    // For binary operators, return the location of the operator.
-    if (const auto *B = dyn_cast<BinaryOperator>(S))
-      return PathDiagnosticLocation::createOperatorLoc(B, SM);
-
-    if (P.getAs<PostStmtPurgeDeadSymbols>())
-      return PathDiagnosticLocation::createEnd(S, SM, LC);
-
-    if (S->getBeginLoc().isValid())
-      return PathDiagnosticLocation(S, SM, LC);
-    return PathDiagnosticLocation(getValidSourceLocation(S, LC), SM);
-  }
-
-  return createDeclEnd(N->getLocationContext(), SM);
-}
-
 PathDiagnosticLocation PathDiagnosticLocation::createSingleLocation(
                                            const PathDiagnosticLocation &PDL) {
   FullSourceLoc L = PDL.asLocation();