Author: Artem Dergachev
Date: 2019-11-21T18:59:46-08:00
New Revision: 0b58b80edb81bf8fb401f8a6e057ca9d50abc0f7

URL: 
https://github.com/llvm/llvm-project/commit/0b58b80edb81bf8fb401f8a6e057ca9d50abc0f7
DIFF: 
https://github.com/llvm/llvm-project/commit/0b58b80edb81bf8fb401f8a6e057ca9d50abc0f7.diff

LOG: [analyzer] Fix Objective-C accessor body farms after 2073dd2d.

Fix a canonicalization problem for the newly added property accessor stubs that
was causing a wrong decl to be used for 'self' in the accessor's body farm.

Fix a crash when constructing a body farm for accessors of a property
that is declared and @synthesize'd in different (but related) interfaces.

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

Added: 
    

Modified: 
    clang/lib/Analysis/BodyFarm.cpp
    clang/lib/StaticAnalyzer/Core/CallEvent.cpp
    clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist
    clang/test/Analysis/nullability-notes.m
    clang/test/Analysis/properties.m

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/BodyFarm.cpp b/clang/lib/Analysis/BodyFarm.cpp
index be065ed9330f..694913b3ac93 100644
--- a/clang/lib/Analysis/BodyFarm.cpp
+++ b/clang/lib/Analysis/BodyFarm.cpp
@@ -737,50 +737,65 @@ static const ObjCIvarDecl *findBackingIvar(const 
ObjCPropertyDecl *Prop) {
 }
 
 static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
-                                      const ObjCPropertyDecl *Prop) {
-  // First, find the backing ivar.
-  const ObjCIvarDecl *IVar = findBackingIvar(Prop);
-  if (!IVar)
-    return nullptr;
+                                      const ObjCMethodDecl *MD) {
+    // First, find the backing ivar.
+  const ObjCIvarDecl *IVar = nullptr;
+
+  // Property accessor stubs sometimes do not correspond to any property.
+  if (MD->isSynthesizedAccessorStub()) {
+    const ObjCInterfaceDecl *IntD = MD->getClassInterface();
+    const ObjCImplementationDecl *ImpD = IntD->getImplementation();
+    for (const auto *V: ImpD->ivars()) {
+      if (V->getName() == MD->getSelector().getNameForSlot(0))
+        IVar = V;
+    }
+  }
 
-  // Ignore weak variables, which have special behavior.
-  if (Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak)
-    return nullptr;
+  if (!IVar) {
+    const ObjCPropertyDecl *Prop = MD->findPropertyDecl();
+    IVar = findBackingIvar(Prop);
+    if (!IVar)
+      return nullptr;
+
+    // Ignore weak variables, which have special behavior.
+    if (Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_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());
+    // 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;
+    // 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 = Prop->getGetterMethodDecl()->getSelfDecl();
+  const VarDecl *selfVar = MD->getSelfDecl();
   if (!selfVar)
     return nullptr;
 
@@ -791,7 +806,7 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
         selfVar->getType()),
       IVar);
 
-  if (!Prop->getType()->isReferenceType())
+  if (!MD->getReturnType()->isReferenceType())
     loadedIVar = M.makeLvalueToRvalue(loadedIVar, IVar->getType());
 
   return M.makeReturn(loadedIVar);
@@ -814,10 +829,6 @@ Stmt *BodyFarm::getBody(const ObjCMethodDecl *D) {
     return Val.getValue();
   Val = nullptr;
 
-  const ObjCPropertyDecl *Prop = D->findPropertyDecl();
-  if (!Prop)
-    return nullptr;
-
   // For now, we only synthesize getters.
   // Synthesizing setters would cause false negatives in the
   // RetainCountChecker because the method body would bind the parameter
@@ -840,7 +851,7 @@ Stmt *BodyFarm::getBody(const ObjCMethodDecl *D) {
         return nullptr;
     }
 
-  Val = createObjCPropertyGetter(C, Prop);
+  Val = createObjCPropertyGetter(C, D);
 
   return Val.getValue();
 }

