r284084 - [analyzer] DeallocChecker: Don't warn about directly-set IBOutlet ivars on macOS

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Oct 12 18:57:05 2016
New Revision: 284084

URL: http://llvm.org/viewvc/llvm-project?rev=284084&view=rev
Log:
[analyzer] DeallocChecker: Don't warn about directly-set IBOutlet ivars on macOS

On macOS (but not iOS), if an ObjC property has no setter, the nib-loading code
for an IBOutlet is documented as directly setting the backing ivar without
retaining the value -- even if the property is 'retain'.

This resulted in false positives from the DeallocChecker for code that did not
release such ivars in -dealloc.

To avoid these false positives, treat IBOutlet ivars that back a property
without a setter as having an unknown release requirement in macOS.

rdar://problem/28507353

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
    cfe/trunk/test/Analysis/DeallocMissingRelease.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp?rev=284084&r1=284083&r2=284084&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp Wed Oct 12 18:57:05 2016
@@ -34,6 +34,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
@@ -180,6 +181,7 @@ private:
   bool classHasSeparateTeardown(const ObjCInterfaceDecl *ID) const;
 
   bool isReleasedByCIFilterDealloc(const ObjCPropertyImplDecl *PropImpl) const;
+  bool isNibLoadedIvarWithoutRetain(const ObjCPropertyImplDecl *PropImpl) const;
 };
 } // End anonymous namespace.
 
@@ -935,6 +937,9 @@ ReleaseRequirement ObjCDeallocChecker::g
     if (isReleasedByCIFilterDealloc(PropImpl))
       return ReleaseRequirement::MustNotReleaseDirectly;
 
+    if (isNibLoadedIvarWithoutRetain(PropImpl))
+      return ReleaseRequirement::Unknown;
+
     return ReleaseRequirement::MustRelease;
 
   case ObjCPropertyDecl::Weak:
@@ -1091,6 +1096,32 @@ bool ObjCDeallocChecker::isReleasedByCIF
   return false;
 }
 
+/// Returns whether the ivar backing the property is an IBOutlet that
+/// has its value set by nib loading code without retaining the value.
+///
+/// On macOS, if there is no setter, the nib-loading code sets the ivar
+/// directly, without retaining the value,
+///
+/// On iOS and its derivatives, the nib-loading code will call
+/// -setValue:forKey:, which retains the value before directly setting the ivar.
+bool ObjCDeallocChecker::isNibLoadedIvarWithoutRetain(
+    const ObjCPropertyImplDecl *PropImpl) const {
+  const ObjCIvarDecl *IvarDecl = PropImpl->getPropertyIvarDecl();
+  if (!IvarDecl->hasAttr<IBOutletAttr>())
+    return false;
+
+  const llvm::Triple &Target =
+      IvarDecl->getASTContext().getTargetInfo().getTriple();
+
+  if (!Target.isMacOSX())
+    return false;
+
+  if (PropImpl->getPropertyDecl()->getSetterMethodDecl())
+    return false;
+
+  return true;
+}
+
 void ento::registerObjCDeallocChecker(CheckerManager &Mgr) {
   const LangOptions &LangOpts = Mgr.getLangOpts();
   // These checker only makes sense under MRR.

Modified: cfe/trunk/test/Analysis/DeallocMissingRelease.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/DeallocMissingRelease.m?rev=284084&r1=284083&r2=284084&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/DeallocMissingRelease.m (original)
+++ cfe/trunk/test/Analysis/DeallocMissingRelease.m Wed Oct 12 18:57:05 2016
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=osx.cocoa.Dealloc -fblocks -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=osx.cocoa.Dealloc -fblocks -triple x86_64-apple-ios4.0 -DMACOS=0 -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=osx.cocoa.Dealloc -fblocks -triple x86_64-apple-macosx10.6.0 -DMACOS=1 -verify %s
 // RUN: %clang_cc1 -analyze -analyzer-checker=osx.cocoa.Dealloc -fblocks -triple x86_64-apple-darwin10 -fobjc-arc -fobjc-runtime-has-weak -verify %s
 
 #include "Inputs/system-header-simulator-for-objc-dealloc.h"
@@ -938,3 +939,71 @@ __attribute__((objc_root_class))
 @implementation NotMissingDeallocCIFilter // no-warning
 @synthesize inputIvar = inputIvar;
 @end
+
+
+ at interface ClassWithRetainPropWithIBOutletIvarButNoSetter : NSObject {
+  // On macOS, the nib-loading code will set the ivar directly without
+  // retaining value (unike iOS, where it is retained). This means that
+  // on macOS we should not warn about a missing release for a property backed
+  // by an IBOutlet ivar when that property does not have a setter.
+  IBOutlet NSObject *ivarForOutlet;
+}
+
+ at property (readonly, retain) NSObject *ivarForOutlet;
+#if NON_ARC && !MACOS
+// expected-note at -2 {{Property is declared here}}
+#endif
+ at end
+
+ at implementation ClassWithRetainPropWithIBOutletIvarButNoSetter
+
+ at synthesize ivarForOutlet;
+#if NON_ARC && !MACOS
+// expected-note at -2 {{Property is synthesized here}}
+#endif
+- (void)dealloc {
+
+#if NON_ARC
+  [super dealloc];
+#if !MACOS
+// expected-warning at -2{{The 'ivarForOutlet' ivar in 'ClassWithRetainPropWithIBOutletIvarButNoSetter' was retained by a synthesized property but not released before '[super dealloc]'}}
+#endif
+#endif
+}
+
+ at end
+
+ at interface ClassWithRetainPropWithIBOutletIvarAndShadowingReadWrite : NSObject {
+  IBOutlet NSObject *ivarForOutlet;
+}
+
+ at property (readonly, retain) NSObject *ivarForOutlet;
+
+ at end
+
+ at interface ClassWithRetainPropWithIBOutletIvarAndShadowingReadWrite ()
+
+// Since there is a shadowing readwrite property, there will be a retaining
+// setter and so the ivar will be retained by nib-loading code even on
+// macOS and therefore must be released.
+ at property (readwrite, retain) NSObject *ivarForOutlet;
+#if NON_ARC
+// expected-note at -2 {{Property is declared here}}
+#endif
+ at end
+
+ at implementation ClassWithRetainPropWithIBOutletIvarAndShadowingReadWrite
+
+ at synthesize ivarForOutlet;
+#if NON_ARC
+// expected-note at -2 {{Property is synthesized here}}
+#endif
+- (void)dealloc {
+
+#if NON_ARC
+  [super dealloc];
+// expected-warning at -1{{The 'ivarForOutlet' ivar in 'ClassWithRetainPropWithIBOutletIvarAndShadowingReadWrite' was retained by a synthesized property but not released before '[super dealloc]'}}
+#endif
+}
+
+ at end