Skip to content

Commit 367586d

Browse files
gottesmmgottesmm
authored
Merge pull request #35105 from gottesmm/release/5.4-sr13926
[5.4] Cherry-pick #35093 to 5.4
2 parents 5ad4fe2 + 9f8d974 commit 367586d

File tree

3 files changed

+144
-24
lines changed

3 files changed

+144
-24
lines changed

‎lib/SILGen/SILGenPattern.cpp

Lines changed: 62 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,6 +1398,25 @@ getManagedSubobject(SILGenFunction &SGF, SILValue value,
13981398
llvm_unreachable("covered switch");
13991399
}
14001400

1401+
/// Given that we've broken down a source value into this subobject,
1402+
/// and that we were supposed to use the given consumption rules on
1403+
/// it, construct an appropriate managed value.
1404+
static ConsumableManagedValue
1405+
getManagedSubobject(SILGenFunction &SGF, ManagedValue value,
1406+
CastConsumptionKind consumption) {
1407+
switch (consumption) {
1408+
case CastConsumptionKind::BorrowAlways:
1409+
case CastConsumptionKind::CopyOnSuccess:
1410+
return {value.unmanagedBorrow(), consumption};
1411+
case CastConsumptionKind::TakeAlways:
1412+
case CastConsumptionKind::TakeOnSuccess: {
1413+
auto loc = RegularLocation::getAutoGeneratedLocation();
1414+
return {value.ensurePlusOne(SGF, loc), consumption};
1415+
}
1416+
}
1417+
llvm_unreachable("covered switch");
1418+
}
1419+
14011420
static ConsumableManagedValue
14021421
emitReabstractedSubobject(SILGenFunction &SGF, SILLocation loc,
14031422
ConsumableManagedValue value,
@@ -2064,29 +2083,34 @@ void PatternMatchEmission::emitEnumElementDispatch(
20642083
}
20652084

20662085
// Emit the switch_enum_addr instruction.
2067-
SILValue srcValue = src.getFinalManagedValue().forward(SGF);
2068-
SGF.B.createSwitchEnumAddr(loc, srcValue, blocks.getDefaultBlock(),
2086+
//
2087+
// NOTE: switch_enum_addr does not actually consume the underlying value.
2088+
SGF.B.createSwitchEnumAddr(loc, src.getValue(), blocks.getDefaultBlock(),
20692089
blocks.getCaseBlocks(), blocks.getCounts(),
20702090
defaultCaseCount);
20712091

20722092
// Okay, now emit all the cases.
2073-
blocks.forEachCase([&](EnumElementDecl *elt, SILBasicBlock *caseBB,
2093+
blocks.forEachCase([&](EnumElementDecl *eltDecl, SILBasicBlock *caseBB,
20742094
const CaseInfo &caseInfo) {
20752095
SILLocation loc = caseInfo.FirstMatcher;
20762096
auto &specializedRows = caseInfo.SpecializedRows;
20772097

20782098
SGF.B.setInsertionPoint(caseBB);
20792099

2100+
// We need to make sure our cleanup stays around long enough for us to emit
2101+
// our destroy, so setup a cleanup state restoration scope for each case.
2102+
CleanupStateRestorationScope srcScope(SGF.Cleanups);
2103+
forwardIntoIrrefutableSubtree(SGF, srcScope, src);
2104+
20802105
// We're in conditionally-executed code; enter a scope.
20812106
Scope scope(SGF.Cleanups, CleanupLocation::get(loc));
2082-
2107+
20832108
// Create a BB argument or 'unchecked_take_enum_data_addr'
20842109
// instruction to receive the enum case data if it has any.
2085-
20862110
SILType eltTy;
20872111
bool hasElt = false;
2088-
if (elt->hasAssociatedValues()) {
2089-
eltTy = src.getType().getEnumElementType(elt, SGF.SGM.M,
2112+
if (eltDecl->hasAssociatedValues()) {
2113+
eltTy = src.getType().getEnumElementType(eltDecl, SGF.SGM.M,
20902114
SGF.getTypeExpansionContext());
20912115
hasElt = !eltTy.getASTType()->isVoid();
20922116
}
@@ -2108,6 +2132,15 @@ void PatternMatchEmission::emitEnumElementDispatch(
21082132
}
21092133
}
21102134

2135+
// Forward src along this path so we don't emit a destroy_addr on our
2136+
// subject value for this case.
2137+
//
2138+
// FIXME: Do we actually want to do this? SILGen tests today assume this
2139+
// pattern. It might be worth leaving the destroy_addr there to create
2140+
// additional liveness information. For now though, we maintain the
2141+
// current behavior.
2142+
src.getFinalManagedValue().forward(SGF);
2143+
21112144
SILValue result;
21122145
if (hasNonAny) {
21132146
result = SGF.emitEmptyTuple(loc);
@@ -2132,16 +2165,18 @@ void PatternMatchEmission::emitEnumElementDispatch(
21322165
eltConsumption = CastConsumptionKind::TakeAlways;
21332166
}
21342167

2135-
SILValue eltValue;
2168+
ManagedValue eltValue;
21362169
// We can only project destructively from an address-only enum, so
21372170
// copy the value if we can't consume it.
21382171
// TODO: Should have a more efficient way to copy payload
21392172
// nondestructively from an enum.
21402173
switch (eltConsumption) {
2141-
case CastConsumptionKind::TakeAlways:
2142-
eltValue =
2143-
SGF.B.createUncheckedTakeEnumDataAddr(loc, srcValue, elt, eltTy);
2174+
case CastConsumptionKind::TakeAlways: {
2175+
auto finalValue = src.getFinalManagedValue();
2176+
eltValue = SGF.B.createUncheckedTakeEnumDataAddr(loc, finalValue,
2177+
eltDecl, eltTy);
21442178
break;
2179+
}
21452180
case CastConsumptionKind::BorrowAlways:
21462181
// If we reach this point, we know that we have a loadable
21472182
// element type from an enum with mixed address
@@ -2150,11 +2185,15 @@ void PatternMatchEmission::emitEnumElementDispatch(
21502185
// address only types do not support BorrowAlways.
21512186
llvm_unreachable("not allowed");
21522187
case CastConsumptionKind::CopyOnSuccess: {
2153-
auto copy = SGF.emitTemporaryAllocation(loc, srcValue->getType());
2154-
SGF.B.createCopyAddr(loc, srcValue, copy, IsNotTake, IsInitialization);
2188+
auto temp = SGF.emitTemporary(loc, SGF.getTypeLowering(src.getType()));
2189+
SGF.B.createCopyAddr(loc, src.getValue(), temp->getAddress(), IsNotTake,
2190+
IsInitialization);
2191+
temp->finishInitialization(SGF);
2192+
21552193
// We can always take from the copy.
21562194
eltConsumption = CastConsumptionKind::TakeAlways;
2157-
eltValue = SGF.B.createUncheckedTakeEnumDataAddr(loc, copy, elt, eltTy);
2195+
eltValue = SGF.B.createUncheckedTakeEnumDataAddr(
2196+
loc, temp->getManagedAddress(), eltDecl, eltTy);
21582197
break;
21592198
}
21602199

@@ -2170,23 +2209,22 @@ void PatternMatchEmission::emitEnumElementDispatch(
21702209
if (eltTL->isLoadable()) {
21712210
// If we do not have a loadable value, just use getManagedSubObject
21722211
// Load a loadable data value.
2173-
auto managedEltValue = ManagedValue::forUnmanaged(eltValue);
21742212
if (eltConsumption == CastConsumptionKind::CopyOnSuccess) {
2175-
managedEltValue = SGF.B.createLoadBorrow(loc, managedEltValue);
2213+
eltValue = SGF.B.createLoadBorrow(loc, eltValue);
21762214
eltConsumption = CastConsumptionKind::BorrowAlways;
21772215
} else {
21782216
assert(eltConsumption == CastConsumptionKind::TakeAlways);
2179-
managedEltValue = SGF.B.createLoadTake(loc, managedEltValue);
2217+
eltValue = SGF.B.createLoadTake(loc, eltValue);
21802218
}
2181-
origCMV = {managedEltValue, eltConsumption};
2219+
origCMV = {eltValue, eltConsumption};
21822220
} else {
2183-
origCMV = getManagedSubobject(SGF, eltValue, *eltTL, eltConsumption);
2221+
origCMV = getManagedSubobject(SGF, eltValue, eltConsumption);
21842222
}
21852223

21862224
eltCMV = origCMV;
21872225

21882226
// If the payload is boxed, project it.
2189-
if (elt->isIndirect() || elt->getParentEnum()->isIndirect()) {
2227+
if (eltDecl->isIndirect() || eltDecl->getParentEnum()->isIndirect()) {
21902228
ManagedValue boxedValue =
21912229
SGF.B.createProjectBox(loc, origCMV.getFinalManagedValue(), 0);
21922230
eltTL = &SGF.getTypeLowering(boxedValue.getType());
@@ -2202,14 +2240,14 @@ void PatternMatchEmission::emitEnumElementDispatch(
22022240

22032241
// Reabstract to the substituted type, if needed.
22042242
CanType substEltTy =
2205-
sourceType->getTypeOfMember(SGF.SGM.M.getSwiftModule(), elt,
2206-
elt->getArgumentInterfaceType())
2243+
sourceType->getTypeOfMember(SGF.SGM.M.getSwiftModule(), eltDecl,
2244+
eltDecl->getArgumentInterfaceType())
22072245
->getCanonicalType();
22082246

22092247
AbstractionPattern origEltTy =
2210-
(elt->getParentEnum()->isOptionalDecl()
2248+
(eltDecl->getParentEnum()->isOptionalDecl()
22112249
? AbstractionPattern(substEltTy)
2212-
: SGF.SGM.M.Types.getAbstractionPattern(elt));
2250+
: SGF.SGM.M.Types.getAbstractionPattern(eltDecl));
22132251

22142252
eltCMV = emitReabstractedSubobject(SGF, loc, eltCMV, *eltTL,
22152253
origEltTy, substEltTy);

‎test/Interpreter/switch_default.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// RUN: %target-run-simple-swift
2+
3+
// REQUIRES: executable_test
4+
5+
import StdlibUnittest
6+
7+
var SwitchDefaultTestSuite = TestSuite("Switch.Default")
8+
defer { runAllTests() }
9+
10+
class Klass {}
11+
protocol Protocol {}
12+
13+
enum Enum {
14+
case value1(LifetimeTracked)
15+
case value2(Protocol)
16+
}
17+
18+
SwitchDefaultTestSuite.test("do not leak default case payload") {
19+
// We are passing in Enum.value1 so we go down the default and leak our
20+
// lifetime tracked.
21+
func f(_ e: Enum?) {
22+
switch (e) {
23+
case .value2: return
24+
default: return
25+
}
26+
}
27+
f(.value1(LifetimeTracked(0)))
28+
}

‎test/SILGen/switch_default.swift

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// RUN: %target-swift-emit-silgen -module-name switch_default %s | %FileCheck %s
2+
3+
class Klass {}
4+
protocol Protocol {}
5+
6+
enum Enum {
7+
case value1(Klass)
8+
case value2(Protocol)
9+
}
10+
11+
// CHECK-LABEL: sil hidden [ossa] @$s14switch_default33testAddressOnlySubjectDefaultCaseyyAA4EnumOSgF : $@convention(thin) (@in_guaranteed Optional<Enum>) -> () {
12+
// CHECK: bb0([[ARG:%.*]] :
13+
// CHECK: [[STACK:%.*]] = alloc_stack $Optional<Enum>
14+
// CHECK: copy_addr [[ARG]] to [initialization] [[STACK]]
15+
// CHECK: switch_enum_addr [[STACK]] : $*Optional<Enum>, case #Optional.some!enumelt: [[SOME_BB:bb[0-9]*]], default [[DEFAULT_BB:bb[0-9]*]]
16+
//
17+
// CHECK: [[SOME_BB]]:
18+
// CHECK: [[STACK_1:%.*]] = alloc_stack $Optional<Enum>
19+
// CHECK: copy_addr [[STACK]] to [initialization] [[STACK_1]]
20+
// CHECK: [[TAKEN_ADDR:%.*]] = unchecked_take_enum_data_addr [[STACK_1]]
21+
// CHECK: switch_enum_addr [[TAKEN_ADDR]] : $*Enum, case #Enum.value2!enumelt: [[VALUE2_BB:bb[0-9]*]], default [[DEFAULT_BB_2:bb[0-9]*]]
22+
//
23+
// CHECK: [[VALUE2_BB]]:
24+
// CHECK: [[TAKEN_TAKEN_ADDR:%.*]] = unchecked_take_enum_data_addr [[TAKEN_ADDR]]
25+
// CHECK: destroy_addr [[TAKEN_TAKEN_ADDR]]
26+
// CHECK: dealloc_stack [[STACK_1]]
27+
// CHECK: destroy_addr [[STACK]]
28+
// CHECK: dealloc_stack [[STACK]]
29+
// CHECK: br [[EXIT_BB:bb[0-9]+]]
30+
//
31+
// We used to leak here!
32+
// CHECK: [[DEFAULT_BB_2]]:
33+
// CHECK: destroy_addr [[TAKEN_ADDR]]
34+
// CHECK: dealloc_stack [[STACK_1]]
35+
// CHECK: br [[CONT_BB:bb[0-9]*]]
36+
//
37+
// CHECK: [[DEFAULT_BB]]:
38+
// CHECK: br [[CONT_BB]]
39+
//
40+
// CHECK: [[CONT_BB]]:
41+
// CHECK: destroy_addr [[STACK]]
42+
// CHECK: dealloc_stack [[STACK]]
43+
// CHECK: br [[EXIT_BB]]
44+
//
45+
// CHECK: [[EXIT_BB]]:
46+
// CHECK-NEXT: tuple
47+
// CHECK-NEXT: return
48+
// } // end sil function '$s14switch_default33testAddressOnlySubjectDefaultCaseyyAA4EnumOSgF'
49+
func testAddressOnlySubjectDefaultCase(_ e: Enum?) {
50+
switch (e) {
51+
case .value2: return
52+
default: return
53+
}
54+
}

0 commit comments

Comments
 (0)