[clang] 0b58b80 - [analyzer] Fix Objective-C accessor body farms after 2073dd2d.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 21 18:59:56 PST 2019
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 at -1{{SYMBOL DEAD}}
+ // expected-note at -2 {{SYMBOL DEAD}}
takesNonnull(x); // expected-warning{{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
// expected-note at -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
+
+ at interface Shadowed
+ at property (assign) NSObject *o;
+- (NSObject *)getShadowedIvar;
+- (void)clearShadowedIvar;
+- (NSObject *)getShadowedProp;
+- (void)clearShadowedProp;
+ at end
+
+ at implementation Shadowed
+- (NSObject *)getShadowedIvar {
+ return self->_o;
+}
+- (void)clearShadowedIvar {
+ self->_o = nil;
+}
+- (NSObject *)getShadowedProp {
+ return self.o;
+}
+- (void)clearShadowedProp {
+ self.o = nil;
+}
+ at end
+
+ at interface Shadowing : Shadowed
+ at end
+
+ at 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.
+ at 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}}
+}
+ at end
More information about the cfe-commits
mailing list