[PATCH] [analyzer] Detect duplicate [super dealloc] calls

David Kilzer ddkilzer at kilzer.net
Fri Jun 12 11:41:01 PDT 2015

Thanks for the feedback!

I will look at reducing the state stored by SuperDeallocState.

I will also add stack hints like "[super dealloc] was called here first" for the first call, and "[super dealloc] was called a second time here", or similar.

Comment at: lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp:156
@@ +155,3 @@
+  // FIXME: A callback should disable checkers at the start of functions.
+  if (!shouldRunOnFunctionOrMethod(
zaks.anna wrote:
> Is this done for performance purposes? Can [super dealloc] be called from functions? isSuperDeallocMessage check seems faster.
This check was modeled after ObjCSelfInitChecker::checkPostObjCMessage() in ObjCSelfInitChecker.cpp.

Heh, I see.  The checks in shouldRunOnFunctionOrMethod() are redundant because we're already passed a reference to an ObjCMethodCall.

I don't believe [super dealloc] is valid inside a plain C function context.

Comment at: lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp:161
@@ +160,3 @@
+  // Check for [super dealloc] method call.
+  if (isSuperDeallocMessage(M)) {
zaks.anna wrote:
> 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;
Sure, will fix.

Comment at: lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp:182
@@ +181,3 @@
+  SymbolRef SelfSymbol = M.getSelfSVal().getAsSymbol();
+  const SuperDeallocState *SDState = State->get<CalledSuperDealloc>(SelfSymbol);
+  if (!SDState)
zaks.anna wrote:
> These should be guarded on isSuperDeallocMessage. Otherwise, we would do a lookup every time we see a method call.
Got it!  Will fix.



More information about the cfe-commits mailing list