Don't emit redundant retain-release sequence for Obj-C alloc result

when calling Kotlin constructor generated for Objective-C initializer.

retain-release here is suboptimal and also may provoke specific bugs
to reproduce.
This commit is contained in:
Svyatoslav Scherbina
2019-08-06 16:42:40 +03:00
committed by SvyatoslavScherbina
parent 7efab59bc1
commit 9e149b6d91
6 changed files with 104 additions and 28 deletions
@@ -287,6 +287,10 @@ private fun KotlinToCCallBuilder.buildCall(
returnValue(cCallBuilder.build(targetFunctionName))
}
internal sealed class ObjCCallReceiver {
class Regular(val rawPtr: IrExpression) : ObjCCallReceiver()
class Retained(val rawPtr: IrExpression) : ObjCCallReceiver()
}
internal fun KotlinStubs.generateObjCCall(
builder: IrBuilderWithScope,
@@ -295,7 +299,7 @@ internal fun KotlinStubs.generateObjCCall(
selector: String,
call: IrFunctionAccessExpression,
superQualifier: IrClassSymbol?,
receiver: IrExpression,
receiver: ObjCCallReceiver,
arguments: List<IrExpression?>
) = builder.irBlock {
val callBuilder = KotlinToCCallBuilder(builder, this@generateObjCCall, isObjCMethod = true)
@@ -320,11 +324,31 @@ internal fun KotlinStubs.generateObjCCall(
val targetFunctionName = "targetPtr"
val preparedReceiver = if (method.consumesReceiver()) {
irCall(symbols.interopObjCRetain.owner).apply {
putValueArgument(0, receiver)
when (receiver) {
is ObjCCallReceiver.Regular -> irCall(symbols.interopObjCRetain.owner).apply {
putValueArgument(0, receiver.rawPtr)
}
is ObjCCallReceiver.Retained -> receiver.rawPtr
}
} else {
receiver
when (receiver) {
is ObjCCallReceiver.Regular -> receiver.rawPtr
is ObjCCallReceiver.Retained -> {
// Note: shall not happen: Retained is used only for alloc result currently,
// which is used only as receiver for init methods, which are always receiver-consuming.
// Can't even add a test for the code below.
val rawPtrVar = scope.createTemporaryVariable(receiver.rawPtr)
callBuilder.bridgeCallBuilder.prepare += rawPtrVar
callBuilder.bridgeCallBuilder.cleanup += {
irCall(symbols.interopObjCRelease).apply {
putValueArgument(0, irGet(rawPtrVar)) // Balance retained pointer.
}
}
irGet(rawPtrVar)
}
}
}
val receiverOrSuper = if (superQualifier != null) {
@@ -26,7 +26,6 @@ import org.jetbrains.kotlin.backend.konan.llvm.tryGetIntrinsicType
import org.jetbrains.kotlin.builtins.UnsignedTypes
import org.jetbrains.kotlin.descriptors.*
import org.jetbrains.kotlin.descriptors.ClassDescriptor
import org.jetbrains.kotlin.descriptors.annotations.Annotations
import org.jetbrains.kotlin.ir.IrElement
import org.jetbrains.kotlin.ir.IrStatement
import org.jetbrains.kotlin.ir.builders.*
@@ -547,7 +546,7 @@ internal class InteropLoweringPart1(val context: Context) : BaseInteropIrTransfo
val initCall = builder.genLoweredObjCMethodCall(
initMethodInfo,
superQualifier = delegatingCallConstructingClass.symbol,
receiver = builder.getRawPtr(builder.irGet(constructedClass.thisReceiver!!)),
receiver = builder.irGet(constructedClass.thisReceiver!!),
arguments = initMethod.valueParameters.map { expression.getValueArgument(it.index) },
call = expression,
method = initMethod
@@ -584,6 +583,22 @@ internal class InteropLoweringPart1(val context: Context) : BaseInteropIrTransfo
arguments: List<IrExpression?>,
call: IrFunctionAccessExpression,
method: IrSimpleFunction
): IrExpression = genLoweredObjCMethodCall(
info = info,
superQualifier = superQualifier,
receiver = ObjCCallReceiver.Regular(rawPtr = getRawPtr(receiver)),
arguments = arguments,
call = call,
method = method
)
private fun IrBuilderWithScope.genLoweredObjCMethodCall(
info: ObjCMethodInfo,
superQualifier: IrClassSymbol?,
receiver: ObjCCallReceiver,
arguments: List<IrExpression?>,
call: IrFunctionAccessExpression,
method: IrSimpleFunction
): IrExpression = generateWithStubs(call) {
if (method.parent !is IrClass) {
// Category-provided.
@@ -669,7 +684,7 @@ internal class InteropLoweringPart1(val context: Context) : BaseInteropIrTransfo
return builder.genLoweredObjCMethodCall(
methodInfo,
superQualifier = expression.superQualifierSymbol,
receiver = builder.getRawPtr(expression.dispatchReceiver ?: expression.extensionReceiver!!),
receiver = expression.dispatchReceiver ?: expression.extensionReceiver!!,
arguments = arguments,
call = expression,
method = callee as IrSimpleFunction
@@ -736,25 +751,14 @@ internal class InteropLoweringPart1(val context: Context) : BaseInteropIrTransfo
arguments: List<IrExpression?>,
call: IrFunctionAccessExpression,
initMethod: IrSimpleFunction
): IrExpression = irBlock(startOffset, endOffset) {
val allocated = irTemporaryVar(callAlloc(classPtr))
val initCall = genLoweredObjCMethodCall(
initMethodInfo,
superQualifier = null,
receiver = irGet(allocated),
arguments = arguments,
call = call,
method = initMethod
)
+IrTryImpl(startOffset, endOffset, initCall.type).apply {
tryResult = initCall
finallyExpression = irCall(symbols.interopObjCRelease).apply {
putValueArgument(0, irGet(allocated)) // Balance pointer retained by alloc.
}
}
}
): IrExpression = genLoweredObjCMethodCall(
initMethodInfo,
superQualifier = null,
receiver = ObjCCallReceiver.Retained(rawPtr = callAlloc(classPtr)),
arguments = arguments,
call = call,
method = initMethod
)
private fun IrBuilderWithScope.getRawPtr(receiver: IrExpression) =
irCall(symbols.interopObjCObjectRawValueGetter).apply {
@@ -1,3 +1,3 @@
language = Objective-C
headers = Foundation/NSArray.h Foundation/NSValue.h Foundation/NSString.h
headerFilter = **/smoke.h Foundation/NSArray.h Foundation/NSValue.h Foundation/NSString.h
headers = Foundation/NSArray.h Foundation/NSValue.h Foundation/NSString.h Foundation/NSStream.h
headerFilter = **/smoke.h Foundation/NSArray.h Foundation/NSValue.h Foundation/NSString.h Foundation/NSStream.h
@@ -220,3 +220,7 @@ Protocol* getPrinterProtocol() {
+(instancetype _Nonnull)createCustom;
@end;
@interface TestAllocNoRetain : NSObject
@property BOOL ok;
@end;
@@ -26,6 +26,8 @@ fun run() {
testMultipleInheritanceClash()
testClashingWithAny()
testInitWithCustomSelector()
testAllocNoRetain()
testNSOutputStreamToMemoryConstructor()
assertEquals(2, ForwardDeclaredEnum.TWO.value)
@@ -397,6 +399,35 @@ private class TestInitWithCustomSelectorSubclass : TestInitWithCustomSelector {
companion object : TestInitWithCustomSelectorMeta()
}
fun testAllocNoRetain() {
// Ensure that calling Kotlin constructor generated for Objective-C initializer doesn't result in
// redundant retain-release sequence for `alloc` result, since it may provoke specific bugs to reproduce, e.g.
// the one found in [[NSOutputStream alloc] initToMemory] sequence where initToMemory deallocates its receiver
// forcibly when replacing it with other object: (to be compiled with ARC enabled)
/*
#import <Foundation/Foundation.h>
void* mem;
NSOutputStream* allocated = nil;
int main() {
allocated = [NSOutputStream alloc];
NSOutputStream* initialized = [allocated initToMemory];
mem = calloc(1, 0x10); // To corrupt the 'allocated' object header.
allocated = nil; // Crashes here in objc_release.
return 0;
}
*/
assertTrue(TestAllocNoRetain().ok)
}
fun testNSOutputStreamToMemoryConstructor() {
val stream: Any = NSOutputStream(toMemory = Unit)
assertTrue(stream is NSOutputStream)
}
fun nsArrayOf(vararg elements: Any): NSArray = NSMutableArray().apply {
elements.forEach {
this.addObject(it as ObjCObject)
+13
View File
@@ -276,3 +276,16 @@ static CustomRetainMethodsImpl* retainedCustomRetainMethodsImpl;
}
@end;
@implementation TestAllocNoRetain
-(instancetype)init {
__weak id weakSelf = self;
self = [TestAllocNoRetain alloc];
if (self = [super init]) {
// Ensure that original self value was deallocated:
self.ok = (weakSelf == nil);
// So it's RC was 1, which means there wasn't redundant retain applied to it.
}
return self;
}
@end;