NoQ created this revision.
NoQ added reviewers: dcoughlin, george.karpenkov.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho, a.sidorin,
szepet, baloghadamsoftware, xazax.hun.
The checker was trying to produce a separate `ProgramPointTag` for every dead
symbol it cleans up, so that to potentially be able to produce a separate
`ExplodedNode` for every symbol, and for that purpose it was dumping the symbol
into a string stream.
We don't need a separate node for every symbol, because whenever the first
symbol leaks, a bug is emitted, the analysis is sinked, and the checker
callback immediately returns due to `State` variable turning into null, so we
never get to see the second leaking symbol.
Then, even if we needed a separate node for every symbol, it'd be much better
to print the raw pointer than pretty-print the symbol, because pointers are
more unique. Additionally, we are no longer able to break normal analysis while
experimenting with debug dumps, which is exactly how this behavior got noticed
- we noticed a crash similar to https://reviews.llvm.org/D52756 on a buildbot
that definitely doesn't dump exploded graphs.
Because https://reviews.llvm.org/D52756 has already landed, this patch doesn't
change the actual behavior of the Analyzer, just prevents future bugs.
Repository:
rC Clang
https://reviews.llvm.org/D52804
Files:
lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1318,19 +1318,6 @@
processLeaks(state, Leaked, Ctx, Pred);
}
-const ProgramPointTag *
-RetainCountChecker::getDeadSymbolTag(SymbolRef sym) const {
- const CheckerProgramPointTag *&tag = DeadSymbolTags[sym];
- if (!tag) {
-SmallString<64> buf;
-llvm::raw_svector_ostream out(buf);
-out << "Dead Symbol : ";
-sym->dumpToStream(out);
-tag = new CheckerProgramPointTag(this, out.str());
- }
- return tag;
-}
-
void RetainCountChecker::checkDeadSymbols(SymbolReaper &SymReaper,
CheckerContext &C) const {
ExplodedNode *Pred = C.getPredecessor();
@@ -1346,8 +1333,8 @@
if (const RefVal *T = B.lookup(Sym)){
// Use the symbol as the tag.
// FIXME: This might not be as unique as we would like.
- const ProgramPointTag *Tag = getDeadSymbolTag(Sym);
- state = handleAutoreleaseCounts(state, Pred, Tag, C, Sym, *T);
+ static CheckerProgramPointTag Tag(this, "DeadSymbolAutorelease");
+ state = handleAutoreleaseCounts(state, Pred, &Tag, C, Sym, *T);
if (!state)
return;
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1318,19 +1318,6 @@
processLeaks(state, Leaked, Ctx, Pred);
}
-const ProgramPointTag *
-RetainCountChecker::getDeadSymbolTag(SymbolRef sym) const {
- const CheckerProgramPointTag *&tag = DeadSymbolTags[sym];
- if (!tag) {
-SmallString<64> buf;
-llvm::raw_svector_ostream out(buf);
-out << "Dead Symbol : ";
-sym->dumpToStream(out);
-tag = new CheckerProgramPointTag(this, out.str());
- }
- return tag;
-}
-
void RetainCountChecker::checkDeadSymbols(SymbolReaper &SymReaper,
CheckerContext &C) const {
ExplodedNode *Pred = C.getPredecessor();
@@ -1346,8 +1333,8 @@
if (const RefVal *T = B.lookup(Sym)){
// Use the symbol as the tag.
// FIXME: This might not be as unique as we would like.
- const ProgramPointTag *Tag = getDeadSymbolTag(Sym);
- state = handleAutoreleaseCounts(state, Pred, Tag, C, Sym, *T);
+ static CheckerProgramPointTag Tag(this, "DeadSymbolAutorelease");
+ state = handleAutoreleaseCounts(state, Pred, &Tag, C, Sym, *T);
if (!state)
return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits