[analyzer] Fix body farm for Obj-C++ properties

[vsavchenko]

Summary

When property is declared in a superclass (or in a protocol),
it still can be of CXXRecord type and Sema could've already
generated a body for us. This patch joins two branches and
two ways of acquiring IVar in order to reuse the existing code.
And prevent us from generating l-value to r-value casts for
C++ types.

rdar://67416721

[steakhal]

I'm not familiar with Objective-C.

[NoQ]

clang/lib/Analysis/BodyFarm.cpp

757

At this point Prop may contain a completely unrelated property decl. It doesn't necessarily correspond to the IVar or have the right name. We keep it after the end of the loop and use it later. This doesn't sound right.

[vsavchenko]

clang/lib/Analysis/BodyFarm.cpp

757

It is actually intentional. To reuse that bit of code that used to be under if(!IVar) condition.

[vsavchenko]

Set Prop and IVar at the same time

[NoQ]

Great, thanks!~

clang/lib/Analysis/BodyFarm.cpp

if (ShadowingProp && ShadowingProp != Prop) {
     IVar = ShadowingProp->getPropertyIvarDecl();
   }
 
   return IVar;
 }
 
 static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
                                       const ObjCMethodDecl *MD) {
-    // First, find the backing ivar.
   const ObjCIvarDecl *IVar = nullptr;
+  const ObjCPropertyDecl *Prop = nullptr;
 
   // Property accessor stubs sometimes do not correspond to any property decl
   // in the current interface (but in a superclass). They still have a
   // corresponding property impl decl in this case.
   if (MD->isSynthesizedAccessorStub()) {
     const ObjCInterfaceDecl *IntD = MD->getClassInterface();
     const ObjCImplementationDecl *ImpD = IntD->getImplementation();
-    for (const auto *PI: ImpD->property_impls()) {
-      if (const ObjCPropertyDecl *P = PI->getPropertyDecl()) {
-        if (P->getGetterName() == MD->getSelector())
-          IVar = P->getPropertyIvarDecl();
+    for (const auto *PI : ImpD->property_impls()) {
+      if (const ObjCPropertyDecl *Candidate = PI->getPropertyDecl()) {
+        if (Candidate->getGetterName() == MD->getSelector()) {
+          Prop = Candidate;
+          IVar = Prop->getPropertyIvarDecl();
+        }
       }
     }
   }
 
   if (!IVar) {
-    const ObjCPropertyDecl *Prop = MD->findPropertyDecl();
+    Prop = MD->findPropertyDecl();
     IVar = findBackingIvar(Prop);
-    if (!IVar)
-      return nullptr;
+  }
+
+  if (!IVar || !Prop)
 
-    // Ignore weak variables, which have special behavior.
-    if (Prop->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak)
-      return nullptr;
 
-    // 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) {
-      for (const auto *I : ImplDecl->property_impls()) {
-        if (I->getPropertyDecl() != Prop)
-          continue;
 
-        if (I->getGetterCXXConstructor()) {
-          ASTMaker M(Ctx);
-          return M.makeReturn(I->getGetterCXXConstructor());
-        }
-      }
-    }
 
-    // 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 nullptr;
-    if (!IVar->getType()->isObjCLifetimeType() &&
-        !IVar->getType().isTriviallyCopyableType(Ctx))
-      return nullptr;
-  }
 
   // Generate our body:
   //   return self->_ivar;
   ASTMaker M(Ctx);
 
   const VarDecl *selfVar = MD->getSelfDecl();
   if (!selfVar)
     return nullptr;
 
-  Expr *loadedIVar =
-    M.makeObjCIvarRef(
-      M.makeLvalueToRvalue(
-        M.makeDeclRefExpr(selfVar),
-        selfVar->getType()),
+  Expr *loadedIVar = M.makeObjCIvarRef(
+      M.makeLvalueToRvalue(M.makeDeclRefExpr(selfVar), selfVar->getType()),
       IVar);
 
   if (!MD->getReturnType()->isReferenceType())
     loadedIVar = M.makeLvalueToRvalue(loadedIVar, IVar->getType());
 
   return M.makeReturn(loadedIVar);
 }
 

clang/test/Analysis/properties.mm

void testConsistencyCustomCopy(CustomCopyWrapper *w) {
   clang_analyzer_eval(w.inner.value == w.inner.value); // expected-warning{{TRUE}}
 
   int origValue = w.inner.value;
   if (origValue != 42)
     return;
 
   clang_analyzer_eval(w.inner.value == 42); // expected-warning{{TRUE}}
 }
+
+@protocol NoDirectPropertyDecl
+@property IntWrapperStruct inner;
+@end
+@interface NoDirectPropertyDecl <NoDirectPropertyDecl>
+@end
+@implementation NoDirectPropertyDecl
+@synthesize inner;
+@end
+
+// rdar://67416721
+void testNoDirectPropertyDecl(NoDirectPropertyDecl *w) {
+  clang_analyzer_eval(w.inner.value == w.inner.value); // expected-warning{{TRUE}}
+
+  int origValue = w.inner.value;
+  if (origValue != 42)
+    return;
+
+  clang_analyzer_eval(w.inner.value == 42); // expected-warning{{TRUE}}
+}