From 30ff21e757a54224edf7e1aff5f956aa4376467a Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Tue, 24 Aug 2021 19:31:35 -0700 Subject: [PATCH] [silgen] Ensure that the outer cleanup is emitted along failure paths when initializing sub-tuple patterns Previously, we would leak in this case along the inner failure path since we had already forwarded the outer cleanup. Instead in this patch, I change the outer cleanup to be persistently active (ensuring that failure paths along the sub-pattern are cleaned up appropriately) and forward it manually afterwards ensuring that we do not /actually/ emit the cleanup along the success path. rdar://81817725 --- lib/SILGen/SILGenDecl.cpp | 13 +++++++++++-- test/SILGen/enum.swift | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/SILGen/SILGenDecl.cpp b/lib/SILGen/SILGenDecl.cpp index 6943ec9201f69..c7d8e94ec2334 100644 --- a/lib/SILGen/SILGenDecl.cpp +++ b/lib/SILGen/SILGenDecl.cpp @@ -69,9 +69,16 @@ void TupleInitialization::copyOrInitValueInto(SILGenFunction &SGF, // In the address case, we forward the underlying value and store it // into memory and then create a +1 cleanup. since we assume here // that we have a +1 value since we are forwarding into memory. + // + // In order to ensure that we properly clean up along any failure paths, we + // need to mark value as being persistently active. We then unforward it once + // we are done. assert(value.isPlusOne(SGF) && "Can not store a +0 value into memory?!"); - value = ManagedValue::forUnmanaged(value.forward(SGF)); - return copyOrInitValueIntoHelper( + CleanupStateRestorationScope valueScope(SGF.Cleanups); + if (value.hasCleanup()) + valueScope.pushCleanupState(value.getCleanup(), + CleanupState::PersistentlyActive); + copyOrInitValueIntoHelper( SGF, loc, value, isInit, SubInitializations, [&](ManagedValue aggregate, unsigned i, SILType fieldType) -> ManagedValue { @@ -83,6 +90,8 @@ void TupleInitialization::copyOrInitValueInto(SILGenFunction &SGF, return SGF.emitManagedRValueWithCleanup(elt.getValue()); }); + std::move(valueScope).pop(); + value.forward(SGF); } void TupleInitialization::finishUninitialized(SILGenFunction &SGF) { diff --git a/test/SILGen/enum.swift b/test/SILGen/enum.swift index 00b1b21cada77..5a9ca073cdb66 100644 --- a/test/SILGen/enum.swift +++ b/test/SILGen/enum.swift @@ -219,3 +219,23 @@ func sr7799_1(bar: SR7799??) { default: print("default") } } + +// Make sure that we handle enum, tuple initialization composed +// correctly. Previously, we leaked down a failure path due to us misusing +// scopes. +enum rdar81817725 { + case localAddress + case setOption(Int, Any) + + static func takeAny(_:Any) -> Bool { return true } + + static func testSwitchCleanup(syscall: rdar81817725, expectedLevel: Int, + valueMatcher: (Any) -> Bool) + throws -> Bool { + if case .setOption(expectedLevel, let value) = syscall { + return rdar81817725.takeAny(value) + } else { + return false + } + } +}