[cfe-dev] [analyzer] Zombie symbols.

[Anna Zaks via cfe-dev]

> On Mar 29, 2016, at 7:10 AM, Artem Dergachev via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> 
> > You might want to store this in the repo by
> > adding a document or extending this one:
> > ./docs/analyzer/RegionStore.txt
> 
> Hmm, i wish i could fix the bugs rather than document them (= Will have a look!
> 

Ted pointed out that isLive covers the symbols that do not exist in the state. Have you tried addressing the issue with something like this:

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index 30481ea..1c5a984 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -550,8 +550,8 @@ public:
   ///
   /// This should only be called once all marking of dead symbols has completed.
   /// (For checkers, this means only in the evalDeadSymbols callback.)
-  bool isDead(SymbolRef sym) const {
-    return TheDead.count(sym);
+  bool isDead(SymbolRef sym) {
+    return TheDead.count(sym) || !isLive(sym);
   }

Almost all tests pass with this change and the one that does not looks like a legitimate new leak reported.

There are a couple of issues to further investigate here:
-  What are the performance implications? I'd expect some, but they are probably not very bad. Still we should measure.
-  We should probably eliminate the use of dead_begin dead_end, which some checkers reply on.
-  removeDeadBindings is not run right after the last reference to the object is lost, which leads to imprecise error reports and failure to report the leak in some cases. It's because of how hasDeadSymbols() is implemented. That problem is solved if we disable the optimization, but I do not know how that effects performance. Maybe we can come up with something more clever.

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 0b66933..e43c5b7 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -380,14 +380,7 @@ void ExprEngine::removeDead(ExplodedNode *Pred, ExplodedNodeSet &Out,
   // Process any special transfer function for dead symbols.
   // A tag to track convenience transitions, which can be removed at cleanup.
   static SimpleProgramPointTag cleanupTag(TagProviderName, "Clean Node");
-  if (!SymReaper.hasDeadSymbols()) {
-    // Generate a CleanedNode that has the environment and store cleaned
-    // up. Since no symbols are dead, we can optimize and not clean out
-    // the constraint manager.
-    StmtNodeBuilder Bldr(Pred, Out, *currBldrCtx);
-    Bldr.generateNode(DiagnosticStmt, Pred, CleanedState, &cleanupTag, K);
-
-  } else {
+  {

Artem, let me know if you'd like to finish the work on this; otherwise either me or Devin could conduct the remaining investigations.

Cheers,
Anna.

> > Taint has never been fully implemented. I also
> > want to investigate using flow-sensitive analysis
> > for taint tracking instead of the approach the
> > current half-implementation uses.
> 
> The current approach seems very sensible to me, i like it, and i'm not quite aware of any significant theoretical problems.
> 
> The implementation gore of sharing the state trait across checkers is annoying, but since we want to eventually share all traits in a generic manner anyway, this problem would eventually be gone. It is also a bit hard to work with tainted strings, but that seems to be an SVal hierarchy thing rather than a taint implementation problem.
> 
> Taint analysis becomes the most effective with heavy IPA, probably better even with inter-unit analysis and good call graph sorting to ensure top-bottom analysis, implying huge node limits, because input-output and usage are often spread out very far away from each other, through many function calls. It wouldn't find many things if it only considers small scopes.
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160330/ad581b80/attachment.html>