Forbid suspension points in critical sections

#KT-26480: Fixed

compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java

@@ -2432,6 +2432,11 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
             return;
         }
 
+        if (resolvedCall.getResultingDescriptor() instanceof FunctionDescriptor &&
+            ((FunctionDescriptor) resolvedCall.getResultingDescriptor()).isSuspend()) {
+            state.getGlobalCoroutinesContext().checkSuspendCall(resolvedCall);
+        }
+
         boolean isSuspendNoInlineCall =
                 CoroutineCodegenUtilKt.isSuspendNoInlineCall(resolvedCall, this, state.getLanguageVersionSettings());
         boolean isConstructor = resolvedCall.getResultingDescriptor() instanceof ConstructorDescriptor;

compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/GlobalCoroutinesContext.kt

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
+ * that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.codegen.coroutines
+
+import org.jetbrains.kotlin.backend.common.peek
+import org.jetbrains.kotlin.backend.common.pop
+import org.jetbrains.kotlin.diagnostics.DiagnosticSink
+import org.jetbrains.kotlin.diagnostics.Errors.SUSPENSION_POINT_INSIDE_MONITOR
+import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
+
+class GlobalCoroutinesContext(private val diagnostics: DiagnosticSink) {
+    private var monitorsDepth = 0
+
+    private val inlineLambdaInsideMonitorSourceArgumentIndexes = arrayListOf<Set<Int>>()
+
+    fun pushArgumentIndexes(indexes: Set<Int>) {
+        inlineLambdaInsideMonitorSourceArgumentIndexes.add(indexes)
+    }
+
+    fun popArgumentIndexes() {
+        inlineLambdaInsideMonitorSourceArgumentIndexes.pop()
+    }
+
+    private fun enterMonitor() {
+        monitorsDepth++
+    }
+
+    fun enterMonitorIfNeeded(index: Int?) {
+        if (index == null) return
+        if (inlineLambdaInsideMonitorSourceArgumentIndexes.peek()?.contains(index) != true) return
+        enterMonitor()
+    }
+
+    private fun exitMonitor() {
+        assert(monitorsDepth > 0) {
+            "exitMonitor without corresponding enterMonitor"
+        }
+        monitorsDepth--
+    }
+
+    fun exitMonitorIfNeeded(index: Int?) {
+        if (index == null) return
+        if (inlineLambdaInsideMonitorSourceArgumentIndexes.peek()?.contains(index) != true) return
+        exitMonitor()
+    }
+
+    fun checkSuspendCall(call: ResolvedCall<*>) {
+        if (monitorsDepth != 0) {
+            diagnostics.report(SUSPENSION_POINT_INSIDE_MONITOR.on(call.call.callElement, call.resultingDescriptor))
+        }
+    }
+}

compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineCodegen.kt

@@ -8,6 +8,7 @@ package org.jetbrains.kotlin.codegen.inline
 import com.intellij.psi.PsiElement
 import com.intellij.util.ArrayUtil
 import org.jetbrains.kotlin.backend.common.isBuiltInIntercepted
+import org.jetbrains.kotlin.backend.common.isTopLevelInPackage
 import org.jetbrains.kotlin.builtins.BuiltInsPackageFragment
 import org.jetbrains.kotlin.codegen.*
 import org.jetbrains.kotlin.codegen.AsmUtil.getMethodAsmFlags
@@ -19,6 +20,8 @@ import org.jetbrains.kotlin.codegen.coroutines.createMethodNodeForSuspendCorouti
 import org.jetbrains.kotlin.codegen.coroutines.isBuiltInSuspendCoroutineUninterceptedOrReturnInJvm
 import org.jetbrains.kotlin.codegen.intrinsics.bytecode
 import org.jetbrains.kotlin.codegen.intrinsics.classId
+import org.jetbrains.kotlin.codegen.optimization.common.ControlFlowGraph
+import org.jetbrains.kotlin.codegen.optimization.common.asSequence
 import org.jetbrains.kotlin.codegen.state.GenerationState
 import org.jetbrains.kotlin.codegen.state.KotlinTypeMapper
 import org.jetbrains.kotlin.config.isReleaseCoroutines
@@ -244,7 +247,18 @@ abstract class InlineCodegen<out T : BaseExpressionCodegen>(
             }
         }
         val reificationResult = reifiedTypeInliner.reifyInstructions(node)
