From: Anna Zaks Date: Wed, 5 Dec 2012 01:14:37 +0000 (+0000) Subject: [analyzer] Implement an opt-in variant of direct ivar assignment. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=39a62fcd3003785d9cc913ab2820be2f6f27bb40;p=clang [analyzer] Implement an opt-in variant of direct ivar assignment. This will only check the direct ivar assignments in the annotated methods. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@169349 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td index 235e63306f..10881b35ec 100644 --- a/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/lib/StaticAnalyzer/Checkers/Checkers.td @@ -417,7 +417,11 @@ def IvarInvalidationChecker : Checker<"InstanceVariableInvalidation">, DescFile<"IvarInvalidationChecker.cpp">; def DirectIvarAssignment : Checker<"DirectIvarAssignment">, - HelpText<"Check that the invalidatable instance variables are invalidated in the methods annotated with objc_instance_variable_invalidator">, + HelpText<"Check for direct assignments to instance variables">, + DescFile<"DirectIvarAssignment.cpp">; + +def DirectIvarAssignmentForAnnotatedFunctions : Checker<"DirectIvarAssignmentForAnnotatedFunctions">, + HelpText<"Check for direct assignments to instance variables in the methods annotated with objc_no_direct_instance_variable_assignmemt">, DescFile<"DirectIvarAssignment.cpp">; def ObjCSuperCallChecker : Checker<"MissingSuperCall">, diff --git a/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp b/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp index 2b35a74ed4..f7535090bd 100644 --- a/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp +++ b/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp @@ -14,6 +14,7 @@ //===----------------------------------------------------------------------===// #include "ClangSACheckers.h" +#include "clang/AST/Attr.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/StmtVisitor.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" @@ -26,6 +27,26 @@ using namespace ento; namespace { +/// The default method filter, which is used to filter out the methods on which +/// the check should not be performed. +/// +/// Checks for the init, dealloc, and any other functions that might be allowed +/// to perform direct instance variable assignment based on their name. +struct MethodFilter { + virtual bool operator()(ObjCMethodDecl *M) { + if (M->getMethodFamily() == OMF_init || + M->getMethodFamily() == OMF_dealloc || + M->getMethodFamily() == OMF_copy || + M->getMethodFamily() == OMF_mutableCopy || + M->getSelector().getNameForSlot(0).find("init") != StringRef::npos || + M->getSelector().getNameForSlot(0).find("Init") != StringRef::npos) + return true; + return false; + } +}; + +static MethodFilter DefaultMethodFilter; + class DirectIvarAssignment : public Checker > { @@ -59,6 +80,10 @@ class DirectIvarAssignment : }; public: + MethodFilter *ShouldSkipMethod; + + DirectIvarAssignment() : ShouldSkipMethod(&DefaultMethodFilter) {} + void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& Mgr, BugReporter &BR) const; }; @@ -118,14 +143,7 @@ void DirectIvarAssignment::checkASTDecl(const ObjCImplementationDecl *D, ObjCMethodDecl *M = *I; AnalysisDeclContext *DCtx = Mgr.getAnalysisDeclContext(M); - // Skip the init, dealloc functions and any functions that might be doing - // initialization based on their name. - if (M->getMethodFamily() == OMF_init || - M->getMethodFamily() == OMF_dealloc || - M->getMethodFamily() == OMF_copy || - M->getMethodFamily() == OMF_mutableCopy || - M->getSelector().getNameForSlot(0).find("init") != StringRef::npos || - M->getSelector().getNameForSlot(0).find("Init") != StringRef::npos) + if ((*ShouldSkipMethod)(M)) continue; const Stmt *Body = M->getBody(); @@ -175,6 +193,32 @@ void DirectIvarAssignment::MethodCrawler::VisitBinaryOperator( } } +// Register the checker that checks for direct accesses in all functions, +// except for the initialization and copy routines. void ento::registerDirectIvarAssignment(CheckerManager &mgr) { mgr.registerChecker(); } + +// Register the checker that checks for direct accesses in functions annotated +// with __attribute__((annotate("objc_no_direct_instance_variable_assignmemt"))). +namespace { +struct InvalidatorMethodFilter : MethodFilter { + virtual bool operator()(ObjCMethodDecl *M) { + for (specific_attr_iterator + AI = M->specific_attr_begin(), + AE = M->specific_attr_end(); AI != AE; ++AI) { + const AnnotateAttr *Ann = *AI; + if (Ann->getAnnotation() == "objc_no_direct_instance_variable_assignmemt") + return false; + } + return true; + } +}; + +InvalidatorMethodFilter AttrFilter; +} + +void ento::registerDirectIvarAssignmentForAnnotatedFunctions( + CheckerManager &mgr) { + mgr.registerChecker()->ShouldSkipMethod = &AttrFilter; +} diff --git a/test/Analysis/objc/direct-ivar-assignment-in-annotated-functions.m b/test/Analysis/objc/direct-ivar-assignment-in-annotated-functions.m new file mode 100644 index 0000000000..7a90fb4dac --- /dev/null +++ b/test/Analysis/objc/direct-ivar-assignment-in-annotated-functions.m @@ -0,0 +1,56 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.osx.cocoa.DirectIvarAssignmentForAnnotatedFunctions -fobjc-default-synthesize-properties -verify -fblocks %s + +typedef signed char BOOL; +@protocol NSObject - (BOOL)isEqual:(id)object; @end +@interface NSObject {} ++(id)alloc; +-(id)init; +-(id)autorelease; +-(id)copy; +-(id)retain; +@end + +@interface MyClass; +@end + +@interface AnnotatedClass :NSObject { +} + - (void) someMethod: (MyClass*)In __attribute__((annotate("objc_no_direct_instance_variable_assignmemt"))); + - (void) someMethodNotAnnaotated: (MyClass*)In; +@end + + +@interface TestProperty :AnnotatedClass { + MyClass *_Z; + id _nonSynth; +} + + @property (assign, nonatomic) MyClass* A; // explicitely synthesized, not implemented, non-default ivar name + + @property (assign) MyClass* X; // automatically synthesized, not implemented + + @property (assign, nonatomic) MyClass* Y; // automatically synthesized, implemented + + @property (assign, nonatomic) MyClass* Z; // non synthesized ivar, implemented setter + @property (readonly) id nonSynth; // non synthesized, explicitly implemented to return ivar with expected name + @end + +@implementation TestProperty + @synthesize A = __A; + + - (void) someMethod: (MyClass*)In { + (__A) = In; // expected-warning {{Direct assignment to an instance variable backing a property; use the setter instead}} + _X = In; // expected-warning {{Direct assignment to an instance variable backing a property; use the setter instead}} + _Y = In; // expected-warning {{Direct assignment to an instance variable backing a property; use the setter instead}} + _Z = In; // expected-warning {{Direct assignment to an instance variable backing a property; use the setter instead}} + _nonSynth = 0; // expected-warning {{Direct assignment to an instance variable backing a property; use the setter instead}} + } + - (void) someMethodNotAnnaotated: (MyClass*)In { + (__A) = In; + _X = In; // no-warning + _Y = In; // no-warning + _Z = In; // no-warning + _nonSynth = 0; // no-warning + } + +@end \ No newline at end of file