Skip to content

Commit e2f3c30

Browse files
committed
[sil] Disable MemoryLifetimeVerifier on alloc_stacks that are enum typed.
Currently in this verifier, we stop verifying if we find a switch_enum_addr use. This creates a problem since no one has gone through and changed the frontend/optimizer to understand that it needs to insert destroy_addr on alloc_stack even if dynamically we know that the enum has a trivial case or non-payloaded case due to a switch_enum_addr. So if one then performs the completely unrelated, valid optimization of promoting the switch_enum_addr to a switch_enum (lets say using a load_borrow), then this verifier will see a leaked value along the non-payloaded path. Disable this verification for now until a complete analysis of enums is provided to unblock further work. https://bugs.swift.org/browse/SR-14123
1 parent 645cac4 commit e2f3c30

File tree

1 file changed

+37
-6
lines changed

1 file changed

+37
-6
lines changed

lib/SIL/Verifier/MemoryLifetime.cpp

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,36 @@ int MemoryLocations::getLocationIdx(SILValue addr) const {
132132
return iter->second;
133133
}
134134

135+
static bool canHandleAllocStack(AllocStackInst *asi) {
136+
assert(asi);
137+
138+
// An alloc_stack with dynamic lifetime set has a lifetime that relies on
139+
// unrelated conditional control flow for correctness. This means that we may
140+
// statically leak along paths that were known by the emitter to never be
141+
// taken if the value is live. So bail since we can't verify this.
142+
if (asi->hasDynamicLifetime())
143+
return false;
144+
145+
// Currently in this verifier, we stop verifying if we find a switch_enum_addr
146+
// use. This creates a problem since no one has gone through and changed the
147+
// frontend/optimizer to understand that it needs to insert destroy_addr on
148+
// alloc_stack even if dynamically we know that the enum has a trivial case or
149+
// non-payloaded case due to a switch_enum_addr. So if one then performs the
150+
// completely unrelated, valid optimization of promoting the switch_enum_addr
151+
// to a switch_enum (lets say using a load_borrow), then this verifier will
152+
// see a leaked value along the non-payloaded path.
153+
//
154+
// Disable this verification for now until a complete analysis of enums is
155+
// implemented.
156+
//
157+
// https://bugs.swift.org/browse/SR-14123
158+
if (asi->getType().getEnumOrBoundGenericEnum())
159+
return false;
160+
161+
// Otherwise we can optimize!
162+
return true;
163+
}
164+
135165
void MemoryLocations::analyzeLocations(SILFunction *function) {
136166
// As we have to limit the set of handled locations to memory, which is
137167
// guaranteed to be not aliased, we currently only handle indirect function
@@ -152,12 +182,13 @@ void MemoryLocations::analyzeLocations(SILFunction *function) {
152182
}
153183
for (SILBasicBlock &BB : *function) {
154184
for (SILInstruction &I : BB) {
155-
auto *ASI = dyn_cast<AllocStackInst>(&I);
156-
if (ASI && !ASI->hasDynamicLifetime()) {
157-
if (allUsesInSameBlock(ASI)) {
158-
singleBlockLocations.push_back(ASI);
159-
} else {
160-
analyzeLocation(ASI);
185+
if (auto *ASI = dyn_cast<AllocStackInst>(&I)) {
186+
if (canHandleAllocStack(ASI)) {
187+
if (allUsesInSameBlock(ASI)) {
188+
singleBlockLocations.push_back(ASI);
189+
} else {
190+
analyzeLocation(ASI);
191+
}
161192
}
162193
}
163194
}

0 commit comments

Comments
 (0)