Skip to content

Commit a7fa45c

Browse files
committed
DiagnoseLifetimeIssues: handle ObjectiveC weak properties
Issue warnings if an object is stored to an ObjectiveC weak property (via a setter) and destroyed before the property is ever used again. rdar://74620325
1 parent bdacb68 commit a7fa45c

File tree

2 files changed

+72
-19
lines changed

2 files changed

+72
-19
lines changed

lib/SILOptimizer/Mandatory/DiagnoseLifetimeIssues.cpp

Lines changed: 59 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "swift/SILOptimizer/Utils/PrunedLiveness.h"
3838
#include "swift/Demangling/Demangler.h"
3939
#include "llvm/Support/Debug.h"
40+
#include "clang/AST/DeclObjC.h"
4041

4142
using namespace swift;
4243

@@ -51,7 +52,7 @@ class DiagnoseLifetimeIssues {
5152

5253
bool computeCanonicalLiveness(SILValue def);
5354

54-
bool isDeadStore(StoreWeakInst *store);
55+
void reportDeadStore(SILInstruction *storeInst, SILValue src);
5556

5657
public:
5758
DiagnoseLifetimeIssues() {}
@@ -96,7 +97,8 @@ bool DiagnoseLifetimeIssues::computeCanonicalLiveness(SILValue def) {
9697
auto *user = use->getUser();
9798

9899
// Recurse through copies and enums.
99-
if (isa<CopyValueInst>(user) || isa<EnumInst>(user)) {
100+
if (isa<CopyValueInst>(user) || isa<EnumInst>(user) ||
101+
isa<InitExistentialRefInst>(user)) {
100102
defUseWorklist.insert(cast<SingleValueInstruction>(user));
101103
continue;
102104
}
@@ -156,25 +158,30 @@ static SILValue getCanonicalDef(SILValue val) {
156158
continue;
157159
}
158160
}
161+
if (auto *initExRef = dyn_cast<InitExistentialRefInst>(val)) {
162+
val = initExRef->getOperand();
163+
continue;
164+
}
159165
return val;
160166
}
161167
}
162168

163-
/// Returns true if the stored object of \p store is never loaded within the
164-
/// lifetime of the stored object.
165-
bool DiagnoseLifetimeIssues::isDeadStore(StoreWeakInst *store) {
166-
SILValue storedDef = getCanonicalDef(store->getSrc());
169+
/// Reports a warning if the stored object \p storedObj is never loaded within
170+
/// the lifetime of the stored object.
171+
void DiagnoseLifetimeIssues::reportDeadStore(SILInstruction *storeInst,
172+
SILValue storedObj) {
173+
SILValue storedDef = getCanonicalDef(storedObj);
167174

168175
// Only for allocations we know that a destroy will actually deallocate the
169176
// object. Otherwise the object could be kept alive by other references and
170177
// we would issue a false alarm.
171178
if (!isAllocation(storedDef))
172-
return false;
179+
return;
173180

181+
liveness.clear();
174182
liveness.initializeDefBlock(storedDef->getParentBlock());
175183
if (!computeCanonicalLiveness(storedDef))
176-
return false;
177-
184+
return;
178185

179186
// Check if the lifetime of the stored object ends at the store_weak.
180187
//
@@ -186,35 +193,68 @@ bool DiagnoseLifetimeIssues::isDeadStore(StoreWeakInst *store) {
186193
// always see a potential load if the lifetime of the object goes beyond the
187194
// store_weak.
188195

189-
SILBasicBlock *storeBlock = store->getParent();
196+
SILBasicBlock *storeBlock = storeInst->getParent();
190197
if (liveness.getBlockLiveness(storeBlock) != PrunedLiveBlocks::LiveWithin)
191-
return false;
198+
return;
192199

193200
// If there are any uses after the store_weak, it means that the liferange of
194201
// the object goes beyond the store_weak.
195-
for (SILInstruction &inst : make_range(std::next(store->getIterator()),
202+
for (SILInstruction &inst : make_range(std::next(storeInst->getIterator()),
196203
storeBlock->end())) {
197204
switch (liveness.isInterestingUser(&inst)) {
198205
case PrunedLiveness::NonUser:
199206
break;
200207
case PrunedLiveness::NonLifetimeEndingUse:
201208
case PrunedLiveness::LifetimeEndingUse:
202-
return false;
209+
return;
203210
}
204211
}
205-
return true;
212+
213+
// Issue the warning.
214+
storeInst->getModule().getASTContext().Diags.diagnose(
215+
storeInst->getLoc().getSourceLoc(), diag::warn_dead_weak_store);
216+
}
217+
218+
/// Returns true if \p inst is a call of an ObjC setter to a weak property.
219+
static bool isStoreObjcWeak(SILInstruction *inst) {
220+
auto *apply = dyn_cast<ApplyInst>(inst);
221+
if (!apply || apply->getNumArguments() < 1)
222+
return false;
223+
224+
auto *method = dyn_cast<ObjCMethodInst>(apply->getCallee());
225+
if (!method)
226+
return false;
227+
228+
Decl *decl = method->getMember().getDecl();
229+
auto *accessor = dyn_cast<AccessorDecl>(decl);
230+
if (!accessor)
231+
return false;
232+
233+
auto *var = dyn_cast<VarDecl>(accessor->getStorage());
234+
if (!var)
235+
return false;
236+
237+
ClangNode clangNode = var->getClangNode();
238+
if (!clangNode)
239+
return false;
240+
241+
auto *objcDecl = dyn_cast_or_null<clang::ObjCPropertyDecl>(clangNode.getAsDecl());
242+
if (!objcDecl)
243+
return false;
244+
245+
return objcDecl->getSetterKind() == clang::ObjCPropertyDecl::Weak;
206246
}
207247

208248
/// Prints warnings for dead weak stores in \p function.
209249
void DiagnoseLifetimeIssues::diagnose(SILFunction *function) {
210250
for (SILBasicBlock &block : *function) {
211251
for (SILInstruction &inst : block) {
212252
if (auto *stWeak = dyn_cast<StoreWeakInst>(&inst)) {
213-
if (isDeadStore(stWeak)) {
214-
function->getModule().getASTContext().Diags.diagnose(
215-
stWeak->getLoc().getSourceLoc(), diag::warn_dead_weak_store);
216-
}
217-
liveness.clear();
253+
reportDeadStore(stWeak, stWeak->getSrc());
254+
continue;
255+
}
256+
if (isStoreObjcWeak(&inst)) {
257+
reportDeadStore(&inst, cast<ApplyInst>(&inst)->getArgument(0));
218258
continue;
219259
}
220260
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// RUN: %target-swift-frontend -emit-sil %s -o /dev/null -verify
2+
// REQUIRES: objc_interop
3+
4+
import Foundation
5+
6+
class MyServiceDelegate : NSObject, NSXPCListenerDelegate { }
7+
8+
public func warningForDeadDelegate() {
9+
let delegate = MyServiceDelegate()
10+
let listener = NSXPCListener.service()
11+
listener.delegate = delegate // expected-warning {{weak reference will always be nil because the referenced object is deallocated here}}
12+
listener.resume()
13+
}

0 commit comments

Comments
 (0)