[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 ↗

(On Diff #219455)

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

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

276 ↗

(On Diff #219455)

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

282 ↗

(On Diff #219455)

before this node?

288 ↗

(On Diff #219455)

before this node?

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

2185 ↗

(On Diff #219455)

Why delete the assert?

clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp

302–303 ↗

(On Diff #219455)

But still fails...

clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

565–566 ↗

(On Diff #219455)

I guess didn't change much :^)

[NoQ]

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

2185 ↗

(On Diff #219455)

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 ↗

(On Diff #219455)

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 ↗

(On Diff #219455)

Well, not everything is a statement.

clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

565–566 ↗

(On Diff #219455)

Indeed :)

cfe/trunk/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);
 
+  /// 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;
   }
 
   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,

cfe/trunk/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 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); }
 };
 
 using InterExplodedGraphMap =
     llvm::DenseMap<const ExplodedNode *, const ExplodedNode *>;
 

cfe/trunk/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.

cfe/trunk/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 '";

cfe/trunk/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 '";

cfe/trunk/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();
 

cfe/trunk/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,

cfe/trunk/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

cfe/trunk/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:

cfe/trunk/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")

cfe/trunk/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

cfe/trunk/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,

cfe/trunk/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");
 }

cfe/trunk/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) &&

cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp

std::string StackHintGeneratorForSymbol::getMessageForArg(const Expr *ArgE,
   // Printed parameters start at 1, not 0.
   ++ArgIndex;
 
   return (llvm::Twine(Msg) + " via " + std::to_string(ArgIndex) +
           llvm::getOrdinalSuffix(ArgIndex) + " parameter").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.

cfe/trunk/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,

cfe/trunk/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.
+  // 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,
                                      bool* IsNew) {
   // Profile 'State' to determine if we already have an existing node.
   llvm::FoldingSetNodeID profile;
   void *InsertPos = nullptr;
 

cfe/trunk/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.

cfe/trunk/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 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.
   if (!L.isValid()) {
     AnalysisDeclContext *ADC;
     if (LAC.is<const LocationContext*>())
       ADC = LAC.get<const LocationContext*>()->getAnalysisDeclContext();
     else
do {
         const Stmt *Body = ADC->getBody();
         if (Body)
           L = Body->getBeginLoc();
         else
           L = ADC->getDecl()->getEndLoc();
         break;
       }
 
-      L = UseEnd ? Parent->getEndLoc() : Parent->getBeginLoc();
+      L = UseEndOfStatement ? Parent->getEndLoc() : Parent->getBeginLoc();
     } while (!L.isValid());
   }
 
   // FIXME: Ironically, this assert actually fails in some cases.
   //assert(L.isValid());
   return L;
 }
 
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,