Skip to content

Commit f7f5f9a

Browse files
committed
SILCombine: fix an ownership violation when replacing open existential apply arguments
The value of the opened existential must outlive the apply. Otherwise the optimization creates invalid SIL. rdar://148259849
1 parent b176ee4 commit f7f5f9a

File tree

2 files changed

+60
-1
lines changed

2 files changed

+60
-1
lines changed

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -765,6 +765,14 @@ void SILCombiner::replaceWitnessMethodInst(
765765
eraseInstFromFunction(*WMI);
766766
}
767767

768+
static bool hasUse(SILValue v, Operand *op) {
769+
for (Operand *use : v->getUses()) {
770+
if (use == op)
771+
return true;
772+
}
773+
return false;
774+
}
775+
768776
// This function determines concrete type of an opened existential argument
769777
// using ProtocolConformanceAnalysis. The concrete type of the argument can be a
770778
// class, struct, or an enum.
@@ -839,6 +847,13 @@ SILCombiner::buildConcreteOpenedExistentialInfoFromSoleConformingType(
839847
// Prepare the code by adding UncheckedCast instructions that cast opened
840848
// existentials to concrete types. Set the ConcreteValue of CEI.
841849
if (auto *OER = dyn_cast<OpenExistentialRefInst>(OAI.OpenedArchetypeValue)) {
850+
// Make sure that the existential value outlives the apply. This is the case
851+
// if the apply uses it as argument operand.
852+
if (OER->getOwnershipKind() == OwnershipKind::Owned &&
853+
!hasUse(OER, &ArgOperand)) {
854+
return std::nullopt;
855+
}
856+
842857
SILBuilderWithScope b(std::next(OER->getIterator()), Builder);
843858
auto loc = RegularLocation::getAutoGeneratedLocation();
844859
SoleCEI.ConcreteValue = b.createUncheckedRefCast(loc, OER, concreteSILType);
@@ -880,6 +895,11 @@ SILCombiner::buildConcreteOpenedExistentialInfo(Operand &ArgOperand) {
880895
auto ConcreteValue = COEI.CEI->ConcreteValue;
881896
if (ConcreteValue->getFunction()->hasOwnership() &&
882897
ConcreteValue->getOwnershipKind() == OwnershipKind::Owned) {
898+
// Make sure that the existential value outlives the apply. This is the case
899+
// if the apply uses it as argument operand.
900+
if (!hasUse(COEI.OAI.OpenedArchetypeValue, &ArgOperand))
901+
return std::nullopt;
902+
883903
SILBuilderWithScope b(COEI.OAI.OpenedArchetypeValue->getNextInstruction(),
884904
Builder);
885905
auto loc = RegularLocation::getAutoGeneratedLocation();

test/SILOptimizer/sil_combine_ossa.sil

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-sil-opt -sil-print-types -enable-copy-propagation=requested-passes-only -enable-objc-interop -enforce-exclusivity=none -enable-sil-verify-all %s -update-borrowed-from -sil-combine | %FileCheck %s
1+
// RUN: %target-sil-opt -sil-print-types -enable-copy-propagation=requested-passes-only -enable-objc-interop -enforce-exclusivity=none -enable-sil-verify-all %s -update-borrowed-from -sil-combine -wmo | %FileCheck %s
22
// RUN: %target-sil-opt -sil-print-types -enable-copy-propagation=requested-passes-only -enable-objc-interop -enforce-exclusivity=none -enable-sil-verify-all %s -update-borrowed-from -sil-combine -generic-specializer | %FileCheck %s --check-prefix=CHECK_FORWARDING_OWNERSHIP_KIND
33
//
44
// FIXME: check copy-propagation output instead once it is the default mode
@@ -5447,3 +5447,42 @@ bb0(%0 : @owned $C):
54475447
return %6 : $()
54485448
}
54495449

5450+
private protocol Pr: AnyObject {
5451+
}
5452+
5453+
private class Cr: Pr {
5454+
}
5455+
5456+
sil @use_pr : $@convention(thin) <τ_0_0 where τ_0_0 : Pr> (@in τ_0_0) -> ()
5457+
5458+
// CHECK-LABEL: sil [ossa] @existential_consumed_too_early1 :
5459+
// CHECK: apply {{.*}}<@opened
5460+
// CHECK: } // end sil function 'existential_consumed_too_early1'
5461+
sil [ossa] @existential_consumed_too_early1 : $@convention(thin) (@owned Pr) -> () {
5462+
bb0(%0 : @owned $Pr):
5463+
%2 = open_existential_ref %0 to $@opened("5554ACAA-0F2B-11F0-86AA-0EA13E3AABB1", any Pr) Self
5464+
%3 = alloc_stack $@opened("5554ACAA-0F2B-11F0-86AA-0EA13E3AABB1", any Pr) Self
5465+
store %2 to [init] %3
5466+
%5 = function_ref @use_pr : $@convention(thin) <τ_0_0 where τ_0_0 : Pr> (@in τ_0_0) -> ()
5467+
%6 = apply %5<@opened("5554ACAA-0F2B-11F0-86AA-0EA13E3AABB1", any Pr) Self>(%3) : $@convention(thin) <τ_0_0 where τ_0_0 : Pr> (@in τ_0_0) -> ()
5468+
dealloc_stack %3
5469+
%8 = tuple ()
5470+
return %8
5471+
}
5472+
5473+
// CHECK-LABEL: sil [ossa] @existential_consumed_too_early2 :
5474+
// CHECK: apply {{.*}}<@opened
5475+
// CHECK: } // end sil function 'existential_consumed_too_early2'
5476+
sil [ossa] @existential_consumed_too_early2 : $@convention(thin) (@owned Cr) -> () {
5477+
bb0(%0 : @owned $Cr):
5478+
%1 = init_existential_ref %0 : $Cr : $Cr, $any Pr
5479+
%2 = open_existential_ref %1 to $@opened("5554ACAA-0F2B-11F0-86AA-0EA13E3AABB1", any Pr) Self
5480+
%3 = alloc_stack $@opened("5554ACAA-0F2B-11F0-86AA-0EA13E3AABB1", any Pr) Self
5481+
store %2 to [init] %3
5482+
%5 = function_ref @use_pr : $@convention(thin) <τ_0_0 where τ_0_0 : Pr> (@in τ_0_0) -> ()
5483+
%6 = apply %5<@opened("5554ACAA-0F2B-11F0-86AA-0EA13E3AABB1", any Pr) Self>(%3) : $@convention(thin) <τ_0_0 where τ_0_0 : Pr> (@in τ_0_0) -> ()
5484+
dealloc_stack %3
5485+
%8 = tuple ()
5486+
return %8
5487+
}
5488+

0 commit comments

Comments
 (0)