[analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic.

[NoQ]

Summary

These static functions deal with ExplodedNodes which is something i don't want the PathDiagnostic interface to know anything about, as it's planned to be moved out of libStaticAnalyzer.

PathDiagnosticLocation::getStmt(N), a function commonly used in bug visitors, is now known as ExplodedNode::getStmtForDiagnostics(). Because it contains a hack that allows it to avoid body-farmed locations, it should only be used for diagnostics and is useless for most other purposes, hence the name. I added a few FIXMEs in places where this may be already causing problems. PathDiagnosticLocation::getNextStmt(N) and a couple of helper functions from BugReporter.cpp now live nearby.

PathDiagnosticLocation::createEndOfPath(N) has a similar purpose but is more sophisticated and is used mostly by end-of-path visitors. I removed this function entirely in favor of using PathSensitiveBugReport::getLocation() instead, which is literally the same thing as long as N is the bug node, which is always true. This creates a certain problem in RetainCountChecker (surprise!!~) because it has its own PathSensitiveBugReport sub-class that overrides getLocation() to not be equal to end of path, but still needs to obtain the end-of-path location in the visitor. I worked around that by giving it a separate method.

[Szelethus]

LGTM! I think code readability improved geatly.

This creates a certain problem in RetainCountChecker (surprise!!~)

We constantly bully this checker, but still not enough :^)

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

284

Lets explain what UseEnd is: Whether to use getEndLoc() rather then getBeginLoc() for \p S.

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

276

Is this correct? Shouldn't we instead say that "Find the next statement that was symbolically executed on this path of execution"?

282

before this node?

288

before this node?

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

2185

Why delete the assert?

clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp

302–303

But still fails...

clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

565–566

I guess didn't change much :^)

[NoQ]

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

2185

Because it's enforced by the type system these days. This is a method on PathSensitiveBugReport that always has an error node, as asserted in its constructor. I should have removed this assert in D66572.

[NoQ]

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

2185

Previously this assert was on the generic BugReport::Profile and it was there to state that "either a report has an error node, or a manually set location". Now have a clear separation between path-sensitive and path-insensitive reports in our type system, so it doesn't make sense to enforce this alternative manually.

Also, no, it wasn't failing :)

[NoQ]

clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp

302–303

Well, not everything is a statement.

clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

565–566