diff  --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp 
b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index e7408b805fa8..b89bbe3f54c5 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -1309,6 +1309,8 @@ RuntimeDefinition ObjCMethodCall::getRuntimeDefinition() 
const {
         }
 
         const ObjCMethodDecl *MD = Val.getValue();
+        if (MD && !MD->hasBody())
+          MD = MD->getCanonicalDecl();
         if (CanBeSubClassed)
           return RuntimeDefinition(MD, Receiver);
         else

diff  --git 
a/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist 
b/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist
index 5e915d63ffde..88cf2fa882ad 100644
--- a/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist
+++ b/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist
@@ -16,12 +16,46 @@
         <key>start</key>
          <array>
           <dict>
-           <key>line</key><integer>16</integer>
+           <key>line</key><integer>31</integer>
            <key>col</key><integer>3</integer>
            <key>file</key><integer>0</integer>
           </dict>
           <dict>
-           <key>line</key><integer>16</integer>
+           <key>line</key><integer>31</integer>
+           <key>col</key><integer>33</integer>
+           <key>file</key><integer>0</integer>
+          </dict>
+         </array>
+        <key>end</key>
+         <array>
+          <dict>
+           <key>line</key><integer>33</integer>
+           <key>col</key><integer>3</integer>
+           <key>file</key><integer>0</integer>
+          </dict>
+          <dict>
+           <key>line</key><integer>33</integer>
+           <key>col</key><integer>10</integer>
+           <key>file</key><integer>0</integer>
+          </dict>
+         </array>
+       </dict>
+      </array>
+    </dict>
+    <dict>
+     <key>kind</key><string>control</string>
+     <key>edges</key>
+      <array>
+       <dict>
+        <key>start</key>
+         <array>
+          <dict>
+           <key>line</key><integer>33</integer>
+           <key>col</key><integer>3</integer>
+           <key>file</key><integer>0</integer>
+          </dict>
+          <dict>
+           <key>line</key><integer>33</integer>
            <key>col</key><integer>10</integer>
            <key>file</key><integer>0</integer>
           </dict>
@@ -29,12 +63,12 @@
         <key>end</key>
          <array>
           <dict>
-           <key>line</key><integer>16</integer>
+           <key>line</key><integer>33</integer>
            <key>col</key><integer>22</integer>
            <key>file</key><integer>0</integer>
           </dict>
           <dict>
-           <key>line</key><integer>16</integer>
+           <key>line</key><integer>33</integer>
            <key>col</key><integer>22</integer>
            <key>file</key><integer>0</integer>
           </dict>
@@ -46,7 +80,7 @@
      <key>kind</key><string>event</string>
      <key>location</key>
      <dict>
-      <key>line</key><integer>16</integer>
+      <key>line</key><integer>33</integer>
       <key>col</key><integer>22</integer>
       <key>file</key><integer>0</integer>
      </dict>
@@ -54,12 +88,12 @@
      <array>
        <array>
         <dict>
-         <key>line</key><integer>16</integer>
+         <key>line</key><integer>33</integer>
          <key>col</key><integer>22</integer>
          <key>file</key><integer>0</integer>
         </dict>
         <dict>
-         <key>line</key><integer>16</integer>
+         <key>line</key><integer>33</integer>
          <key>col</key><integer>22</integer>
          <key>file</key><integer>0</integer>
         </dict>
@@ -79,12 +113,12 @@
         <key>start</key>
          <array>
           <dict>
-           <key>line</key><integer>16</integer>
+           <key>line</key><integer>33</integer>
            <key>col</key><integer>22</integer>
            <key>file</key><integer>0</integer>
           </dict>
           <dict>
-           <key>line</key><integer>16</integer>
+           <key>line</key><integer>33</integer>
            <key>col</key><integer>22</integer>
            <key>file</key><integer>0</integer>
           </dict>
@@ -92,12 +126,12 @@
         <key>end</key>
          <array>
           <dict>
-           <key>line</key><integer>16</integer>
+           <key>line</key><integer>33</integer>
            <key>col</key><integer>3</integer>
            <key>file</key><integer>0</integer>
           </dict>
           <dict>