-        generateClosuresBodies()
+
+        val hasMonitor = node.instructions.asSequence().any { it.opcode == Opcodes.MONITORENTER }
+        if (hasMonitor) {
+            state.globalCoroutinesContext.pushArgumentIndexes(findInlineLambdasInsideMonitor(node))
+        }
+        try {
+            generateClosuresBodies()
+        } finally {
+            if (hasMonitor) {
+                state.globalCoroutinesContext.popArgumentIndexes()
+            }
+        }
 
         //through generation captured parameters will be added to invocationParamBuilder
         putClosureParametersOnStack()
@@ -301,6 +315,45 @@ abstract class InlineCodegen<out T : BaseExpressionCodegen>(
         return result
     }
 
+    private fun findInlineLambdasInsideMonitor(node: MethodNode): Set<Int> {
+        val sources = MethodInliner.analyzeMethodNodeBeforeInline(node)
+
+        val cfg = ControlFlowGraph.build(node)
+        val monitorDepthMap = hashMapOf<AbstractInsnNode, Int>()
+        val result = hashSetOf<Int>()
+
+        fun addMonitorDepthToSuccs(index: Int, depth: Int) {
+            val insn = node.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[node.instructions[succIndex]] == null) {
+                    addMonitorDepthToSuccs(succIndex, newDepth)
+                }
+            }
+        }
+
+        addMonitorDepthToSuccs(0, 0)
+
+        for (insn in node.instructions.asSequence()) {
+            if (insn !is MethodInsnNode) continue
+            if (!isInvokeOnLambda(insn.owner, insn.name)) continue
+            if (monitorDepthMap[insn]?.let { it > 0 } != true) continue
+            val frame = sources[node.instructions.indexOf(insn)] ?: continue
+            for (source in frame.getStack(frame.stackSize - Type.getArgumentTypes(insn.desc).size - 1).insns) {
+                if (source.opcode == Opcodes.ALOAD) {
+                    result.add((source as VarInsnNode).`var`)
+                }
+            }
+        }
+
+        return result
+    }
+
     private fun isInlinedToInlineFunInKotlinRuntime(): Boolean {
         val codegen = this.codegen as? ExpressionCodegen ?: return false
         val caller = codegen.context.functionDescriptor
@@ -310,8 +363,16 @@ abstract class InlineCodegen<out T : BaseExpressionCodegen>(
     }
 
     private fun generateClosuresBodies() {
+        val parameters = invocationParamBuilder.buildParameters()
+
         for (info in expressionMap.values) {
-            info.generateLambdaBody(sourceCompiler, reifiedTypeInliner)
+            val index = parameters.find { it.lambda == info }?.index
+            state.globalCoroutinesContext.enterMonitorIfNeeded(index)
+            try {
+                info.generateLambdaBody(sourceCompiler, reifiedTypeInliner)
+            } finally {
+                state.globalCoroutinesContext.exitMonitorIfNeeded(index)
+            }
         }
     }
 

compiler/backend/src/org/jetbrains/kotlin/codegen/inline/MethodInliner.kt

@@ -945,7 +945,7 @@ class MethodInliner(
             )
         }
 
