From 9f54e9cd1fb21b6f5fde87a2c12ea26cd85d879d Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Thu, 19 Jan 2023 09:46:56 -0800 Subject: [PATCH] SILGen: Fix double-free of `__owned` parameters of functions with `@_backDeploy`. The following program crashed when compiled with the Swift 5.7 and 5.8 compilers: ``` @available(macOS 12, *) @_backDeploy(before: macOS 99) public func foo(_ t: __owned T) { print("foo") } class C { deinit { print("deinit") } } foo(C()) print("done") ``` ``` > ./test foo deinit [1] 49162 segmentation fault ./test ``` The root cause is that generated SIL for the back deployment thunk for `foo(_:)` included its own `destroy_addr` instruction for the value of `t`, but didn't copy the parameter before passing it to the real function implementation which also destroys the value. The fix is to forward ownership of the parameter values to the called function, which causes cleanup generation to be skipped. Resolves rdar://104436515 --- lib/SILGen/SILGenBackDeploy.cpp | 28 ++++++---- .../back_deploy_attribute_generic_func.swift | 52 +++++++++++++++++-- test/attr/Inputs/BackDeployHelper.swift | 4 +- 3 files changed, 67 insertions(+), 17 deletions(-) diff --git a/lib/SILGen/SILGenBackDeploy.cpp b/lib/SILGen/SILGenBackDeploy.cpp index 261539f1f0fed..c6e767604cf60 100644 --- a/lib/SILGen/SILGenBackDeploy.cpp +++ b/lib/SILGen/SILGenBackDeploy.cpp @@ -178,20 +178,28 @@ void SILGenFunction::emitBackDeploymentThunk(SILDeclRef thunk) { F.setGenericEnvironment(SGM.Types.getConstantGenericEnvironment(thunk)); - emitBasicProlog(FD->getParameters(), FD->getImplicitSelfDecl(), - FD->getResultInterfaceType(), FD, FD->hasThrows(), - FD->getThrowsLoc()); - prepareEpilog(FD->getResultInterfaceType(), FD->hasThrows(), - CleanupLocation(FD)); + // Generate the thunk prolog by collecting parameters. + SmallVector params; + SmallVector indirectParams; + collectThunkParams(loc, params, &indirectParams); - // Gather the entry block's arguments up so that we can forward them. + // Build up the list of arguments that we're going to invoke the the real + // function with. SmallVector paramsForForwarding; - SILBasicBlock *entryBlock = getFunction().getEntryBlock(); - for (SILArgument *arg : - make_range(entryBlock->args_begin(), entryBlock->args_end())) { - paramsForForwarding.emplace_back(arg); + for (auto indirectParam : indirectParams) { + paramsForForwarding.emplace_back(indirectParam); + } + + for (auto param : params) { + // We're going to directly call either the original function or the fallback + // function with these arguments and then return. Therefore we just forward + // the arguments instead of handling their ownership conventions. + paramsForForwarding.emplace_back(param.forward(*this)); } + prepareEpilog(FD->getResultInterfaceType(), FD->hasThrows(), + CleanupLocation(FD)); + SILBasicBlock *availableBB = createBasicBlock("availableBB"); SILBasicBlock *unavailableBB = createBasicBlock("unavailableBB"); diff --git a/test/SILGen/back_deploy_attribute_generic_func.swift b/test/SILGen/back_deploy_attribute_generic_func.swift index 3cfb303eaaae2..5831d85215ec2 100644 --- a/test/SILGen/back_deploy_attribute_generic_func.swift +++ b/test/SILGen/back_deploy_attribute_generic_func.swift @@ -5,14 +5,14 @@ // REQUIRES: OS=macosx -// -- Fallback definition of genericFunc() +// -- Fallback definition of genericFunc(_:) // CHECK-LABEL: sil non_abi [serialized] [ossa] @$s11back_deploy11genericFuncyxxlFTwB : $@convention(thin) (@in_guaranteed T) -> @out T // CHECK: bb0([[OUT_ARG:%.*]] : $*T, [[IN_ARG:%.*]] : $*T): // CHECK: copy_addr [[IN_ARG]] to [init] [[OUT_ARG]] : $*T // CHECK: [[RESULT:%.*]] = tuple () // CHECK: return [[RESULT]] : $() -// -- Back deployment thunk for genericFunc() +// -- Back deployment thunk for genericFunc(_:) // CHECK-LABEL: sil non_abi [serialized] [thunk] [ossa] @$s11back_deploy11genericFuncyxxlFTwb : $@convention(thin) (@in_guaranteed T) -> @out T // CHECK: bb0([[OUT_ARG:%.*]] : $*T, [[IN_ARG:%.*]] : $*T): // CHECK: [[MAJOR:%.*]] = integer_literal $Builtin.Word, 10 @@ -36,16 +36,58 @@ // CHECK: [[RESULT:%.*]] = tuple () // CHECK: return [[RESULT]] : $() -// -- Original definition of genericFunc() +// -- Original definition of genericFunc(_:) // CHECK-LABEL: sil [available 10.52] [ossa] @$s11back_deploy11genericFuncyxxlF : $@convention(thin) (@in_guaranteed T) -> @out T @_backDeploy(before: macOS 10.52) public func genericFunc(_ t: T) -> T { return t } +// -- Fallback definition of genericFuncWithOwnedParam(_:) +// CHECK-LABEL: sil non_abi [serialized] [ossa] @$s11back_deploy25genericFuncWithOwnedParamyyxnlFTwB : $@convention(thin) (@in T) -> () +// CHECK: bb0([[IN_ARG:%.*]] : $*T): +// CHECK: destroy_addr [[IN_ARG]] : $*T +// CHECK: [[RESULT:%.*]] = tuple () +// CHECK: return [[RESULT]] : $() + +// -- Back deployment thunk for genericFuncWithOwnedParam(_:) +// CHECK-LABEL: sil non_abi [serialized] [thunk] [ossa] @$s11back_deploy25genericFuncWithOwnedParamyyxnlFTwb : $@convention(thin) (@in T) -> () +// CHECK: bb0([[IN_ARG:%.*]] : $*T): +// CHECK: [[MAJOR:%.*]] = integer_literal $Builtin.Word, 10 +// CHECK: [[MINOR:%.*]] = integer_literal $Builtin.Word, 52 +// CHECK: [[PATCH:%.*]] = integer_literal $Builtin.Word, 0 +// CHECK: [[OSVFN:%.*]] = function_ref @$ss26_stdlib_isOSVersionAtLeastyBi1_Bw_BwBwtF : $@convention(thin) (Builtin.Word, Builtin.Word, Builtin.Word) -> Builtin.Int1 +// CHECK: [[AVAIL:%.*]] = apply [[OSVFN]]([[MAJOR]], [[MINOR]], [[PATCH]]) : $@convention(thin) (Builtin.Word, Builtin.Word, Builtin.Word) -> Builtin.Int1 +// CHECK: cond_br [[AVAIL]], [[AVAIL_BB:bb[0-9]+]], [[UNAVAIL_BB:bb[0-9]+]] +// +// CHECK: [[UNAVAIL_BB]]: +// CHECK: [[FALLBACKFN:%.*]] = function_ref @$s11back_deploy25genericFuncWithOwnedParamyyxnlFTwB : $@convention(thin) <τ_0_0> (@in τ_0_0) -> () +// CHECK: {{%.*}} = apply [[FALLBACKFN]]([[IN_ARG]]) : $@convention(thin) <τ_0_0> (@in τ_0_0) -> () +// CHECK: br [[RETURN_BB:bb[0-9]+]] +// +// CHECK: [[AVAIL_BB]]: +// CHECK: [[ORIGFN:%.*]] = function_ref @$s11back_deploy25genericFuncWithOwnedParamyyxnlF : $@convention(thin) <τ_0_0> (@in τ_0_0) -> () +// CHECK: {{%.*}} = apply [[ORIGFN]]([[IN_ARG]]) : $@convention(thin) <τ_0_0> (@in τ_0_0) -> () +// CHECK: br [[RETURN_BB]] +// +// CHECK: [[RETURN_BB]] +// CHECK-NOT: destroy_addr +// CHECK: [[RESULT:%.*]] = tuple () +// CHECK: return [[RESULT]] : $() + +// -- Original definition of genericFuncWithOwnedParam(_:) +// CHECK-LABEL: sil [available 10.52] [ossa] @$s11back_deploy25genericFuncWithOwnedParamyyxnlF : $@convention(thin) (@in T) -> () +@_backDeploy(before: macOS 10.52) +public func genericFuncWithOwnedParam(_ t: __owned T) { } + +struct S {} + // CHECK-LABEL: sil hidden [ossa] @$s11back_deploy6calleryyF : $@convention(thin) () -> () func caller() { - // -- Verify the thunk is called + // -- Verify the thunks are called // CHECK: {{%.*}} = function_ref @$s11back_deploy11genericFuncyxxlFTwb : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0 - _ = genericFunc(Int32(1)) + _ = genericFunc(S()) + + // CHECK: {{%.*}} = function_ref @$s11back_deploy25genericFuncWithOwnedParamyyxnlFTwb : $@convention(thin) <τ_0_0> (@in τ_0_0) -> () + genericFuncWithOwnedParam(S()) } diff --git a/test/attr/Inputs/BackDeployHelper.swift b/test/attr/Inputs/BackDeployHelper.swift index 7aa5b7916f4e7..eaab7a3d231b8 100644 --- a/test/attr/Inputs/BackDeployHelper.swift +++ b/test/attr/Inputs/BackDeployHelper.swift @@ -35,7 +35,7 @@ public func v2APIsAreStripped() -> Bool { /// Describes types that can be appended to. public protocol Appendable { associatedtype Element - mutating func append(_ x: Element) + mutating func append(_ x: __owned Element) } /// Describes types that can be counted. @@ -107,7 +107,7 @@ public func pleaseThrow(_ shouldThrow: Bool) throws -> Bool { @_backDeploy(before: BackDeploy 2.0) public func genericAppend( _ a: inout T, - _ x: T.Element + _ x: __owned T.Element ) { return a.append(x) }