[analyzer] Tighten up sanity checks on Objective-C property getter synthesis.

If there are non-trivially-copyable types /other/ than C++ records, we
won't have a synthesized copy expression, but we can't just use a simple
load/return.

Also, add comments and shore up tests, making sure to test in both ARC
and non-ARC.

llvm-svn: 199869

clang/lib/Analysis/BodyFarm.cpp

@@ -387,12 +387,20 @@ Stmt *BodyFarm::getBody(const FunctionDecl *D) {
 
 static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
                                       const ObjCPropertyDecl *Prop) {
+  // First, find the backing ivar.
   const ObjCIvarDecl *IVar = Prop->getPropertyIvarDecl();
   if (!IVar)
     return 0;
+
+  // Ignore weak variables, which have special behavior.
   if (Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak)
     return 0;
 
+  // Look to see if Sema has synthesized a body for us. This happens in
+  // Objective-C++ because the return value may be a C++ class type with a
+  // non-trivial copy constructor. We can only do this if we can find the
+  // @synthesize for this property, though (or if we know it's been auto-
+  // synthesized).
   const ObjCImplementationDecl *ImplDecl =
     IVar->getContainingInterface()->getImplementation();
   if (ImplDecl) {
@@ -410,10 +418,18 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
     }
   }
 
-  if (IVar->getType().getCanonicalType() !=
-      Prop->getType().getNonReferenceType().getCanonicalType())
+  // Sanity check that the property is the same type as the ivar, or a
+  // reference to it, and that it is either an object pointer or trivially
+  // copyable.
+  if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
+                                  Prop->getType().getNonReferenceType()))
+    return 0;
+  if (!IVar->getType()->isObjCLifetimeType() &&
+      !IVar->getType().isTriviallyCopyableType(Ctx))
     return 0;
 
+  // Generate our body:
+  //   return self->_ivar;
   ASTMaker M(Ctx);
 
   const VarDecl *selfVar = Prop->getGetterMethodDecl()->getSelfDecl();
@@ -431,7 +447,8 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
   return M.makeReturn(loadedIVar);
 }
 
