Skip to content

Commit 0b58b80

Browse files
haoNoQhaoNoQ
committed
[analyzer] Fix Objective-C accessor body farms after 2073dd2.
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
1 parent bbc8662 commit 0b58b80

File tree

5 files changed

+181
-66
lines changed

5 files changed

+181
-66
lines changed

‎clang/lib/Analysis/BodyFarm.cpp

Lines changed: 51 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -737,50 +737,65 @@ static const ObjCIvarDecl *findBackingIvar(const ObjCPropertyDecl *Prop) {
737737
}
738738

739739
static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
740-
const ObjCPropertyDecl *Prop) {
741-
// First, find the backing ivar.
742-
const ObjCIvarDecl *IVar = findBackingIvar(Prop);
743-
if (!IVar)
744-
return nullptr;
740+
const ObjCMethodDecl *MD) {
741+
// First, find the backing ivar.
742+
const ObjCIvarDecl *IVar = nullptr;
743+
744+
// Property accessor stubs sometimes do not correspond to any property.
745+
if (MD->isSynthesizedAccessorStub()) {
746+
const ObjCInterfaceDecl *IntD = MD->getClassInterface();
747+
const ObjCImplementationDecl *ImpD = IntD->getImplementation();
748+
for (const auto *V: ImpD->ivars()) {
749+
if (V->getName() == MD->getSelector().getNameForSlot(0))
750+
IVar = V;
751+
}
752+
}
745753

746-
// Ignore weak variables, which have special behavior.
747-
if (Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak)
748-
return nullptr;
754+
if (!IVar) {
755+
const ObjCPropertyDecl *Prop = MD->findPropertyDecl();
756+
IVar = findBackingIvar(Prop);
757+
if (!IVar)
758+
return nullptr;
759+
760+
// Ignore weak variables, which have special behavior.
761+
if (Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak)
762+
return nullptr;
749763

750-
// Look to see if Sema has synthesized a body for us. This happens in
751-
// Objective-C++ because the return value may be a C++ class type with a
752-
// non-trivial copy constructor. We can only do this if we can find the
753-
// @synthesize for this property, though (or if we know it's been auto-
754-
// synthesized).
755-
const ObjCImplementationDecl *ImplDecl =
756-
IVar->getContainingInterface()->getImplementation();
757-
if (ImplDecl) {
758-
for (const auto *I : ImplDecl->property_impls()) {
759-
if (I->getPropertyDecl() != Prop)
760-
continue;
761-
762-
if (I->getGetterCXXConstructor()) {
763-
ASTMaker M(Ctx);
764-
return M.makeReturn(I->getGetterCXXConstructor());
764+
// Look to see if Sema has synthesized a body for us. This happens in
765+
// Objective-C++ because the return value may be a C++ class type with a
766+
// non-trivial copy constructor. We can only do this if we can find the
767+
// @synthesize for this property, though (or if we know it's been auto-
768+
// synthesized).
769+
const ObjCImplementationDecl *ImplDecl =
770+
IVar->getContainingInterface()->getImplementation();
771+
if (ImplDecl) {
772+
for (const auto *I : ImplDecl->property_impls()) {
773+
if (I->getPropertyDecl() != Prop)
774+
continue;
775+
776+
if (I->getGetterCXXConstructor()) {
777+
ASTMaker M(Ctx);
778+
return M.makeReturn(I->getGetterCXXConstructor());
779+
}
765780
}
766781
}
767-
}
768782

769-
// Sanity check that the property is the same type as the ivar, or a
770-
// reference to it, and that it is either an object pointer or trivially
771-
// copyable.
772-
if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
773-
Prop->getType().getNonReferenceType()))
774-
return nullptr;
775-
if (!IVar->getType()->isObjCLifetimeType() &&
776-
!IVar->getType().isTriviallyCopyableType(Ctx))
777-
return nullptr;
783+
// Sanity check that the property is the same type as the ivar, or a
784+
// reference to it, and that it is either an object pointer or trivially
785+
// copyable.
786+
if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
787+
Prop->getType().getNonReferenceType()))
788+
return nullptr;
789+
if (!IVar->getType()->isObjCLifetimeType() &&
790+
!IVar->getType().isTriviallyCopyableType(Ctx))
791+
return nullptr;
792+
}
778793

