Skip to content

Commit 7849e0f

Browse files
committed
[ownership] Change "AnyForwardingInst" with trivial return type to return OwnershipKind::None even if they forward non-OwnershipKind::none ownership.
Otherwise the forwarding instruction will return a trivial value with non-OwnershipKind::None ownership. This inevitably causes an ownership violation since any place that we use that trivial value will expect the value to have OwnershipKind::None, showing the inconsistency that this problem yields.
1 parent 62d5546 commit 7849e0f

File tree

2 files changed

+30
-1
lines changed

2 files changed

+30
-1
lines changed

lib/SIL/IR/ValueOwnership.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,9 @@ ValueOwnershipKindClassifier::visitForwardingInst(SILInstruction *i,
241241
#define FORWARDING_OWNERSHIP_INST(INST) \
242242
ValueOwnershipKind ValueOwnershipKindClassifier::visit##INST##Inst( \
243243
INST##Inst *I) { \
244-
return I->getForwardingOwnershipKind(); \
244+
return I->getType().isTrivial(*I->getFunction()) \
245+
? ValueOwnershipKind(OwnershipKind::None) \
246+
: I->getForwardingOwnershipKind(); \
245247
}
246248
FORWARDING_OWNERSHIP_INST(BridgeObjectToRef)
247249
FORWARDING_OWNERSHIP_INST(ConvertFunction)

test/SIL/ownership-verifier/use_verifier.sil

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1431,3 +1431,30 @@ bb0(%0 : @guaranteed $Builtin.NativeObject):
14311431
%2 = unmanaged_to_ref %1 : $@sil_unmanaged Builtin.NativeObject to $Builtin.NativeObject
14321432
return %2 : $Builtin.NativeObject
14331433
}
1434+
1435+
sil [ossa] @nontrivial_enum_unchecked_enum_data_trivial_payload_owned : $@convention(thin) (@owned ThreeDifferingPayloadEnum) -> Builtin.Int32 {
1436+
bb0(%0 : @owned $ThreeDifferingPayloadEnum):
1437+
// NOTE: It may be surprising that %0 is consumed by this unchecked_enum_data
1438+
// despite the result of the instruction being trivial... after all... aren't
1439+
// we leaking something?! If one takes a step back, one will realize that even
1440+
// though %0 is statically owned, it dynamically is known to be a value with
1441+
// OwnershipKind::None and thus there is nothing to clean up. So in a certain
1442+
// sense by converting its non-trivial input type to a trivial output type,
1443+
// the instruction is stating that it knows that %0 is really dynamically
1444+
// OwnershipKind::None and thus it is safe to not propagate a value with
1445+
// non-OwnershipKind::None to be cleaned up to cleanup %0.
1446+
%1 = unchecked_enum_data %0 : $ThreeDifferingPayloadEnum, #ThreeDifferingPayloadEnum.trivial_payload!enumelt
1447+
return %1 : $Builtin.Int32
1448+
}
1449+
1450+
sil [ossa] @nontrivial_enum_unchecked_enum_data_trivial_payload_guaranteed : $@convention(thin) (@guaranteed ThreeDifferingPayloadEnum) -> Builtin.Int32 {
1451+
bb0(%0 : @guaranteed $ThreeDifferingPayloadEnum):
1452+
%1 = unchecked_enum_data %0 : $ThreeDifferingPayloadEnum, #ThreeDifferingPayloadEnum.trivial_payload!enumelt
1453+
return %1 : $Builtin.Int32
1454+
}
1455+
1456+
sil [ossa] @nontrivial_enum_unchecked_enum_data_trivial_payload_unowned : $@convention(thin) (ThreeDifferingPayloadEnum) -> Builtin.Int32 {
1457+
bb0(%0 : @unowned $ThreeDifferingPayloadEnum):
1458+
%1 = unchecked_enum_data %0 : $ThreeDifferingPayloadEnum, #ThreeDifferingPayloadEnum.trivial_payload!enumelt
1459+
return %1 : $Builtin.Int32
1460+
}

0 commit comments

Comments
 (0)