Skip to content

Commit 507bd39

Browse files
kavonkavon
authored
Merge pull request #66352 from kavon/5.9-use-after-discard
[5.9🍒] prevent reinitialization of self after discard
2 parents 7b0a8a2 + 55f50cd commit 507bd39

37 files changed

+1136
-27
lines changed

‎SwiftCompilerSources/Sources/SIL/Instruction.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,10 @@ final public class MoveValueInst : SingleValueInstruction, UnaryInstruction {
647647
public var fromValue: Value { operand.value }
648648
}
649649

650+
final public class DropDeinitInst : SingleValueInstruction, UnaryInstruction {
651+
public var fromValue: Value { operand.value }
652+
}
653+
650654
final public class StrongCopyUnownedValueInst : SingleValueInstruction, UnaryInstruction {}
651655

652656
final public class StrongCopyUnmanagedValueInst : SingleValueInstruction, UnaryInstruction {}

‎SwiftCompilerSources/Sources/SIL/Registration.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ public func registerSILClasses() {
125125
register(ProjectBoxInst.self)
126126
register(CopyValueInst.self)
127127
register(MoveValueInst.self)
128+
register(DropDeinitInst.self)
128129
register(EndCOWMutationInst.self)
129130
register(ClassifyBridgeObjectInst.self)
130131
register(PartialApplyInst.self)

‎docs/SIL.rst

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6148,6 +6148,29 @@ a type `T` into the move only value space.
61486148
The ``lexical`` attribute specifies that the value corresponds to a local
61496149
variable in the Swift source.
61506150

6151+
6152+
drop_deinit
6153+
```````````
6154+
6155+
::
6156+
6157+
sil-instruction ::= 'drop_deinit' sil-operand
6158+
6159+
%1 = drop_deinit %0 : $T
6160+
// T must be a move-only type
6161+
// %1 is an @owned T
6162+
%3 = drop_deinit %2 : $*T
6163+
// T must be a move-only type
6164+
// %2 has type *T
6165+
6166+
This instruction is a marker for a following destroy instruction to suppress
6167+
the call of the move-only type's deinitializer.
6168+
The instruction accepts an object or address type.
6169+
If its argument is an object type it takes in an `@owned T` and produces a new
6170+
`@owned T`. If its argument is an address type, it's an identity projection.
6171+
6172+
The instruction is only valid in ownership SIL.
6173+
61516174
release_value
61526175
`````````````
61536176

‎include/swift/AST/DiagnosticsSIL.def

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,13 @@ ERROR(sil_movechecking_notconsumable_but_assignable_was_consumed, none,
776776
ERROR(sil_movechecking_cannot_destructure_has_deinit, none,
777777
"cannot partially consume '%0' when it has a deinitializer",
778778
(StringRef))
779+
ERROR(sil_movechecking_discard_missing_consume_self, none,
780+
"must consume 'self' before exiting method that discards self", ())
781+
ERROR(sil_movechecking_reinit_after_discard, none,
782+
"cannot reinitialize 'self' after 'discard self'", ())
779783

784+
NOTE(sil_movechecking_discard_self_here, none,
785+
"discarded self here", ())
780786
NOTE(sil_movechecking_partial_consume_here, none,
781787
"partially consumed here", ())
782788
NOTE(sil_movechecking_consuming_use_here, none,

‎include/swift/SIL/MemAccessUtils.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,6 +1632,7 @@ inline bool isAccessStorageIdentityCast(SingleValueInstruction *svi) {
16321632
// Simply pass-thru the incoming address.
16331633
case SILInstructionKind::MarkUninitializedInst:
16341634
case SILInstructionKind::MarkMustCheckInst:
1635+
case SILInstructionKind::DropDeinitInst:
16351636
case SILInstructionKind::MarkUnresolvedReferenceBindingInst:
16361637
case SILInstructionKind::MarkDependenceInst:
16371638
case SILInstructionKind::CopyValueInst:

‎include/swift/SIL/SILBuilder.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,6 +1364,14 @@ class SILBuilder {
13641364
operand, isLexical));
13651365
}
13661366

1367+
DropDeinitInst *createDropDeinit(SILLocation loc, SILValue operand) {
1368+
assert(getFunction().hasOwnership());
1369+
assert(!operand->getType().isTrivial(getFunction()) &&
1370+
"Should not be passing trivial values to this api.");
1371+
return insert(new (getModule()) DropDeinitInst(getSILDebugLocation(loc),
1372+
operand));
1373+
}
1374+
13671375
MarkUnresolvedMoveAddrInst *createMarkUnresolvedMoveAddr(SILLocation loc,
13681376
SILValue srcAddr,
13691377
SILValue takeAddr) {

‎include/swift/SIL/SILCloner.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1896,6 +1896,17 @@ void SILCloner<ImplClass>::visitMoveValueInst(MoveValueInst *Inst) {
18961896
recordClonedInstruction(Inst, MVI);
18971897
}
18981898

1899+
template <typename ImplClass>
1900+
void SILCloner<ImplClass>::visitDropDeinitInst(DropDeinitInst *Inst) {
1901+
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
1902+
if (!getBuilder().hasOwnership()) {
1903+
return recordFoldedValue(Inst, getOpValue(Inst->getOperand()));
1904+
}
1905+
auto *MVI = getBuilder().createDropDeinit(getOpLocation(Inst->getLoc()),
1906+
getOpValue(Inst->getOperand()));
1907+
recordClonedInstruction(Inst, MVI);
1908+
}
1909+
18991910
template <typename ImplClass>
19001911
void SILCloner<ImplClass>::visitMarkMustCheckInst(MarkMustCheckInst *Inst) {
19011912
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));

‎include/swift/SIL/SILInstruction.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8301,6 +8301,15 @@ class MoveValueInst
83018301
void removeIsLexical() { lexical = false; }
83028302
};
83038303

8304+
class DropDeinitInst
8305+
: public UnaryInstructionBase<SILInstructionKind::DropDeinitInst,
8306+
SingleValueInstruction> {
8307+
friend class SILBuilder;
8308+
8309+
DropDeinitInst(SILDebugLocation DebugLoc, SILValue operand)
8310+
: UnaryInstructionBase(DebugLoc, operand, operand->getType()) {}
8311+
};
8312+
83048313
/// Equivalent to a copy_addr to [init] except that it is used for diagnostics
83058314
/// and should not be pattern matched. During the diagnostic passes, the "move
83068315
/// function" checker for addresses always converts this to a copy_addr [init]

‎include/swift/SIL/SILNodes.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,8 @@ ABSTRACT_VALUE_AND_INST(SingleValueInstruction, ValueBase, SILInstruction)
463463
// effects relative to other OSSA values like copy_value.
464464
SINGLE_VALUE_INST(MoveValueInst, move_value, SingleValueInstruction, None,
465465
DoesNotRelease)
466+
SINGLE_VALUE_INST(DropDeinitInst, drop_deinit, SingleValueInstruction, None,
467+
DoesNotRelease)
466468
// A canary value inserted by a SIL generating frontend to signal to the move
467469
// checker to check a specific value. Valid only in Raw SIL. The relevant
468470
// checkers should remove the mark_must_check instruction after successfully

‎lib/IRGen/IRGenSIL.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,6 +1248,10 @@ class IRGenSILFunction :
12481248
auto e = getLoweredExplosion(i->getOperand());
12491249
setLoweredExplosion(i, e);
12501250
}
1251+
void visitDropDeinitInst(DropDeinitInst *i) {
1252+
auto e = getLoweredExplosion(i->getOperand());
1253+
setLoweredExplosion(i, e);
1254+
}
12511255
void visitMarkMustCheckInst(MarkMustCheckInst *i) {
12521256
llvm_unreachable("Invalid in Lowered SIL");
12531257
}

