diff options
6 files changed, 74 insertions, 37 deletions
diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h index 73958a6ef48..2e7abfa5cc3 100644 --- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h +++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h @@ -350,10 +350,17 @@ public: }; -class PathPieces : - public std::deque<IntrusiveRefCntPtr<PathDiagnosticPiece> > { +class PathPieces : public std::deque<IntrusiveRefCntPtr<PathDiagnosticPiece> > { + void flattenTo(PathPieces &Primary, PathPieces &Current, + bool ShouldFlattenMacros) const; public: - ~PathPieces(); + ~PathPieces(); + + PathPieces flatten(bool ShouldFlattenMacros) const { + PathPieces Result; + flattenTo(Result, Result, ShouldFlattenMacros); + return Result; + } }; class PathDiagnosticSpotPiece : public PathDiagnosticPiece { diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index ded341caf9f..3dedcb3edf0 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -707,6 +707,9 @@ ConditionBRVisitor::VisitConditionVariable(StringRef LhsString, BugReporterContext &BRC, BugReport &report, const ExplodedNode *N) { + // FIXME: If there's already a constraint tracker for this variable, + // we shouldn't emit anything here (c.f. the double note in + // test/Analysis/inlining/path-notes.c) SmallString<256> buf; llvm::raw_svector_ostream Out(buf); Out << "Assuming " << LhsString << " is "; diff --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp index f6e8dccdfb7..0152e328e50 100644 --- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -95,37 +95,6 @@ void HTMLDiagnostics::FlushDiagnosticsImpl( } } -static void flattenPath(PathPieces &primaryPath, PathPieces ¤tPath, - const PathPieces &oldPath) { - for (PathPieces::const_iterator it = oldPath.begin(), et = oldPath.end(); - it != et; ++it ) { - PathDiagnosticPiece *piece = it->getPtr(); - if (const PathDiagnosticCallPiece *call = - dyn_cast<PathDiagnosticCallPiece>(piece)) { - IntrusiveRefCntPtr<PathDiagnosticEventPiece> callEnter = - call->getCallEnterEvent(); - if (callEnter) - currentPath.push_back(callEnter); - flattenPath(primaryPath, primaryPath, call->path); - IntrusiveRefCntPtr<PathDiagnosticEventPiece> callExit = - call->getCallExitEvent(); - if (callExit) - currentPath.push_back(callExit); - continue; - } - if (PathDiagnosticMacroPiece *macro = - dyn_cast<PathDiagnosticMacroPiece>(piece)) { - currentPath.push_back(piece); - PathPieces newPath; - flattenPath(primaryPath, newPath, macro->subPieces); - macro->subPieces = newPath; - continue; - } - - currentPath.push_back(piece); - } -} - void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D, SmallVectorImpl<std::string> *FilesMade) { @@ -152,8 +121,7 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D, return; // First flatten out the entire path to make it easier to use. - PathPieces path; - flattenPath(path, path, D.path); + PathPieces path = D.path.flatten(/*ShouldFlattenMacros=*/false); // The path as already been prechecked that all parts of the path are // from the same file and that it is non-empty. diff --git a/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp index c482d6796c0..7d52aac71c2 100644 --- a/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ b/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -59,6 +59,48 @@ PathDiagnosticMacroPiece::~PathDiagnosticMacroPiece() {} PathPieces::~PathPieces() {} + +void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current, + bool ShouldFlattenMacros) const { + for (PathPieces::const_iterator I = begin(), E = end(); I != E; ++I) { + PathDiagnosticPiece *Piece = I->getPtr(); + + switch (Piece->getKind()) { + case PathDiagnosticPiece::Call: { + PathDiagnosticCallPiece *Call = cast<PathDiagnosticCallPiece>(Piece); + IntrusiveRefCntPtr<PathDiagnosticEventPiece> CallEnter = + Call->getCallEnterEvent(); + if (CallEnter) + Current.push_back(CallEnter); + Call->path.flattenTo(Primary, Primary, ShouldFlattenMacros); + IntrusiveRefCntPtr<PathDiagnosticEventPiece> callExit = + Call->getCallExitEvent(); + if (callExit) + Current.push_back(callExit); + break; + } + case PathDiagnosticPiece::Macro: { + PathDiagnosticMacroPiece *Macro = cast<PathDiagnosticMacroPiece>(Piece); + if (ShouldFlattenMacros) { + Macro->subPieces.flattenTo(Primary, Primary, ShouldFlattenMacros); + } else { + Current.push_back(Piece); + PathPieces NewPath; + Macro->subPieces.flattenTo(Primary, NewPath, ShouldFlattenMacros); + // FIXME: This probably shouldn't mutate the original path piece. + Macro->subPieces = NewPath; + } + break; + } + case PathDiagnosticPiece::Event: + case PathDiagnosticPiece::ControlFlow: + Current.push_back(Piece); + break; + } + } +} + + PathDiagnostic::~PathDiagnostic() {} PathDiagnostic::PathDiagnostic(const Decl *declWithIssue, diff --git a/clang/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp index fe912dfd21f..e5b8553aeda 100644 --- a/clang/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp @@ -42,6 +42,7 @@ public: bool supportsLogicalOpControlFlow() const { return true; } bool supportsAllBlockEdges() const { return true; } virtual bool useVerboseDescription() const { return true; } + virtual bool supportsCrossFileDiagnostics() const { return true; } }; } // end anonymous namespace @@ -58,7 +59,9 @@ void TextPathDiagnostics::FlushDiagnosticsImpl( for (std::vector<const PathDiagnostic *>::iterator it = Diags.begin(), et = Diags.end(); it != et; ++it) { const PathDiagnostic *D = *it; - for (PathPieces::const_iterator I = D->path.begin(), E = D->path.end(); + + PathPieces FlatPath = D->path.flatten(/*ShouldFlattenMacros=*/true); + for (PathPieces::const_iterator I = FlatPath.begin(), E = FlatPath.end(); I != E; ++I) { unsigned diagID = Diag.getDiagnosticIDs()->getCustomDiagID(DiagnosticIDs::Note, diff --git a/clang/test/Analysis/inlining/path-notes.c b/clang/test/Analysis/inlining/path-notes.c new file mode 100644 index 00000000000..1db3c5aab8d --- /dev/null +++ b/clang/test/Analysis/inlining/path-notes.c @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-ipa=inlining -analyzer-output=text -verify %s + +void zero(int **p) { + *p = 0; + // expected-note@-1 {{Null pointer value stored to 'a'}} +} + +void testZero(int *a) { + zero(&a); + // expected-note@-1 {{Calling 'zero'}} + // expected-note@-2 {{Returning from 'zero'}} + *a = 1; // expected-warning{{Dereference of null pointer}} + // expected-note@-1 {{Dereference of null pointer (loaded from variable 'a')}} +}
\ No newline at end of file |