779794
// Generate our body:
780795
// return self->_ivar;
781796
ASTMaker M(Ctx);
782797

783-
const VarDecl *selfVar = Prop->getGetterMethodDecl()->getSelfDecl();
798+
const VarDecl *selfVar = MD->getSelfDecl();
784799
if (!selfVar)
785800
return nullptr;
786801

@@ -791,7 +806,7 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
791806
selfVar->getType()),
792807
IVar);
793808

794-
if (!Prop->getType()->isReferenceType())
809+
if (!MD->getReturnType()->isReferenceType())
795810
loadedIVar = M.makeLvalueToRvalue(loadedIVar, IVar->getType());
796811

797812
return M.makeReturn(loadedIVar);
@@ -814,10 +829,6 @@ Stmt *BodyFarm::getBody(const ObjCMethodDecl *D) {
814829
return Val.getValue();
815830
Val = nullptr;
816831

817-
const ObjCPropertyDecl *Prop = D->findPropertyDecl();
818-
if (!Prop)
819-
return nullptr;
820-
821832
// For now, we only synthesize getters.
822833
// Synthesizing setters would cause false negatives in the
823834
// RetainCountChecker because the method body would bind the parameter
@@ -840,7 +851,7 @@ Stmt *BodyFarm::getBody(const ObjCMethodDecl *D) {
840851
return nullptr;
841852
}
842853

843-
Val = createObjCPropertyGetter(C, Prop);
854+
Val = createObjCPropertyGetter(C, D);
844855

845856
return Val.getValue();
846857
}

‎clang/lib/StaticAnalyzer/Core/CallEvent.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,6 +1309,8 @@ RuntimeDefinition ObjCMethodCall::getRuntimeDefinition() const {
13091309
}
13101310

13111311
const ObjCMethodDecl *MD = Val.getValue();
1312+
if (MD && !MD->hasBody())
1313+
MD = MD->getCanonicalDecl();
13121314
if (CanBeSubClassed)
13131315
return RuntimeDefinition(MD, Receiver);
13141316
else

‎clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist

