[PATCH] D28023: [analyzer] Fix leak false positives before no-return functions caused by incomplete analyses.

Devin Coughlin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 21 08:49:34 PST 2016


dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

With the change Aleksei suggested (can you get the CFGStmtMap from the AnalysisDeclContext?), looks good to me.

I especially like the test!



================
Comment at: lib/StaticAnalyzer/Core/BugReporter.cpp:3363
+    // we're post-dominated by.
+    // FIXME: This is far from enough to handle the incomplete analysis case.
+    // We may be post-dominated in subsequent blocks, or even
----------------
a.sidorin wrote:
> I took a brief look and found that we have Domination analysis for clang CFG but not PostDomination analysis. However, `llvm::DominatorTreeBase` that is used internally in `clang::DominatorTree` may be constructed for post-domination analysis. Is it possible to implement such analysis quickly (or modify `clang::DominatorTree` to support it)? If the answer is no, don't mind.
Do you actually want a FIXME here? Are you certain it would it be a good use of some future contributor's time to rewrite this? If not then you might want to soften it to a normal comment. (New contributors often search the codebase for FIXMEs to address as their first patch -- so a FIXME is a recommendation that someone  actually fix it, rather than an acknowledgement/documentation of some known limitation).


================
Comment at: test/Analysis/max-nodes-suppress-on-sink.c:6
+
+// If we throw a warning of a bug-type with "suppress on sink" attribute set
+// (such as MallocChecker's memory leak warning), then failing to reach the
----------------
Using "throw" is not correct here. I would suggest "emit" or"report".


================
Comment at: test/Analysis/max-nodes-suppress-on-sink.c:27
+  // the max-nodes run-line helps.
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  clang_analyzer_warnIfReached(); // no-warning
----------------
This is great.


https://reviews.llvm.org/D28023





More information about the cfe-commits mailing list