Skip to content

Commit eda3c8d

Browse files
tkremenektkremenek
committed
Merge pull request #1967 from slavapestov/partial-apply-fix-2.2
Fix for memory leak when partially applying function with @autoreleased return value
2 parents dd2e024 + 2d29049 commit eda3c8d

File tree

8 files changed

+159
-25
lines changed

8 files changed

+159
-25
lines changed

‎lib/SIL/SILBuilder.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,26 @@ SILType SILBuilder::getPartialApplyResultType(SILType origTy, unsigned argCount,
3232
auto extInfo = SILFunctionType::ExtInfo(
3333
SILFunctionType::Representation::Thick,
3434
/*noreturn*/ FTI->isNoReturn());
35-
35+
36+
// If the original method has an @unowned_inner_pointer return, the partial
37+
// application thunk will lifetime-extend 'self' for us, converting the
38+
// return value to @unowned.
39+
//
40+
// If the original method has an @autoreleased return, the partial application
41+
// thunk will retain it for us, converting the return value to @owned.
42+
SILResultInfo result = FTI->getResult();
43+
if (result.getConvention() == ResultConvention::UnownedInnerPointer)
44+
result = SILResultInfo(result.getType(), ResultConvention::Unowned);
45+
else if (result.getConvention() == ResultConvention::Autoreleased)
46+
result = SILResultInfo(result.getType(), ResultConvention::Owned);
47+
3648
auto appliedFnType = SILFunctionType::get(nullptr, extInfo,
3749
ParameterConvention::Direct_Owned,
3850
newParams,
39-
FTI->getResult(),
51+
result,
4052
FTI->getOptionalErrorResult(),
4153
M.getASTContext());
54+
4255
return SILType::getPrimitiveObjectType(appliedFnType);
4356
}
4457

