From a0c83316638ca691572dd719ddfa93673a72a2a4 Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Thu, 27 Sep 2012 21:57:14 +0000 Subject: [PATCH] [analyzer] IvarInvalidation: track synthesized ivars and allow escape through property getters. llvm-svn: 164802 --- .../Checkers/IvarInvalidationChecker.cpp | 100 ++++++++++++++------- clang/test/Analysis/objc_invalidation.m | 8 ++ 2 files changed, 77 insertions(+), 31 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp index 0bf8073..836860e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp @@ -41,6 +41,8 @@ class IvarInvalidationChecker : const ObjCIvarDecl*> MethToIvarMapTy; typedef llvm::DenseMap PropToIvarMapTy; + typedef llvm::DenseMap IvarToPropMapTy; /// Statement visitor, which walks the method body and flags the ivars /// referenced in it (either directly or via property). @@ -49,18 +51,19 @@ class IvarInvalidationChecker : /// The set of Ivars which need to be invalidated. IvarSet &IVars; - /// Property setter to ivar mapping. - MethToIvarMapTy &PropertySetterToIvarMap; + /// Property setter/getter to ivar mapping. + MethToIvarMapTy &PropertyAccessorToIvarMap; // Property to ivar mapping. PropToIvarMapTy &PropertyToIvarMap; public: MethodCrawler(const ObjCInterfaceDecl *InID, - IvarSet &InIVars, MethToIvarMapTy &InPropertySetterToIvarMap, + IvarSet &InIVars, + MethToIvarMapTy &InPropertyAccessorToIvarMap, PropToIvarMapTy &InPropertyToIvarMap) : IVars(InIVars), - PropertySetterToIvarMap(InPropertySetterToIvarMap), + PropertyAccessorToIvarMap(InPropertyAccessorToIvarMap), PropertyToIvarMap(InPropertyToIvarMap) {} void VisitStmt(const Stmt *S) { VisitChildren(S); } @@ -80,7 +83,11 @@ class IvarInvalidationChecker : /// Check if the any of the methods inside the interface are annotated with /// the invalidation annotation. - bool containsInvalidationMethod(const ObjCContainerDecl *D) const; + static bool containsInvalidationMethod(const ObjCContainerDecl *D); + + /// Check if ivar should be tracked and add to TrackedIvars if positive. + /// Returns true if ivar should be tracked. + static bool trackIvar(const ObjCIvarDecl *Iv, IvarSet &TrackedIvars); /// Given the property declaration, and the list of tracked ivars, finds /// the ivar backing the property when possible. Returns '0' when no such @@ -88,7 +95,7 @@ class IvarInvalidationChecker : static const ObjCIvarDecl *findPropertyBackingIvar( const ObjCPropertyDecl *Prop, const ObjCInterfaceDecl *InterfaceD, - IvarSet TrackedIvars); + IvarSet &TrackedIvars); public: void checkASTDecl(const ObjCMethodDecl *D, AnalysisManager& Mgr, @@ -109,7 +116,7 @@ bool isInvalidationMethod(const ObjCMethodDecl *M) { } bool IvarInvalidationChecker::containsInvalidationMethod ( - const ObjCContainerDecl *D) const { + const ObjCContainerDecl *D) { // TODO: Cache the results. @@ -152,16 +159,36 @@ bool IvarInvalidationChecker::containsInvalidationMethod ( llvm_unreachable("One of the casts above should have succeeded."); } +bool IvarInvalidationChecker::trackIvar(const ObjCIvarDecl *Iv, + IvarSet &TrackedIvars) { + QualType IvQTy = Iv->getType(); + const ObjCObjectPointerType *IvTy = IvQTy->getAs(); + if (!IvTy) + return false; + const ObjCInterfaceDecl *IvInterf = IvTy->getInterfaceDecl(); + if (containsInvalidationMethod(IvInterf)) { + TrackedIvars[cast(Iv->getCanonicalDecl())] = false; + return true; + } + return false; +} + const ObjCIvarDecl *IvarInvalidationChecker::findPropertyBackingIvar( const ObjCPropertyDecl *Prop, const ObjCInterfaceDecl *InterfaceD, - IvarSet TrackedIvars) { + IvarSet &TrackedIvars) { const ObjCIvarDecl *IvarD = 0; // Lookup for the synthesized case. IvarD = Prop->getPropertyIvarDecl(); - if (IvarD) - return IvarD; + if (IvarD) { + if (TrackedIvars.count(IvarD)) { + return IvarD; + } + // If the ivar is synthesized we still want to track it. + if (trackIvar(IvarD, TrackedIvars)) + return IvarD; + } // Lookup IVars named "_PropName"or "PropName" among the tracked Ivars. StringRef PropName = Prop->getIdentifier()->getName(); @@ -202,19 +229,14 @@ void IvarInvalidationChecker::checkASTDecl(const ObjCMethodDecl *D, 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->getInterfaceDecl(); - if (containsInvalidationMethod(IvInterf)) - Ivars[cast(Iv->getCanonicalDecl())] = false; + trackIvar(Iv, Ivars); } - // Construct Property/Property Setter to Ivar maps to assist checking if an + // Construct Property/Property Accessor to Ivar maps to assist checking if an // ivar which is backing a property has been reset. - MethToIvarMapTy PropSetterToIvarMap; + MethToIvarMapTy PropAccessorToIvarMap; PropToIvarMapTy PropertyToIvarMap; + IvarToPropMapTy IvarToPopertyMap; for (ObjCInterfaceDecl::prop_iterator I = InterfaceD->prop_begin(), E = InterfaceD->prop_end(); I != E; ++I) { @@ -224,24 +246,31 @@ void IvarInvalidationChecker::checkASTDecl(const ObjCMethodDecl *D, 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) - continue; // Store the mappings. PD = cast(PD->getCanonicalDecl()); - SetterD = cast(SetterD->getCanonicalDecl()); PropertyToIvarMap[PD] = ID; - PropSetterToIvarMap[SetterD] = ID; + IvarToPopertyMap[ID] = PD; + + // Find the setter and the getter. + const ObjCMethodDecl *SetterD = PD->getSetterMethodDecl(); + if (SetterD) { + SetterD = cast(SetterD->getCanonicalDecl()); + PropAccessorToIvarMap[SetterD] = ID; + } + + const ObjCMethodDecl *GetterD = PD->getGetterMethodDecl(); + if (GetterD) { + GetterD = cast(GetterD->getCanonicalDecl()); + PropAccessorToIvarMap[GetterD] = 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()); + PropAccessorToIvarMap, 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){ @@ -254,8 +283,17 @@ void IvarInvalidationChecker::checkASTDecl(const ObjCMethodDecl *D, SmallString<128> sbuf; llvm::raw_svector_ostream os(sbuf); - os << "Instance variable "<< IvarDecl->getName() - << " needs to be invalidated"; + + // Construct the warning message. + if (IvarDecl->getSynthesize()) { + const ObjCPropertyDecl *PD = IvarToPopertyMap[IvarDecl]; + assert(PD && + "Do we synthesize ivars for something other than properties?"); + os << "Property "<< PD->getName() << " needs to be invalidated"; + } else { + os << "Instance variable "<< IvarDecl->getName() + << " needs to be invalidated"; + } BR.EmitBasicReport(D, "Incomplete invalidation", @@ -282,7 +320,7 @@ void IvarInvalidationChecker::MethodCrawler::VisitObjCMessageExpr( const ObjCMethodDecl *MD = ME->getMethodDecl(); if (MD) { MD = cast(MD->getCanonicalDecl()); - IVars[PropertySetterToIvarMap[MD]] = true; + IVars[PropertyAccessorToIvarMap[MD]] = true; } VisitStmt(ME); } @@ -305,7 +343,7 @@ void IvarInvalidationChecker::MethodCrawler::VisitObjCPropertyRefExpr( const ObjCMethodDecl *MD = PA->getImplicitPropertySetter(); if (MD) { MD = cast(MD->getCanonicalDecl()); - IVars[PropertySetterToIvarMap[MD]] = true; + IVars[PropertyAccessorToIvarMap[MD]] = true; VisitStmt(PA); return; } diff --git a/clang/test/Analysis/objc_invalidation.m b/clang/test/Analysis/objc_invalidation.m index e0ce28b..ededae6 100644 --- a/clang/test/Analysis/objc_invalidation.m +++ b/clang/test/Analysis/objc_invalidation.m @@ -50,6 +50,7 @@ SomeInvalidationImplementingObject *_propIvar; Invalidation1Class *MultipleProtocols; Invalidation2Class *MultInheritance; + SomeInvalidationImplementingObject *_Prop5; // invalidate via getter method // No warnings on these. NSObject *NObj1; @@ -63,6 +64,10 @@ @property (assign) SomeInvalidationImplementingObject* Prop2; @property (assign) SomeInvalidationImplementingObject* Prop3; @property (assign) SomeInvalidationImplementingObject* Prop4; +@property (assign) SomeInvalidationImplementingObject* Prop5; +@property (assign) SomeInvalidationImplementingObject *SynthIvarProp; + + @property (assign) NSObject* NProp0; @property (nonatomic, assign) NSObject* NProp1; @property (assign) NSObject* NProp2; @@ -98,13 +103,16 @@ - (void) invalidate { [Obj3 invalidate]; + self.Prop0 = 0; self.Prop1 = 0; [self setProp2:0]; [self setProp3:0]; self.Prop4 = 0; + [[self Prop5] invalidate]; [super invalidate]; }// expected-warning {{Instance variable Obj1 needs to be invalidated}} // expected-warning@-1 {{Instance variable Obj2 needs to be invalidated}} // expected-warning@-2 {{Instance variable MultipleProtocols needs to be invalidated}} // expected-warning@-3 {{Instance variable MultInheritance needs to be invalidated}} + // expected-warning@-4 {{Property SynthIvarProp needs to be invalidated}} @end -- 2.7.4