From e14999e768fe55f620719fc4fbc361759e990e80 Mon Sep 17 00:00:00 2001 From: Jordan Rose <jordan_rose@apple.com> Date: Thu, 13 Dec 2012 03:06:45 +0000 Subject: [PATCH] [analyzer] Generalize ObjCMissingSuperCallChecker. We now check a few methods for UIResponder, NSResponder, and NSDocument. Patch by Julian Mayer! git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@170089 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Checkers/ObjCMissingSuperCallChecker.cpp | 152 +++++++++++++----- .../{viewcontroller.m => superclass.m} | 95 ++++++++++- 2 files changed, 200 insertions(+), 47 deletions(-) rename test/Analysis/{viewcontroller.m => superclass.m} (62%) diff --git a/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp index 53187ec8ee2..fc4add3b35f 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp @@ -29,15 +29,11 @@ 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; +namespace { +struct SelectorDescriptor { + const char *SelectorName; + unsigned ArgumentCount; +}; } //===----------------------------------------------------------------------===// @@ -71,9 +67,102 @@ namespace { class ObjCSuperCallChecker : public Checker< check::ASTDecl<ObjCImplementationDecl> > { public: + ObjCSuperCallChecker() : IsInitialized(false) {} + void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager &Mgr, BugReporter &BR) const; +private: + bool isCheckableClass(const ObjCImplementationDecl *D, + StringRef &SuperclassName) const; + void initializeSelectors(ASTContext &Ctx) const; + void fillSelectors(ASTContext &Ctx, ArrayRef<SelectorDescriptor> Sel, + StringRef ClassName) const; + mutable llvm::StringMap<llvm::SmallSet<Selector, 16> > SelectorsForClass; + mutable bool IsInitialized; }; + +} + +/// \brief Determine whether the given class has a superclass that we want +/// to check. The name of the found superclass is stored in SuperclassName. +/// +/// \param ObjCImplementationDecl The declaration to check for superclasses. +/// \param[out] SuperclassName On return, the found superclass name. +bool ObjCSuperCallChecker::isCheckableClass(const ObjCImplementationDecl *D, + StringRef &SuperclassName) const { + const ObjCInterfaceDecl *ID = D->getClassInterface(); + for ( ; ID ; ID = ID->getSuperClass()) + { + SuperclassName = ID->getIdentifier()->getName(); + if (SelectorsForClass.count(SuperclassName)) + return true; + } + return false; +} + +void ObjCSuperCallChecker::fillSelectors(ASTContext &Ctx, + ArrayRef<SelectorDescriptor> Sel, + StringRef ClassName) const { + llvm::SmallSet<Selector, 16> &ClassSelectors = SelectorsForClass[ClassName]; + // Fill the Selectors SmallSet with all selectors we want to check. + for (ArrayRef<SelectorDescriptor>::iterator I = Sel.begin(), E = Sel.end(); + I != E; ++I) { + SelectorDescriptor Descriptor = *I; + assert(Descriptor.ArgumentCount <= 1); // No multi-argument selectors yet. + + // Get the selector. + IdentifierInfo *II = &Ctx.Idents.get(Descriptor.SelectorName); + + Selector Sel = Ctx.Selectors.getSelector(Descriptor.ArgumentCount, &II); + ClassSelectors.insert(Sel); + } +} + +void ObjCSuperCallChecker::initializeSelectors(ASTContext &Ctx) const { + + { // Initialize selectors for: UIViewController + const SelectorDescriptor Selectors[] = { + { "addChildViewController", 1 }, + { "viewDidAppear", 1 }, + { "viewDidDisappear", 1 }, + { "viewWillAppear", 1 }, + { "viewWillDisappear", 1 }, + { "removeFromParentViewController", 0 }, + { "didReceiveMemoryWarning", 0 }, + { "viewDidUnload", 0 }, + { "viewDidLoad", 0 }, + { "viewWillUnload", 0 }, + { "updateViewConstraints", 0 }, + { "encodeRestorableStateWithCoder", 1 }, + { "restoreStateWithCoder", 1 }}; + + fillSelectors(Ctx, Selectors, "UIViewController"); + } + + { // Initialize selectors for: UIResponder + const SelectorDescriptor Selectors[] = { + { "resignFirstResponder", 0 }}; + + fillSelectors(Ctx, Selectors, "UIResponder"); + } + + { // Initialize selectors for: NSResponder + const SelectorDescriptor Selectors[] = { + { "encodeRestorableStateWithCoder", 1 }, + { "restoreStateWithCoder", 1 }}; + + fillSelectors(Ctx, Selectors, "NSResponder"); + } + + { // Initialize selectors for: NSDocument + const SelectorDescriptor Selectors[] = { + { "encodeRestorableStateWithCoder", 1 }, + { "restoreStateWithCoder", 1 }}; + + fillSelectors(Ctx, Selectors, "NSDocument"); + } + + IsInitialized = true; } void ObjCSuperCallChecker::checkASTDecl(const ObjCImplementationDecl *D, @@ -81,29 +170,15 @@ void ObjCSuperCallChecker::checkASTDecl(const ObjCImplementationDecl *D, 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); + // We need to initialize the selector table once. + if (!IsInitialized) + initializeSelectors(Ctx); - // Fill the Selectors SmallSet with all selectors we want to check. - llvm::SmallSet<Selector, 16> Selectors; - for (size_t i = 0; i < SelectorCount; i++) { - unsigned ArgumentCount = SelectorArgumentCounts[i]; - const char *SelectorCString = SelectorNames[i]; + // Find out whether this class has a superclass that we are supposed to check. + StringRef SuperclassName; + if (!isCheckableClass(D, SuperclassName)) + return; - // 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(), @@ -111,7 +186,7 @@ void ObjCSuperCallChecker::checkASTDecl(const ObjCImplementationDecl *D, I != E; ++I) { Selector S = (*I)->getSelector(); // Find out whether this is a selector that we want to check. - if (!Selectors.count(S)) + if (!SelectorsForClass[SuperclassName].count(S)) continue; ObjCMethodDecl *MD = *I; @@ -130,12 +205,12 @@ void ObjCSuperCallChecker::checkASTDecl(const ObjCImplementationDecl *D, Mgr.getAnalysisDeclContext(D)); const char *Name = "Missing call to superclass"; - SmallString<256> Buf; + SmallString<320> Buf; llvm::raw_svector_ostream os(Buf); os << "The '" << S.getAsString() - << "' instance method in UIViewController subclass '" << *D - << "' is missing a [super " << S.getAsString() << "] call"; + << "' instance method in " << SuperclassName.str() << " subclass '" + << *D << "' is missing a [super " << S.getAsString() << "] call"; BR.EmitBasicReport(MD, Name, categories::CoreFoundationObjectiveC, os.str(), DLoc); @@ -161,15 +236,6 @@ void ento::registerObjCSuperCallChecker(CheckerManager &Mgr) { 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) diff --git a/test/Analysis/viewcontroller.m b/test/Analysis/superclass.m similarity index 62% rename from test/Analysis/viewcontroller.m rename to test/Analysis/superclass.m index a8c45806db1..ba5ea40aceb 100644 --- a/test/Analysis/viewcontroller.m +++ b/test/Analysis/superclass.m @@ -1,5 +1,6 @@ // RUN: %clang_cc1 -fblocks -analyze -analyzer-checker=alpha.osx.cocoa.MissingSuperCall -verify -Wno-objc-root-class %s +// Define used Classes @protocol NSObject - (id)retain; - (oneway void)release; @@ -8,13 +9,15 @@ - (id)init; + (id)alloc; @end - typedef char BOOL; typedef double NSTimeInterval; typedef enum UIViewAnimationOptions { UIViewAnimationOptionLayoutSubviews = 1 << 0 } UIViewAnimationOptions; +@interface NSCoder : NSObject {} +@end +// Define the Superclasses for our Checks @interface UIViewController : NSObject {} - (void)addChildViewController:(UIViewController *)childController; - (void)viewDidAppear:(BOOL)animated; @@ -32,8 +35,21 @@ typedef enum UIViewAnimationOptions { animations:(void (^)(void))animations completion:(void (^)(BOOL finished))completion; @end +@interface UIResponder : NSObject {} +- (BOOL)resignFirstResponder; +@end +@interface NSResponder : NSObject {} +- (void)restoreStateWithCoder:(NSCoder *)coder; +- (void)encodeRestorableStateWithCoder:(NSCoder *)coder; +@end +@interface NSDocument : NSObject {} +- (void)restoreStateWithCoder:(NSCoder *)coder; +- (void)encodeRestorableStateWithCoder:(NSCoder *)coder; +@end -// Do not warn if UIViewController isn't our superclass +// Checks + +// Do not warn if UIViewController/*Responder/NSDocument is not our superclass @interface TestA @end @implementation TestA @@ -48,7 +64,9 @@ typedef enum UIViewAnimationOptions { - (void)viewWillDisappear:(BOOL)animated {} - (void)didReceiveMemoryWarning {} - (void)removeFromParentViewController {} - +- (BOOL)resignFirstResponder { return 0; } +- (void)restoreStateWithCoder:(NSCoder *)coder {} +- (void)encodeRestorableStateWithCoder:(NSCoder *)coder {} @end // Warn if UIViewController is our superclass and we do not call super @@ -72,7 +90,7 @@ typedef enum UIViewAnimationOptions { - (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 {}; +- (void)shouldAutorotate {} @end // Do not warn if UIViewController is our superclass but we did call super @@ -133,3 +151,72 @@ typedef enum UIViewAnimationOptions { [self methodDoingStuff]; } // expected-warning {{The 'removeFromParentViewController' instance method in UIViewController subclass 'TestC' is missing a [super removeFromParentViewController] call}} @end + + +// Do warn for UIResponder subclasses that don't call super +@interface TestD : UIResponder {} +@end +@implementation TestD + +- (BOOL)resignFirstResponder { + return 0; +} // expected-warning {{The 'resignFirstResponder' instance method in UIResponder subclass 'TestD' is missing a [super resignFirstResponder] call}} +@end + +// Do not warn for UIResponder subclasses that do the right thing +@interface TestE : UIResponder {} +@end +@implementation TestE + +- (BOOL)resignFirstResponder { + return [super resignFirstResponder]; +} +@end + +// Do warn for NSResponder subclasses that don't call super +@interface TestF : NSResponder {} +@end +@implementation TestF + +- (void)restoreStateWithCoder:(NSCoder *)coder { +} // expected-warning {{The 'restoreStateWithCoder:' instance method in NSResponder subclass 'TestF' is missing a [super restoreStateWithCoder:] call}} +- (void)encodeRestorableStateWithCoder:(NSCoder *)coder { +} // expected-warning {{The 'encodeRestorableStateWithCoder:' instance method in NSResponder subclass 'TestF' is missing a [super encodeRestorableStateWithCoder:] call}} +@end + +// Do not warn for NSResponder subclasses that do the right thing +@interface TestG : NSResponder {} +@end +@implementation TestG + +- (void)restoreStateWithCoder:(NSCoder *)coder { + [super restoreStateWithCoder:coder]; +} +- (void)encodeRestorableStateWithCoder:(NSCoder *)coder { + [super encodeRestorableStateWithCoder:coder]; +} +@end + +// Do warn for NSDocument subclasses that don't call super +@interface TestH : NSDocument {} +@end +@implementation TestH + +- (void)restoreStateWithCoder:(NSCoder *)coder { +} // expected-warning {{The 'restoreStateWithCoder:' instance method in NSDocument subclass 'TestH' is missing a [super restoreStateWithCoder:] call}} +- (void)encodeRestorableStateWithCoder:(NSCoder *)coder { +} // expected-warning {{The 'encodeRestorableStateWithCoder:' instance method in NSDocument subclass 'TestH' is missing a [super encodeRestorableStateWithCoder:] call}} +@end + +// Do not warn for NSDocument subclasses that do the right thing +@interface TestI : NSDocument {} +@end +@implementation TestI + +- (void)restoreStateWithCoder:(NSCoder *)coder { + [super restoreStateWithCoder:coder]; +} +- (void)encodeRestorableStateWithCoder:(NSCoder *)coder { + [super encodeRestorableStateWithCoder:coder]; +} +@end \ No newline at end of file -- GitLab