r180170 - [analyzer] IvarInvalidation: correctly handle cases where only partial invalidators exist

[Anna Zaks]

Author: zaks
Date: Tue Apr 23 21:49:16 2013
New Revision: 180170

URL: http://llvm.org/viewvc/llvm-project?rev=180170&view=rev
Log:
[analyzer] IvarInvalidation: correctly handle cases where only partial invalidators exist

 - If only partial invalidators exist and there are no full invalidators in @implementation, report every ivar that has
not been invalidated. (Previously, we reported the first Ivar in the list, which could actually have been invalidated
by a partial invalidator. The code assumed you cannot have only partial invalidators.)

- Do not report missing invalidation method declaration if a partial invalidation method declaration exists.

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=180170&r1=180169&r2=180170&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp Tue Apr 23 21:49:16 2013
@@ -437,6 +437,7 @@ visit(const ObjCImplementationDecl *Impl
 
   // Remove ivars invalidated by the partial invalidation methods. They do not
   // need to be invalidated in the regular invalidation methods.
+  bool AtImplementationContainsAtLeastOnePartialInvalidationMethod = false;
   for (MethodSet::iterator
       I = PartialInfo.InvalidationMethods.begin(),
       E = PartialInfo.InvalidationMethods.end(); I != E; ++I) {
@@ -446,6 +447,8 @@ visit(const ObjCImplementationDecl *Impl
     const ObjCMethodDecl *D = ImplD->getMethod(InterfD->getSelector(),
                                                InterfD->isInstanceMethod());
     if (D && D->hasBody()) {
+      AtImplementationContainsAtLeastOnePartialInvalidationMethod = true;
+
       bool CalledAnotherInvalidationMethod = false;
       // The MethodCrowler is going to remove the invalidated ivars.
       MethodCrawler(Ivars,
@@ -471,7 +474,7 @@ visit(const ObjCImplementationDecl *Impl
   containsInvalidationMethod(InterfaceD, Info, /*LookForPartial*/ false);
 
   // Report an error in case none of the invalidation methods are declared.
-  if (!Info.needsInvalidation()) {
+  if (!Info.needsInvalidation() && !PartialInfo.needsInvalidation()) {
     if (Filter.check_MissingInvalidationMethod)
       reportNoInvalidationMethod(FirstIvarDecl, IvarToPopertyMap, InterfaceD,
                                  /*MissingDeclaration*/ true);
@@ -520,9 +523,19 @@ visit(const ObjCImplementationDecl *Impl
   }
 
   // Report an error in case none of the invalidation methods are implemented.
-  if (!AtImplementationContainsAtLeastOneInvalidationMethod)
-    reportNoInvalidationMethod(FirstIvarDecl, IvarToPopertyMap, InterfaceD,
-                               /*MissingDeclaration*/ false);
+  if (!AtImplementationContainsAtLeastOneInvalidationMethod) {
+    if (AtImplementationContainsAtLeastOnePartialInvalidationMethod) {
+      // Warn on the ivars that were not invalidated by the prrtial
+      // invalidation methods.
+      for (IvarSet::const_iterator
+           I = Ivars.begin(), E = Ivars.end(); I != E; ++I)
+        reportIvarNeedsInvalidation(I->first, IvarToPopertyMap, 0);
+    } else {
+      // Otherwise, no invalidation methods were implemented.
+      reportNoInvalidationMethod(FirstIvarDecl, IvarToPopertyMap, InterfaceD,
+                                 /*MissingDeclaration*/ false);
+    }
+  }
 }
 
 void IvarInvalidationCheckerImpl::
@@ -551,19 +564,27 @@ reportNoInvalidationMethod(const ObjCIva
 
 void IvarInvalidationCheckerImpl::
 reportIvarNeedsInvalidation(const ObjCIvarDecl *IvarD,
-                                    const IvarToPropMapTy &IvarToPopertyMap,
-                                    const ObjCMethodDecl *MethodD) const {
+                            const IvarToPropMapTy &IvarToPopertyMap,
+                            const ObjCMethodDecl *MethodD) const {
   SmallString<128> sbuf;
   llvm::raw_svector_ostream os(sbuf);
   printIvar(os, IvarD, IvarToPopertyMap);
   os << "needs to be invalidated or set to nil";
-  PathDiagnosticLocation MethodDecLocation =
-                         PathDiagnosticLocation::createEnd(MethodD->getBody(),
-                         BR.getSourceManager(),
-                         Mgr.getAnalysisDeclContext(MethodD));
-  BR.EmitBasicReport(MethodD, "Incomplete invalidation",
-                     categories::CoreFoundationObjectiveC, os.str(),
-                     MethodDecLocation);
+  if (MethodD) {
+    PathDiagnosticLocation MethodDecLocation =
+                           PathDiagnosticLocation::createEnd(MethodD->getBody(),
+                           BR.getSourceManager(),
+                           Mgr.getAnalysisDeclContext(MethodD));
+    BR.EmitBasicReport(MethodD, "Incomplete invalidation",
+                       categories::CoreFoundationObjectiveC, os.str(),
+                       MethodDecLocation);
+  } else {
+    BR.EmitBasicReport(IvarD, "Incomplete invalidation",
+                       categories::CoreFoundationObjectiveC, os.str(),
+                       PathDiagnosticLocation::createBegin(IvarD,
+                                                        BR.getSourceManager()));
+                       
+  }
 }
 
 void IvarInvalidationCheckerImpl::MethodCrawler::markInvalidated(

Modified: cfe/trunk/test/Analysis/objc_invalidation.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objc_invalidation.m?rev=180170&r1=180169&r2=180170&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/objc_invalidation.m (original)
+++ cfe/trunk/test/Analysis/objc_invalidation.m Tue Apr 23 21:49:16 2013
@@ -322,6 +322,47 @@ extern void NSLog(NSString *format, ...)
 #endif
 @end
 
+ at interface SomeNotInvalidatedInPartial : SomeInvalidationImplementingObject {
+  SomeInvalidationImplementingObject *Ivar1;
+  SomeInvalidationImplementingObject *Ivar2;
+#if RUN_IVAR_INVALIDATION
+  // expected-warning at -2 {{Instance variable Ivar2 needs to be invalidated or set to nil}}
+#endif
+}
+-(void)partialInvalidator __attribute__((annotate("objc_instance_variable_invalidator_partial")));
+-(void)partialInvalidatorCallsPartial __attribute__((annotate("objc_instance_variable_invalidator_partial")));
+ at end
+ at implementation SomeNotInvalidatedInPartial {
+  SomeInvalidationImplementingObject *Ivar3;
+#if RUN_IVAR_INVALIDATION
+  // expected-warning at -2 {{Instance variable Ivar3 needs to be invalidated or set to nil}}
+#endif
+}
+-(void)partialInvalidator {
+  Ivar1 = 0;
+}
+-(void)partialInvalidatorCallsPartial {
+  [self partialInvalidator];
+}
+ at end
+
+ at interface OnlyPartialDeclsBase : NSObject
+-(void)partialInvalidator __attribute__((annotate("objc_instance_variable_invalidator_partial")));
+ at end
+ at implementation OnlyPartialDeclsBase
+-(void)partialInvalidator {}
+ at end
+
+ at interface OnlyPartialDecls : OnlyPartialDeclsBase {
+  SomeInvalidationImplementingObject *Ivar1;
+#if RUN_IVAR_INVALIDATION
+  // expected-warning at -2 {{Instance variable Ivar1 needs to be invalidated; no invalidation method is defined in the @implementation for OnlyPartialDecls}}
+#endif
+}
+ at end
+ at implementation OnlyPartialDecls
+ at end
+
 // False negative.
 @interface PartialCallsFull : SomeInvalidationImplementingObject {
   SomeInvalidationImplementingObject *Ivar1;