[analyzer] Improve pattern matching in ObjCDealloc checker.

Look through PseudoObjectExpr and OpaqueValueExprs when scanning for
release-like operations. This commit also adds additional tests in anticipation
of re-writing this as a path-sensitive checker.

llvm-svn: 260608

3 files changed, 86 insertions, 8 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
index 1a3519307a2..902babfe502 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -28,6 +28,16 @@
 using namespace clang;
 using namespace ento;
 
+// FIXME: This was taken from IvarInvalidationChecker.cpp
+static const Expr *peel(const Expr *E) {
+  E = E->IgnoreParenCasts();
+  if (const PseudoObjectExpr *POE = dyn_cast<PseudoObjectExpr>(E))
+    E = POE->getSyntacticForm()->IgnoreParenCasts();
+  if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(E))
+    E = OVE->getSourceExpr()->IgnoreParenCasts();
+  return E;
+}
+
 static bool scan_ivar_release(Stmt *S, const ObjCIvarDecl *ID,
                               const ObjCPropertyDecl *PD,
                               Selector Release,
@@ -38,30 +48,29 @@ static bool scan_ivar_release(Stmt *S, const ObjCIvarDecl *ID,
   if (ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(S))
     if (ME->getSelector() == Release)
       if (ME->getInstanceReceiver())
-        if (Expr *Receiver = ME->getInstanceReceiver()->IgnoreParenCasts())
-          if (ObjCIvarRefExpr *E = dyn_cast<ObjCIvarRefExpr>(Receiver))
+        if (const Expr *Receiver = peel(ME->getInstanceReceiver()))
+          if (auto *E = dyn_cast<ObjCIvarRefExpr>(Receiver))
             if (E->getDecl() == ID)
               return true;
 
   // [self setMyIvar:nil];
   if (ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(S))
     if (ME->getInstanceReceiver())
-      if (Expr *Receiver = ME->getInstanceReceiver()->IgnoreParenCasts())
-        if (DeclRefExpr *E = dyn_cast<DeclRefExpr>(Receiver))
+      if (const Expr *Receiver = peel(ME->getInstanceReceiver()))
+        if (auto *E = dyn_cast<DeclRefExpr>(Receiver))
           if (E->getDecl()->getIdentifier() == SelfII)
             if (ME->getMethodDecl() == PD->getSetterMethodDecl() &&
                 ME->getNumArgs() == 1 &&
-                ME->getArg(0)->isNullPointerConstant(Ctx,
+                peel(ME->getArg(0))->isNullPointerConstant(Ctx,
                                               Expr::NPC_ValueDependentIsNull))
               return true;
 
   // self.myIvar = nil;
   if (BinaryOperator* BO = dyn_cast<BinaryOperator>(S))
     if (BO->isAssignmentOp())
-      if (ObjCPropertyRefExpr *PRE =
-           dyn_cast<ObjCPropertyRefExpr>(BO->getLHS()->IgnoreParenCasts()))
+      if (auto *PRE = dyn_cast<ObjCPropertyRefExpr>(peel(BO->getLHS())))
         if (PRE->isExplicitProperty() && PRE->getExplicitProperty() == PD)
-            if (BO->getRHS()->isNullPointerConstant(Ctx,
+            if (peel(BO->getRHS())->isNullPointerConstant(Ctx,
                                             Expr::NPC_ValueDependentIsNull)) {
               // This is only a 'release' if the property kind is not
               // 'assign'.
diff --git a/clang/test/Analysis/DeallocMissingRelease.m b/clang/test/Analysis/DeallocMissingRelease.m
index 3a2b556c11d..e782f9968f9 100644
--- a/clang/test/Analysis/DeallocMissingRelease.m
+++ b/clang/test/Analysis/DeallocMissingRelease.m
@@ -189,4 +189,64 @@ typedef struct objc_selector *SEL;
 #endif
 }
 @end
+
+@interface ClassWithControlFlowInRelease : NSObject {
+  BOOL _ivar1;
+}
+@property (retain) NSObject *ivar2;
+@end
+
+@implementation ClassWithControlFlowInRelease
+- (void)dealloc; {
+  if (_ivar1) {
+    // We really should warn because there is a path through -dealloc on which
+    // _ivar2 is not released.
+#if NON_ARC
+    [_ivar2 release]; // no-warning
+#endif
+  }
+
+#if NON_ARC
+  [super dealloc];
+#endif
+}
+
+@end
+
+//===------------------------------------------------------------------------===
+// Don't warn when the property is nil'd out in -dealloc
+
+@interface ClassWithNildOutProperty : NSObject
+@property (retain) NSObject *ivar;  // no-warning
+@end
+
+@implementation ClassWithNildOutProperty
+- (void)dealloc; {
+  self.ivar = nil;
+
+#if NON_ARC
+  [super dealloc];
+#endif
+}
+
+@end
+
+//===------------------------------------------------------------------------===
+// Don't warn when the property is nil'd out with a setter in -dealloc
+
+@interface ClassWithNildOutPropertyViaSetter : NSObject
+@property (retain) NSObject *ivar;  // no-warning
+@end
+
+@implementation ClassWithNildOutPropertyViaSetter
+- (void)dealloc; {
+  [self setIvar:nil];
+
+#if NON_ARC
+  [super dealloc];
+#endif
+}
+
+@end
+
 // CHECK: 4 warnings generated.
diff --git a/clang/test/Analysis/PR2978.m b/clang/test/Analysis/PR2978.m
index 1fbd9723a2b..2067b3e85af 100644
--- a/clang/test/Analysis/PR2978.m
+++ b/clang/test/Analysis/PR2978.m
@@ -17,6 +17,8 @@
   id _L;
   id _N;
   id _M;
+  id _P;
+  id _Q;
   id _V;
   id _W;
 }
@@ -27,6 +29,9 @@
 @property(weak) id L;
 @property(readonly) id N;
 @property(retain) id M;
+@property(weak) id P; // expected-warning {{'_P' instance variable in 'MyClass' was not retained by a synthesized property but was released in 'dealloc'}}
+@property(weak) id Q;
+
 @property(retain) id V;
 @property(retain) id W;
 -(id) O;
@@ -41,6 +46,7 @@
 @synthesize L = _L; // no-warning
 @synthesize N = _N; // no-warning
 @synthesize M = _M;
+@synthesize Q = _Q; // expected-warning {{'_Q' instance variable in 'MyClass' was not retained by a synthesized property but was released in 'dealloc'}}
 @synthesize V = _V;
 @synthesize W = _W; // expected-warning{{The '_W' instance variable in 'MyClass' was retained by a synthesized property but was not released in 'dealloc'}}
 
@@ -57,6 +63,9 @@
   [self setV:0]; // This will release '_V'
   [self setW:@"newW"]; // This will release '_W', but retain the new value
   self.O = 0; // no-warning  
+
+  [_Q release];
+  self.P = 0;
   [super dealloc];
   return 0;
 }