diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index fad684238b3f..8e455de3bf46 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -48,6 +48,7 @@ add_clang_library(clangStaticAnalyzerCheckers ObjCAtSyncChecker.cpp ObjCContainersASTChecker.cpp ObjCContainersChecker.cpp + ObjCMissingSuperCallChecker.cpp ObjCSelfInitChecker.cpp ObjCUnusedIVarsChecker.cpp PointerArithChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/Checkers.td b/clang/lib/StaticAnalyzer/Checkers/Checkers.td index 71b8931ba09c..b4b40fae2b51 100644 --- a/clang/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/lib/StaticAnalyzer/Checkers/Checkers.td @@ -420,6 +420,10 @@ def DirectIvarAssignment : Checker<"DirectIvarAssignment">, 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 { diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp new file mode 100644 index 000000000000..e906e8aa3016 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp @@ -0,0 +1,203 @@ +//==- 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 "llvm/ADT/SmallString.h" +#include "llvm/ADT/SmallSet.h" +#include "llvm/Support/raw_ostream.h" + +using namespace clang; +using namespace ento; + +static bool isUIViewControllerSubclass(ASTContext &Ctx, + const ObjCImplementationDecl *D) { + IdentifierInfo *ViewControllerII = &Ctx.Idents.get("UIViewController"); + const ObjCInterfaceDecl *ID = D->getClassInterface(); + + for ( ; ID; ID = ID->getSuperClass()) + if (ID->getIdentifier() == ViewControllerII) + return true; + return false; +} + +//===----------------------------------------------------------------------===// +// FindSuperCallVisitor - Identify specific calls to the superclass. +//===----------------------------------------------------------------------===// + +class FindSuperCallVisitor : public RecursiveASTVisitor { +public: + explicit FindSuperCallVisitor(Selector S) : DoesCallSuper(false), Sel(S) {} + + bool VisitObjCMessageExpr(ObjCMessageExpr *E) { + if (E->getSelector() == Sel) + if (E->getReceiverKind() == ObjCMessageExpr::SuperInstance) + DoesCallSuper = true; + + // Recurse if we didn't find the super call yet. + return !DoesCallSuper; + } + + bool DoesCallSuper; + +private: + Selector Sel; +}; + +//===----------------------------------------------------------------------===// +// ObjCSuperCallChecker +//===----------------------------------------------------------------------===// + +namespace { +class ObjCSuperCallChecker : public Checker< + check::ASTDecl > { +public: + void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager &Mgr, + BugReporter &BR) const; +}; +} + +void ObjCSuperCallChecker::checkASTDecl(const ObjCImplementationDecl *D, + AnalysisManager &Mgr, + BugReporter &BR) const { + ASTContext &Ctx = BR.getContext(); + + if (!isUIViewControllerSubclass(Ctx, D)) + return; + + const char *SelectorNames[] = + {"addChildViewController", "viewDidAppear", "viewDidDisappear", + "viewWillAppear", "viewWillDisappear", "removeFromParentViewController", + "didReceiveMemoryWarning", "viewDidUnload", "viewWillUnload", + "viewDidLoad"}; + const unsigned SelectorArgumentCounts[] = + {1, 1, 1, 1, 1, 0, 0, 0, 0, 0}; + const size_t SelectorCount = llvm::array_lengthof(SelectorNames); + assert(llvm::array_lengthof(SelectorArgumentCounts) == SelectorCount); + + // Fill the Selectors SmallSet with all selectors we want to check. + llvm::SmallSet Selectors; + for (size_t i = 0; i < SelectorCount; i++) { + unsigned ArgumentCount = SelectorArgumentCounts[i]; + const char *SelectorCString = SelectorNames[i]; + + // Get the selector. + IdentifierInfo *II = &Ctx.Idents.get(SelectorCString); + Selectors.insert(Ctx.Selectors.getSelector(ArgumentCount, &II)); + } + + // Iterate over all instance methods. + for (ObjCImplementationDecl::instmeth_iterator I = D->instmeth_begin(), + E = D->instmeth_end(); + I != E; ++I) { + Selector S = (*I)->getSelector(); + // Find out whether this is a selector that we want to check. + if (!Selectors.count(S)) + continue; + + ObjCMethodDecl *MD = *I; + + // Check if the method calls its superclass implementation. + if (MD->getBody()) + { + FindSuperCallVisitor Visitor(S); + Visitor.TraverseDecl(MD); + + // It doesn't call super, emit a diagnostic. + if (!Visitor.DoesCallSuper) { + PathDiagnosticLocation DLoc = + PathDiagnosticLocation::createEnd(MD->getBody(), + BR.getSourceManager(), + Mgr.getAnalysisDeclContext(D)); + + const char *Name = "Missing call to superclass"; + SmallString<256> Buf; + llvm::raw_svector_ostream os(Buf); + + os << "The '" << S.getAsString() + << "' instance method in UIViewController subclass '" << *D + << "' is missing a [super " << S.getAsString() << "] call"; + + BR.EmitBasicReport(MD, Name, categories::CoreFoundationObjectiveC, + os.str(), DLoc); + } + } + } +} + + +//===----------------------------------------------------------------------===// +// Check registration. +//===----------------------------------------------------------------------===// + +void ento::registerObjCSuperCallChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + + +/* + 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: +UIResponder subclasses +- resignFirstResponder + +NSResponder subclasses +- cursorUpdate + +*** more difficult cases: + +UIDocument subclasses +- finishedHandlingError:recovered: (is multi-arg) +- finishedHandlingError:recovered: (is multi-arg) + +UIViewController subclasses +- loadView (should *never* call super) +- transitionFromViewController:toViewController: + duration:options:animations:completion: (is multi-arg) + +UICollectionViewController subclasses +- loadView (take care because UIViewController subclasses should NOT call super + in loadView, but UICollectionViewController subclasses should) + +NSObject subclasses +- doesNotRecognizeSelector (it only has to call super if it doesn't throw) + +UIPopoverBackgroundView subclasses (some of those are class methods) +- arrowDirection (should *never* call super) +- arrowOffset (should *never* call super) +- arrowBase (should *never* call super) +- arrowHeight (should *never* call super) +- contentViewInsets (should *never* call super) + +UITextSelectionRect subclasses (some of those are properties) +- rect (should *never* call super) +- range (should *never* call super) +- writingDirection (should *never* call super) +- isVertical (should *never* call super) +- containsStart (should *never* call super) +- containsEnd (should *never* call super) +*/ diff --git a/clang/test/Analysis/viewcontroller.m b/clang/test/Analysis/viewcontroller.m new file mode 100644 index 000000000000..a8c45806db15 --- /dev/null +++ b/clang/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 {} +- (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 { + int addChildViewController = 5; + for (int i = 0; i < addChildViewController; i++) + [self viewDidAppear:i]; +} // expected-warning {{The 'addChildViewController:' instance method in UIViewController subclass 'TestB' is missing a [super addChildViewController:] call}} +- (void)viewDidAppear:(BOOL)animated {} // expected-warning {{The 'viewDidAppear:' instance method in UIViewController subclass 'TestB' is missing a [super viewDidAppear:] call}} +- (void)viewDidDisappear:(BOOL)animated {} // expected-warning {{The 'viewDidDisappear:' instance method in UIViewController subclass 'TestB' is missing a [super viewDidDisappear:] call}} +- (void)viewDidUnload {} // expected-warning {{The 'viewDidUnload' instance method in UIViewController subclass 'TestB' is missing a [super viewDidUnload] call}} +- (void)viewDidLoad {} // expected-warning {{The 'viewDidLoad' instance method in UIViewController subclass 'TestB' is missing a [super viewDidLoad] call}} +- (void)viewWillUnload {} // expected-warning {{The 'viewWillUnload' instance method in UIViewController subclass 'TestB' is missing a [super viewWillUnload] call}} +- (void)viewWillAppear:(BOOL)animated {} // expected-warning {{The 'viewWillAppear:' instance method in UIViewController subclass 'TestB' is missing a [super viewWillAppear:] call}} +- (void)viewWillDisappear:(BOOL)animated {} // expected-warning {{The 'viewWillDisappear:' instance method in UIViewController subclass 'TestB' is missing a [super viewWillDisappear:] call}} +- (void)didReceiveMemoryWarning {} // expected-warning {{The 'didReceiveMemoryWarning' instance method in UIViewController subclass 'TestB' is missing a [super didReceiveMemoryWarning] call}} +- (void)removeFromParentViewController {} // expected-warning {{The 'removeFromParentViewController' instance method in UIViewController subclass 'TestB' is missing a [super removeFromParentViewController] call}} + +// 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 { + [self methodDoingStuff]; +} // expected-warning {{The 'removeFromParentViewController' instance method in UIViewController subclass 'TestC' is missing a [super removeFromParentViewController] call}} +@end