Skip to content

Commit a3709b0

Browse files
Merge pull request #41450 from atrick/rauw-fix-rebase
Temporary fix for OSSA RAUW utilities.
2 parents a0fbce0 + 9dba1c1 commit a3709b0

File tree

3 files changed

+32
-10
lines changed

3 files changed

+32
-10
lines changed

lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ bool swift::canOpcodeForwardOwnedValues(Operand *use) {
9090
// Skip over nested borrow scopes. Their scope-ending instructions are their use
9191
// points. Transitively find all nested scope-ending instructions by looking
9292
// through nested reborrows. Nested reborrows are not use points.
93+
//
94+
// FIXME: handle inner reborrows, which aren't dominated by
95+
// guaranteedValue. Audit all users to handle reborrows.
9396
bool swift::findInnerTransitiveGuaranteedUses(
9497
SILValue guaranteedValue, SmallVectorImpl<Operand *> *usePoints) {
9598

@@ -183,13 +186,18 @@ bool swift::findInnerTransitiveGuaranteedUses(
183186
break;
184187
}
185188
case OperandOwnership::Borrow:
186-
// FIXME: visitExtendedScopeEndingUses can't return false here once dead
189+
// FIXME: Use visitExtendedScopeEndingUses and audit all clients to handle
190+
// reborrows.
191+
//
192+
// FIXME: visit[Extended]ScopeEndingUses can't return false here once dead
187193
// borrows are disallowed.
188-
if (!BorrowingOperand(use).visitExtendedScopeEndingUses(
189-
[&](Operand *endUse) {
190-
leafUse(endUse);
191-
return true;
192-
})) {
194+
if (!BorrowingOperand(use).visitScopeEndingUses([&](Operand *endUse) {
195+
if (endUse->getOperandOwnership() == OperandOwnership::Reborrow) {
196+
foundPointerEscape = true;
197+
}
198+
leafUse(endUse);
199+
return true;
200+
})) {
193201
// Special case for dead borrows. This is dangerous because clients
194202
// don't expect a begin_borrow to be in the use list.
195203
leafUse(use);

lib/SILOptimizer/Utils/OwnershipOptUtils.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -392,18 +392,27 @@ bool OwnershipRAUWHelper::hasValidRAUWOwnership(SILValue oldValue,
392392
static bool canFixUpOwnershipForRAUW(SILValue oldValue, SILValue newValue,
393393
OwnershipFixupContext &context) {
394394
switch (oldValue.getOwnershipKind()) {
395-
case OwnershipKind::Guaranteed:
395+
case OwnershipKind::Guaranteed: {
396396
// Check that the old lifetime can be extended and record the necessary
397397
// book-keeping in the OwnershipFixupContext.
398398
context.clear();
399399

400400
// Check that no transitive uses have a PointerEscape, and record the leaf
401401
// uses for liveness extension.
402-
if (!findExtendedTransitiveGuaranteedUses(oldValue,
403-
context.guaranteedUsePoints))
402+
//
403+
// FIXME: Use findExtendedTransitiveGuaranteedUses and switch the
404+
// implementation of borrowCopyOverGuaranteedUses to
405+
// GuaranteedOwnershipExtension. Utils then, reborrows are considered
406+
// pointer escapes, causing findTransitiveGuaranteedUses to return false. So
407+
// they can be ignored.
408+
auto visitReborrow = [&](Operand *reborrow) {};
409+
if (!findTransitiveGuaranteedUses(oldValue, context.guaranteedUsePoints,
410+
visitReborrow)) {
404411
return false;
412+
}
405413
return OwnershipRAUWHelper::hasValidRAUWOwnership(
406414
oldValue, newValue, context.guaranteedUsePoints);
415+
}
407416
default: {
408417
SmallVector<Operand *, 8> ownedUsePoints;
409418
// If newValue is lexical, find the uses of oldValue so that it can be

test/SILOptimizer/cse_ossa_nontrivial.sil

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -609,10 +609,15 @@ struct RecurStruct {
609609
}
610610

611611
// Test to make sure we do not crash on a use after free when the branch instructions in bb1 and bb2 are deleted
612+
//
613+
// FIXME: Fix OwnershipRAUW to handle guaranteed uses of the old CSE'd
614+
// value that are reborrows. The reborrowed uses are not dominated by
615+
// the old value, but they are still within the new values borrow scope.
616+
//
612617
// CHECK-LABEL: sil [ossa] @test_useafterfreeinrauw :
613618
// CHECK: bb0
614619
// CHECK: struct_extract
615-
// CHECK-NOT: struct_extract
620+
// FIXME-CHECK-NOT: struct_extract
616621
// CHECK: cond_br
617622
// CHECK-LABEL: } // end sil function 'test_useafterfreeinrauw'
618623
sil [ossa] @test_useafterfreeinrauw : $@convention(thin) (@guaranteed RecurStruct) -> @owned NonTrivialStruct {

0 commit comments

Comments
 (0)