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.

llvm-svn: 82198
This commit is contained in:
Ted Kremenek 2009-09-18 05:37:41 +00:00
parent 01ce06fbbc
commit 82f7f9c080
5 changed files with 150 additions and 25 deletions

View File

@ -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>
@ -25,11 +26,16 @@
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 @@ class PathDiagnosticClient : public DiagnosticClient {
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 @@ public:
void flatten();
const SourceManager& getManager() const { assert(isValid()); return *SM; }
void Profile(llvm::FoldingSetNodeID &ID) const;
};
class PathDiagnosticLocationPair {
@ -142,6 +147,11 @@ public:
Start.flatten();
End.flatten();
}
void Profile(llvm::FoldingSetNodeID &ID) const {
Start.Profile(ID);
End.Profile(ID);
}
};
//===----------------------------------------------------------------------===//
@ -220,6 +230,8 @@ public:
static inline bool classof(const PathDiagnosticPiece* P) {
return true;
}
virtual void Profile(llvm::FoldingSetNodeID &ID) const;
};
class PathDiagnosticSpotPiece : public PathDiagnosticPiece {
@ -238,6 +250,8 @@ public:
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 @@ public:
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 @@ public:
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 @@ public:
}
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 @@ public:
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 @@ public:
void flattenLocations() {
for (iterator I = begin(), E = end(); I != E; ++I) I->flattenLocations();
}
void Profile(llvm::FoldingSetNodeID &ID) const;
};
} //end clang namespace
#endif

View File

@ -1732,6 +1732,50 @@ static BugReport *FindReportInEquivalenceClass(BugReportEquivClass& EQ) {
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 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) {
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)

View File

@ -239,4 +239,66 @@ void PathDiagnosticLocation::flatten() {
}
}
//===----------------------------------------------------------------------===//
// 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);
}

View File

@ -293,8 +293,7 @@ static void ActionWarnUninitVals(AnalysisManager& mgr, Decl *D) {
static void ActionGRExprEngine(AnalysisManager& mgr, Decl *D,
GRTransferFuncs* tf,
bool StandardWarnings = true) {
GRTransferFuncs* tf) {
llvm::OwningPtr<GRTransferFuncs> TF(tf);
@ -303,17 +302,13 @@ static void ActionGRExprEngine(AnalysisManager& mgr, Decl *D,
mgr.DisplayFunction(D);
// Construct the analysis engine.
LiveVariables* L = mgr.getLiveVariables(D);
if (!L) return;
GRExprEngine Eng(mgr);
Eng.setTransferFunctions(tf);
if (StandardWarnings) {
Eng.RegisterInternalChecks();
Eng.RegisterInternalChecks(); // FIXME: Internal checks should just
// automatically register.
RegisterAppleChecks(Eng, *D);
}
// Set the graph auditor.
llvm::OwningPtr<ExplodedNode::Auditor> Auditor;
@ -338,13 +333,13 @@ static void ActionGRExprEngine(AnalysisManager& mgr, Decl *D,
}
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 @@ static void ActionCheckerCFRef(AnalysisManager& mgr, Decl *D) {
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;
}
}

View File

@ -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