[cfe-commits] r82198 - in /cfe/trunk: include/clang/Analysis/PathDiagnostic.h lib/Analysis/BugReporter.cpp lib/Analysis/PathDiagnostic.cpp lib/Frontend/AnalysisConsumer.cpp test/Analysis/misc-ps.m
Ted Kremenek
kremenek at apple.com
Thu Sep 17 22:37:41 PDT 2009
Author: kremenek
Date: Fri Sep 18 00:37:41 2009
New Revision: 82198
URL: http://llvm.org/viewvc/llvm-project?rev=82198&view=rev
Log:
Introduce caching of diagnostics in BugReporter. This provides extra
pruning of diagnostics that may be emitted multiple times. This is
accomplished by adding FoldingSet profiling support to PathDiagnostic,
and then having BugReporter record what diagnostics have been issued.
This was motived to a serious bug introduced by moving the
'divide-by-zero' checking outside of GRExprEngine into a separate
'Checker' class. When analyzing code using the '-fobjc-gc' option, a
given function would be analyzed twice, but the second time various
"internal checks" would be disabled to avoid emitting multiple
diagnostics (e.g., "null dereference") for the same issue. The
problem is that such checks also effect path pruning and don't just
emit diagnostics. This resulted in an assertion failure involving a
real divide-by-zero in some analyzed code where we would get an
assertion failure in APInt because the 'DivZero' check was disabled
and didn't prune the logic that resulted in the divide-by-zero in the
analyzer.
The implemented solution is somewhat of a hack, and may not perform
extremely well. This will need to be cleaned up over time.
As a regression test, 'misc-ps.m' has been modified so that its tests
are run using -fobjc-gc to test this diagnostic pruning behavior.
Modified:
cfe/trunk/include/clang/Analysis/PathDiagnostic.h
cfe/trunk/lib/Analysis/BugReporter.cpp
cfe/trunk/lib/Analysis/PathDiagnostic.cpp
cfe/trunk/lib/Frontend/AnalysisConsumer.cpp
cfe/trunk/test/Analysis/misc-ps.m
Modified: cfe/trunk/include/clang/Analysis/PathDiagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathDiagnostic.h?rev=82198&r1=82197&r2=82198&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathDiagnostic.h (original)
+++ cfe/trunk/include/clang/Analysis/PathDiagnostic.h Fri Sep 18 00:37:41 2009
@@ -17,6 +17,7 @@
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/Diagnostic.h"
#include "llvm/ADT/OwningPtr.h"
+#include "llvm/ADT/FoldingSet.h"
#include <vector>
#include <deque>
@@ -24,12 +25,17 @@
#include <algorithm>
namespace clang {
-
+
+class Stmt;
+class Decl;
+class Preprocessor;
+
//===----------------------------------------------------------------------===//
// High-level interface for handlers of path-sensitive diagnostics.
//===----------------------------------------------------------------------===//
class PathDiagnostic;
+
class Stmt;
class Decl;
class Preprocessor;
@@ -38,12 +44,9 @@
public:
PathDiagnosticClient() {}
virtual ~PathDiagnosticClient() {}
-
virtual void SetPreprocessor(Preprocessor *PP) {}
-
virtual void HandleDiagnostic(Diagnostic::Level DiagLevel,
const DiagnosticInfo &Info);
-
virtual void HandlePathDiagnostic(const PathDiagnostic* D) = 0;
enum PathGenerationScheme { Minimal, Extensive };
@@ -125,6 +128,8 @@
void flatten();
const SourceManager& getManager() const { assert(isValid()); return *SM; }
+
+ void Profile(llvm::FoldingSetNodeID &ID) const;
};
class PathDiagnosticLocationPair {
@@ -142,6 +147,11 @@
Start.flatten();
End.flatten();
}
+
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ Start.Profile(ID);
+ End.Profile(ID);
+ }
};
//===----------------------------------------------------------------------===//
@@ -220,6 +230,8 @@
static inline bool classof(const PathDiagnosticPiece* P) {
return true;
}
+
+ virtual void Profile(llvm::FoldingSetNodeID &ID) const;
};
class PathDiagnosticSpotPiece : public PathDiagnosticPiece {
@@ -238,6 +250,8 @@
PathDiagnosticLocation getLocation() const { return Pos; }
virtual void flattenLocations() { Pos.flatten(); }
+
+ virtual void Profile(llvm::FoldingSetNodeID &ID) const;
};
class PathDiagnosticEventPiece : public PathDiagnosticSpotPiece {
@@ -317,6 +331,8 @@
static inline bool classof(const PathDiagnosticPiece* P) {
return P->getKind() == ControlFlow;
}
+
+ virtual void Profile(llvm::FoldingSetNodeID &ID) const;
};
class PathDiagnosticMacroPiece : public PathDiagnosticSpotPiece {
@@ -347,12 +363,14 @@
static inline bool classof(const PathDiagnosticPiece* P) {
return P->getKind() == Macro;
}
+
+ virtual void Profile(llvm::FoldingSetNodeID &ID) const;
};
/// PathDiagnostic - PathDiagnostic objects represent a single path-sensitive
/// diagnostic. It represents an ordered-collection of PathDiagnosticPieces,
/// each which represent the pieces of the path.
-class PathDiagnostic {
+class PathDiagnostic : public llvm::FoldingSetNode {
std::deque<PathDiagnosticPiece*> path;
unsigned Size;
std::string BugType;
@@ -386,11 +404,13 @@
}
void push_front(PathDiagnosticPiece* piece) {
+ assert(piece);
path.push_front(piece);
++Size;
}
void push_back(PathDiagnosticPiece* piece) {
+ assert(piece);
path.push_back(piece);
++Size;
}
@@ -453,7 +473,7 @@
bool operator==(const const_iterator& X) const { return I == X.I; }
bool operator!=(const const_iterator& X) const { return I != X.I; }
- reference operator*() const { return **I; }
+ reference operator*() const { assert(*I); return **I; }
pointer operator->() const { return *I; }
const_iterator& operator++() { ++I; return *this; }
@@ -480,8 +500,8 @@
void flattenLocations() {
for (iterator I = begin(), E = end(); I != E; ++I) I->flattenLocations();
}
-};
-
-
+
+ void Profile(llvm::FoldingSetNodeID &ID) const;
+};
} //end clang namespace
#endif
Modified: cfe/trunk/lib/Analysis/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BugReporter.cpp?rev=82198&r1=82197&r2=82198&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BugReporter.cpp (original)
+++ cfe/trunk/lib/Analysis/BugReporter.cpp Fri Sep 18 00:37:41 2009
@@ -1732,6 +1732,50 @@
return NULL;
}
+
+//===----------------------------------------------------------------------===//
+// DiagnosticCache. This is a hack to cache analyzer diagnostics. It
+// uses global state, which eventually should go elsewhere.
+//===----------------------------------------------------------------------===//
+namespace {
+class VISIBILITY_HIDDEN DiagCacheItem : public llvm::FoldingSetNode {
+ llvm::FoldingSetNodeID ID;
+public:
+ DiagCacheItem(BugReport *R, PathDiagnostic *PD) {
+ ID.AddString(R->getBugType().getName());
+ ID.AddString(R->getBugType().getCategory());
+ ID.AddString(R->getDescription());
+ ID.AddInteger(R->getLocation().getRawEncoding());
+ PD->Profile(ID);
+ }
+
+ void Profile(llvm::FoldingSetNodeID &id) {
+ id = ID;
+ }
+
+ llvm::FoldingSetNodeID &getID() { return ID; }
+};
+}
+
+static bool IsCachedDiagnostic(BugReport *R, PathDiagnostic *PD) {
+ // FIXME: Eventually this diagnostic cache should reside in something
+ // like AnalysisManager instead of being a static variable. This is
+ // really unsafe in the long term.
+ typedef llvm::FoldingSet<DiagCacheItem> DiagnosticCache;
+ static DiagnosticCache DC;
+
+ void *InsertPos;
+ DiagCacheItem *Item = new DiagCacheItem(R, PD);
+
+ if (DC.FindNodeOrInsertPos(Item->getID(), InsertPos)) {
+ delete Item;
+ return true;
+ }
+
+ DC.InsertNode(Item, InsertPos);
+ return false;
+}
+
void BugReporter::FlushReport(BugReportEquivClass& EQ) {
BugReport *R = FindReportInEquivalenceClass(EQ);
@@ -1752,6 +1796,9 @@
GeneratePathDiagnostic(*D.get(), EQ);
+ if (IsCachedDiagnostic(R, D.get()))
+ return;
+
// Get the meta data.
std::pair<const char**, const char**> Meta = R->getExtraDescriptiveText();
for (const char** s = Meta.first; s != Meta.second; ++s)
Modified: cfe/trunk/lib/Analysis/PathDiagnostic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PathDiagnostic.cpp?rev=82198&r1=82197&r2=82198&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/Analysis/PathDiagnostic.cpp Fri Sep 18 00:37:41 2009
@@ -239,4 +239,66 @@
}
}
+//===----------------------------------------------------------------------===//
+// FoldingSet profiling methods.
+//===----------------------------------------------------------------------===//
+void PathDiagnosticLocation::Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddInteger((unsigned) K);
+ switch (K) {
+ case RangeK:
+ ID.AddInteger(R.getBegin().getRawEncoding());
+ ID.AddInteger(R.getEnd().getRawEncoding());
+ break;
+ case SingleLocK:
+ ID.AddInteger(R.getBegin().getRawEncoding());
+ break;
+ case StmtK:
+ ID.Add(S);
+ break;
+ case DeclK:
+ ID.Add(D);
+ break;
+ }
+ return;
+}
+
+void PathDiagnosticPiece::Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddInteger((unsigned) getKind());
+ ID.AddString(str);
+ // FIXME: Add profiling support for code hints.
+ ID.AddInteger((unsigned) getDisplayHint());
+ for (range_iterator I = ranges_begin(), E = ranges_end(); I != E; ++I) {
+ ID.AddInteger(I->getBegin().getRawEncoding());
+ ID.AddInteger(I->getEnd().getRawEncoding());
+ }
+}
+
+void PathDiagnosticSpotPiece::Profile(llvm::FoldingSetNodeID &ID) const {
+ PathDiagnosticPiece::Profile(ID);
+ ID.Add(Pos);
+}
+
+void PathDiagnosticControlFlowPiece::Profile(llvm::FoldingSetNodeID &ID) const {
+ PathDiagnosticPiece::Profile(ID);
+ for (const_iterator I = begin(), E = end(); I != E; ++I)
+ ID.Add(*I);
+}
+
+void PathDiagnosticMacroPiece::Profile(llvm::FoldingSetNodeID &ID) const {
+ PathDiagnosticSpotPiece::Profile(ID);
+ for (const_iterator I = begin(), E = end(); I != E; ++I)
+ ID.Add(**I);
+}
+
+void PathDiagnostic::Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddInteger(Size);
+ ID.AddString(BugType);
+ ID.AddString(Desc);
+ ID.AddString(Category);
+ for (const_iterator I = begin(), E = end(); I != E; ++I)
+ ID.Add(*I);
+
+ for (meta_iterator I = meta_begin(), E = meta_end(); I != E; ++I)
+ ID.AddString(*I);
+}
Modified: cfe/trunk/lib/Frontend/AnalysisConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/AnalysisConsumer.cpp?rev=82198&r1=82197&r2=82198&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/AnalysisConsumer.cpp (original)
+++ cfe/trunk/lib/Frontend/AnalysisConsumer.cpp Fri Sep 18 00:37:41 2009
@@ -293,8 +293,7 @@
static void ActionGRExprEngine(AnalysisManager& mgr, Decl *D,
- GRTransferFuncs* tf,
- bool StandardWarnings = true) {
+ GRTransferFuncs* tf) {
llvm::OwningPtr<GRTransferFuncs> TF(tf);
@@ -303,17 +302,13 @@
mgr.DisplayFunction(D);
// Construct the analysis engine.
- LiveVariables* L = mgr.getLiveVariables(D);
- if (!L) return;
-
GRExprEngine Eng(mgr);
Eng.setTransferFunctions(tf);
+ Eng.RegisterInternalChecks(); // FIXME: Internal checks should just
+ // automatically register.
+ RegisterAppleChecks(Eng, *D);
- if (StandardWarnings) {
- Eng.RegisterInternalChecks();
- RegisterAppleChecks(Eng, *D);
- }
// Set the graph auditor.
llvm::OwningPtr<ExplodedNode::Auditor> Auditor;
@@ -338,13 +333,13 @@
}
static void ActionCheckerCFRefAux(AnalysisManager& mgr, Decl *D,
- bool GCEnabled, bool StandardWarnings) {
+ bool GCEnabled) {
GRTransferFuncs* TF = MakeCFRefCountTF(mgr.getASTContext(),
GCEnabled,
mgr.getLangOptions());
- ActionGRExprEngine(mgr, D, TF, StandardWarnings);
+ ActionGRExprEngine(mgr, D, TF);
}
static void ActionCheckerCFRef(AnalysisManager& mgr, Decl *D) {
@@ -353,16 +348,16 @@
default:
assert (false && "Invalid GC mode.");
case LangOptions::NonGC:
- ActionCheckerCFRefAux(mgr, D, false, true);
+ ActionCheckerCFRefAux(mgr, D, false);
break;
case LangOptions::GCOnly:
- ActionCheckerCFRefAux(mgr, D, true, true);
+ ActionCheckerCFRefAux(mgr, D, true);
break;
case LangOptions::HybridGC:
- ActionCheckerCFRefAux(mgr, D, false, true);
- ActionCheckerCFRefAux(mgr, D, true, false);
+ ActionCheckerCFRefAux(mgr, D, false);
+ ActionCheckerCFRefAux(mgr, D, true);
break;
}
}
Modified: cfe/trunk/test/Analysis/misc-ps.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps.m?rev=82198&r1=82197&r2=82198&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/misc-ps.m (original)
+++ cfe/trunk/test/Analysis/misc-ps.m Fri Sep 18 00:37:41 2009
@@ -1,4 +1,5 @@
-// RUN: clang-cc -analyze -checker-cfref --analyzer-store=basic -analyzer-constraints=basic --verify -fblocks %s &&
+// NOTE: Use '-fobjc-gc' to test the analysis being run twice, and multiple reports are not issued.
+// RUN: clang-cc -analyze -checker-cfref --analyzer-store=basic -fobjc-gc -analyzer-constraints=basic --verify -fblocks %s &&
// RUN: clang-cc -analyze -checker-cfref --analyzer-store=basic -analyzer-constraints=range --verify -fblocks %s &&
// RUN: clang-cc -analyze -checker-cfref --analyzer-store=region -analyzer-constraints=basic --verify -fblocks %s &&
// RUN: clang-cc -analyze -checker-cfref --analyzer-store=region -analyzer-constraints=range --verify -fblocks %s
More information about the cfe-commits
mailing list