[analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagn…

…ostic.

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

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

@@ -52,11 +52,6 @@ class SourceManager;
 
 namespace ento {
 
-class ExplodedNode;
-class SymExpr;
-
-using SymbolRef = const SymExpr *;
-
 //===----------------------------------------------------------------------===https://
 // High-level interface for handlers of path-sensitive diagnostics.
 //===----------------------------------------------------------------------===https://
@@ -276,18 +271,21 @@ class PathDiagnosticLocation {
  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 @@ class PathDiagnosticLocation {
  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 {

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h

@@ -267,6 +267,30 @@ class ExplodedNode : public llvm::FoldingSetNode {
  /// 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); }

clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp

@@ -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;
 

clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp

@@ -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;
 

clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp

@@ -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;
 

clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp

@@ -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;
 

clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp

@@ -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,

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

@@ -532,8 +532,7 @@ class MallocChecker : public Checker<check::DeadSymbols,
  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))

clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp

@@ -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());
 

clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

@@ -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)

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp

@@ -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;

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h

@@ -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

clang/lib/StaticAnalyzer/Checkers/Taint.cpp

@@ -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;
 

clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp

@@ -83,8 +83,7 @@ class ValistChecker : public Checker<check::PreCall, check::PreStmt<VAArgExpr>,
  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;
 

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

@@ -334,26 +334,6 @@ std::string StackHintGeneratorForSymbol::getMessageForArg(const Expr *ArgE,
  llvm::getOrdinalSuffix(ArgIndex) + " parameter").str();
 }
 
-//===----------------------------------------------------------------------===https://
-// Helper routines for walking the ExplodedGraph and fetching statements.
-//===----------------------------------------------------------------------===https://
-
-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);
-}
-
 //===----------------------------------------------------------------------===https://
 // Diagnostic cleanup.
 //===----------------------------------------------------------------------===https://
@@ -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);
 }
 
 //===----------------------------------------------------------------------===https://
@@ -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.