[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