diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineCodegen.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineCodegen.kt index 39df85b2caf..3d1e091ad6f 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineCodegen.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineCodegen.kt @@ -448,7 +448,8 @@ class CoroutineCodegenForLambda private constructor( return CoroutineTransformerMethodVisitor( mv, access, name, desc, null, null, obtainClassBuilderForCoroutineState = { v }, - lineNumber = CodegenUtil.getLineNumberForElement(element, false) ?: 0, + element = element, + diagnostics = state.diagnostics, shouldPreserveClassInitialization = constructorCallNormalizationMode.shouldPreserveClassInitialization, containingClassInternalName = v.thisName, isForNamedFunction = false, diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt index 4ea1a7fb1b4..1963a6b0334 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt @@ -6,6 +6,7 @@ package org.jetbrains.kotlin.codegen.coroutines import com.intellij.util.containers.Stack +import org.jetbrains.kotlin.backend.common.CodegenUtil import org.jetbrains.kotlin.codegen.AsmUtil import org.jetbrains.kotlin.codegen.ClassBuilder import org.jetbrains.kotlin.codegen.StackValue @@ -19,7 +20,10 @@ import org.jetbrains.kotlin.codegen.optimization.fixStack.top import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer import org.jetbrains.kotlin.config.LanguageVersionSettings import org.jetbrains.kotlin.config.isReleaseCoroutines +import org.jetbrains.kotlin.diagnostics.DiagnosticSink +import org.jetbrains.kotlin.psi.KtElement import org.jetbrains.kotlin.resolve.jvm.AsmTypes +import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOrigin import org.jetbrains.kotlin.utils.addToStdlib.cast import org.jetbrains.kotlin.utils.addToStdlib.safeAs @@ -56,9 +60,11 @@ class CoroutineTransformerMethodVisitor( obtainClassBuilderForCoroutineState: () -> ClassBuilder, private val isForNamedFunction: Boolean, private val shouldPreserveClassInitialization: Boolean, - private val lineNumber: Int, private val languageVersionSettings: LanguageVersionSettings, private val sourceFile: String, + // These two are needed to report diagnostics about suspension points inside critical section + private val element: KtElement?, + private val diagnostics: DiagnosticSink, // It's only matters for named functions, may differ from '!isStatic(access)' in case of DefaultImpls private val needDispatchReceiver: Boolean = false, // May differ from containingClassInternalName in case of DefaultImpls @@ -68,6 +74,7 @@ class CoroutineTransformerMethodVisitor( ) : TransformationMethodVisitor(delegate, access, name, desc, signature, exceptions) { private val classBuilderForCoroutineState: ClassBuilder by lazy(obtainClassBuilderForCoroutineState) + private val lineNumber = element?.let { CodegenUtil.getLineNumberForElement(it, false) } ?: 0 private var continuationIndex = if (isForNamedFunction) -1 else 0 private var dataIndex = if (isForNamedFunction) -1 else 1 @@ -90,6 +97,8 @@ class CoroutineTransformerMethodVisitor( val suspensionPoints = collectSuspensionPoints(methodNode) + checkForSuspensionPointInsideMonitor(methodNode, suspensionPoints) + // First instruction in the method node may change in case of named function val actualCoroutineStart = methodNode.instructions.first @@ -191,6 +200,37 @@ class CoroutineTransformerMethodVisitor( } } + private fun checkForSuspensionPointInsideMonitor(methodNode: MethodNode, suspensionPoints: List) { + if (methodNode.instructions.asSequence().none { it.opcode == Opcodes.MONITORENTER }) return + + val cfg = ControlFlowGraph.build(methodNode) + val monitorDepthMap = hashMapOf() + fun addMonitorDepthToSuccs(index: Int, depth: Int) { + val insn = methodNode.instructions[index] + monitorDepthMap[insn] = depth + val newDepth = when (insn.opcode) { + Opcodes.MONITORENTER -> depth + 1 + Opcodes.MONITOREXIT -> depth - 1 + else -> depth + } + for (succIndex in cfg.getSuccessorsIndices(index)) { + if (monitorDepthMap[methodNode.instructions[succIndex]] == null) { + addMonitorDepthToSuccs(succIndex, newDepth) + } + } + } + + addMonitorDepthToSuccs(0, 0) + + for (suspensionPoint in suspensionPoints) { + if (monitorDepthMap[suspensionPoint.suspensionCallBegin]?.let { it > 0 } == true) { + // TODO: Support crossinline suspend lambdas + element?.let { diagnostics.report(ErrorsJvm.SUSPENSION_POINT_INSIDE_MONITOR.on(it)) } + return + } + } + } + private fun fixLvtForParameters(methodNode: MethodNode, startLabel: LabelNode, endLabel: LabelNode) { // We need to skip continuation, since the inliner likes to remap variables there. // But this is not a problem, since we have separate $continuation LVT entry diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/SuspendFunctionGenerationStrategy.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/SuspendFunctionGenerationStrategy.kt index 20385d27f98..733bee7dccf 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/SuspendFunctionGenerationStrategy.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/SuspendFunctionGenerationStrategy.kt @@ -66,7 +66,8 @@ open class SuspendFunctionGenerationStrategy( return CoroutineTransformerMethodVisitor( mv, access, name, desc, null, null, containingClassInternalName, this::classBuilderForCoroutineState, isForNamedFunction = true, - lineNumber = CodegenUtil.getLineNumberForElement(declaration, false) ?: 0, + element = declaration, + diagnostics = state.diagnostics, shouldPreserveClassInitialization = constructorCallNormalizationMode.shouldPreserveClassInitialization, needDispatchReceiver = originalSuspendDescriptor.dispatchReceiverParameter != null, internalNameForDispatchReceiver = containingClassInternalNameOrNull(), diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/AnonymousObjectTransformer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/AnonymousObjectTransformer.kt index a4852c403cc..2e0d4e811a5 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/AnonymousObjectTransformer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/AnonymousObjectTransformer.kt @@ -448,7 +448,8 @@ class AnonymousObjectTransformer( ArrayUtil.toStringArray(original.exceptions) ), original.access, original.name, original.desc, null, null, obtainClassBuilderForCoroutineState = { builder }, - lineNumber = 0, // <- TODO + element = null, + diagnostics = state.diagnostics, languageVersionSettings = languageVersionSettings, shouldPreserveClassInitialization = state.constructorCallNormalizationMode.shouldPreserveClassInitialization, containingClassInternalName = builder.thisName, @@ -477,7 +478,8 @@ class AnonymousObjectTransformer( ArrayUtil.toStringArray(original.exceptions) ), original.access, original.name, original.desc, null, null, obtainClassBuilderForCoroutineState = { (inliningContext as RegeneratedClassContext).continuationBuilders[continuationClassName]!! }, - lineNumber = 0, // <- TODO + element = null, + diagnostics = state.diagnostics, languageVersionSettings = languageVersionSettings, shouldPreserveClassInitialization = state.constructorCallNormalizationMode.shouldPreserveClassInitialization, containingClassInternalName = builder.thisName, diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java index d940baddcc1..5a7e0a6c8d3 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java @@ -147,7 +147,7 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension { MAP.put(USAGE_OF_JVM_DEFAULT_THROUGH_SUPER_CALL, "Super calls of '@JvmDefault' members are only allowed with -Xjvm-default option"); MAP.put(NON_JVM_DEFAULT_OVERRIDES_JAVA_DEFAULT, "Non-@JvmDefault interface method cannot override default Java method. Please annotate this method with @JvmDefault"); MAP.put(EXPLICIT_METADATA_IS_DISALLOWED, "Explicit @Metadata is disallowed"); - MAP.put(SUSPENSION_POINT_INSIDE_MONITOR, "The ''{0}'' suspension point is inside a critical section", NAME); + MAP.put(SUSPENSION_POINT_INSIDE_MONITOR, "A suspension point is inside a critical section"); } @NotNull diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java index c5004fb981a..af54b15c3b4 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java @@ -133,7 +133,7 @@ public interface ErrorsJvm { DiagnosticFactory1 ANNOTATION_TARGETS_NON_EXISTENT_ACCESSOR = DiagnosticFactory1.create(WARNING); - DiagnosticFactory1 SUSPENSION_POINT_INSIDE_MONITOR = DiagnosticFactory1.create(ERROR, DEFAULT); + DiagnosticFactory0 SUSPENSION_POINT_INSIDE_MONITOR = DiagnosticFactory0.create(ERROR); @SuppressWarnings("UnusedDeclaration") Object _initializer = new Object() { diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/output.txt b/compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/output.txt index 2d8085c78a2..e0018b4ce6b 100644 --- a/compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/output.txt +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/output.txt @@ -1,10 +1,13 @@ -compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt:10:13: error: the 'suspensionPoint' suspension point is inside a critical section - suspensionPoint() +compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt:8:13: error: a suspension point is inside a critical section + builder { ^ -compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt:14:13: error: the 'suspensionPoint' suspension point is inside a critical section - suspensionPoint() +compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt:14:13: error: a suspension point is inside a critical section + builder { ^ -compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt:19:15: error: the 'suspensionPoint' suspension point is inside a critical section - { suspensionPoint() } - ^ +compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt:20:13: error: a suspension point is inside a critical section + builder { + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt:42:9: error: a suspension point is inside a critical section + override suspend fun run() { + ^ COMPILATION_ERROR diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt b/compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt index 283dc0bdcfe..feae7e6d102 100644 --- a/compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt @@ -9,14 +9,44 @@ fun test() { synchronized(lock) { suspensionPoint() } + } + builder { inlineMe { suspensionPoint() } + } + builder { monitorInFinally( - { suspensionPoint() }, + {}, { suspensionPoint() } ) } + + synchronized(lock) { + builder { + suspensionPoint() + } + } + + synchronized(lock) { + object : SuspendRunnable { + override suspend fun run() { + suspensionPoint() + } + } + } + + object : SuspendRunnable { + override suspend fun run() { + synchronized(lock) { + suspensionPoint() + } + } + } +} + +interface SuspendRunnable { + suspend fun run() }