-Stmt *BodyFarm::getBody(const ObjCMethodDecl *D, const ObjCPropertyDecl *Prop) {
+Stmt *BodyFarm::getBody(const ObjCMethodDecl *D) {
+  // We currently only know how to synthesize property accessors.
   if (!D->isPropertyAccessor())
     return 0;
 
@@ -442,11 +459,11 @@ Stmt *BodyFarm::getBody(const ObjCMethodDecl *D, const ObjCPropertyDecl *Prop) {
     return Val.getValue();
   Val = 0;
 
-  if (!Prop)
-    Prop = D->findPropertyDecl();
+  const ObjCPropertyDecl *Prop = D->findPropertyDecl();
   if (!Prop)
     return 0;
 
+  // For now, we only synthesize getters.
   if (D->param_size() != 0)
     return 0;
 

clang/lib/Analysis/BodyFarm.h

@@ -36,7 +36,7 @@ public:
   Stmt *getBody(const FunctionDecl *D);
 
   /// Factory method for creating bodies for Objective-C properties.
-  Stmt *getBody(const ObjCMethodDecl *D, const ObjCPropertyDecl *Prop = 0);
+  Stmt *getBody(const ObjCMethodDecl *D);
 
 private:
   typedef llvm::DenseMap<const Decl *, Optional<Stmt *> > BodyMap;

clang/test/Analysis/properties.m

@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class -fobjc-arc %s
 
 void clang_analyzer_eval(int);
 
@@ -39,6 +40,8 @@ typedef struct _NSZone NSZone;
 @end
 
 
+#if !__has_feature(objc_arc)
+
 @implementation Test1
 
 @synthesize text;
@@ -117,6 +120,8 @@ NSNumber* numberFromMyNumberProperty(MyNumber* aMyNumber)
   return [result autorelease]; // expected-warning {{Object autoreleased too many times}}
 }
 
+#endif
+
 
 // rdar://6611873
 
@@ -124,18 +129,23 @@ NSNumber* numberFromMyNumberProperty(MyNumber* aMyNumber)
   NSString *_name;
 }
 @property (retain) NSString * name;
+@property (assign) id friend;
 @end
 
 @implementation Person
 @synthesize name = _name;
 @end
 
+#if !__has_feature(objc_arc)
 void rdar6611873() {
   Person *p = [[[Person alloc] init] autorelease];
   
   p.name = [[NSString string] retain]; // expected-warning {{leak}}
   p.name = [[NSString alloc] init]; // expected-warning {{leak}}
+
+  p.friend = [[Person alloc] init]; // expected-warning {{leak}}
 }
+#endif
 
 @interface SubPerson : Person
 -(NSString *)foo;
@@ -147,6 +157,8 @@ void rdar6611873() {
 }
 @end
 
+
+#if !__has_feature(objc_arc)
 // <rdar://problem/9241180> Static analyzer doesn't detect uninitialized variable issues for property accesses
 @interface RDar9241180
 @property (readwrite,assign) id x;
@@ -167,13 +179,14 @@ void rdar6611873() {
   self.x = y;  // expected-warning {{Argument for property setter is an uninitialized value}}
 }
 @end
+#endif
 
 
 //------
 // Property accessor synthesis
 //------
 
-void testConsistency(Person *p) {
+void testConsistencyRetain(Person *p) {
   clang_analyzer_eval(p.name == p.name); // expected-warning{{TRUE}}
 
   extern void doSomethingWithPerson(Person *p);
@@ -183,10 +196,25 @@ void testConsistency(Person *p) {
   clang_analyzer_eval(p.name == origName); // expected-warning{{UNKNOWN}}
 }
 
-void testOverrelease(Person *p) {
-  [p.name release]; // expected-warning{{not owned}}
+void testConsistencyAssign(Person *p) {
+  clang_analyzer_eval(p.friend == p.friend); // expected-warning{{TRUE}}
+
+  extern void doSomethingWithPerson(Person *p);
+  id origFriend = p.friend;
+  clang_analyzer_eval(p.friend == origFriend); // expected-warning{{TRUE}}
+  doSomethingWithPerson(p);
+  clang_analyzer_eval(p.friend == origFriend); // expected-warning{{UNKNOWN}}
 }
 
+#if !__has_feature(objc_arc)
+void testOverrelease(Person *p, int coin) {
+  if (coin)
+    [p.name release]; // expected-warning{{not owned}}
+  else
+    [p.friend release]; // expected-warning{{not owned}}
+}
+#endif
+
 @interface IntWrapper
 @property int value;
 @end
@@ -212,6 +240,32 @@ void testConsistencyInt2(IntWrapper *w) {
   clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}}
 }
 
+
+@interface IntWrapperAuto
+@property int value;
+@end
+
+@implementation IntWrapperAuto
+@end
+
+void testConsistencyIntAuto(IntWrapperAuto *w) {
+  clang_analyzer_eval(w.value == w.value); // expected-warning{{TRUE}}
+
+  int origValue = w.value;
+  if (origValue != 42)
+    return;
+
+  clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}}
+}
+
+void testConsistencyIntAuto2(IntWrapperAuto *w) {
+  if (w.value != 42)
+    return;
+
+  clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}}
+}
+
+
 typedef struct {
   int value;
 } IntWrapperStruct;

clang/test/Analysis/properties.mm

@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class -fobjc-arc %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_checkInlined(bool);
@@ -27,8 +28,6 @@ void testReferenceAssignment(IntWrapper *w) {
 }
 
 
-// FIXME: Handle C++ structs, which need to go through the copy constructor.
-
 struct IntWrapperStruct {
   int value;
 };
@@ -66,7 +65,7 @@ public:
 @end
 
 @implementation CustomCopyWrapper
-@synthesize inner;
+//@synthesize inner;
 @end
 
 void testConsistencyCustomCopy(CustomCopyWrapper *w) {