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

[Valeriy Savchenko via cfe-commits]

Author: Valeriy Savchenko
Date: 2021-04-07T13:44:43+03:00
New Revision: 4821c15691bab9efaef871c957a8ba73697cdda9

URL: https://github.com/llvm/llvm-project/commit/4821c15691bab9efaef871c957a8ba73697cdda9
DIFF: https://github.com/llvm/llvm-project/commit/4821c15691bab9efaef871c957a8ba73697cdda9.diff

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

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

Differential Revision: https://reviews.llvm.org/D99194

Added: 
    

Modified: 
    clang/lib/Analysis/BodyFarm.cpp
    clang/test/Analysis/properties.mm

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/BodyFarm.cpp b/clang/lib/Analysis/BodyFarm.cpp
index 603da67156254..b2110f6b5450c 100644
--- a/clang/lib/Analysis/BodyFarm.cpp
+++ b/clang/lib/Analysis/BodyFarm.cpp
@@ -742,8 +742,9 @@ static const ObjCIvarDecl *findBackingIvar(const ObjCPropertyDecl *Prop) {
 
 static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
                                       const ObjCMethodDecl *MD) {
-    // First, find the backing ivar.
+  // 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
@@ -751,54 +752,57 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
   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;
+  }
 
-    // Ignore weak variables, which have special behavior.
-    if (Prop->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak)
-      return nullptr;
+  if (!IVar || !Prop)
+    return nullptr;
+
+  // 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 =
+  // 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());
-        }
+  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;
   }
 
+  // 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);
@@ -807,11 +811,8 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
   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())

diff  --git a/clang/test/Analysis/properties.mm b/clang/test/Analysis/properties.mm
index 2d93d6dc63856..fc3be967115e1 100644
--- a/clang/test/Analysis/properties.mm
+++ b/clang/test/Analysis/properties.mm
@@ -77,3 +77,23 @@ void testConsistencyCustomCopy(CustomCopyWrapper *w) {
 
   clang_analyzer_eval(w.inner.value == 42); // expected-warning{{TRUE}}
 }
+
+ at protocol NoDirectPropertyDecl
+ at property IntWrapperStruct inner;
+ at end
+ at interface NoDirectPropertyDecl <NoDirectPropertyDecl>
+ at end
+ at implementation NoDirectPropertyDecl
+ at synthesize inner;
+ at 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}}
+}