Hi jordan_rose,
attached is a new clang-analyzer checker that verifies ObjC API usage in
UIViewController subclasses, more specifically if implemented methods correctly
call the superclass implementations in methods where this is mandatory (and
omitting these calls can lead to real and difficult-to-find bugs).
the code is largely based on the dealloc checker which includes the check for a
missing super call as part its functions.
the code currently only checks UIViewController subclasses but should be
extended to be more general in the future, there are a variety of classes in
the iOS and Mac OS X APIs that require some methods in their subclass
implementations to either call super or not call super. the ToDo list in this
regard is at the bottom of the file.
this is the third revision of the patch, it addresses the points raised in the
review from jordan rose. the most significant change is to use a
RecursiveASTVisitor now.
http://llvm-reviews.chandlerc.com/D78
Files:
test/Analysis/viewcontroller.m
lib/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp
lib/StaticAnalyzer/Checkers/CMakeLists.txt
Index: test/Analysis/viewcontroller.m
===================================================================
--- test/Analysis/viewcontroller.m
+++ test/Analysis/viewcontroller.m
@@ -0,0 +1,135 @@
+// RUN: %clang_cc1 -fblocks -analyze -analyzer-checker=alpha.osx.cocoa.MissingSuperCall -verify -Wno-objc-root-class %s
+
+@protocol NSObject
+- (id)retain;
+- (oneway void)release;
+@end
+@interface NSObject <NSObject> {}
+- (id)init;
++ (id)alloc;
+@end
+
+typedef char BOOL;
+typedef double NSTimeInterval;
+typedef enum UIViewAnimationOptions {
+ UIViewAnimationOptionLayoutSubviews = 1 << 0
+} UIViewAnimationOptions;
+
+@interface UIViewController : NSObject {}
+- (void)addChildViewController:(UIViewController *)childController;
+- (void)viewDidAppear:(BOOL)animated;
+- (void)viewDidDisappear:(BOOL)animated;
+- (void)viewDidUnload;
+- (void)viewDidLoad;
+- (void)viewWillUnload;
+- (void)viewWillAppear:(BOOL)animated;
+- (void)viewWillDisappear:(BOOL)animated;
+- (void)didReceiveMemoryWarning;
+- (void)removeFromParentViewController;
+- (void)transitionFromViewController:(UIViewController *)fromViewController
+ toViewController:(UIViewController *)toViewController
+ duration:(NSTimeInterval)duration options:(UIViewAnimationOptions)options
+ animations:(void (^)(void))animations
+ completion:(void (^)(BOOL finished))completion;
+@end
+
+// Do not warn if UIViewController isn't our superclass
+@interface TestA
+@end
+@implementation TestA
+
+- (void)addChildViewController:(UIViewController *)childController {}
+- (void)viewDidAppear:(BOOL)animated {}
+- (void)viewDidDisappear:(BOOL)animated {}
+- (void)viewDidUnload {}
+- (void)viewDidLoad {}
+- (void)viewWillUnload {}
+- (void)viewWillAppear:(BOOL)animated {}
+- (void)viewWillDisappear:(BOOL)animated {}
+- (void)didReceiveMemoryWarning {}
+- (void)removeFromParentViewController {}
+
+@end
+
+// Warn if UIViewController is our superclass and we do not call super
+@interface TestB : UIViewController {}
+@end
+@implementation TestB
+
+- (void)addChildViewController:(UIViewController *)childController { // expected-warning {{The 'addChildViewController:' instance method in UIViewController subclass 'TestB' does not send a 'addChildViewController:' message to its super class (missing [super addChildViewController:]}}
+ int addChildViewController = 5;
+ for (int i = 0; i < addChildViewController; i++)
+ [self viewDidAppear:i];
+}
+- (void)viewDidAppear:(BOOL)animated {} // expected-warning {{The 'viewDidAppear:' instance method in UIViewController subclass 'TestB' does not send a 'viewDidAppear:' message to its super class (missing [super viewDidAppear:])}}
+- (void)viewDidDisappear:(BOOL)animated {} // expected-warning {{The 'viewDidDisappear:' instance method in UIViewController subclass 'TestB' does not send a 'viewDidDisappear:' message to its super class (missing [super viewDidDisappear:])}}
+- (void)viewDidUnload {} // expected-warning {{The 'viewDidUnload' instance method in UIViewController subclass 'TestB' does not send a 'viewDidUnload' message to its super class (missing [super viewDidUnload])}}
+- (void)viewDidLoad {} // expected-warning {{The 'viewDidLoad' instance method in UIViewController subclass 'TestB' does not send a 'viewDidLoad' message to its super class (missing [super viewDidLoad])}}
+- (void)viewWillUnload {} // expected-warning {{The 'viewWillUnload' instance method in UIViewController subclass 'TestB' does not send a 'viewWillUnload' message to its super class (missing [super viewWillUnload])}}
+- (void)viewWillAppear:(BOOL)animated {} // expected-warning {{The 'viewWillAppear:' instance method in UIViewController subclass 'TestB' does not send a 'viewWillAppear:' message to its super class (missing [super viewWillAppear:])}}
+- (void)viewWillDisappear:(BOOL)animated {} // expected-warning {{The 'viewWillDisappear:' instance method in UIViewController subclass 'TestB' does not send a 'viewWillDisappear:' message to its super class (missing [super viewWillDisappear:])}}
+- (void)didReceiveMemoryWarning {} // expected-warning {{The 'didReceiveMemoryWarning' instance method in UIViewController subclass 'TestB' does not send a 'didReceiveMemoryWarning' message to its super class (missing [super didReceiveMemoryWarning])}}
+- (void)removeFromParentViewController {} // expected-warning {{The 'removeFromParentViewController' instance method in UIViewController subclass 'TestB' does not send a 'removeFromParentViewController' message to its super class (missing [super removeFromParentViewController])}}
+
+// Do not warn for methods were it shouldn't
+- (void)shouldAutorotate {};
+@end
+
+// Do not warn if UIViewController is our superclass but we did call super
+@interface TestC : UIViewController {}
+@end
+@implementation TestC
+
+- (BOOL)methodReturningStuff {
+ return 1;
+}
+
+- (void)methodDoingStuff {
+ [super removeFromParentViewController];
+}
+
+- (void)addChildViewController:(UIViewController *)childController {
+ [super addChildViewController:childController];
+}
+
+- (void)viewDidAppear:(BOOL)animated {
+ [super viewDidAppear:animated];
+}
+
+- (void)viewDidDisappear:(BOOL)animated {
+ [super viewDidDisappear:animated];
+}
+
+- (void)viewDidUnload {
+ [super viewDidUnload];
+}
+
+- (void)viewDidLoad {
+ [super viewDidLoad];
+}
+
+- (void)viewWillUnload {
+ [super viewWillUnload];
+}
+
+- (void)viewWillAppear:(BOOL)animated {
+ int i = 0; // Also don't start warning just because we do additional stuff
+ i++;
+ [self viewDidDisappear:i];
+ [super viewWillAppear:animated];
+}
+
+- (void)viewWillDisappear:(BOOL)animated {
+ [super viewWillDisappear:[self methodReturningStuff]];
+}
+
+- (void)didReceiveMemoryWarning {
+ [super didReceiveMemoryWarning];
+}
+
+// We expect a warning here because at the moment the super-call can't be
+// done from another method.
+- (void)removeFromParentViewController { // expected-warning {{The 'removeFromParentViewController' instance method in UIViewController subclass 'TestC' does not send a 'removeFromParentViewController' message to its super class (missing [super removeFromParentViewController])}}
+ [self methodDoingStuff];
+}
+@end
\ No newline at end of file
Index: lib/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- lib/StaticAnalyzer/Checkers/Checkers.td
+++ lib/StaticAnalyzer/Checkers/Checkers.td
@@ -416,6 +416,10 @@
HelpText<"Check that the invalidatable instance variables are invalidated in the methods annotated with objc_instance_variable_invalidator">,
DescFile<"DirectIvarAssignment.cpp">;
+def ObjCSuperCallChecker : Checker<"MissingSuperCall">,
+ HelpText<"Warn about Objective-C methods that lack a necessary call to super">,
+ DescFile<"ObjCMissingSuperCallChecker.cpp">;
+
} // end "alpha.osx.cocoa"
let ParentPackage = CoreFoundation in {
Index: lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp
@@ -0,0 +1,207 @@
+//==- ObjCMissingSuperCallChecker.cpp - Check missing super-calls in ObjC -==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines a ObjCMissingSuperCallChecker, a checker that
+// analyzes a UIViewController implementation to determine if it
+// correctly calls super in the methods where this is mandatory.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/AST/ExprObjC.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/LangOptions.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang;
+using namespace ento;
+
+class FindSupercallVisitor
+ : public RecursiveASTVisitor<FindSupercallVisitor> {
+ public:
+
+ explicit FindSupercallVisitor(ASTContext *Context, Selector Sel)
+ : DoesCallSuper(0)
+ ,Context(Context)
+ , Sel(Sel) {}
+
+ bool VisitObjCMessageExpr(ObjCMessageExpr *E) {
+ if (E->getSelector() == Sel) {
+ if (E->getReceiverKind() == ObjCMessageExpr::SuperInstance) {
+ DoesCallSuper = true;
+ }
+ }
+ return !DoesCallSuper; // Recurse if we didn't find the super call yet.
+ }
+ bool DoesCallSuper;
+
+ private:
+ ASTContext *Context;
+ Selector Sel;
+ };
+
+
+static void checkObjCSuperCall(const ObjCImplementationDecl *D,
+ const LangOptions& LOpts, BugReporter& BR) {
+
+ ASTContext &Ctx = BR.getContext();
+ const ObjCInterfaceDecl *ID = D->getClassInterface();
+
+
+ // Determine if the class subclasses UIViewController.
+ IdentifierInfo *NSObjectII = &Ctx.Idents.get("NSObject");
+ IdentifierInfo *ViewControllerII = &Ctx.Idents.get("UIViewController");
+ bool isViewController = false;
+
+ for ( ; ID ; ID = ID->getSuperClass()) {
+ IdentifierInfo *II = ID->getIdentifier();
+
+ if (II == NSObjectII)
+ break;
+
+ if (II == ViewControllerII)
+ isViewController = true;
+ }
+
+ if (!isViewController)
+ return;
+
+ if (!ID)
+ return;
+
+ const int selectorCount = 10;
+ const char *selectorNames[selectorCount] =
+ {"addChildViewController", "viewDidAppear", "viewDidDisappear",
+ "viewWillAppear", "viewWillDisappear", "removeFromParentViewController",
+ "didReceiveMemoryWarning", "viewDidUnload", "viewWillUnload", "viewDidLoad"};
+ const int selectorArgumentCounts[selectorCount] =
+ {1, 1, 1, 1, 1, 0, 0, 0, 0, 0};
+
+
+ // Iterate over all every method we want to check.
+ for (size_t i = 0; i < selectorCount; i++) {
+ int argumentCount = selectorArgumentCounts[i];
+ const char *selectorCString = selectorNames[i];
+
+ // Get the selector.
+ IdentifierInfo *II = &Ctx.Idents.get(selectorCString);
+ Selector S = Ctx.Selectors.getSelector(argumentCount, &II);
+ ObjCMethodDecl *MD = 0;
+
+ // Scan the instance methods for the selector.
+ for (ObjCImplementationDecl::instmeth_iterator I = D->instmeth_begin(),
+ E = D->instmeth_end(); I!=E; ++I) {
+ if ((*I)->getSelector() == S) {
+ MD = *I;
+ break;
+ }
+ }
+
+ if (MD && MD->getBody())
+ {
+ FindSupercallVisitor Visitor = FindSupercallVisitor(&Ctx, S);
+ Visitor.TraverseDecl(MD);
+ if (!Visitor.DoesCallSuper) {
+ PathDiagnosticLocation DLoc =
+ PathDiagnosticLocation::createBegin(MD, BR.getSourceManager());
+
+ const char* name = "missing call to superclass";
+
+ std::string buf;
+ llvm::raw_string_ostream os(buf);
+ os << "The '" << S.getAsString()
+ << "' instance method in UIViewController subclass '" << *D
+ << "' does not send a '" << S.getAsString()
+ << "' message to its super class (missing [super "
+ << S.getAsString() << "])";
+
+ BR.EmitBasicReport(MD, name, categories::CoreFoundationObjectiveC,
+ os.str(), DLoc);
+ }
+ }
+ }
+}
+
+//===----------------------------------------------------------------------===//
+// ObjCViewControllerChecker
+//===----------------------------------------------------------------------===//
+
+namespace {
+class ObjCSuperCallChecker : public Checker<
+ check::ASTDecl<ObjCImplementationDecl> > {
+public:
+ void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& mgr,
+ BugReporter &BR) const {
+ checkObjCSuperCall(cast<ObjCImplementationDecl>(D), mgr.getLangOpts(), BR);
+ }
+};
+}
+
+void ento::registerObjCSuperCallChecker(CheckerManager &mgr) {
+ mgr.registerChecker<ObjCSuperCallChecker>();
+}
+
+
+/*
+ ToDo list for expanding this check in the future, the list is not exhaustive.
+ There are also cases where calling super is suggested but not "mandatory".
+ In addition to be able to check the classes and methods below, architectural
+ improvements like being able to allow for the super-call to be done in a called
+ method would be good too.
+
+*** trivial cases:
+UIView subclasses
++ initWithFrame
+
+UIResponder subclasses
++ resignFirstResponder
+
+NSResponder subclasses
++ cursorUpdate
+
+*** more difficult cases:
+
+UIDocument subclasses
++ finishedHandlingError:recovered: (no support for more than one argument now)
++ finishedHandlingError:recovered: (no support for more than one argument now)
+
+UIViewController subclasses
+- loadView (minus indicates it should never call super)
++ transitionFromViewController:toViewController:
+ duration:options:animations:completion: (difficult because multi-arg)
+
+UICollectionViewController subclasses
++ loadView (take care because UIViewController subclasses should NOT call
+ super in loadView, but UICollectionViewController should)
+
+NSObject subclasses
++ init* (init* methods have to call *a* init method in self /or/ super)
++ doesNotRecognizeSelector (it only has to call super if it doesnt throw)
+
+UIPopoverBackgroundView subclasses (some of those are class methods)
+- arrowDirection
+- arrowOffset
+- arrowBase
+- arrowHeight
+- contentViewInsets
+
+UITextSelectionRect subclasses (take care because some of those are properties)
+- rect
+- range
+- writingDirection
+- isVertical
+- containsStart
+- containsEnd
+*/
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -16,6 +16,7 @@
CallAndMessageChecker.cpp
CastSizeChecker.cpp
CastToStructChecker.cpp
+ CheckObjCViewController.cpp
CheckObjCDealloc.cpp
CheckObjCInstMethSignature.cpp
CheckSecuritySyntaxOnly.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
