[cfe-commits] r164802 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp test/Analysis/objc_invalidation.m

Anna Zaks ganna at apple.com
Thu Sep 27 14:57:14 PDT 2012


Author: zaks
Date: Thu Sep 27 16:57:14 2012
New Revision: 164802

URL: http://llvm.org/viewvc/llvm-project?rev=164802&view=rev
Log:
[analyzer] IvarInvalidation: track synthesized ivars and allow escape
through property getters.

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
    cfe/trunk/test/Analysis/objc_invalidation.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp?rev=164802&r1=164801&r2=164802&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp Thu Sep 27 16:57:14 2012
@@ -41,6 +41,8 @@
                          const ObjCIvarDecl*> MethToIvarMapTy;
   typedef llvm::DenseMap<const ObjCPropertyDecl*,
                          const ObjCIvarDecl*> PropToIvarMapTy;
+  typedef llvm::DenseMap<const ObjCIvarDecl*,
+                         const ObjCPropertyDecl*> IvarToPropMapTy;
 
   /// Statement visitor, which walks the method body and flags the ivars
   /// referenced in it (either directly or via property).
@@ -49,18 +51,19 @@
     /// 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 @@
 
   /// 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 @@
   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 IvarInvalidationChecker::containsInvalidationMethod (
-    const ObjCContainerDecl *D) const {
+    const ObjCContainerDecl *D) {
 
   // TODO: Cache the results.
 
@@ -152,16 +159,36 @@
   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<ObjCObjectPointerType>();
+  if (!IvTy)
+    return false;
+  const ObjCInterfaceDecl *IvInterf = IvTy->getInterfaceDecl();
+  if (containsInvalidationMethod(IvInterf)) {
+    TrackedIvars[cast<ObjCIvarDecl>(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 @@
       II = InterfaceD->ivar_begin(),
       IE = InterfaceD->ivar_end(); II != IE; ++II) {
     const ObjCIvarDecl *Iv = *II;
-    QualType IvQTy = Iv->getType();
-    const ObjCObjectPointerType *IvTy = IvQTy->getAs<ObjCObjectPointerType>();
-    if (!IvTy)
-      continue;
-    const ObjCInterfaceDecl *IvInterf = IvTy->getInterfaceDecl();
-    if (containsInvalidationMethod(IvInterf))
-      Ivars[cast<ObjCIvarDecl>(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 @@
     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<ObjCPropertyDecl>(PD->getCanonicalDecl());
-    SetterD = cast<ObjCMethodDecl>(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<ObjCMethodDecl>(SetterD->getCanonicalDecl());
+      PropAccessorToIvarMap[SetterD] = ID;
+    }
+
+    const ObjCMethodDecl *GetterD = PD->getGetterMethodDecl();
+    if (GetterD) {
+      GetterD = cast<ObjCMethodDecl>(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 @@
 
       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 @@
   const ObjCMethodDecl *MD = ME->getMethodDecl();
   if (MD) {
     MD = cast<ObjCMethodDecl>(MD->getCanonicalDecl());
-    IVars[PropertySetterToIvarMap[MD]] = true;
+    IVars[PropertyAccessorToIvarMap[MD]] = true;
   }
   VisitStmt(ME);
 }
@@ -305,7 +343,7 @@
     const ObjCMethodDecl *MD = PA->getImplicitPropertySetter();
     if (MD) {
       MD = cast<ObjCMethodDecl>(MD->getCanonicalDecl());
-      IVars[PropertySetterToIvarMap[MD]] = true;
+      IVars[PropertyAccessorToIvarMap[MD]] = true;
       VisitStmt(PA);
       return;
     }

Modified: cfe/trunk/test/Analysis/objc_invalidation.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objc_invalidation.m?rev=164802&r1=164801&r2=164802&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/objc_invalidation.m (original)
+++ cfe/trunk/test/Analysis/objc_invalidation.m Thu Sep 27 16:57:14 2012
@@ -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;
+ at property (assign) SomeInvalidationImplementingObject* Prop5;
+ at 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 at -1 {{Instance variable Obj2 needs to be invalidated}}
  // expected-warning at -2 {{Instance variable MultipleProtocols needs to be invalidated}}
  // expected-warning at -3 {{Instance variable MultInheritance needs to be invalidated}}
+ // expected-warning at -4 {{Property SynthIvarProp needs to be invalidated}}
 @end





More information about the cfe-commits mailing list