[analyzer] Disable all retain count diagnostics on values that come from ivars.

This is imitating a pre-r228174 state where ivars are not considered tracked by
default, but with the addition that even ivars /with/ retain count information
(e.g. "[_ivar retain]; [ivar _release];") are not being tracked as well. This is
to ensure that we don't regress on values accessed through both properties and
ivars, which is what r228174 was trying to fix.

The issue occurs in code like this:

  [_contentView retain];
  [_contentView removeFromSuperview];
  [self addSubview:_contentView]; // invalidates 'self'
  [_contentView release];

In this case, the call to -addSubview: may change the value of self->_contentView,
and so the analyzer can't be sure that we didn't leak the original _contentView.
This is a correct conservative view of the world, but not a useful one. Until we
have a heuristic that allows us to not consider this a leak, not emitting a
diagnostic is our best bet.

This commit disables all of the ivar-related retain count tests, but does not
remove them to ensure that we don't crash trying to evaluate either valid or
erroneous code. The next commit will add a new test for the example above so
that this commit (and the previous one) can be reverted wholesale when a better
solution is implemented.

Rest of rdar://problem/20335433

llvm-svn: 233592

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

@@ -3237,6 +3237,16 @@ void RetainCountChecker::processNonLeakError(ProgramStateRef St,
                                              RefVal::Kind ErrorKind,
                                              SymbolRef Sym,
                                              CheckerContext &C) const {
+  // HACK: Ignore retain-count issues on values accessed through ivars,
+  // because of cases like this:
+  //   [_contentView retain];
+  //   [_contentView removeFromSuperview];
+  //   [self addSubview:_contentView]; // invalidates 'self'
+  //   [_contentView release];
+  if (const RefVal *RV = getRefBinding(St, Sym))
+    if (RV->getIvarAccessHistory() != RefVal::IvarAccessHistory::None)
+      return;
+
   ExplodedNode *N = C.generateSink(St);
   if (!N)
     return;
@@ -3463,6 +3473,15 @@ void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S,
                                                   RetEffect RE, RefVal X,
                                                   SymbolRef Sym,
                                               ProgramStateRef state) const {
+  // HACK: Ignore retain-count issues on values accessed through ivars,
+  // because of cases like this:
+  //   [_contentView retain];
+  //   [_contentView removeFromSuperview];
+  //   [self addSubview:_contentView]; // invalidates 'self'
+  //   [_contentView release];
+  if (X.getIvarAccessHistory() != RefVal::IvarAccessHistory::None)
+    return;
+
   // Any leaks or other errors?
   if (X.isReturnedOwned() && X.getCount() == 0) {
     if (RE.getKind() != RetEffect::NoRet) {
@@ -3700,6 +3719,15 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state,
     return setRefBinding(state, Sym, V);
   }
 
+  // HACK: Ignore retain-count issues on values accessed through ivars,
+  // because of cases like this:
+  //   [_contentView retain];
+  //   [_contentView removeFromSuperview];
+  //   [self addSubview:_contentView]; // invalidates 'self'
+  //   [_contentView release];
+  if (V.getIvarAccessHistory() != RefVal::IvarAccessHistory::None)
+    return state;
+
   // Woah!  More autorelease counts then retain counts left.
   // Emit hard error.
   V = V ^ RefVal::ErrorOverAutorelease;
@@ -3733,11 +3761,22 @@ ProgramStateRef
 RetainCountChecker::handleSymbolDeath(ProgramStateRef state,
                                       SymbolRef sid, RefVal V,
                                     SmallVectorImpl<SymbolRef> &Leaked) const {
-  bool hasLeak = false;
-  if (V.isOwned())
+  bool hasLeak;
+
+  // HACK: Ignore retain-count issues on values accessed through ivars,
+  // because of cases like this:
+  //   [_contentView retain];
+  //   [_contentView removeFromSuperview];
+  //   [self addSubview:_contentView]; // invalidates 'self'
+  //   [_contentView release];
+  if (V.getIvarAccessHistory() != RefVal::IvarAccessHistory::None)
+    hasLeak = false;
+  else if (V.isOwned())
     hasLeak = true;
   else if (V.isNotOwned() || V.isReturnedOwned())
     hasLeak = (V.getCount() > 0);
+  else
+    hasLeak = false;
 
   if (!hasLeak)
     return removeRefBinding(state, sid);

clang/test/Analysis/properties.m

@@ -376,7 +376,7 @@ void testOpaqueConsistency(OpaqueIntWrapper *w) {
   [_ownedProp retain];
   [_ownedProp release];
   [_ownedProp release];
-  [_ownedProp release]; // expected-warning{{used after it is released}}
+  [_ownedProp release]; // FIXME-warning{{used after it is released}}
 }
 
 - (void)testOverreleaseUnownedIvar {
@@ -389,21 +389,21 @@ void testOpaqueConsistency(OpaqueIntWrapper *w) {
   [_ivarOnly retain];
   [_ivarOnly release];
   [_ivarOnly release];
-  [_ivarOnly release]; // expected-warning{{used after it is released}}
+  [_ivarOnly release]; // FIXME-warning{{used after it is released}}
 }
 
 - (void)testOverreleaseReadonlyIvar {
   [_readonlyProp retain];
   [_readonlyProp release];
   [_readonlyProp release];
-  [_readonlyProp release]; // expected-warning{{used after it is released}}
+  [_readonlyProp release]; // FIXME-warning{{used after it is released}}
 }
 
 - (void)testOverreleaseImplicitManualIvar {
   [_implicitManualProp retain];
   [_implicitManualProp release];
   [_implicitManualProp release];
-  [_implicitManualProp release]; // expected-warning{{used after it is released}}
+  [_implicitManualProp release]; // FIXME-warning{{used after it is released}}
 }
 
 - (void)testOverreleaseImplicitSynthIvar {
@@ -416,21 +416,21 @@ void testOpaqueConsistency(OpaqueIntWrapper *w) {
   CFRetain(_cfProp);
   CFRelease(_cfProp);
   CFRelease(_cfProp);
-  CFRelease(_cfProp); // expected-warning{{used after it is released}}
+  CFRelease(_cfProp); // FIXME-warning{{used after it is released}}
 }
 
 - (void)testOverreleaseOwnedIvarUse {
   [_ownedProp retain];
   [_ownedProp release];
   [_ownedProp release];
-  [_ownedProp myMethod]; // expected-warning{{used after it is released}}
+  [_ownedProp myMethod]; // FIXME-warning{{used after it is released}}
 }
 
 - (void)testOverreleaseIvarOnlyUse {
   [_ivarOnly retain];
   [_ivarOnly release];
   [_ivarOnly release];
-  [_ivarOnly myMethod]; // expected-warning{{used after it is released}}
+  [_ivarOnly myMethod]; // FIXME-warning{{used after it is released}}
 }
 
 - (void)testOverreleaseCFUse {
@@ -439,7 +439,7 @@ void testOpaqueConsistency(OpaqueIntWrapper *w) {
   CFRelease(_cfProp);
 
   extern void CFUse(CFTypeRef);
-  CFUse(_cfProp); // expected-warning{{used after it is released}}
+  CFUse(_cfProp); // FIXME-warning{{used after it is released}}
 }
 
 - (void)testOverreleaseOwnedIvarAutoreleaseOkay {
@@ -459,14 +459,14 @@ void testOpaqueConsistency(OpaqueIntWrapper *w) {
   [_ownedProp release];
   [_ownedProp autorelease];
   [_ownedProp autorelease];
-} // expected-warning{{Object autoreleased too many times}}
+} // FIXME-warning{{Object autoreleased too many times}}
 
 - (void)testOverreleaseIvarOnlyAutorelease {
   [_ivarOnly retain];
   [_ivarOnly release];
   [_ivarOnly autorelease];
   [_ivarOnly autorelease];
-} // expected-warning{{Object autoreleased too many times}}
+} // FIXME-warning{{Object autoreleased too many times}}
 
 - (void)testPropertyAccessThenReleaseOwned {
   id owned = [self.ownedProp retain];