From e03d00f8b53eadf0456423cbf66e8d06b9fd41f6 Mon Sep 17 00:00:00 2001 From: Artem Dergachev Date: Wed, 11 Sep 2019 20:54:21 +0000 Subject: [PATCH] [analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic. 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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@371659 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/BugReporter/PathDiagnostic.h | 25 ++-- .../Core/PathSensitive/ExplodedGraph.h | 24 ++++ .../DeleteWithNonVirtualDtorChecker.cpp | 2 +- .../Checkers/DynamicTypeChecker.cpp | 2 +- .../Checkers/DynamicTypePropagation.cpp | 2 +- .../Checkers/InnerPointerChecker.cpp | 2 +- .../Checkers/MacOSKeychainAPIChecker.cpp | 2 +- lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 7 +- lib/StaticAnalyzer/Checkers/MoveChecker.cpp | 4 +- .../Checkers/NullabilityChecker.cpp | 2 +- .../RetainCountDiagnostics.cpp | 8 +- .../RetainCountDiagnostics.h | 5 +- lib/StaticAnalyzer/Checkers/Taint.cpp | 2 +- lib/StaticAnalyzer/Checkers/ValistChecker.cpp | 7 +- lib/StaticAnalyzer/Core/BugReporter.cpp | 77 +++++++---- .../Core/BugReporterVisitors.cpp | 8 +- lib/StaticAnalyzer/Core/ExplodedGraph.cpp | 90 ++++++++++++- lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 4 +- lib/StaticAnalyzer/Core/PathDiagnostic.cpp | 126 +----------------- 19 files changed, 204 insertions(+), 195 deletions(-) diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h index dd65af452f..aaeeb48cf2 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h @@ -52,11 +52,6 @@ class SourceManager; namespace ento { -class ExplodedNode; -class SymExpr; - -using SymbolRef = const SymExpr *; - //===----------------------------------------------------------------------===// // High-level interface for handlers of path-sensitive diagnostics. //===----------------------------------------------------------------------===// @@ -276,18 +271,21 @@ public: static PathDiagnosticLocation createDeclEnd(const LocationContext *LC, const SourceManager &SM); - /// Create a location corresponding to the given valid ExplodedNode. + /// Create a location corresponding to the given valid ProgramPoint. static PathDiagnosticLocation create(const ProgramPoint &P, const SourceManager &SMng); - /// Create a location corresponding to the next valid ExplodedNode as end - /// of path location. - static PathDiagnosticLocation createEndOfPath(const ExplodedNode* N); - /// Convert the given location into a single kind location. static PathDiagnosticLocation createSingleLocation( const PathDiagnosticLocation &PDL); + /// Construct a source location that corresponds to either the beginning + /// or the end of the given statement, or a nearby valid source location + /// if the statement does not have a valid source location of its own. + static SourceLocation + getValidSourceLocation(const Stmt *S, LocationOrAnalysisDeclContext LAC, + bool UseEndOfStatement = false); + bool operator==(const PathDiagnosticLocation &X) const { return K == X.K && Loc == X.Loc && Range == X.Range; } @@ -332,13 +330,6 @@ public: void Profile(llvm::FoldingSetNodeID &ID) const; void dump() const; - - /// Given an exploded node, retrieve the statement that should be used - /// for the diagnostic location. - static const Stmt *getStmt(const ExplodedNode *N); - - /// Retrieve the statement corresponding to the successor node. - static const Stmt *getNextStmt(const ExplodedNode *N); }; class PathDiagnosticLocationPair { diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h index 598e2f632c..9773a552bf 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h @@ -267,6 +267,30 @@ public: /// Trivial nodes may be skipped while printing exploded graph. bool isTrivial() const; + /// If the node's program point corresponds to a statement, retrieve that + /// statement. Useful for figuring out where to put a warning or a note. + /// If the statement belongs to a body-farmed definition, + /// retrieve the call site for that definition. + const Stmt *getStmtForDiagnostics() const; + + /// Find the next statement that was executed on this node's execution path. + /// Useful for explaining control flow that follows the current node. + /// If the statement belongs to a body-farmed definition, retrieve the + /// call site for that definition. + const Stmt *getNextStmtForDiagnostics() const; + + /// Find the statement that was executed immediately before this node. + /// Useful when the node corresponds to a CFG block entrance. + /// If the statement belongs to a body-farmed definition, retrieve the + /// call site for that definition. + const Stmt *getPreviousStmtForDiagnostics() const; + + /// Find the statement that was executed at or immediately before this node. + /// Useful when any nearby statement will do. + /// If the statement belongs to a body-farmed definition, retrieve the + /// call site for that definition. + const Stmt *getCurrentOrPreviousStmtForDiagnostics() const; + private: void replaceSuccessor(ExplodedNode *node) { Succs.replaceNode(node); } void replacePredecessor(ExplodedNode *node) { Preds.replaceNode(node); } diff --git a/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp b/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp index 882df3e1f9..8010c85897 100644 --- a/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp +++ b/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; diff --git a/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp b/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp index d7a725c1a5..8cc38f9735 100644 --- a/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp +++ b/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; diff --git a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp index ca2283d3a3..cce3449b88 100644 --- a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ b/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; diff --git a/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp b/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp index 9cb77c0420..b0d101c885 100644 --- a/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp +++ b/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; diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index 9947b57e27..6be6762468 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/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, diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 7763c53fd5..260e61ee48 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -532,8 +532,7 @@ private: if (!IsLeak) return nullptr; - PathDiagnosticLocation L = - PathDiagnosticLocation::createEndOfPath(EndPathNode); + PathDiagnosticLocation L = BR.getLocation(); // Do not add the statement itself as a range in case of leak. return std::make_shared(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(Sym); const RefState *RSPrev = statePrev->get(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)) diff --git a/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/lib/StaticAnalyzer/Checkers/MoveChecker.cpp index a429123ddc..1473c05d7e 100644 --- a/lib/StaticAnalyzer/Checkers/MoveChecker.cpp +++ b/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()); diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp index 4b68a4ad75..2f46e813ba 100644 --- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/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) diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp index d3c60112ab..9853758f7f 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp +++ b/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(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; diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h index 323b50ad9f..3b212127c9 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h +++ b/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 diff --git a/lib/StaticAnalyzer/Checkers/Taint.cpp b/lib/StaticAnalyzer/Checkers/Taint.cpp index 2a9456dd84..574d4ed1e6 100644 --- a/lib/StaticAnalyzer/Checkers/Taint.cpp +++ b/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; diff --git a/lib/StaticAnalyzer/Checkers/ValistChecker.cpp b/lib/StaticAnalyzer/Checkers/ValistChecker.cpp index ca0091205e..0c83af145a 100644 --- a/lib/StaticAnalyzer/Checkers/ValistChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ValistChecker.cpp @@ -83,8 +83,7 @@ private: if (!IsLeak) return nullptr; - PathDiagnosticLocation L = - PathDiagnosticLocation::createEndOfPath(EndPathNode); + PathDiagnosticLocation L = BR.getLocation(); // Do not add the statement itself as a range in case of leak. return std::make_shared(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; diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index c3b76b42ba..57f639afba 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -334,26 +334,6 @@ std::string StackHintGeneratorForSymbol::getMessageForArg(const Expr *ArgE, 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. //===----------------------------------------------------------------------===// @@ -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 BE = ProgP.getAs()) { 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 PIE = P.getAs()) + return PathDiagnosticLocation(PIE->getLocation(), SM); + if (auto FE = P.getAs()) { + 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(S)) + return PathDiagnosticLocation::createMemberLoc(ME, SM); + + // For binary operators, return the location of the operator. + if (const auto *B = dyn_cast(S)) + return PathDiagnosticLocation::createOperatorLoc(B, SM); + + if (P.getAs()) + return PathDiagnosticLocation::createEnd(S, SM, LC); + + if (S->getBeginLoc().isValid()) + return PathDiagnosticLocation(S, SM, LC); + + return PathDiagnosticLocation( + PathDiagnosticLocation::getValidSourceLocation(S, LC), SM); + } + + return PathDiagnosticLocation::createDeclEnd(ErrorNode->getLocationContext(), + SM); } //===----------------------------------------------------------------------===// @@ -3070,7 +3091,7 @@ findExecutedLines(const SourceManager &SM, const ExplodedNode *N) { // Inlined function: show signature. const Decl* D = CE->getCalleeContext()->getDecl(); populateExecutedLinesWithFunctionSignature(D, SM, *ExecutedLines); - } else if (const Stmt *S = PathDiagnosticLocation::getStmt(N)) { + } else if (const Stmt *S = N->getStmtForDiagnostics()) { populateExecutedLinesWithStmt(S, SM, *ExecutedLines); // Show extra context for some parent kinds. diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 7b26b8fabb..bf923daeec 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -308,9 +308,7 @@ PathDiagnosticPieceRef BugReporterVisitor::getDefaultEndPath(const BugReporterContext &BRC, const ExplodedNode *EndPathNode, const PathSensitiveBugReport &BR) { - PathDiagnosticLocation L = - PathDiagnosticLocation::createEndOfPath(EndPathNode); - + PathDiagnosticLocation L = BR.getLocation(); const auto &Ranges = BR.getRanges(); // Only add the statement itself as a range if we didn't specify any @@ -852,7 +850,7 @@ private: /// \return Source location of right hand side of an assignment /// into \c RegionOfInterest, empty optional if none found. Optional matchAssignment(const ExplodedNode *N) { - const Stmt *S = PathDiagnosticLocation::getStmt(N); + const Stmt *S = N->getStmtForDiagnostics(); ProgramStateRef State = N->getState(); auto *LCtx = N->getLocationContext(); if (!S) @@ -1919,7 +1917,7 @@ static const Expr *peelOffOuterExpr(const Expr *Ex, static const ExplodedNode* findNodeForExpression(const ExplodedNode *N, const Expr *Inner) { while (N) { - if (PathDiagnosticLocation::getStmt(N) == Inner) + if (N->getStmtForDiagnostics() == Inner) return N; N = N->getFirstPred(); } diff --git a/lib/StaticAnalyzer/Core/ExplodedGraph.cpp b/lib/StaticAnalyzer/Core/ExplodedGraph.cpp index 03e813e1e6..a8fa7ad2d9 100644 --- a/lib/StaticAnalyzer/Core/ExplodedGraph.cpp +++ b/lib/StaticAnalyzer/Core/ExplodedGraph.cpp @@ -299,7 +299,9 @@ const CFGBlock *ExplodedNode::getCFGBlock() const { return BEP->getBlock(); // Find the node's current statement in the CFG. - if (const Stmt *S = PathDiagnosticLocation::getStmt(this)) + // FIXME: getStmtForDiagnostics() does nasty things in order to provide + // a valid statement for body farms, do we need this behavior here? + if (const Stmt *S = getStmtForDiagnostics()) return getLocationContext() ->getAnalysisDeclContext() ->getCFGStmtMap() @@ -308,6 +310,92 @@ const CFGBlock *ExplodedNode::getCFGBlock() const { return nullptr; } +static const LocationContext * +findTopAutosynthesizedParentContext(const LocationContext *LC) { + assert(LC->getAnalysisDeclContext()->isBodyAutosynthesized()); + const LocationContext *ParentLC = LC->getParent(); + assert(ParentLC && "We don't start analysis from autosynthesized code"); + while (ParentLC->getAnalysisDeclContext()->isBodyAutosynthesized()) { + LC = ParentLC; + ParentLC = LC->getParent(); + assert(ParentLC && "We don't start analysis from autosynthesized code"); + } + return LC; +} + +const Stmt *ExplodedNode::getStmtForDiagnostics() const { + // We cannot place diagnostics on autosynthesized code. + // Put them onto the call site through which we jumped into autosynthesized + // code for the first time. + const LocationContext *LC = getLocationContext(); + if (LC->getAnalysisDeclContext()->isBodyAutosynthesized()) { + // It must be a stack frame because we only autosynthesize functions. + return cast(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()) + return SP->getStmt(); + if (auto BE = P.getAs()) + return BE->getSrc()->getTerminatorStmt(); + if (auto CE = P.getAs()) + return CE->getCallExpr(); + if (auto CEE = P.getAs()) + return CEE->getCalleeContext()->getCallSite(); + if (auto PIPP = P.getAs()) + return PIPP->getInitializer()->getInit(); + if (auto CEB = P.getAs()) + return CEB->getReturnStmt(); + if (auto FEP = P.getAs()) + 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(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, diff --git a/lib/StaticAnalyzer/Core/LoopUnrolling.cpp b/lib/StaticAnalyzer/Core/LoopUnrolling.cpp index 9838249ae8..1a09a521f1 100644 --- a/lib/StaticAnalyzer/Core/LoopUnrolling.cpp +++ b/lib/StaticAnalyzer/Core/LoopUnrolling.cpp @@ -165,7 +165,9 @@ static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) { return true; while (!N->pred_empty()) { - const Stmt *S = PathDiagnosticLocation::getStmt(N); + // FIXME: getStmtForDiagnostics() does nasty things in order to provide + // a valid statement for body farms, do we need this behavior here? + const Stmt *S = N->getStmtForDiagnostics(); if (!S) { N = N->getFirstPred(); continue; diff --git a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp index 94d1c095b8..853029ac7b 100644 --- a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -30,7 +30,6 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/FoldingSet.h" #include "llvm/ADT/None.h" @@ -524,12 +523,12 @@ PathDiagnosticConsumer::FilesMade::getFiles(const PathDiagnostic &PD) { // PathDiagnosticLocation methods. //===----------------------------------------------------------------------===// -static SourceLocation getValidSourceLocation(const Stmt* S, - LocationOrAnalysisDeclContext LAC, - bool UseEnd = false) { - SourceLocation L = UseEnd ? S->getEndLoc() : S->getBeginLoc(); - assert(!LAC.isNull() && "A valid LocationContext or AnalysisDeclContext should " - "be passed to PathDiagnosticLocation upon creation."); +SourceLocation PathDiagnosticLocation::getValidSourceLocation( + const Stmt *S, LocationOrAnalysisDeclContext LAC, bool UseEndOfStatement) { + SourceLocation L = UseEndOfStatement ? S->getEndLoc() : S->getBeginLoc(); + assert(!LAC.isNull() && + "A valid LocationContext or AnalysisDeclContext should be passed to " + "PathDiagnosticLocation upon creation."); // S might be a temporary statement that does not have a location in the // source code, so find an enclosing statement and use its location. @@ -559,7 +558,7 @@ static SourceLocation getValidSourceLocation(const Stmt* S, break; } - L = UseEnd ? Parent->getEndLoc() : Parent->getBeginLoc(); + L = UseEndOfStatement ? Parent->getEndLoc() : Parent->getBeginLoc(); } while (!L.isValid()); } @@ -778,117 +777,6 @@ PathDiagnosticLocation::create(const ProgramPoint& P, return PathDiagnosticLocation(S, SMng, P.getLocationContext()); } -static const LocationContext * -findTopAutosynthesizedParentContext(const LocationContext *LC) { - assert(LC->getAnalysisDeclContext()->isBodyAutosynthesized()); - const LocationContext *ParentLC = LC->getParent(); - assert(ParentLC && "We don't start analysis from autosynthesized code"); - while (ParentLC->getAnalysisDeclContext()->isBodyAutosynthesized()) { - LC = ParentLC; - ParentLC = LC->getParent(); - assert(ParentLC && "We don't start analysis from autosynthesized code"); - } - return LC; -} - -const Stmt *PathDiagnosticLocation::getStmt(const ExplodedNode *N) { - // We cannot place diagnostics on autosynthesized code. - // Put them onto the call site through which we jumped into autosynthesized - // code for the first time. - const LocationContext *LC = N->getLocationContext(); - if (LC->getAnalysisDeclContext()->isBodyAutosynthesized()) { - // It must be a stack frame because we only autosynthesize functions. - return cast(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()) - return SP->getStmt(); - if (auto BE = P.getAs()) - return BE->getSrc()->getTerminatorStmt(); - if (auto CE = P.getAs()) - return CE->getCallExpr(); - if (auto CEE = P.getAs()) - return CEE->getCalleeContext()->getCallSite(); - if (auto PIPP = P.getAs()) - return PIPP->getInitializer()->getInit(); - if (auto CEB = P.getAs()) - return CEB->getReturnStmt(); - if (auto FEP = P.getAs()) - 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(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 PIE = N->getLocationAs()) - return PathDiagnosticLocation(PIE->getLocation(), SM); - if (auto FE = N->getLocationAs()) { - 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(S)) - return PathDiagnosticLocation::createMemberLoc(ME, SM); - - // For binary operators, return the location of the operator. - if (const auto *B = dyn_cast(S)) - return PathDiagnosticLocation::createOperatorLoc(B, SM); - - if (P.getAs()) - 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(); -- 2.50.1