[cfe-commits] r126272 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h include/clang/StaticAnalyzer/Core/BugReporter/BugType.h lib/StaticAnalyzer/Checkers/ExprEngine.cpp lib/StaticAnalyzer/Core/BugReporter.cpp
Argyrios Kyrtzidis
akyrtzi at gmail.com
Tue Feb 22 16:16:01 PST 2011
Author: akirtzidis
Date: Tue Feb 22 18:16:01 2011
New Revision: 126272
URL: http://llvm.org/viewvc/llvm-project?rev=126272&view=rev
Log:
[analyzer] Refactor BugTypes and their ownership model.
-In general, don't have the BugReporter deleting BugTypes, BugTypes will eventually become owned by checkers
and outlive the BugReporter. In the meantime, there will be some leaks since some checkers assume that
the BugTypes they create will be destroyed by the BugReporter.
-Have BugReporter::EmitBasicReport create BugTypes that are reused if the same name & category strings
are passed to EmitBasicReport. These BugTypes are owned and destroyed by the BugReporter.
This allows bugs reported through EmitBasicReport to be coalesced.
-Remove the llvm::FoldingSet<BugReportEquivClass> from BugType and move it into the BugReporter.
For uniquing BugReportEquivClass also use the BugType* so that we can iterate over all of them using only one set.
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
cfe/trunk/lib/StaticAnalyzer/Checkers/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=126272&r1=126271&r2=126272&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Tue Feb 22 18:16:01 2011
@@ -72,6 +72,7 @@
friend class BugReportEquivClass;
virtual void Profile(llvm::FoldingSetNodeID& hash) const {
+ hash.AddPointer(&BT);
hash.AddInteger(getLocation().getRawEncoding());
hash.AddString(Description);
}
@@ -277,6 +278,8 @@
void FlushReport(BugReportEquivClass& EQ);
+ llvm::FoldingSet<BugReportEquivClass> EQClasses;
+
protected:
BugReporter(BugReporterData& d, Kind k) : BugTypes(F.getEmptySet()), kind(k),
D(d) {}
@@ -302,6 +305,10 @@
iterator begin() { return BugTypes.begin(); }
iterator end() { return BugTypes.end(); }
+ typedef llvm::FoldingSet<BugReportEquivClass>::iterator EQClasses_iterator;
+ EQClasses_iterator EQClasses_begin() { return EQClasses.begin(); }
+ EQClasses_iterator EQClasses_end() { return EQClasses.end(); }
+
ASTContext& getContext() { return D.getASTContext(); }
SourceManager& getSourceManager() { return D.getSourceManager(); }
@@ -344,6 +351,13 @@
}
static bool classof(const BugReporter* R) { return true; }
+
+private:
+ llvm::StringMap<BugType *> StrBugTypes;
+
+ /// \brief Returns a BugType that is associated with the given name and
+ /// category.
+ BugType *getBugTypeForName(llvm::StringRef name, llvm::StringRef category);
};
// FIXME: Get rid of GRBugReporter. It's the wrong abstraction.
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h?rev=126272&r1=126271&r2=126272&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h Tue Feb 22 18:16:01 2011
@@ -29,8 +29,6 @@
private:
const std::string Name;
const std::string Category;
- llvm::FoldingSet<BugReportEquivClass> EQClasses;
- friend class BugReporter;
bool SuppressonSink;
public:
BugType(llvm::StringRef name, llvm::StringRef cat)
@@ -48,14 +46,6 @@
void setSuppressOnSink(bool x) { SuppressonSink = x; }
virtual void FlushReports(BugReporter& BR);
-
- typedef llvm::FoldingSet<BugReportEquivClass>::iterator iterator;
- iterator begin() { return EQClasses.begin(); }
- iterator end() { return EQClasses.end(); }
-
- typedef llvm::FoldingSet<BugReportEquivClass>::const_iterator const_iterator;
- const_iterator begin() const { return EQClasses.begin(); }
- const_iterator end() const { return EQClasses.end(); }
};
class BuiltinBug : public BugType {
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ExprEngine.cpp?rev=126272&r1=126271&r2=126272&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ExprEngine.cpp Tue Feb 22 18:16:01 2011
@@ -3630,14 +3630,12 @@
const_cast<BugType*>(*I)->FlushReports(BR);
// Iterate through the reports and get their nodes.
- for (BugReporter::iterator I=BR.begin(), E=BR.end(); I!=E; ++I) {
- for (BugType::const_iterator I2=(*I)->begin(), E2=(*I)->end();
- I2!=E2; ++I2) {
- const BugReportEquivClass& EQ = *I2;
- const BugReport &R = **EQ.begin();
- ExplodedNode *N = const_cast<ExplodedNode*>(R.getErrorNode());
- if (N) Src.push_back(N);
- }
+ for (BugReporter::EQClasses_iterator
+ EI = BR.EQClasses_begin(), EE = BR.EQClasses_end(); EI != EE; ++EI) {
+ BugReportEquivClass& EQ = *EI;
+ const BugReport &R = **EQ.begin();
+ ExplodedNode *N = const_cast<ExplodedNode*>(R.getErrorNode());
+ if (N) Src.push_back(N);
}
ViewGraph(&Src[0], &Src[0]+Src.size());
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=126272&r1=126271&r2=126272&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Tue Feb 22 18:16:01 2011
@@ -1202,16 +1202,8 @@
//===----------------------------------------------------------------------===//
// Methods for BugType and subclasses.
//===----------------------------------------------------------------------===//
-BugType::~BugType() {
- // Free up the equivalence class objects. Observe that we get a pointer to
- // the object first before incrementing the iterator, as destroying the
- // node before doing so means we will read from freed memory.
- for (iterator I = begin(), E = end(); I !=E; ) {
- BugReportEquivClass *EQ = &*I;
- ++I;
- delete EQ;
- }
-}
+BugType::~BugType() { }
+
void BugType::FlushReports(BugReporter &BR) {}
//===----------------------------------------------------------------------===//
@@ -1315,28 +1307,30 @@
return;
// First flush the warnings for each BugType. This may end up creating new
- // warnings and new BugTypes. Because ImmutableSet is a functional data
- // structure, we do not need to worry about the iterators being invalidated.
+ // warnings and new BugTypes.
+ // FIXME: Only NSErrorChecker needs BugType's FlushReports.
+ // Turn NSErrorChecker into a proper checker and remove this.
+ llvm::SmallVector<const BugType*, 16> bugTypes;
for (BugTypesTy::iterator I=BugTypes.begin(), E=BugTypes.end(); I!=E; ++I)
+ bugTypes.push_back(*I);
+ for (llvm::SmallVector<const BugType*, 16>::iterator
+ I = bugTypes.begin(), E = bugTypes.end(); I != E; ++I)
const_cast<BugType*>(*I)->FlushReports(*this);
- // Iterate through BugTypes a second time. BugTypes may have been updated
- // with new BugType objects and new warnings.
- for (BugTypesTy::iterator I=BugTypes.begin(), E=BugTypes.end(); I!=E; ++I) {
- BugType *BT = const_cast<BugType*>(*I);
-
- typedef llvm::FoldingSet<BugReportEquivClass> SetTy;
- SetTy& EQClasses = BT->EQClasses;
-
- for (SetTy::iterator EI=EQClasses.begin(), EE=EQClasses.end(); EI!=EE;++EI){
- BugReportEquivClass& EQ = *EI;
- FlushReport(EQ);
- }
-
- // Delete the BugType object.
- delete BT;
+ typedef llvm::FoldingSet<BugReportEquivClass> SetTy;
+ for (SetTy::iterator EI=EQClasses.begin(), EE=EQClasses.end(); EI!=EE;++EI){
+ BugReportEquivClass& EQ = *EI;
+ FlushReport(EQ);
}
+ // BugReporter owns and deletes only BugTypes created implicitly through
+ // EmitBasicReport.
+ // FIXME: There are leaks from checkers that assume that the BugTypes they
+ // create will be destroyed by the BugReporter.
+ for (llvm::StringMap<BugType*>::iterator
+ I = StrBugTypes.begin(), E = StrBugTypes.end(); I != E; ++I)
+ delete I->second;
+
// Remove all references to the BugType objects.
BugTypes = F.getEmptySet();
}
@@ -1632,11 +1626,11 @@
BugType& BT = R->getBugType();
Register(&BT);
void *InsertPos;
- BugReportEquivClass* EQ = BT.EQClasses.FindNodeOrInsertPos(ID, InsertPos);
+ BugReportEquivClass* EQ = EQClasses.FindNodeOrInsertPos(ID, InsertPos);
if (!EQ) {
EQ = new BugReportEquivClass(R);
- BT.EQClasses.InsertNode(EQ, InsertPos);
+ EQClasses.InsertNode(EQ, InsertPos);
}
else
EQ->AddReport(R);
@@ -1887,10 +1881,24 @@
llvm::StringRef str, SourceLocation Loc,
SourceRange* RBeg, unsigned NumRanges) {
- // 'BT' will be owned by BugReporter as soon as we call 'EmitReport'.
- BugType *BT = new BugType(name, category);
+ // 'BT' is owned by BugReporter.
+ BugType *BT = getBugTypeForName(name, category);
FullSourceLoc L = getContext().getFullLoc(Loc);
RangedBugReport *R = new DiagBugReport(*BT, str, L);
for ( ; NumRanges > 0 ; --NumRanges, ++RBeg) R->addRange(*RBeg);
EmitReport(R);
}
+
+BugType *BugReporter::getBugTypeForName(llvm::StringRef name,
+ llvm::StringRef category) {
+ llvm::SmallString<136> fullDesc;
+ llvm::raw_svector_ostream(fullDesc) << name << ":" << category;
+ llvm::StringMapEntry<BugType *> &
+ entry = StrBugTypes.GetOrCreateValue(fullDesc);
+ BugType *BT = entry.getValue();
+ if (!BT) {
+ BT = new BugType(name, category);
+ entry.setValue(BT);
+ }
+ return BT;
+}
More information about the cfe-commits
mailing list