Indeed :)

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

 class Expr;
 class LocationContext;
 class MemberExpr;
 class ProgramPoint;
 class SourceManager;
 
 namespace ento {
 
-class ExplodedNode;
-class SymExpr;
-
-using SymbolRef = const SymExpr *;
-
 //===----------------------------------------------------------------------===//
 // High-level interface for handlers of path-sensitive diagnostics.
 //===----------------------------------------------------------------------===//
 
 class PathDiagnostic;
 
 class PathDiagnosticConsumer {
 public:
public:
   static PathDiagnosticLocation createDeclBegin(const LocationContext *LC,
                                                 const SourceManager &SM);
 
   /// Constructs a location for the end of the enclosing declaration body.
   /// Defaults to the end of brace.
   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);
 
+  static SourceLocation
+  getValidSourceLocation(const Stmt *S, LocationOrAnalysisDeclContext LAC,
+                         bool UseEnd = false);
+
   bool operator==(const PathDiagnosticLocation &X) const {
     return K == X.K && Loc == X.Loc && Range == X.Range;
   }
 
   bool operator!=(const PathDiagnosticLocation &X) const {
     return !(*this == X);
   }
 
public:
 
   void flatten();
 
   const SourceManager& getManager() const { assert(isValid()); return *SM; }
 
   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 {
 private:
   PathDiagnosticLocation Start, End;
 
 public:
   PathDiagnosticLocationPair(const PathDiagnosticLocation &start,

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

public:
 
   /// The node is trivial if it has only one successor, only one predecessor,
   /// it's predecessor has only one successor,
   /// and its program state is the same as the program state of the previous
   /// node.
   /// 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 is going to be executed after this node.
+  /// 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 that 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 that 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); }
 };
 
 using InterExplodedGraphMap =
     llvm::DenseMap<const ExplodedNode *, const ExplodedNode *>;
 

clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp

 PathDiagnosticPieceRef
 DeleteWithNonVirtualDtorChecker::DeleteBugVisitor::VisitNode(
     const ExplodedNode *N, BugReporterContext &BRC,
     PathSensitiveBugReport &BR) {
   // Stop traversal after the first conversion was found on a path.
   if (Satisfied)
     return nullptr;
 
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
 
   const auto *CastE = dyn_cast<CastExpr>(S);
   if (!CastE)
     return nullptr;
 
   // Only interested in DerivedToBase implicit casts.

clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp

PathDiagnosticPieceRef DynamicTypeChecker::DynamicTypeBugVisitor::VisitNode(
   if (!TrackedType.isValid())
     return nullptr;
 
   if (TrackedTypePrev.isValid() &&
       TrackedTypePrev.getType() == TrackedType.getType())
     return nullptr;
 
   // Retrieve the associated statement.
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
 
   const LangOptions &LangOpts = BRC.getASTContext().getLangOpts();
 
   SmallString<256> Buf;
   llvm::raw_svector_ostream OS(Buf);
   OS << "Type '";

clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp

const ObjCObjectPointerType *const *TrackedTypePrev =
       statePrev->get<MostSpecializedTypeArgsMap>(Sym);
   if (!TrackedType)
     return nullptr;
 
   if (TrackedTypePrev && *TrackedTypePrev == *TrackedType)
     return nullptr;
 
   // Retrieve the associated statement.
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
 
   const LangOptions &LangOpts = BRC.getASTContext().getLangOpts();
 
   SmallString<256> Buf;
   llvm::raw_svector_ostream OS(Buf);
   OS << "Type '";

clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp

 } // end namespace clang
 
 PathDiagnosticPieceRef InnerPointerChecker::InnerPointerBRVisitor::VisitNode(
     const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &) {
   if (!isSymbolTracked(N->getState(), PtrToBuf) ||
       isSymbolTracked(N->getFirstPred()->getState(), PtrToBuf))
     return nullptr;
 
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
 
   const MemRegion *ObjRegion =
       allocation_state::getContainerObjRegion(N->getState(), PtrToBuf);
   const auto *TypedRegion = cast<TypedValueRegion>(ObjRegion);
   QualType ObjTy = TypedRegion->getValueType();
 

clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp

MacOSKeychainAPIChecker::generateAllocatedDataNotReleasedReport(
   os << "Allocated data is not released: missing a call to '"
       << FunctionsToTrack[FI.DeallocatorIdx].Name << "'.";
 
   // Most bug reports are cached at the location where they occurred.
   // With leaks, we want to unique them by the location where they were
   // 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,
                                               C.getSourceManager(),
                                               AllocNode->getLocationContext());
 
   auto Report = std::make_unique<PathSensitiveBugReport>(
       *BT, os.str(), N, LocUsedForUniqueing,

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                      PathSensitiveBugReport &BR) override;
 
     PathDiagnosticPieceRef getEndPath(BugReporterContext &BRC,
                                       const ExplodedNode *EndPathNode,
                                       PathSensitiveBugReport &BR) override {
       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);
     }
 
   private:
     class StackHintGeneratorForReallocationFailed
         : public StackHintGeneratorForSymbol {
void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N,
   // Most bug reports are cached at the location where they occurred.
   // With leaks, we want to unique them by the location where they were
   // allocated, and only report a single path.
   PathDiagnosticLocation LocUsedForUniqueing;
   const ExplodedNode *AllocNode = nullptr;
   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(),
                                               AllocNode->getLocationContext());
 
   SmallString<200> buf;
   llvm::raw_svector_ostream os(buf);
   if (Region && Region->canPrintPretty()) {
MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
                                            PathSensitiveBugReport &BR) {
 
   ProgramStateRef state = N->getState();
   ProgramStateRef statePrev = N->getFirstPred()->getState();
 
   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))
     return nullptr;
 
   const LocationContext *CurrentLC = N->getLocationContext();
 
   // If we find an atomic fetch_add or fetch_sub within the destructor in which

clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp

MoveChecker::MovedBugVisitor::VisitNode(const ExplodedNode *N,
   const RegionState *TrackedObjectPrev =
       StatePrev->get<TrackedRegionMap>(Region);
   if (!TrackedObject)
     return nullptr;
   if (TrackedObjectPrev && TrackedObject)
     return nullptr;
 
   // Retrieve the associated statement.
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
   Found = true;
 
   SmallString<128> Str;
   llvm::raw_svector_ostream OS(Str);
 
   ObjectKind OK = Chk.classifyObject(Region, RD);
if (ExplodedNode *N = misuseCausesCrash(MK) ? C.generateErrorNode()
     if (!BT)
       BT.reset(new BugType(this, "Use-after-move",
                            "C++ move semantics"));
 
     // Uniqueing report to the same object.
     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());
 
     // Creating the error message.
     llvm::SmallString<128> Str;
     llvm::raw_svector_ostream OS(Str);
     switch(MK) {
       case MK_FunCall:

clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

PathDiagnosticPieceRef NullabilityChecker::NullabilityBugVisitor::VisitNode(
 
   if (TrackedNullabPrev &&
       TrackedNullabPrev->getValue() == TrackedNullab->getValue())
     return nullptr;
 
   // Retrieve the associated statement.
   const Stmt *S = TrackedNullab->getNullabilitySource();
   if (!S || S->getBeginLoc().isInvalid()) {
-    S = PathDiagnosticLocation::getStmt(N);
+    S = N->getStmtForDiagnostics();
   }
 
   if (!S)
     return nullptr;
 
   std::string InfoText =
       (llvm::Twine("Nullability '") +
        getNullabilityString(TrackedNullab->getValue()) + "' is inferred")

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

class RefLeakReport : public RefCountReport {
   // Finds the location where a leak warning for 'sym' should be raised.
   void deriveAllocLocation(CheckerContext &Ctx, SymbolRef sym);
   // Produces description of a leak warning which is printed on the console.
   void createDescription(CheckerContext &Ctx);
 
 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
 } // end namespace ento
 } // end namespace clang
 
 #endif

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

RefLeakReportVisitor::getEndPath(BugReporterContext &BRC,
   // We are reporting a leak.  Walk up the graph to get to the first node where
   // the symbol appeared, and also get the first VarDecl that tracked object
   // is stored to.
   AllocationInfo AllocI = GetAllocationSite(BRC.getStateManager(), EndN, Sym);
 
   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);
 
   os << "Object leaked: ";
 
   Optional<std::string> RegionDescription = describeRegion(FirstBinding);
   if (RegionDescription) {
void RefLeakReport::deriveAllocLocation(CheckerContext &Ctx,
   AllocNode = AllocI.N;
   AllocBinding = AllocI.R;
   markInteresting(AllocI.InterestingMethodContext);
 
   // Get the SourceLocation for the allocation site.
   // 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;
     return;
   }
 
   PathDiagnosticLocation AllocLocation =
     PathDiagnosticLocation::createBegin(AllocStmt, SMgr,

clang/lib/StaticAnalyzer/Checkers/Taint.cpp

PathDiagnosticPieceRef TaintBugVisitor::VisitNode(const ExplodedNode *N,
                                                   BugReporterContext &BRC,
                                                   PathSensitiveBugReport &BR) {
 
   // Find the ExplodedNode where the taint was first introduced
   if (!isTainted(N->getState(), V) ||
       isTainted(N->getFirstPred()->getState(), V))
     return nullptr;
 
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
 
   const LocationContext *NCtx = N->getLocationContext();
   PathDiagnosticLocation L =
       PathDiagnosticLocation::createBegin(S, BRC.getSourceManager(), NCtx);
   if (!L.isValid() || !L.asLocation().isValid())
     return nullptr;
 
   return std::make_shared<PathDiagnosticEventPiece>(L, "Taint originated here");
 }

clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp

void Profile(llvm::FoldingSetNodeID &ID) const override {
       ID.AddPointer(Reg);
     }
     PathDiagnosticPieceRef getEndPath(BugReporterContext &BRC,
                                       const ExplodedNode *EndPathNode,
                                       PathSensitiveBugReport &BR) override {
       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);
     }
     PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                      BugReporterContext &BRC,
                                      PathSensitiveBugReport &BR) override;
 
if (!BT_leakedvalist) {
                           : CheckNames[CK_Unterminated],
                       "Leaked va_list", categories::MemoryError,
                       /*SuppressOnSink=*/true));
     }
 
     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());
 
     SmallString<100> Buf;
     llvm::raw_svector_ostream OS(Buf);
     OS << Msg1;
     std::string VariableName = Reg->getDescriptiveName();
     if (!VariableName.empty())
void ValistChecker::checkVAListEndCall(const CallEvent &Call,
   C.addTransition(State);
 }
 
 PathDiagnosticPieceRef ValistChecker::ValistBugVisitor::VisitNode(
     const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &) {
   ProgramStateRef State = N->getState();
   ProgramStateRef StatePrev = N->getFirstPred()->getState();
 
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  const Stmt *S = N->getStmtForDiagnostics();
   if (!S)
     return nullptr;
 
   StringRef Msg;
   if (State->contains<InitializedVALists>(Reg) &&
       !StatePrev->contains<InitializedVALists>(Reg))
     Msg = "Initialized va_list";
   else if (!State->contains<InitializedVALists>(Reg) &&

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

std::string StackHintGeneratorForSymbol::getMessageForArg(const Expr *ArgE,
 
   os << Msg << " via " << ArgIndex << llvm::getOrdinalSuffix(ArgIndex)
      << " parameter";
 
   return os.str();
 }
 
 //===----------------------------------------------------------------------===//
-// 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.
 //===----------------------------------------------------------------------===//
 
 static PathDiagnosticEventPiece *
 eventsDescribeSameCondition(PathDiagnosticEventPiece *X,
                             PathDiagnosticEventPiece *Y) {
   // Prefer diagnostics that come from ConditionBRVisitor over
   // those that came from TrackConstraintBRVisitor,
if (!(*I)->getLocation().isValid() ||
       continue;
     }
     I++;
   }
 }
 
 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());
 
   return PathDiagnosticLocation::createDeclEnd(C.getCurrLocationContext(),
                                                getSourceManager());
 }
 
 PathDiagnosticLocation PathDiagnosticBuilder::ExecutionContinues(
void PathDiagnosticBuilder::generateMinimalDiagForBlockEdge(
 
   auto Start = PathDiagnosticLocation::createBegin(T, SM, LC);
   switch (T->getStmtClass()) {
   default:
     break;
 
   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;
   }
 
   case Stmt::SwitchStmtClass: {
     C.getActivePath().push_front(generateDiagForSwitchOP(C, Dst, Start));
     break;
   }

 void PathSensitiveBugReport::Profile(llvm::FoldingSetNodeID &hash) const {
   hash.AddInteger(static_cast<int>(getKind()));
   hash.AddPointer(&BT);
   hash.AddString(Description);
   PathDiagnosticLocation UL = getUniqueingLocation();
   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) {
     if (!range.isValid())
       continue;
     hash.AddInteger(range.getBegin().getRawEncoding());
     hash.AddInteger(range.getEnd().getRawEncoding());
   }
if (!ErrorNode)
     return nullptr;
 
   ProgramPoint ProgP = ErrorNode->getLocation();
   const Stmt *S = nullptr;
 
   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;
 }
 
 ArrayRef<SourceRange>
 PathSensitiveBugReport::getRanges() const {
   // If no custom ranges, add the range of the statement corresponding to
   // the error node.
   if (Ranges.empty() && isa_and_nonnull<Expr>(getStmt()))
       return ErrorNodeRange;
 
   return Ranges;
 }
 
 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);
 }
 
 //===----------------------------------------------------------------------===//
 // Methods for BugReporter and subclasses.
 //===----------------------------------------------------------------------===//
 
 const ExplodedGraph &PathSensitiveBugReporter::getGraph() const {
   return Eng.getGraph();
while (N) {
     if (N->getFirstPred() == nullptr) {
       // First node: show signature of the entrance point.
       const Decl *D = N->getLocationContext()->getDecl();
       populateExecutedLinesWithFunctionSignature(D, SM, *ExecutedLines);
     } else if (auto CE = N->getLocationAs<CallEnter>()) {
       // 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.
       const Stmt *P = N->getParentMap().getParent(S);
 
       // The path exploration can die before the node with the associated
       // return statement is generated, but we do want to show the whole
       // return.

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

 void BugReporterVisitor::finalizeVisitor(BugReporterContext &,
                                          const ExplodedNode *,
                                          PathSensitiveBugReport &) {}
 
 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
   // special ranges for this report.
   auto P = std::make_shared<PathDiagnosticEventPiece>(
       L, BR.getDescription(), Ranges.begin() == Ranges.end());
   for (SourceRange Range : Ranges)
     P->addRange(Range);
public:
   void Profile(llvm::FoldingSetNodeID &ID) const override {
     ID.AddPointer(getTag());
   }
 
 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)
       return None;
 
     if (const auto *DS = dyn_cast<DeclStmt>(S)) {
       if (const auto *VD = dyn_cast<VarDecl>(DS->getSingleDecl()))
         if (const Expr *RHS = VD->getInit())
static const Expr *peelOffOuterExpr(const Expr *Ex,
   return Ex;
 }
 
 /// Find the ExplodedNode where the lvalue (the value of 'Ex')
 /// was computed.
 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();
   }
   return N;
 }
 
 bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
                                        const Expr *E,

clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp

 }
 
 const CFGBlock *ExplodedNode::getCFGBlock() const {
   ProgramPoint P = getLocation();
   if (auto BEP = P.getAs<BlockEntrance>())
     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()
         ->getBlock(S);
 
   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.
+  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,
                                      bool* IsNew) {
   // Profile 'State' to determine if we already have an existing node.
   llvm::FoldingSetNodeID profile;
   void *InsertPos = nullptr;
 

clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp

 }
 
 static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
   // Global variables assumed as escaped variables.
   if (VD->hasGlobalStorage())
     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;
     }
 
     if (const DeclStmt *DS = dyn_cast<DeclStmt>(S)) {
       for (const Decl *D : DS->decls()) {
         // Once we reach the declaration of the VD we can return.

clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LLVM.h"
 #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"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
if (!Entry)
     return nullptr;
   return &Entry->files;
 }
 
 //===----------------------------------------------------------------------===//
 // PathDiagnosticLocation methods.
 //===----------------------------------------------------------------------===//
 
-static SourceLocation getValidSourceLocation(const Stmt* S,
-                                             LocationOrAnalysisDeclContext LAC,
-                                             bool UseEnd = false) {
+SourceLocation PathDiagnosticLocation::getValidSourceLocation(
+    const Stmt *S, LocationOrAnalysisDeclContext LAC, bool UseEnd) {
   SourceLocation L = UseEnd ? S->getEndLoc() : S->getBeginLoc();
-  assert(!LAC.isNull() && "A valid LocationContext or AnalysisDeclContext should "
-                          "be passed to PathDiagnosticLocation upon creation.");
+  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.
   if (!L.isValid()) {
     AnalysisDeclContext *ADC;
     if (LAC.is<const LocationContext*>())
       ADC = LAC.get<const LocationContext*>()->getAnalysisDeclContext();
     else
do {
           L = ADC->getDecl()->getEndLoc();
         break;
       }
 
       L = UseEnd ? Parent->getEndLoc() : Parent->getBeginLoc();
     } while (!L.isValid());
   }
 
   // FIXME: Ironically, this assert actually fails in some cases.
   //assert(L.isValid());
   return L;
 }
 
 static PathDiagnosticLocation
 getLocationForCaller(const StackFrameContext *SFC,
                      const LocationContext *CallerCtx,
                      const SourceManager &SM) {
   const CFGBlock &Block = *SFC->getCallSiteBlock();
return PathDiagnosticLocation(FE->getStmt(), SMng,
                                   FE->getLocationContext());
   } else {
     llvm_unreachable("Unexpected ProgramPoint");
   }
 
   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();
   return PathDiagnosticLocation(L, L.getManager(), SingleLocK);
 }
 
 FullSourceLoc
   PathDiagnosticLocation::genLocation(SourceLocation L,