[PATCH] D52804: [analyzer] NFC: RetainCountChecker: Avoid dumping symbols during normal operation.

2018-10-15 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC344538: [analyzer] NFC: RetainCountChecker: Dont 
dump() symbols into program point… (authored by dergachev, committed by ).
Herald added a subscriber: donat.nagy.

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
@@ -1314,19 +1314,6 @@
   processLeaks(state, Leaked, Ctx, Pred);
 }
 
-const ProgramPointTag *
-RetainCountChecker::getDeadSymbolTag(SymbolRef sym) const {
-  const CheckerProgramPointTag * = 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 ,
   CheckerContext ) const {
   ExplodedNode *Pred = C.getPredecessor();
@@ -1342,8 +1329,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, , 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
@@ -1314,19 +1314,6 @@
   processLeaks(state, Leaked, Ctx, Pred);
 }
 
-const ProgramPointTag *
-RetainCountChecker::getDeadSymbolTag(SymbolRef sym) const {
-  const CheckerProgramPointTag * = 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 ,
   CheckerContext ) const {
   ExplodedNode *Pred = C.getPredecessor();
@@ -1342,8 +1329,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, , 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


[PATCH] D52804: [analyzer] NFC: RetainCountChecker: Avoid dumping symbols during normal operation.

2018-10-02 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment.

This should also prevent leaks.


Repository:
  rC Clang

https://reviews.llvm.org/D52804



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52804: [analyzer] NFC: RetainCountChecker: Avoid dumping symbols during normal operation.

2018-10-02 Thread Artem Dergachev via Phabricator via cfe-commits
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 * = 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 ,
   CheckerContext ) 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, , 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 * = 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 ,
   CheckerContext ) 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, , 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