From: Anna Zaks Date: Wed, 26 Sep 2012 18:55:16 +0000 (+0000) Subject: [analyzer] Add experimental ObjC invalidation method checker. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9802f9fb2a64df1c27251a3284394c2748122128;p=platform%2Fupstream%2Fllvm.git [analyzer] Add experimental ObjC invalidation method checker. This checker is annotation driven. It checks that the annotated invalidation method accesses all ivars of the enclosing objects that are objects of type, which in turn contains an invalidation method. This is driven by __attribute((annotation("objc_instance_variable_invalidator")). llvm-svn: 164716 --- diff --git a/clang/lib/Sema/SemaObjCProperty.cpp b/clang/lib/Sema/SemaObjCProperty.cpp index 82b8094..f51ec72 100644 --- a/clang/lib/Sema/SemaObjCProperty.cpp +++ b/clang/lib/Sema/SemaObjCProperty.cpp @@ -931,7 +931,6 @@ Decl *Sema::ActOnPropertyImplDecl(Scope *S, Ivar->setInvalidDecl(); ClassImpDecl->addDecl(Ivar); IDecl->makeDeclVisibleInContext(Ivar); - property->setPropertyIvarDecl(Ivar); if (getLangOpts().ObjCRuntime.isFragile()) Diag(PropertyDiagLoc, diag::error_missing_property_ivar_decl) @@ -947,6 +946,8 @@ Decl *Sema::ActOnPropertyImplDecl(Scope *S, << Ivar << Ivar->getName(); // Note! I deliberately want it to fall thru so more errors are caught. } + property->setPropertyIvarDecl(Ivar); + QualType IvarType = Context.getCanonicalType(Ivar->getType()); // Check that type of property and its ivar are type compatible. diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index ba39ccc..a1151ea 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -33,6 +33,7 @@ add_clang_library(clangStaticAnalyzerCheckers FixedAddressChecker.cpp GenericTaintChecker.cpp IdempotentOperationChecker.cpp + IvarInvalidationChecker.cpp LLVMConventionsChecker.cpp MacOSKeychainAPIChecker.cpp MacOSXAPIChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/Checkers.td b/clang/lib/StaticAnalyzer/Checkers/Checkers.td index 92b5a16..6e78560 100644 --- a/clang/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/lib/StaticAnalyzer/Checkers/Checkers.td @@ -116,7 +116,7 @@ def SizeofPointerChecker : Checker<"SizeofPtr">, HelpText<"Warn about unintended use of sizeof() on pointer expressions">, DescFile<"CheckSizeofPointer.cpp">; -} // end "core.experimental" +} // end "alpha.core" //===----------------------------------------------------------------------===// // Evaluate "builtin" functions. @@ -172,7 +172,7 @@ def VirtualCallChecker : Checker<"VirtualCall">, HelpText<"Check virtual function calls during construction or destruction">, DescFile<"VirtualCallChecker.cpp">; -} // end: "cplusplus.experimental" +} // end: "alpha.cplusplus" //===----------------------------------------------------------------------===// // Deadcode checkers. @@ -195,7 +195,7 @@ def UnreachableCodeChecker : Checker<"UnreachableCode">, HelpText<"Check unreachable code">, DescFile<"UnreachableCodeChecker.cpp">; -} // end "deadcode.experimental" +} // end "alpha.deadcode" //===----------------------------------------------------------------------===// // Security checkers. @@ -251,7 +251,7 @@ def MallocOverflowSecurityChecker : Checker<"MallocOverflow">, HelpText<"Check for overflows in the arguments to malloc()">, DescFile<"MallocOverflowSecurityChecker.cpp">; -} // end "security.experimental" +} // end "alpha.security" //===----------------------------------------------------------------------===// // Taint checkers. @@ -263,7 +263,7 @@ def GenericTaintChecker : Checker<"TaintPropagation">, HelpText<"Generate taint information used by other checkers">, DescFile<"GenericTaintChecker.cpp">; -} // end "experimental.security.taint" +} // end "alpha.security.taint" //===----------------------------------------------------------------------===// // Unix API checkers. @@ -303,7 +303,7 @@ def StreamChecker : Checker<"Stream">, HelpText<"Check stream handling functions">, DescFile<"StreamChecker.cpp">; -} // end "unix.experimental" +} // end "alpha.unix" let ParentPackage = CString in { @@ -413,7 +413,11 @@ def ObjCDeallocChecker : Checker<"Dealloc">, HelpText<"Warn about Objective-C classes that lack a correct implementation of -dealloc">, DescFile<"CheckObjCDealloc.cpp">; -} // end "cocoa.alpha" +def IvarInvalidationChecker : Checker<"InstanceVariableInvalidation">, + HelpText<"Check that the invalidatable instance variables are invalidated in the methods annotated with objc_instance_variable_invalidator">, + DescFile<"IvarInvalidationCheckerChecker.cpp">; + +} // end "alpha.osx.cocoa" let ParentPackage = CoreFoundation in { diff --git a/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp new file mode 100644 index 0000000..b08a2fb --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp @@ -0,0 +1,319 @@ +//=- IvarInvalidationChecker.cpp - -*- C++ ----*-==// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This checker implements annotation driven invalidation checking. If a class +// contains a method annotated with 'objc_instance_variable_invalidator', +// - (void) foo +// __attribute__((annotate("objc_instance_variable_invalidator"))); +// all the "ivalidatable" instance variables of this class should be +// invalidated. We call an instance variable ivalidatable if it is an object of +// a class which contains an invalidation method. +// +// Note, this checker currently only checks if an ivar was accessed by the +// method, we do not currently support any deeper invalidation checking. +// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" +#include "clang/AST/DeclObjC.h" +#include "clang/AST/StmtVisitor.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang; +using namespace ento; + +namespace { +class IvarInvalidationChecker : + public Checker > { + + typedef llvm::DenseMap IvarSet; + typedef llvm::DenseMap MethToIvarMapTy; + typedef llvm::DenseMap PropToIvarMapTy; + + /// Statement visitor, which walks the method body and flags the ivars + /// referenced in it (either directly or via property). + class MethodCrawler : public ConstStmtVisitor { + const ObjCInterfaceDecl *InterfD; + + /// The set of Ivars which need to be invalidated. + IvarSet &IVars; + + /// Property setter to ivar mapping. + MethToIvarMapTy &PropertySetterToIvarMap; + + // Property to ivar mapping. + PropToIvarMapTy &PropertyToIvarMap; + + public: + MethodCrawler(const ObjCInterfaceDecl *InID, + IvarSet &InIVars, MethToIvarMapTy &InPropertySetterToIvarMap, + PropToIvarMapTy &InPropertyToIvarMap) + : InterfD(InID), IVars(InIVars), + PropertySetterToIvarMap(InPropertySetterToIvarMap), + PropertyToIvarMap(InPropertyToIvarMap) {} + + void VisitStmt(const Stmt *S) { VisitChildren(S); } + + void VisitObjCIvarRefExpr(const ObjCIvarRefExpr *IvarRef); + + void VisitObjCMessageExpr(const ObjCMessageExpr *ME); + + void VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *PA); + + void VisitChildren(const Stmt *S) { + for (Stmt::const_child_range I = S->children(); I; ++I) + if (*I) + static_cast(this)->Visit(*I); + } + }; + + /// Check if the any of the methods inside the interface are annotated with + /// the invalidation annotation. + bool containsInvalidationMethod(const ObjCContainerDecl *D) const; + + /// Given the property declaration, and the list of tracked ivars, finds + /// the ivar backing the property when possible. Returns '0' when no such + /// ivar could be found. + static const ObjCIvarDecl *findPropertyBackingIvar( + const ObjCPropertyDecl *Prop, + const ObjCInterfaceDecl *InterfaceD, + IvarSet TrackedIvars); + +public: + void checkASTDecl(const ObjCMethodDecl *D, AnalysisManager& Mgr, + BugReporter &BR) const; + +}; + +bool isInvalidationMethod(const ObjCMethodDecl *M) { + const AnnotateAttr *Ann = M->getAttr(); + if (!Ann) + return false; + if (Ann->getAnnotation() == "objc_instance_variable_invalidator") + return true; + return false; +} + +bool IvarInvalidationChecker::containsInvalidationMethod ( + const ObjCContainerDecl *D) const { + + // TODO: Cache the results. + + if (!D) + return false; + + // Check all methods. + for (ObjCContainerDecl::method_iterator + I = D->meth_begin(), + E = D->meth_end(); I != E; ++I) { + const ObjCMethodDecl *MDI = *I; + if (isInvalidationMethod(MDI)) + return true; + } + + // If interface, check all parent protocols and super. + // TODO: Visit all categories in case the invalidation method is declared in + // a category. + if (const ObjCInterfaceDecl *InterfaceD = dyn_cast(D)) { + for (ObjCInterfaceDecl::protocol_iterator + I = InterfaceD->protocol_begin(), + E = InterfaceD->protocol_end(); I != E; ++I) { + if (containsInvalidationMethod(*I)) + return true; + } + return containsInvalidationMethod(InterfaceD->getSuperClass()); + } + + // If protocol, check all parent protocols. + if (const ObjCProtocolDecl *ProtD = dyn_cast(D)) { + for (ObjCInterfaceDecl::protocol_iterator + I = ProtD->protocol_begin(), + E = ProtD->protocol_end(); I != E; ++I) { + if (containsInvalidationMethod(*I)) + return true; + } + return false; + } + + llvm_unreachable("One of the casts above should have succeeded."); +} + +const ObjCIvarDecl *IvarInvalidationChecker::findPropertyBackingIvar( + const ObjCPropertyDecl *Prop, + const ObjCInterfaceDecl *InterfaceD, + IvarSet TrackedIvars) { + const ObjCIvarDecl *IvarD = 0; + + // Lookup for the synthesized case. + IvarD = Prop->getPropertyIvarDecl(); + if (IvarD) + return IvarD; + + // Lookup IVars named "_PropName"or "PropName" among the tracked Ivars. + StringRef PropName = Prop->getIdentifier()->getName(); + for (IvarSet::const_iterator I = TrackedIvars.begin(), + E = TrackedIvars.end(); I != E; ++I) { + const ObjCIvarDecl *Iv = I->first; + StringRef IvarName = Iv->getName(); + + if (IvarName == PropName) + return Iv; + + SmallString<128> PropNameWithUnderscore; + { + llvm::raw_svector_ostream os(PropNameWithUnderscore); + os << '_' << PropName; + } + if (IvarName == PropNameWithUnderscore.str()) + return Iv; + } + + // Note, this is a possible source of false positives. We could look at the + // getter implementation to find the ivar when its name is not derived from + // the property name. + return 0; +} + +void IvarInvalidationChecker::checkASTDecl(const ObjCMethodDecl *D, + AnalysisManager& Mgr, + BugReporter &BR) const { + // We are only interested in checking the cleanup methods. + if (!D->hasBody() || !isInvalidationMethod(D)) + return; + + // Collect all ivars that need cleanup. + IvarSet Ivars; + const ObjCInterfaceDecl *InterfaceD = D->getClassInterface(); + for (ObjCInterfaceDecl::ivar_iterator + II = InterfaceD->ivar_begin(), + IE = InterfaceD->ivar_end(); II != IE; ++II) { + const ObjCIvarDecl *Iv = *II; + QualType IvQTy = Iv->getType(); + const ObjCObjectPointerType *IvTy = IvQTy->getAs(); + if (!IvTy) + continue; + const ObjCInterfaceDecl *IvInterf = IvTy->getObjectType()->getInterface(); + if (containsInvalidationMethod(IvInterf)) + Ivars[cast(Iv->getCanonicalDecl())] = false; + } + + // Construct Property/Property Setter to Ivar maps to assist checking if an + // ivar which is backing a property has been reset. + MethToIvarMapTy PropSetterToIvarMap; + PropToIvarMapTy PropertyToIvarMap; + for (ObjCInterfaceDecl::prop_iterator + I = InterfaceD->prop_begin(), + E = InterfaceD->prop_end(); I != E; ++I) { + const ObjCPropertyDecl *PD = *I; + + const ObjCIvarDecl *ID = findPropertyBackingIvar(PD, InterfaceD, Ivars); + if (!ID) { + continue; + } + // Find the setter. + const ObjCMethodDecl *SetterD = PD->getSetterMethodDecl(); + // If we don't know the setter, do not track this ivar. + if (!SetterD) { + Ivars[cast(ID->getCanonicalDecl())] = true; + continue; + } + + // Store the mappings. + PD = cast(PD->getCanonicalDecl()); + SetterD = cast(SetterD->getCanonicalDecl()); + PropertyToIvarMap[PD] = ID; + PropSetterToIvarMap[SetterD] = ID; + } + + + // Check which ivars have been accessed by the method. + // We assume that if ivar was at least accessed, it was not forgotten. + MethodCrawler(InterfaceD, Ivars, + PropSetterToIvarMap, PropertyToIvarMap).VisitStmt(D->getBody()); + + // Warn on the ivars that were not accessed by the method. + for (IvarSet::const_iterator I = Ivars.begin(), E = Ivars.end(); I != E; ++I){ + if (I->second == false) { + const ObjCIvarDecl *IvarDecl = I->first; + + PathDiagnosticLocation IvarDecLocation = + PathDiagnosticLocation::createBegin(IvarDecl, BR.getSourceManager()); + + SmallString<128> sbuf; + llvm::raw_svector_ostream os(sbuf); + os << "Ivar needs to be invalidated in the '" << + D->getSelector().getAsString()<< "' method"; + + BR.EmitBasicReport(IvarDecl, + "Incomplete invalidation", + categories::CoreFoundationObjectiveC, os.str(), + IvarDecLocation); + } + } +} + +/// Handle the case when an ivar is directly accessed. +void IvarInvalidationChecker::MethodCrawler::VisitObjCIvarRefExpr( + const ObjCIvarRefExpr *IvarRef) { + const Decl *D = IvarRef->getDecl(); + if (D) + IVars[cast(D->getCanonicalDecl())] = true; + VisitStmt(IvarRef); +} + + +/// Handle the case when the property backing ivar is set via a direct call +/// to the setter. +void IvarInvalidationChecker::MethodCrawler::VisitObjCMessageExpr( + const ObjCMessageExpr *ME) { + const ObjCMethodDecl *MD = ME->getMethodDecl(); + if (MD) { + MD = cast(MD->getCanonicalDecl()); + IVars[PropertySetterToIvarMap[MD]] = true; + } + VisitStmt(ME); +} + +/// Handle the case when the property backing ivar is set via the dot syntax. +void IvarInvalidationChecker::MethodCrawler::VisitObjCPropertyRefExpr( + const ObjCPropertyRefExpr *PA) { + + if (PA->isExplicitProperty()) { + const ObjCPropertyDecl *PD = PA->getExplicitProperty(); + if (PD) { + PD = cast(PD->getCanonicalDecl()); + IVars[PropertyToIvarMap[PD]] = true; + VisitStmt(PA); + return; + } + } + + if (PA->isImplicitProperty()) { + const ObjCMethodDecl *MD = PA->getImplicitPropertySetter(); + if (MD) { + MD = cast(MD->getCanonicalDecl()); + IVars[PropertySetterToIvarMap[MD]] = true; + VisitStmt(PA); + return; + } + } + VisitStmt(PA); +} +} + +// Register the checker. +void ento::registerIvarInvalidationChecker(CheckerManager &mgr) { + mgr.registerChecker(); +} diff --git a/clang/test/Analysis/objc_invalidation.m b/clang/test/Analysis/objc_invalidation.m new file mode 100644 index 0000000..7050fdd --- /dev/null +++ b/clang/test/Analysis/objc_invalidation.m @@ -0,0 +1,107 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.osx.cocoa.InstanceVariableInvalidation -fobjc-default-synthesize-properties -verify %s + +@protocol NSObject +@end +@interface NSObject {} ++(id)alloc; ++(id)new; +-(id)init; +-(id)autorelease; +-(id)copy; +- (Class)class; +-(id)retain; +@end + +@protocol Invalidation1 +- (void) invalidate __attribute__((annotate("objc_instance_variable_invalidator"))); +@end + +@protocol Invalidation2 +- (void) invalidate __attribute__((annotate("objc_instance_variable_invalidator"))); +@end + +@protocol Invalidation3 +- (void) invalidate __attribute__((annotate("objc_instance_variable_invalidator"))); +@end + +@interface Invalidation2Class +@end + +@interface Invalidation1Class +@end + +@interface SomeInvalidationImplementingObject: NSObject { + SomeInvalidationImplementingObject *ObjA; +} +@end + +@implementation SomeInvalidationImplementingObject +- (void)invalidate{ + ObjA = 0; +} +@end + +@interface SomeSubclassInvalidatableObject : SomeInvalidationImplementingObject { + SomeInvalidationImplementingObject *Obj1; // expected-warning{{Ivar needs to be invalidated}} + SomeInvalidationImplementingObject *Obj2; // expected-warning{{Ivar needs to be invalidated}} + SomeInvalidationImplementingObject *Obj3; + SomeInvalidationImplementingObject *_Prop1; + SomeInvalidationImplementingObject *_Prop4; + SomeInvalidationImplementingObject *_propIvar; + Invalidation1Class *MultipleProtocols; // expected-warning{{Ivar needs to be invalidated}} + Invalidation2Class *MultInheritance; // expected-warning{{Ivar needs to be invalidated}} + + // No warnings on these. + NSObject *NObj1; + NSObject *NObj2; + NSObject *_NProp1; + NSObject *_NpropIvar; +} + +@property (assign) SomeInvalidationImplementingObject* Prop0; +@property (nonatomic, assign) SomeInvalidationImplementingObject* Prop1; +@property (assign) SomeInvalidationImplementingObject* Prop2; +@property (assign) SomeInvalidationImplementingObject* Prop3; +@property (assign) SomeInvalidationImplementingObject* Prop4; +@property (assign) NSObject* NProp0; +@property (nonatomic, assign) NSObject* NProp1; +@property (assign) NSObject* NProp2; + +-(void)setProp1: (SomeInvalidationImplementingObject*) InO; +-(void)setNProp1: (NSObject*) InO; + +-(void)invalidate; + +@end + +@implementation SomeSubclassInvalidatableObject + +@synthesize Prop2 = _propIvar; +@synthesize Prop3; + +- (void) setProp1: (SomeInvalidationImplementingObject*) InObj { + _Prop1 = InObj; +} + +- (void) setProp4: (SomeInvalidationImplementingObject*) InObj { + _Prop4 = InObj; +} +- (SomeInvalidationImplementingObject*) Prop4 { + return _Prop4; +} + +@synthesize NProp2 = _NpropIvar; + +- (void) setNProp1: (NSObject*) InObj { + _NProp1 = InObj; +} + +- (void) invalidate { + [Obj3 invalidate]; + self.Prop1 = 0; + [self setProp2:0]; + [self setProp3:0]; + self.Prop4 = 0; + [super invalidate]; +} +@end