David,
I've listed some small concerns throughout and have not looked at the tests in
a lot of detail yet.
However, the main problem is that you store too much state in
SuperDeallocState. (We try to keep the states as lean as possible since they
are one of the major bottlenecks and have considerable impact on performance.)
Most of what you store is only consumed by BugReporterVisitor. You might be
able to work around storing this info by lazily reconstructing it at bug
reporting time.
When bug is reported, the path is visited bottom up; this is when your visitor
will get called. You can teach the visitor to detect the first node in which
the SuperDeallocState for the given SymRef has been added (as the path is
visited bottom up). At that point, you look up the statement associated with
the ProgramPoint and if it's a message call, you found the node to which the
diagnostic should be attached. (Take a look at other checkers such as
MallocChecker; they are all based on this lazy approach.)
You might also want to play with Stack Hints in error reporting since your
reports are likely to span multiple functions. These are the notes that get
attached to the call site of the functions that contain the diagnostic. You
should use -analyzer-output=text (and -analyzer-output=plist) when testing the
full path experience. (Ex: See ./test/Analysis/keychainAPI-diagnostic-visitor.m)
================
Comment at: lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp:156
@@ +155,3 @@
+
+ // FIXME: A callback should disable checkers at the start of functions.
+ if (!shouldRunOnFunctionOrMethod(
----------------
Is this done for performance purposes? Can [super dealloc] be called from
functions? isSuperDeallocMessage check seems faster.
================
Comment at: lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp:161
@@ +160,3 @@
+
+ // Check for [super dealloc] method call.
+ if (isSuperDeallocMessage(M)) {
----------------
It would be better to do an early return here as well, unless you plan to
expand this for handling other calls.
if (!isSuperDeallocMessage(M))
return;
================
Comment at: lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp:182
@@ +181,3 @@
+ SymbolRef SelfSymbol = M.getSelfSVal().getAsSymbol();
+ const SuperDeallocState *SDState =
State->get<CalledSuperDealloc>(SelfSymbol);
+ if (!SDState)
----------------
These should be guarded on isSuperDeallocMessage. Otherwise, we would do a
lookup every time we see a method call.
http://reviews.llvm.org/D5238
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