Lines changed: 59 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,59 @@
1616
<key>start</key>
1717
<array>
1818
<dict>
19-
<key>line</key><integer>16</integer>
19+
<key>line</key><integer>31</integer>
2020
<key>col</key><integer>3</integer>
2121
<key>file</key><integer>0</integer>
2222
</dict>
2323
<dict>
24-
<key>line</key><integer>16</integer>
24+
<key>line</key><integer>31</integer>
25+
<key>col</key><integer>33</integer>
26+
<key>file</key><integer>0</integer>
27+
</dict>
28+
</array>
29+
<key>end</key>
30+
<array>
31+
<dict>
32+
<key>line</key><integer>33</integer>
33+
<key>col</key><integer>3</integer>
34+
<key>file</key><integer>0</integer>
35+
</dict>
36+
<dict>
37+
<key>line</key><integer>33</integer>
38+
<key>col</key><integer>10</integer>
39+
<key>file</key><integer>0</integer>
40+
</dict>
41+
</array>
42+
</dict>
43+
</array>
44+
</dict>
45+
<dict>
46+
<key>kind</key><string>control</string>
47+
<key>edges</key>
48+
<array>
49+
<dict>
50+
<key>start</key>
51+
<array>
52+
<dict>
53+
<key>line</key><integer>33</integer>
54+
<key>col</key><integer>3</integer>
55+
<key>file</key><integer>0</integer>
56+
</dict>
57+
<dict>
58+
<key>line</key><integer>33</integer>
2559
<key>col</key><integer>10</integer>
2660
<key>file</key><integer>0</integer>
2761
</dict>
2862
</array>
2963
<key>end</key>
3064
<array>
3165
<dict>
32-
<key>line</key><integer>16</integer>
66+
<key>line</key><integer>33</integer>
3367
<key>col</key><integer>22</integer>
3468
<key>file</key><integer>0</integer>
3569
</dict>
3670
<dict>
37-
<key>line</key><integer>16</integer>
71+
<key>line</key><integer>33</integer>
3872
<key>col</key><integer>22</integer>
3973
<key>file</key><integer>0</integer>
4074
</dict>
@@ -46,20 +80,20 @@
4680
<key>kind</key><string>event</string>
4781
<key>location</key>
4882
<dict>
49-
<key>line</key><integer>16</integer>
83+
<key>line</key><integer>33</integer>
5084
<key>col</key><integer>22</integer>
5185
<key>file</key><integer>0</integer>
5286
</dict>
5387
<key>ranges</key>
5488
<array>
5589
<array>
5690
<dict>
57-
<key>line</key><integer>16</integer>
91+
<key>line</key><integer>33</integer>
5892
<key>col</key><integer>22</integer>
5993
<key>file</key><integer>0</integer>
6094
</dict>
6195
<dict>
62-
<key>line</key><integer>16</integer>
96+
<key>line</key><integer>33</integer>
6397
<key>col</key><integer>22</integer>
6498
<key>file</key><integer>0</integer>
6599
</dict>
@@ -79,25 +113,25 @@
79113
<key>start</key>
80114
<array>
81115
<dict>
82-
<key>line</key><integer>16</integer>
116+
<key>line</key><integer>33</integer>
83117
<key>col</key><integer>22</integer>
84118
<key>file</key><integer>0</integer>
85119
</dict>
86120
<dict>
87-
<key>line</key><integer>16</integer>
121+
<key>line</key><integer>33</integer>
88122
<key>col</key><integer>22</integer>
89123
<key>file</key><integer>0</integer>
90124
</dict>
91125
</array>
92126
<key>end</key>
93127
<array>
94128
<dict>
95-
<key>line</key><integer>16</integer>
129+
<key>line</key><integer>33</integer>
96130
<key>col</key><integer>3</integer>
97131
<key>file</key><integer>0</integer>
98132
</dict>
99133
<dict>
100-
<key>line</key><integer>16</integer>
134+
<key>line</key><integer>33</integer>
101135
<key>col</key><integer>10</integer>
102136
<key>file</key><integer>0</integer>
103137
</dict>
@@ -113,25 +147,25 @@
113147
<key>start</key>
114148
<array>
115149
<dict>
116-
<key>line</key><integer>16</integer>
150+
<key>line</key><integer>33</integer>
117151
<key>col</key><integer>3</integer>
118152
<key>file</key><integer>0</integer>
119153
</dict>
120154
<dict>
121-
<key>line</key><integer>16</integer>
155+
<key>line</key><integer>33</integer>
122156
<key>col</key><integer>10</integer>
123157
<key>file</key><integer>0</integer>
124158
</dict>
125159
</array>
126160
<key>end</key>
127161
<array>
128162
<dict>
129-
<key>line</key><integer>17</integer>
163+
<key>line</key><integer>36</integer>
130164
<key>col</key><integer>3</integer>
131165
<key>file</key><integer>0</integer>
132166
</dict>
133167
<dict>
134-
<key>line</key><integer>17</integer>
168+
<key>line</key><integer>36</integer>
135169
<key>col</key><integer>14</integer>
136170
<key>file</key><integer>0</integer>
137171
</dict>
@@ -143,20 +177,20 @@
143177
<key>kind</key><string>event</string>
144178
<key>location</key>
145179
<dict>
146-
<key>line</key><integer>17</integer>
180+
<key>line</key><integer>36</integer>
147181
<key>col</key><integer>3</integer>
148182
<key>file</key><integer>0</integer>
149183
</dict>
150184
<key>ranges</key>
151185
<array>
152186
<array>
153187
<dict>
154-
<key>line</key><integer>17</integer>
188+
<key>line</key><integer>36</integer>
155189
<key>col</key><integer>16</integer>
156190
<key>file</key><integer>0</integer>
157191
</dict>
158192
<dict>
159-
<key>line</key><integer>17</integer>
193+
<key>line</key><integer>36</integer>
160194
<key>col</key><integer>16</integer>
161195
<key>file</key><integer>0</integer>
162196
</dict>
@@ -177,20 +211,22 @@
177211
<key>issue_hash_content_of_line_in_context</key><string>ff735bea0eb12d4d172b139143c32365</string>
178212
<key>issue_context_kind</key><string>Objective-C method</string>
179213
<key>issue_context</key><string>method</string>
180-
<key>issue_hash_function_offset</key><string>3</string>
214+
<key>issue_hash_function_offset</key><string>6</string>
181215
<key>location</key>
182216
<dict>
183-
<key>line</key><integer>17</integer>
217+
<key>line</key><integer>36</integer>
184218
<key>col</key><integer>3</integer>
185219
<key>file</key><integer>0</integer>
186220
</dict>
187221
<key>ExecutedLines</key>
188222
<dict>
189223
<key>0</key>
190224
<array>
191-
<integer>14</integer>
192-
<integer>16</integer>
193-
<integer>17</integer>
225+
<integer>26</integer>
226+
<integer>30</integer>
227+
<integer>31</integer>
228+
<integer>33</integer>
229+
<integer>36</integer>
194230
</array>
195231
</dict>
196232
</dict>