‎lib/SIL/Verifier.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,18 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
877877
require(resultInfo->getResult() == expectedResult,
878878
"result type of result function type for partially applied "
879879
"@unowned_inner_pointer function should have @unowned convention");
880+
881+
// The "autoreleased" convention doesn't survive through a
882+
// partial application, since the thunk takes responsibility for
883+
// retaining the return value.
884+
} else if (expectedResult.getConvention() == ResultConvention::Autoreleased)
885+
{
886+
expectedResult = SILResultInfo(expectedResult.getType(),
887+
ResultConvention::Owned);
888+
require(resultInfo->getResult() == expectedResult,
889+
"result type of result function type for partially applied "
890+
"@autoreleased function should have @owned convention");
891+
880892
} else {
881893
require(resultInfo->getResult() == expectedResult,
882894
"result type of result function type does not match original "

‎lib/SILGen/SILGenApply.cpp

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4258,24 +4258,10 @@ static SILValue emitDynamicPartialApply(SILGenFunction &gen,
42584258
SILValue method,
42594259
SILValue self,
42604260
CanFunctionType methodTy) {
4261-
// Pop the self type off of the function type.
4262-
// Just to be weird, partially applying an objc method produces a native
4263-
// function (?!)
4264-
auto fnTy = method.getType().castTo<SILFunctionType>();
4265-
// If the original method has an @unowned_inner_pointer return, the partial
4266-
// application thunk will lifetime-extend 'self' for us.
4267-
auto resultInfo = fnTy->getResult();
4268-
if (resultInfo.getConvention() == ResultConvention::UnownedInnerPointer)
4269-
resultInfo = SILResultInfo(resultInfo.getType(), ResultConvention::Unowned);
4270-
4271-
auto partialApplyTy = SILFunctionType::get(fnTy->getGenericSignature(),
4272-
fnTy->getExtInfo()
4273-
.withRepresentation(SILFunctionType::Representation::Thick),
4274-
ParameterConvention::Direct_Owned,
4275-
fnTy->getParameters()
4276-
.slice(0, fnTy->getParameters().size() - 1),
4277-
resultInfo, fnTy->getOptionalErrorResult(),
4278-
gen.getASTContext());
4261+
auto partialApplyTy = SILBuilder::getPartialApplyResultType(method.getType(),
4262+
/*argCount*/1,
4263+
gen.SGM.M,
4264+
/*subs*/{});
42794265

42804266
// Retain 'self' because the partial apply will take ownership.
42814267
// We can't simply forward 'self' because the partial apply is conditional.
@@ -4289,11 +4275,11 @@ static SILValue emitDynamicPartialApply(SILGenFunction &gen,
42894275
#endif
42904276

42914277
SILValue result = gen.B.createPartialApply(loc, method, method.getType(), {},
4292-
self, SILType::getPrimitiveObjectType(partialApplyTy));
4278+
self, partialApplyTy);
42934279
// If necessary, thunk to the native ownership conventions and bridged types.
42944280
auto nativeTy = gen.getLoweredLoadableType(methodTy).castTo<SILFunctionType>();
42954281

4296-
if (nativeTy != partialApplyTy) {
4282+
if (nativeTy != partialApplyTy.getSwiftRValueType()) {
42974283
result = gen.emitBlockToFunc(loc, ManagedValue::forUnmanaged(result),
42984284
nativeTy).forward(gen);
42994285
}

‎test/ClangModules/Inputs/serialization-sil.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
@protocol Test
2+
@optional
3+
- (nonnull id)normalObject;
4+
- (nonnull void *)innerPointer __attribute__((objc_returns_inner_pointer));
5+
@property (nonnull) id normalObjectProp;
6+
@property (nonnull) void *innerPointerProp __attribute__((objc_returns_inner_pointer));
7+
@end

‎test/ClangModules/serialization-sil.swift

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// RUN: rm -rf %t && mkdir -p %t
2+
// RUN: %target-swift-frontend -emit-module-path %t/Test.swiftmodule -emit-sil -o /dev/null -module-name Test %s -sdk "" -import-objc-header %S/Inputs/serialization-sil.h
3+
// RUN: %target-sil-extract %t/Test.swiftmodule -func=_TF4Test16testPartialApplyFPSo4Test_T_ -o - | FileCheck %s
4+
5+
// REQUIRES: objc_interop
6+
7+
// @_transparent to force serialization.
8+
@_transparent
9+
public func testPartialApply(obj: Test) {
10+
// CHECK-LABEL: @_TF4Test16testPartialApplyFPSo4Test_T_ : $@convention(thin) (@owned Test) -> () {
11+
if let curried1 = obj.normalObject {
12+
// CHECK: dynamic_method_br [[CURRIED1_OBJ:%.+]] : $@opened([[CURRIED1_EXISTENTIAL:.+]]) Test, #Test.normalObject!1.foreign, [[CURRIED1_TRUE:[^, ]+]], [[CURRIED1_FALSE:[^, ]+]]
13+
// CHECK: [[CURRIED1_TRUE]]([[CURRIED1_METHOD:%.+]] : $@convention(objc_method) (@opened([[CURRIED1_EXISTENTIAL]]) Test) -> @autoreleased AnyObject):
14+
// CHECK: [[CURRIED1_PARTIAL:%.+]] = partial_apply [[CURRIED1_METHOD]]([[CURRIED1_OBJ]]) : $@convention(objc_method) (@opened([[CURRIED1_EXISTENTIAL]]) Test) -> @autoreleased AnyObject
15+
// CHECK: [[CURRIED1_THUNK:%.+]] = function_ref @_TTRXFo__oPs9AnyObject__XFo_iT__iPS___ : $@convention(thin) (@out AnyObject, @in (), @owned @callee_owned () -> @owned AnyObject) -> ()
16+
// CHECK: = partial_apply [[CURRIED1_THUNK]]([[CURRIED1_PARTIAL]]) : $@convention(thin) (@out AnyObject, @in (), @owned @callee_owned () -> @owned AnyObject) -> ()
17+
// CHECK: [[CURRIED1_FALSE]]:
18+
curried1()
19+
}
20+
if let curried2 = obj.innerPointer {
21+
// CHECK: dynamic_method_br [[CURRIED2_OBJ:%.+]] : $@opened([[CURRIED2_EXISTENTIAL:.+]]) Test, #Test.innerPointer!1.foreign, [[CURRIED2_TRUE:[^, ]+]], [[CURRIED2_FALSE:[^, ]+]]
22+
// CHECK: [[CURRIED2_TRUE]]([[CURRIED2_METHOD:%.+]] : $@convention(objc_method) (@opened([[CURRIED2_EXISTENTIAL]]) Test) -> @unowned_inner_pointer UnsafeMutablePointer<()>):
23+
// CHECK: [[CURRIED2_PARTIAL:%.+]] = partial_apply [[CURRIED2_METHOD]]([[CURRIED2_OBJ]]) : $@convention(objc_method) (@opened([[CURRIED2_EXISTENTIAL]]) Test) -> @unowned_inner_pointer UnsafeMutablePointer<()>
24+
// CHECK: [[CURRIED2_THUNK:%.+]] = function_ref @_TTRXFo__dGSpT___XFo_iT__iGSpT___ : $@convention(thin) (@out UnsafeMutablePointer<()>, @in (), @owned @callee_owned () -> UnsafeMutablePointer<()>) -> ()
25+
// CHECK: = partial_apply [[CURRIED2_THUNK]]([[CURRIED2_PARTIAL]]) : $@convention(thin) (@out UnsafeMutablePointer<()>, @in (), @owned @callee_owned () -> UnsafeMutablePointer<()>) -> ()
26+
// CHECK: [[CURRIED2_FALSE]]:
27+
curried2()
28+
}
29+
if let prop1 = obj.normalObjectProp {
30+
// CHECK: dynamic_method_br [[PROP1_OBJ:%.+]] : $@opened([[PROP1_EXISTENTIAL:.+]]) Test, #Test.normalObjectProp!getter.1.foreign, [[PROP1_TRUE:[^, ]+]], [[PROP1_FALSE:[^, ]+]]
31+
// CHECK: [[PROP1_TRUE]]([[PROP1_METHOD:%.+]] : $@convention(objc_method) (@opened([[PROP1_EXISTENTIAL]]) Test) -> @autoreleased AnyObject):
32+
// CHECK: [[PROP1_PARTIAL:%.+]] = partial_apply [[PROP1_METHOD]]([[PROP1_OBJ]]) : $@convention(objc_method) (@opened([[PROP1_EXISTENTIAL]]) Test) -> @autoreleased AnyObject
33+
// CHECK: = apply [[PROP1_PARTIAL]]() : $@callee_owned () -> @owned AnyObject
34+
// CHECK: [[PROP1_FALSE]]:
35+
_ = prop1
36+
}
37+
if let prop2 = obj.innerPointerProp {
38+
// CHECK: dynamic_method_br [[PROP2_OBJ:%.+]] : $@opened([[PROP2_EXISTENTIAL:.+]]) Test, #Test.innerPointerProp!getter.1.foreign, [[PROP2_TRUE:[^, ]+]], [[PROP2_FALSE:[^, ]+]]
39+
// CHECK: [[PROP2_TRUE]]([[PROP2_METHOD:%.+]] : $@convention(objc_method) (@opened([[PROP2_EXISTENTIAL]]) Test) -> @unowned_inner_pointer UnsafeMutablePointer<()>):
40+
// CHECK: [[PROP2_PARTIAL:%.+]] = partial_apply [[PROP2_METHOD]]([[PROP2_OBJ]]) : $@convention(objc_method) (@opened([[PROP2_EXISTENTIAL]]) Test) -> @unowned_inner_pointer UnsafeMutablePointer<()>
41+
// CHECK: = apply [[PROP2_PARTIAL]]() : $@callee_owned () -> UnsafeMutablePointer<()>
42+
// CHECK: [[PROP2_FALSE]]:
43+
_ = prop2
44+
}
45+
} // CHECK: {{^}$}}

‎test/Interpreter/SDK/objc_protocol_lookup.swift

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,44 @@ func check(x: AnyObject) {
3737
check(NSString()) // CHECK: true true
3838
check(C()) // CHECK: true true
3939
check(D()) // CHECK: true true
40+
41+
// Make sure partial application of methods with @autoreleased
42+
// return values works
43+
44+
var count = 0
45+
46+
class Juice : NSObject {
47+
override init() {
48+
count += 1
49+
}
50+
51+
deinit {
52+
count -= 1
53+
}
54+
}
55+
56+
@objc protocol Fruit {
57+
optional var juice: Juice { get }
58+
}
59+
60+
class Durian : NSObject, Fruit {
61+
init(juice: Juice) {
62+
self.juice = juice
63+
}
64+
65+
var juice: Juice
66+
}
67+
68+
func consume(fruit: Fruit) {
69+
_ = fruit.juice
70+
}
71+
72+
autoreleasepool {
73+
let tasty = Durian(juice: Juice())
74+
print(count) // CHECK: 1
75+
consume(tasty)
76+
}
77+
78+
do {
79+
print(count) // CHECK: 0
80+
}

‎test/SILGen/dynamic_lookup.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,3 +201,33 @@ func downcast(var obj: AnyObject) -> X {
201201
// CHECK-NEXT: return [[X]] : $X
202202
return obj as! X
203203
}
204+
205+
@objc class Juice { }
206+
207+
@objc protocol Fruit {
208+
optional var juice: Juice { get }
209+
}
210+
211+
// CHECK-LABEL: sil hidden @_TF14dynamic_lookup7consumeFPS_5Fruit_T_
212+
// CHECK: bb0(%0 : $Fruit):
213+
// CHECK: [[BOX:%.*]] = alloc_stack $Optional<Juice>
214+
// CHECK: dynamic_method_br [[SELF:%.*]] : $@opened("{{.*}}") Fruit, #Fruit.juice!getter.1.foreign, bb1, bb2
215+
216+
// CHECK: bb1([[FN:%.*]] : $@convention(objc_method) (@opened("{{.*}}") Fruit) -> @autoreleased Juice):
217+
// CHECK: [[METHOD:%.*]] = partial_apply [[FN]]([[SELF]]) : $@convention(objc_method) (@opened("{{.*}}") Fruit) -> @autoreleased Juice
218+
// CHECK: [[RESULT:%.*]] = apply [[METHOD]]() : $@callee_owned () -> @owned Juice
219+
// CHECK: [[PAYLOAD:%.*]] = init_enum_data_addr [[BOX]] : $*Optional<Juice>, #Optional.Some!enumelt.1
220+
// CHECK: store [[RESULT]] to [[PAYLOAD]]
221+
// CHECK: inject_enum_addr [[BOX]] : $*Optional<Juice>, #Optional.Some!enumelt.1
222+
// CHECK: br bb3
223+
224+
// CHECK: bb2:
225+
// CHECK: inject_enum_addr [[BOX]] : $*Optional<Juice>, #Optional.None!enumelt
226+
// CHECK: br bb3
227+
228+
// CHECK: bb3:
229+
// CHECK: return
230+
231+
func consume(fruit: Fruit) {
232+
_ = fruit.juice
233+
}

‎test/SILGen/objc_currying.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func curry_pod_AnyObject(x: AnyObject) -> Int -> Int {
8484
// CHECK: dynamic_method_br [[SELF:%.*]] : $@opened({{.*}}) AnyObject, #CurryTest.normalOwnership!1.foreign, [[HAS_METHOD:bb[0-9]+]]
8585
// CHECK: [[HAS_METHOD]]([[METHOD:%.*]] : $@convention(objc_method) (ImplicitlyUnwrappedOptional<CurryTest>, @opened({{.*}}) AnyObject) -> @autoreleased ImplicitlyUnwrappedOptional<CurryTest>):
8686
// CHECK: [[PA:%.*]] = partial_apply [[METHOD]]([[SELF]])
87-
// CHECK: [[THUNK:%.*]] = function_ref @_TTRXFo_dGSQCSo9CurryTest__aGSQS___XFo_oGSQS___oGSQS___
87+
// CHECK: [[THUNK:%.*]] = function_ref @_TTRXFo_dGSQCSo9CurryTest__oGSQS___XFo_oGSQS___oGSQS___
8888
// CHECK: partial_apply [[THUNK]]([[PA]])
8989
func curry_normalOwnership_AnyObject(x: AnyObject) -> CurryTest! -> CurryTest! {
9090
return x.normalOwnership!
@@ -105,7 +105,7 @@ func curry_weirdOwnership_AnyObject(x: AnyObject) -> CurryTest! -> CurryTest! {
105105
// CHECK: dynamic_method_br [[SELF:%.*]] : $@opened({{.*}}) AnyObject, #CurryTest.bridged!1.foreign, [[HAS_METHOD:bb[0-9]+]]
106106
// CHECK: [[HAS_METHOD]]([[METHOD:%.*]] : $@convention(objc_method) (ImplicitlyUnwrappedOptional<NSString>, @opened({{.*}}) AnyObject) -> @autoreleased ImplicitlyUnwrappedOptional<NSString>):
107107
// CHECK: [[PA:%.*]] = partial_apply [[METHOD]]([[SELF]])
108-
// CHECK: [[THUNK:%.*]] = function_ref @_TTRXFo_dGSQCSo8NSString__aGSQS___XFo_oGSQSS__oGSQSS__
108+
// CHECK: [[THUNK:%.*]] = function_ref @_TTRXFo_dGSQCSo8NSString__oGSQS___XFo_oGSQSS__oGSQSS__
109109
// CHECK: partial_apply [[THUNK]]([[PA]])
110110
func curry_bridged_AnyObject(x: AnyObject) -> String! -> String! {
111111
return x.bridged!
@@ -117,7 +117,7 @@ func curry_bridged_AnyObject(x: AnyObject) -> String! -> String! {
117117
// CHECK: dynamic_method_br [[SELF:%.*]] : $@opened({{.*}}) AnyObject, #CurryTest.returnsSelf!1.foreign, [[HAS_METHOD:bb[0-9]+]]
118118
// CHECK: [[HAS_METHOD]]([[METHOD:%.*]] : $@convention(objc_method) (@opened({{.*}}) AnyObject) -> @autoreleased ImplicitlyUnwrappedOptional<AnyObject>):
119119
// CHECK: [[PA:%.*]] = partial_apply [[METHOD]]([[SELF]])
120-
// CHECK: [[THUNK:%.*]] = function_ref @_TTRXFo__aGSQPs9AnyObject___XFo__oGSQPS____
120+
// CHECK: [[THUNK:%.*]] = function_ref @_TTRXFo__oGSQPs9AnyObject___XFo_iT__iGSQPS____
121121
// CHECK: partial_apply [[THUNK]]([[PA]])
122122
func curry_returnsSelf_AnyObject(x: AnyObject) -> () -> AnyObject! {
123123
return x.returnsSelf!

0 commit comments

Comments
 (0)