-        private fun analyzeMethodNodeBeforeInline(node: MethodNode): Array<Frame<SourceValue>?> {
+        fun analyzeMethodNodeBeforeInline(node: MethodNode): Array<Frame<SourceValue>?> {
             val analyzer = object : Analyzer<SourceValue>(SourceInterpreter()) {
                 override fun newFrame(nLocals: Int, nStack: Int): Frame<SourceValue> {
 

compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt

@@ -13,6 +13,7 @@ import org.jetbrains.kotlin.codegen.`when`.MappingsClassesForWhenByEnum
 import org.jetbrains.kotlin.codegen.binding.CodegenBinding
 import org.jetbrains.kotlin.codegen.context.CodegenContext
 import org.jetbrains.kotlin.codegen.context.RootContext
+import org.jetbrains.kotlin.codegen.coroutines.GlobalCoroutinesContext
 import org.jetbrains.kotlin.codegen.extensions.ClassBuilderInterceptorExtension
 import org.jetbrains.kotlin.codegen.inline.GlobalInlineContext
 import org.jetbrains.kotlin.codegen.inline.InlineCache
@@ -202,6 +203,7 @@ class GenerationState private constructor(
     }
     val samWrapperClasses: SamWrapperClasses = SamWrapperClasses(this)
     val globalInlineContext: GlobalInlineContext = GlobalInlineContext(diagnostics)
+    val globalCoroutinesContext: GlobalCoroutinesContext = GlobalCoroutinesContext(diagnostics)
     val mappingsClassesForWhenByEnum: MappingsClassesForWhenByEnum = MappingsClassesForWhenByEnum(this)
     val jvmRuntimeTypes: JvmRuntimeTypes = JvmRuntimeTypes(module, configuration.languageVersionSettings)
     val factory: ClassFileFactory

compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java

@@ -1037,6 +1037,7 @@ public interface Errors {
     DiagnosticFactory0<KtDeclaration> OVERRIDE_BY_INLINE = DiagnosticFactory0.create(WARNING, DECLARATION_SIGNATURE);
     DiagnosticFactory0<PsiElement> REIFIED_TYPE_PARAMETER_IN_OVERRIDE = DiagnosticFactory0.create(ERROR);
     DiagnosticFactory1<PsiElement, CallableDescriptor> INLINE_CALL_CYCLE = DiagnosticFactory1.create(ERROR, DEFAULT);
+    DiagnosticFactory1<PsiElement, CallableDescriptor> SUSPENSION_POINT_INSIDE_MONITOR = DiagnosticFactory1.create(ERROR, DEFAULT);
     DiagnosticFactory0<PsiElement> NON_LOCAL_RETURN_IN_DISABLED_INLINE = DiagnosticFactory0.create(ERROR);
     DiagnosticFactory0<KtDeclaration> INLINE_PROPERTY_WITH_BACKING_FIELD = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE);
     DiagnosticFactory0<KtAnnotationEntry> NON_INTERNAL_PUBLISHED_API = DiagnosticFactory0.create(ERROR);

compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java

@@ -934,6 +934,7 @@ public class DefaultErrorMessages {
         //Inline non locals
         MAP.put(NON_LOCAL_RETURN_NOT_ALLOWED, "Can''t inline ''{0}'' here: it may contain non-local returns. Add ''crossinline'' modifier to parameter declaration ''{0}''", ELEMENT_TEXT);
         MAP.put(INLINE_CALL_CYCLE, "The ''{0}'' invocation is a part of inline cycle", NAME);
+        MAP.put(SUSPENSION_POINT_INSIDE_MONITOR, "The ''{0}'' suspension point is inside a critical section", NAME);
         MAP.put(NON_LOCAL_RETURN_IN_DISABLED_INLINE, "Non-local returns are not allowed with inlining disabled");
         MAP.put(NON_LOCAL_SUSPENSION_POINT, "Suspension functions can be called only within coroutine body");
         MAP.put(ILLEGAL_SUSPEND_FUNCTION_CALL, "Suspend function ''{0}'' should be called only from a coroutine or another suspend function", NAME);

compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/library/a.kt

@@ -0,0 +1,17 @@
+val lock = Any()
+
+inline fun inlineMe(c: () -> Unit) {
+    synchronized(lock) {
+        c()
+    }
+}
+
+inline fun monitorInFinally(a: () -> Unit, b: () -> Unit) {
+    try {
+        a()
+    } finally {
+        synchronized(lock) {
+            b()
+        }
+    }
+}

compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/output.txt

@@ -0,0 +1,10 @@
+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:14:13: error: the 'suspensionPoint' suspension point is inside a critical section
+            suspensionPoint()
+            ^
+compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt:19:15: error: the 'suspensionPoint' suspension point is inside a critical section
+            { suspensionPoint() }
+              ^
+COMPILATION_ERROR

compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt

@@ -0,0 +1,22 @@
+fun builder(c: suspend () -> Unit) {}
+
+private val lock = Any()
+
+suspend fun suspensionPoint() {}
+
+fun test() {
+    builder {
+        synchronized(lock) {
+            suspensionPoint()
+        }
+
+        inlineMe {
+            suspensionPoint()
+        }
+
+        monitorInFinally(
+            { suspensionPoint() },
+            { suspensionPoint() }
+        )
+    }
+}

compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.kt

@@ -140,6 +140,15 @@ class CompileKotlinAgainstCustomBinariesTest : AbstractKotlinCompilerIntegration
         compileKotlin("main.kt", tmpdir, listOf(compileLibrary("library")))
     }
 
+    fun testSuspensionPointInMonitor() {
+        compileKotlin(
+            "source.kt",
+            tmpdir,
+            listOf(compileLibrary("library", additionalOptions = listOf("-Xskip-metadata-version-check"))),
+            additionalOptions = listOf("-Xskip-metadata-version-check")
+        )
+    }
+
     fun testDuplicateObjectInBinaryAndSources() {
         val allDescriptors = analyzeAndGetAllDescriptors(compileLibrary("library"))
         assertEquals(allDescriptors.toString(), 2, allDescriptors.size)