-           <key>line</key><integer>16</integer>
+           <key>line</key><integer>33</integer>
            <key>col</key><integer>10</integer>
            <key>file</key><integer>0</integer>
           </dict>
@@ -113,12 +147,12 @@
         <key>start</key>
          <array>
           <dict>
-           <key>line</key><integer>16</integer>
+           <key>line</key><integer>33</integer>
            <key>col</key><integer>3</integer>
            <key>file</key><integer>0</integer>
           </dict>
           <dict>
-           <key>line</key><integer>16</integer>
+           <key>line</key><integer>33</integer>
            <key>col</key><integer>10</integer>
            <key>file</key><integer>0</integer>
           </dict>
@@ -126,12 +160,12 @@
         <key>end</key>
          <array>
           <dict>
-           <key>line</key><integer>17</integer>
+           <key>line</key><integer>36</integer>
            <key>col</key><integer>3</integer>
            <key>file</key><integer>0</integer>
           </dict>
           <dict>
-           <key>line</key><integer>17</integer>
+           <key>line</key><integer>36</integer>
            <key>col</key><integer>14</integer>
            <key>file</key><integer>0</integer>
           </dict>
@@ -143,7 +177,7 @@
      <key>kind</key><string>event</string>
      <key>location</key>
      <dict>
-      <key>line</key><integer>17</integer>
+      <key>line</key><integer>36</integer>
       <key>col</key><integer>3</integer>
       <key>file</key><integer>0</integer>
      </dict>
@@ -151,12 +185,12 @@
      <array>
        <array>
         <dict>
-         <key>line</key><integer>17</integer>
+         <key>line</key><integer>36</integer>
          <key>col</key><integer>16</integer>
          <key>file</key><integer>0</integer>
         </dict>
         <dict>
-         <key>line</key><integer>17</integer>
+         <key>line</key><integer>36</integer>
          <key>col</key><integer>16</integer>
          <key>file</key><integer>0</integer>
         </dict>
@@ -177,10 +211,10 @@
    
<key>issue_hash_content_of_line_in_context</key><string>ff735bea0eb12d4d172b139143c32365</string>
   <key>issue_context_kind</key><string>Objective-C method</string>
   <key>issue_context</key><string>method</string>
-  <key>issue_hash_function_offset</key><string>3</string>
+  <key>issue_hash_function_offset</key><string>6</string>
   <key>location</key>
   <dict>
-   <key>line</key><integer>17</integer>
+   <key>line</key><integer>36</integer>
    <key>col</key><integer>3</integer>
    <key>file</key><integer>0</integer>
   </dict>
@@ -188,9 +222,11 @@
   <dict>
    <key>0</key>
    <array>
-    <integer>14</integer>
-    <integer>16</integer>
-    <integer>17</integer>
+    <integer>26</integer>
+    <integer>30</integer>
+    <integer>31</integer>
+    <integer>33</integer>
+    <integer>36</integer>
    </array>
   </dict>
   </dict>

diff  --git a/clang/test/Analysis/nullability-notes.m 
b/clang/test/Analysis/nullability-notes.m
index 39dbb8fd7222..e1b4e8f3d5ce 100644
--- a/clang/test/Analysis/nullability-notes.m
+++ b/clang/test/Analysis/nullability-notes.m
@@ -1,6 +1,22 @@
-// RUN: %clang_analyze_cc1 -fblocks 
-analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced
 -analyzer-output=text -verify %s
-// RUN: %clang_analyze_cc1 -fblocks 
-analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced
 -analyzer-output=plist -o %t.plist %s