‎lib/SIL/IR/OperandOwnership.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,12 @@ OperandOwnershipClassifier::visitStoreBorrowInst(StoreBorrowInst *i) {
457457
return OperandOwnership::TrivialUse;
458458
}
459459

460+
OperandOwnership
461+
OperandOwnershipClassifier::visitDropDeinitInst(DropDeinitInst *i) {
462+
return i->getType().isAddress() ? OperandOwnership::TrivialUse
463+
: OperandOwnership::ForwardingConsume;
464+
}
465+
460466
// Get the OperandOwnership for instantaneous apply, yield, and return uses.
461467
// This does not apply to uses that begin an explicit borrow scope in the
462468
// caller, such as begin_apply.

‎lib/SIL/IR/SILPrinter.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1999,6 +1999,10 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
19991999
*this << getIDAndType(I->getOperand());
20002000
}
20012001

2002+
void visitDropDeinitInst(DropDeinitInst *I) {
2003+
*this << getIDAndType(I->getOperand());
2004+
}
2005+
20022006
void visitMarkMustCheckInst(MarkMustCheckInst *I) {
20032007
using CheckKind = MarkMustCheckInst::CheckKind;
20042008
switch (I->getCheckKind()) {

‎lib/SIL/IR/ValueOwnership.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,10 @@ ValueOwnershipKind ValueOwnershipKindClassifier::visitLoadInst(LoadInst *LI) {
355355
llvm_unreachable("Unhandled LoadOwnershipQualifier in switch.");
356356
}
357357

358+
ValueOwnershipKind ValueOwnershipKindClassifier::visitDropDeinitInst(DropDeinitInst *ddi) {
359+
return ddi->getType().isAddress() ? OwnershipKind::None : OwnershipKind::Owned;
360+
}
361+
358362
ValueOwnershipKind
359363
ValueOwnershipKindClassifier::visitPartialApplyInst(PartialApplyInst *PA) {
360364
// partial_apply instructions are modeled as creating an owned value during

‎lib/SIL/Parser/ParseSIL.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3702,6 +3702,15 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
37023702
break;
37033703
}
37043704

3705+
case SILInstructionKind::DropDeinitInst: {
3706+
if (parseTypedValueRef(Val, B))
3707+
return true;
3708+
if (parseSILDebugLocation(InstLoc, B))
3709+
return true;
3710+
ResultVal = B.createDropDeinit(InstLoc, Val);
3711+
break;
3712+
}
3713+
37053714
case SILInstructionKind::MarkMustCheckInst: {
37063715
StringRef AttrName;
37073716
if (!parseSILOptional(AttrName, *this)) {

‎lib/SIL/Utils/AddressWalker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ AddressUseKind TransitiveAddressWalker::walk(SILValue projectedAddress) && {
157157
isa<BeginAccessInst>(user) || isa<TailAddrInst>(user) ||
158158
isa<IndexAddrInst>(user) || isa<StoreBorrowInst>(user) ||
159159
isa<UncheckedAddrCastInst>(user) || isa<MarkMustCheckInst>(user) ||
160-
isa<MarkUninitializedInst>(user) ||
160+
isa<MarkUninitializedInst>(user) || isa<DropDeinitInst>(user) ||
161161
isa<ProjectBlockStorageInst>(user) || isa<UpcastInst>(user) ||
162162
isa<TuplePackElementAddrInst>(user)) {
163163
transitiveResultUses(op);

‎lib/SIL/Utils/InstructionUtils.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,7 @@ RuntimeEffect swift::getRuntimeEffect(SILInstruction *inst, SILType &impactType)
441441
case SILInstructionKind::ValueToBridgeObjectInst:
442442
case SILInstructionKind::MarkDependenceInst:
443443
case SILInstructionKind::MoveValueInst:
444+
case SILInstructionKind::DropDeinitInst:
444445
case SILInstructionKind::MarkMustCheckInst:
445446
case SILInstructionKind::MarkUnresolvedReferenceBindingInst:
446447
case SILInstructionKind::CopyableToMoveOnlyWrapperValueInst:

‎lib/SIL/Utils/MemAccessUtils.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1866,6 +1866,7 @@ AccessPathDefUseTraversal::visitSingleValueUser(SingleValueInstruction *svi,
18661866
return IgnoredUse;
18671867
}
18681868

1869+
case SILInstructionKind::DropDeinitInst:
18691870
case SILInstructionKind::MarkMustCheckInst: {
18701871
// Mark must check goes on the project_box, so it isn't a ref.
18711872
assert(!dfs.isRef());

‎lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5968,6 +5968,14 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
59685968
"Result and operand must have the same type, today.");
59695969
}
59705970

5971+
void checkDropDeinitInst(DropDeinitInst *ddi) {
5972+
require(ddi->getType() == ddi->getOperand()->getType(),
5973+
"Result and operand must have the same type.");
5974+
require(ddi->getType().isMoveOnlyNominalType(),
5975+
"drop_deinit only allowed for move-only types");
5976+
require(F.hasOwnership(), "drop_deinit only allowed in OSSA");
5977+
}
5978+
59715979
void checkMarkMustCheckInst(MarkMustCheckInst *i) {
59725980
require(i->getModule().getStage() == SILStage::Raw,
59735981
"Only valid in Raw SIL! Should have been eliminated by /some/ "

‎lib/SILGen/SILGenDestructor.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,7 @@ void SILGenFunction::emitMoveOnlyMemberDestruction(SILValue selfValue,
499499
NominalTypeDecl *nom,
500500
CleanupLocation cleanupLoc,
501501
SILBasicBlock *finishBB) {
502+
selfValue = B.createDropDeinit(cleanupLoc, selfValue);
502503
if (selfValue->getType().isAddress()) {
503504
if (auto *structDecl = dyn_cast<StructDecl>(nom)) {
504505
for (VarDecl *vd : nom->getStoredProperties()) {

0 commit comments

Comments
 (0)