Skip to content

Commit 0307b43

Browse files
committed
MemoryLifetime: fix a miscompile in DestroyHoisting
DestroyHoisting moved a destroy_addr above a partial_apply with an inout argument. rdar://75267199
1 parent 4c7059f commit 0307b43

File tree

2 files changed

+54
-1
lines changed

2 files changed

+54
-1
lines changed

lib/SIL/Verifier/MemoryLifetime.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,19 @@ void MemoryLocations::clear() {
281281
nonTrivialLocations.clear();
282282
}
283283

284+
static bool hasInoutArgument(ApplySite AS) {
285+
for (Operand &op : AS.getArgumentOperands()) {
286+
switch (AS.getArgumentConvention(op)) {
287+
case SILArgumentConvention::Indirect_Inout:
288+
case SILArgumentConvention::Indirect_InoutAliasable:
289+
return true;
290+
default:
291+
break;
292+
}
293+
}
294+
return false;
295+
}
296+
284297
bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx,
285298
SmallVectorImpl<SILValue> &collectedVals,
286299
SubLocationMap &subLocationMap) {
@@ -329,6 +342,13 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx
329342
/*fieldNr*/ 0, collectedVals, subLocationMap))
330343
return false;
331344
break;
345+
case SILInstructionKind::PartialApplyInst:
346+
// inout/inout_aliasable conventions means that the argument "escapes".
347+
// This is okay for memory verification, but cannot handled by other
348+
// optimizations, like DestroyHoisting.
349+
if (!handleNonTrivialProjections && hasInoutArgument(ApplySite(user)))
350+
return false;
351+
break;
332352
case SILInstructionKind::InjectEnumAddrInst:
333353
case SILInstructionKind::SelectEnumAddrInst:
334354
case SILInstructionKind::ExistentialMetatypeInst:
@@ -344,7 +364,6 @@ bool MemoryLocations::analyzeLocationUsesRecursively(SILValue V, unsigned locIdx
344364
case SILInstructionKind::CheckedCastAddrBranchInst:
345365
case SILInstructionKind::UncheckedRefCastAddrInst:
346366
case SILInstructionKind::UnconditionalCheckedCastAddrInst:
347-
case SILInstructionKind::PartialApplyInst:
348367
case SILInstructionKind::ApplyInst:
349368
case SILInstructionKind::TryApplyInst:
350369
case SILInstructionKind::BeginApplyInst:

test/SILOptimizer/destroy_hoisting.sil

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,3 +314,37 @@ bb1:
314314
%75 = tuple ()
315315
return %75 : $()
316316
}
317+
318+
sil [ossa] @closure_inout : $@convention(thin) (@inout X) -> ()
319+
sil [ossa] @closure_inout_aliasable : $@convention(thin) (@inout_aliasable X) -> ()
320+
321+
// CHECK-LABEL: sil [ossa] @test_closure_inout :
322+
// CHECK: partial_apply
323+
// CHECK-NEXT: = apply
324+
// CHECK-NEXT: destroy_addr %0
325+
// CHECK: } // end sil function 'test_closure_inout'
326+
sil [ossa] @test_closure_inout : $@convention(thin) (@in X) -> () {
327+
bb0(%0 : $*X):
328+
%func = function_ref @closure_inout : $@convention(thin) (@inout X) -> ()
329+
%pa = partial_apply %func(%0) : $@convention(thin) (@inout X) -> ()
330+
apply %pa() : $@callee_owned () -> ()
331+
destroy_addr %0 : $*X
332+
%res = tuple ()
333+
return %res : $()
334+
}
335+
336+
// CHECK-LABEL: sil [ossa] @test_closure_inout_aliasable :
337+
// CHECK: partial_apply
338+
// CHECK-NEXT: = apply
339+
// CHECK-NEXT: destroy_addr %0
340+
// CHECK: } // end sil function 'test_closure_inout_aliasable'
341+
sil [ossa] @test_closure_inout_aliasable : $@convention(thin) (@in X) -> () {
342+
bb0(%0 : $*X):
343+
%func = function_ref @closure_inout_aliasable : $@convention(thin) (@inout_aliasable X) -> ()
344+
%pa = partial_apply %func(%0) : $@convention(thin) (@inout_aliasable X) -> ()
345+
apply %pa() : $@callee_owned () -> ()
346+
destroy_addr %0 : $*X
347+
%res = tuple ()
348+
return %res : $()
349+
}
350+

0 commit comments

Comments
 (0)