‎clang/test/Analysis/nullability-notes.m

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,22 @@
1-
// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced -analyzer-output=text -verify %s
2-
// 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
3-
// RUN: %normalize_plist <%t.plist | diff -ub %S/Inputs/expected-plists/nullability-notes.m.plist -
1+
// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core \
2+
// RUN: -analyzer-checker=nullability.NullPassedToNonnull \
3+
// RUN: -analyzer-checker=nullability.NullReturnedFromNonnull \
4+
// RUN: -analyzer-checker=nullability.NullablePassedToNonnull \
5+
// RUN: -analyzer-checker=nullability.NullableReturnedFromNonnull \
6+
// RUN: -analyzer-checker=nullability.NullableDereferenced \
7+
// RUN: -analyzer-checker=debug.ExprInspection \
8+
// RUN: -analyzer-output=text -verify %s
9+
// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core \
10+
// RUN: -analyzer-checker=nullability.NullPassedToNonnull \
11+
// RUN: -analyzer-checker=nullability.NullReturnedFromNonnull \
12+
// RUN: -analyzer-checker=nullability.NullablePassedToNonnull \
13+
// RUN: -analyzer-checker=nullability.NullableReturnedFromNonnull \
14+
// RUN: -analyzer-checker=nullability.NullableDereferenced \
15+
// RUN: -analyzer-output=plist -o %t.plist %s
16+
// RUN: %normalize_plist <%t.plist \
17+
// RUN: | diff -ub %S/Inputs/expected-plists/nullability-notes.m.plist -
18+
19+
void clang_analyzer_warnOnDeadSymbol(id);
420

521
#include "Inputs/system-header-simulator-for-nullability.h"
622

@@ -12,8 +28,11 @@ -(void) method;
1228
@end;
1329
@implementation ClassWithProperties
1430
-(void) method {
31+
clang_analyzer_warnOnDeadSymbol(self);
1532
// no-crash
1633
NSObject *x = self.x; // expected-note{{Nullability 'nullable' is inferred}}
34+
// expected-warning@-1{{SYMBOL DEAD}}
35+
// expected-note@-2 {{SYMBOL DEAD}}
1736
takesNonnull(x); // expected-warning{{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
1837
// expected-note@-1{{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
1938
}

0 commit comments

Comments
 (0)