-// RUN: %normalize_plist <%t.plist | 
diff  -ub %S/Inputs/expected-plists/nullability-notes.m.plist -
+// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core \
+// RUN:   -analyzer-checker=nullability.NullPassedToNonnull \
+// RUN:   -analyzer-checker=nullability.NullReturnedFromNonnull \
+// RUN:   -analyzer-checker=nullability.NullablePassedToNonnull \
+// RUN:   -analyzer-checker=nullability.NullableReturnedFromNonnull \
+// RUN:   -analyzer-checker=nullability.NullableDereferenced \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core \
+// RUN:   -analyzer-checker=nullability.NullPassedToNonnull \
+// RUN:   -analyzer-checker=nullability.NullReturnedFromNonnull \
+// RUN:   -analyzer-checker=nullability.NullablePassedToNonnull \
+// RUN:   -analyzer-checker=nullability.NullableReturnedFromNonnull \
+// RUN:   -analyzer-checker=nullability.NullableDereferenced \
+// RUN:   -analyzer-output=plist -o %t.plist %s
+// RUN: %normalize_plist <%t.plist \
+// RUN:   | 
diff  -ub %S/Inputs/expected-plists/nullability-notes.m.plist -
+
+void clang_analyzer_warnOnDeadSymbol(id);
 
 #include "Inputs/system-header-simulator-for-nullability.h"
 
@@ -12,8 +28,11 @@ -(void) method;
 @end;
 @implementation ClassWithProperties
 -(void) method {
+  clang_analyzer_warnOnDeadSymbol(self);
   // no-crash
   NSObject *x = self.x; // expected-note{{Nullability 'nullable' is inferred}}
+                        // expected-warning@-1{{SYMBOL DEAD}}
+                        // expected-note@-2   {{SYMBOL DEAD}}
   takesNonnull(x); // expected-warning{{Nullable pointer is passed to a callee 
that requires a non-null 1st parameter}}
                    // expected-note@-1{{Nullable pointer is passed to a callee 
that requires a non-null 1st parameter}}
 }

diff  --git a/clang/test/Analysis/properties.m 
b/clang/test/Analysis/properties.m
index 17b156035a34..2f427f275182 100644
--- a/clang/test/Analysis/properties.m
+++ b/clang/test/Analysis/properties.m
@@ -3,6 +3,8 @@
 
 void clang_analyzer_eval(int);
 
+#define nil ((id)0)
+
 typedef const void * CFTypeRef;
 extern CFTypeRef CFRetain(CFTypeRef cf);
 void CFRelease(CFTypeRef cf);
@@ -1040,3 +1042,48 @@ - (void)foo {
        clang_analyzer_eval(self.still_no_custom_accessor == 
self.still_no_custom_accessor); // expected-warning{{TRUE}}
 }
 @end
+
+@interface Shadowed
+@property (assign) NSObject *o;
+- (NSObject *)getShadowedIvar;
+- (void)clearShadowedIvar;
+- (NSObject *)getShadowedProp;
+- (void)clearShadowedProp;
+@end
+
+@implementation Shadowed
+- (NSObject *)getShadowedIvar {
+  return self->_o;
+}
+- (void)clearShadowedIvar {
+  self->_o = nil;
+}
+- (NSObject *)getShadowedProp {
+  return self.o;
+}
+- (void)clearShadowedProp {
+  self.o = nil;
+}
+@end
+
+@interface Shadowing : Shadowed
+@end
+
+@implementation Shadowing
+// Property 'o' is declared in the superclass but synthesized here.
+// This creates a separate ivar that shadows the superclass's ivar,
+// but the old ivar is still accessible from the methods of the superclass.
+// The old property, however, is not accessible with the property syntax
+// even from the superclass methods.
+@synthesize o;
+
+-(void)testPropertyShadowing {
+  NSObject *oo = self.o;
+  clang_analyzer_eval(self.o == oo); // expected-warning{{TRUE}}
+  clang_analyzer_eval([self getShadowedIvar] == oo); // 
expected-warning{{UNKNOWN}}
+  [self clearShadowedIvar];
+  clang_analyzer_eval(self.o == oo); // expected-warning{{TRUE}}
+  clang_analyzer_eval([self getShadowedIvar] == oo); // 
expected-warning{{UNKNOWN}}
+  clang_analyzer_eval([self getShadowedIvar] == nil); // 
expected-warning{{TRUE}}
+}
+@end


        
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to