Add call checker to report error more granulary if possible

This, however, works only for calls of 'synchronized' only. Thus, it
does not support inline functions of any kind.
This commit is contained in:
Ilmir Usmanov
2018-09-28 21:20:40 +03:00
parent 281f09f077
commit 9af7316845
21 changed files with 327 additions and 86 deletions
@@ -239,11 +239,3 @@ object CodegenUtil {
)
}
}
fun DeclarationDescriptor.isTopLevelInPackage(name: String, packageName: String): Boolean {
if (name != this.name.asString()) return false
val containingDeclaration = containingDeclaration as? PackageFragmentDescriptor ?: return false
val packageFqName = containingDeclaration.fqName.asString()
return packageName == packageFqName
}
@@ -9,6 +9,7 @@ import org.jetbrains.kotlin.config.LanguageFeature
import org.jetbrains.kotlin.config.LanguageVersionSettings
import org.jetbrains.kotlin.config.coroutinesIntrinsicsPackageFqName
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
import org.jetbrains.kotlin.descriptors.isTopLevelInPackage
import org.jetbrains.kotlin.name.Name
val COROUTINE_SUSPENDED_NAME = Name.identifier("COROUTINE_SUSPENDED")
@@ -5,11 +5,11 @@
package org.jetbrains.kotlin.codegen
import org.jetbrains.kotlin.backend.common.isTopLevelInPackage
import org.jetbrains.kotlin.codegen.coroutines.createCustomCopy
import org.jetbrains.kotlin.codegen.state.KotlinTypeMapper
import org.jetbrains.kotlin.config.JVMAssertionsMode
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
import org.jetbrains.kotlin.descriptors.isTopLevelInPackage
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.resolve.BindingTraceContext
import org.jetbrains.kotlin.resolve.DelegatingBindingTrace
@@ -5,7 +5,6 @@
package org.jetbrains.kotlin.codegen
import org.jetbrains.kotlin.backend.common.isTopLevelInPackage
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
import org.jetbrains.kotlin.builtins.KotlinBuiltIns.RANGES_PACKAGE_FQ_NAME
import org.jetbrains.kotlin.builtins.PrimitiveType
@@ -138,10 +138,7 @@ class CoroutineTransformerMethodVisitor(
val suspendMarkerVarIndex = methodNode.maxLocals++
val suspensionPointLineNumbers =
suspensionPoints.map { suspensionPoint ->
suspensionPoint.suspensionCallBegin.findPreviousOrNull { it is LineNumberNode } as LineNumberNode?
}
val suspensionPointLineNumbers = suspensionPoints.map { findSuspensionPointLineNumber(it) }
val continuationLabels = suspensionPoints.withIndex().map {
transformCallAndReturnContinuationLabel(it.index + 1, it.value, methodNode, suspendMarkerVarIndex)
@@ -200,6 +197,9 @@ class CoroutineTransformerMethodVisitor(
}
}
private fun findSuspensionPointLineNumber(suspensionPoint: SuspensionPoint) =
suspensionPoint.suspensionCallBegin.findPreviousOrNull { it is LineNumberNode } as LineNumberNode?
private fun checkForSuspensionPointInsideMonitor(methodNode: MethodNode, suspensionPoints: List<SuspensionPoint>) {
if (methodNode.instructions.asSequence().none { it.opcode == Opcodes.MONITORENTER }) return
@@ -225,7 +225,13 @@ class CoroutineTransformerMethodVisitor(
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)) }
val stackTraceElement = StackTraceElement(
containingClassInternalName,
methodNode.name,
sourceFile,
findSuspensionPointLineNumber(suspensionPoint)?.line ?: -1
)
element?.let { diagnostics.report(ErrorsJvm.SUSPENSION_POINT_INSIDE_MONITOR.on(it, "$stackTraceElement")) }
return
}
}
@@ -0,0 +1,69 @@
/*
* 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.resolve.jvm.checkers
import com.intellij.psi.PsiElement
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
import org.jetbrains.kotlin.descriptors.isTopLevelInPackage
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtLambdaExpression
import org.jetbrains.kotlin.psi.KtValueArgumentList
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.calls.checkers.CallChecker
import org.jetbrains.kotlin.resolve.calls.checkers.CallCheckerContext
import org.jetbrains.kotlin.resolve.calls.checkers.findEnclosingSuspendFunction
import org.jetbrains.kotlin.resolve.calls.model.ExpressionValueArgument
import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm
import org.jetbrains.kotlin.resolve.source.getPsi
class SuspensionPointInSynchronizedCallChecker : CallChecker {
override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) {
val descriptor = resolvedCall.candidateDescriptor
if (descriptor !is FunctionDescriptor || !descriptor.isSuspend) return
val enclosingSuspendFunctionSource = findEnclosingSuspendFunction(context)?.source?.getPsi() ?: return
// Search for `synchronized` call
var parent = reportOn
var child = reportOn
var insideLambda = false
while (parent != enclosingSuspendFunctionSource) {
if (parent is KtCallExpression) {
if (checkCall(context, parent, child, insideLambda, reportOn, resolvedCall)) break
}
if (parent is KtLambdaExpression) {
insideLambda = true
}
// The lambda is inside parentheses -> keep the child the same to check whether it is the second argument
if (parent !is KtValueArgumentList) {
child = parent
}
parent = parent.parent
}
}
private fun checkCall(
context: CallCheckerContext,
parent: KtCallExpression,
child: PsiElement,
insideLambda: Boolean,
reportOn: PsiElement,
resolvedCall: ResolvedCall<*>
): Boolean {
val call = context.trace[BindingContext.CALL, parent.calleeExpression] ?: return false
val resolved = context.trace[BindingContext.RESOLVED_CALL, call] ?: return false
val isSynchronized = resolved.resultingDescriptor.isTopLevelInPackage("synchronized", "kotlin")
if (isSynchronized) {
val isSecondArgument = (resolved.valueArgumentsByIndex?.get(1) as? ExpressionValueArgument)?.valueArgument == child
if (insideLambda && isSecondArgument) {
context.trace.report(ErrorsJvm.SUSPENSION_POINT_INSIDE_SYNCHRONIZED.on(reportOn, resolvedCall.resultingDescriptor))
}
return true
}
return false
}
}
@@ -1,17 +1,6 @@
/*
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* 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.resolve.jvm.diagnostics;
@@ -147,7 +136,8 @@ 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, "A suspension point is inside a critical section");
MAP.put(SUSPENSION_POINT_INSIDE_MONITOR, "A suspension point at {0} is inside a critical section", STRING);
MAP.put(SUSPENSION_POINT_INSIDE_SYNCHRONIZED, "The ''{0}'' suspension point is inside a synchronized block", NAME);
}
@NotNull
@@ -1,17 +1,6 @@
/*
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* 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.resolve.jvm.diagnostics;
@@ -133,7 +122,8 @@ public interface ErrorsJvm {
DiagnosticFactory1<KtAnnotationEntry, String> ANNOTATION_TARGETS_NON_EXISTENT_ACCESSOR = DiagnosticFactory1.create(WARNING);
DiagnosticFactory0<PsiElement> SUSPENSION_POINT_INSIDE_MONITOR = DiagnosticFactory0.create(ERROR);
DiagnosticFactory1<PsiElement, String> SUSPENSION_POINT_INSIDE_MONITOR = DiagnosticFactory1.create(ERROR);
DiagnosticFactory1<PsiElement, CallableDescriptor> SUSPENSION_POINT_INSIDE_SYNCHRONIZED = DiagnosticFactory1.create(ERROR);
@SuppressWarnings("UnusedDeclaration")
Object _initializer = new Object() {
@@ -1,17 +1,6 @@
/*
* Copyright 2010-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* 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.resolve.jvm.platform
@@ -52,6 +41,7 @@ object JvmPlatformConfigurator : PlatformConfigurator(
additionalCallCheckers = listOf(
JavaAnnotationCallChecker(),
SuspensionPointInSynchronizedCallChecker(),
JavaClassOnCompanionChecker(),
ProtectedInSuperClassCompanionCallChecker(),
UnsupportedSyntheticCallableReferenceChecker(),
@@ -46,9 +46,15 @@ fun FunctionDescriptor.isBuiltInCoroutineContext(languageVersionSettings: Langua
fun PropertyDescriptor.isBuiltInCoroutineContext(languageVersionSettings: LanguageVersionSettings) =
this.fqNameSafe.isBuiltInCoroutineContext(languageVersionSettings)
object CoroutineSuspendCallChecker : CallChecker {
private val ALLOWED_SCOPE_KINDS = setOf(LexicalScopeKind.FUNCTION_INNER_SCOPE, LexicalScopeKind.FUNCTION_HEADER_FOR_DESTRUCTURING)
private val ALLOWED_SCOPE_KINDS = setOf(LexicalScopeKind.FUNCTION_INNER_SCOPE, LexicalScopeKind.FUNCTION_HEADER_FOR_DESTRUCTURING)
fun findEnclosingSuspendFunction(context: CallCheckerContext): FunctionDescriptor? = context.scope
.parentsWithSelf.firstOrNull {
it is LexicalScope && it.kind in ALLOWED_SCOPE_KINDS &&
it.ownerDescriptor.safeAs<FunctionDescriptor>()?.isSuspend == true
}?.cast<LexicalScope>()?.ownerDescriptor?.cast()
object CoroutineSuspendCallChecker : CallChecker {
override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) {
val descriptor = resolvedCall.candidateDescriptor
when (descriptor) {
@@ -61,11 +67,7 @@ object CoroutineSuspendCallChecker : CallChecker {
else -> return
}
val enclosingSuspendFunction = context.scope
.parentsWithSelf.firstOrNull {
it is LexicalScope && it.kind in ALLOWED_SCOPE_KINDS &&
it.ownerDescriptor.safeAs<FunctionDescriptor>()?.isSuspend == true
}?.cast<LexicalScope>()?.ownerDescriptor?.cast<FunctionDescriptor>()
val enclosingSuspendFunction = findEnclosingSuspendFunction(context)
when {
enclosingSuspendFunction != null -> {
@@ -0,0 +1,3 @@
$TESTDATA_DIR$/suspensionPointInMonitor.kt
-d
$TEMP_DIR$
+68
View File
@@ -0,0 +1,68 @@
fun builder(c: suspend () -> Unit) {}
private val lock = Any()
suspend fun suspensionPoint() {}
private inline fun inlineMe(c: () -> Unit) {
synchronized(lock) {
c()
}
}
private inline fun monitorInFinally(a: () -> Unit, b: () -> Unit) {
try {
a()
} finally {
synchronized(lock) {
b()
}
}
}
fun test() {
builder {
synchronized(lock) {
suspensionPoint()
}
}
builder {
inlineMe {
suspensionPoint()
}
}
builder {
monitorInFinally(
{},
{ 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()
}
@@ -0,0 +1,7 @@
compiler/testData/cli/jvm/suspensionPointInMonitor.kt:26:13: error: the 'suspensionPoint' suspension point is inside a synchronized block
suspensionPoint()
^
compiler/testData/cli/jvm/suspensionPointInMonitor.kt:60:17: error: the 'suspensionPoint' suspension point is inside a synchronized block
suspensionPoint()
^
COMPILATION_ERROR
@@ -1,13 +1,7 @@
compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt:8:13: error: a suspension point is inside a critical section
compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt:8:13: error: a suspension point at SourceKt$test$1.invokeSuspend(source.kt:10) is inside a critical section
builder {
^
compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt:14:13: error: a suspension point is inside a critical section
compiler/testData/compileKotlinAgainstCustomBinaries/suspensionPointInMonitor/source.kt:14:13: error: a suspension point at SourceKt$test$2.invokeSuspend(source.kt:17) is inside a critical section
builder {
^
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
@@ -5,12 +5,6 @@ private val lock = Any()
suspend fun suspensionPoint() {}
fun test() {
builder {
synchronized(lock) {
suspensionPoint()
}
}
builder {
inlineMe {
suspensionPoint()
@@ -37,14 +31,6 @@ fun test() {
}
}
}
object : SuspendRunnable {
override suspend fun run() {
synchronized(lock) {
suspensionPoint()
}
}
}
}
interface SuspendRunnable {
@@ -0,0 +1,55 @@
// !DIAGNOSTICS: -UNUSED_PARAMETER, -USELESS_IS_CHECK
// SKIP_TXT
val lock = Any()
fun builder(c: suspend () -> Unit) {}
suspend fun getLock() = lock
suspend fun suspensionPoint() {}
fun test() {
builder {
synchronized(lock) {
<!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>()
}
synchronized(lock) label@{
<!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>()
}
synchronized(lock, { <!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>() })
synchronized(getLock()) {
println("")
}
synchronized(suspend { getLock() } ()) {
println("")
}
synchronized(run { getLock() }) {
println("")
}
}
}
suspend fun run() {
synchronized(lock) {
<!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>()
}
}
suspend fun ifWhenAndOtherNonsence() {
synchronized(lock) {
if (lock == Any()) {
when (1) {
is Int -> {
return@synchronized 1 + <!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>returnsInt<!>()
}
else -> {}
}
} else {}
}
}
suspend fun returnsInt(): Int = 0
@@ -0,0 +1,56 @@
// !DIAGNOSTICS: -UNUSED_PARAMETER, -USELESS_IS_CHECK
// !LANGUAGE: +NewInference
// SKIP_TXT
val lock = Any()
fun builder(c: suspend () -> Unit) {}
suspend fun getLock() = lock
suspend fun suspensionPoint() {}
fun test() {
builder {
synchronized(lock) {
<!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>()
}
synchronized(lock) label@{
<!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>()
}
synchronized(lock, { <!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>() })
synchronized(getLock()) {
println("")
}
synchronized(suspend { getLock() } ()) {
println("")
}
synchronized(run { getLock() }) {
println("")
}
}
}
suspend fun run() {
synchronized(lock) {
<!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>()
}
}
suspend fun ifWhenAndOtherNonsence() {
synchronized(lock) {
if (lock == Any()) {
when (1) {
is Int -> {
return@synchronized 1 + <!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>returnsInt<!>()
}
else -> {}
}
} else {}
}
}
suspend fun returnsInt(): Int = 0
@@ -1658,6 +1658,16 @@ public class DiagnosticsTestWithStdLibGenerated extends AbstractDiagnosticsTestW
runTest("compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendTest.kt");
}
@TestMetadata("suspensionPointInMonitor.kt")
public void testSuspensionPointInMonitor() throws Exception {
runTest("compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.kt");
}
@TestMetadata("suspensionPointInMonitorNewInf.kt")
public void testSuspensionPointInMonitorNewInf() throws Exception {
runTest("compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.kt");
}
@TestMetadata("suspesionInDefaultValue.kt")
public void testSuspesionInDefaultValue() throws Exception {
runTest("compiler/testData/diagnostics/testsWithStdLib/coroutines/suspesionInDefaultValue.kt");
@@ -1658,6 +1658,16 @@ public class DiagnosticsTestWithStdLibUsingJavacGenerated extends AbstractDiagno
runTest("compiler/testData/diagnostics/testsWithStdLib/coroutines/suspendTest.kt");
}
@TestMetadata("suspensionPointInMonitor.kt")
public void testSuspensionPointInMonitor() throws Exception {
runTest("compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.kt");
}
@TestMetadata("suspensionPointInMonitorNewInf.kt")
public void testSuspensionPointInMonitorNewInf() throws Exception {
runTest("compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.kt");
}
@TestMetadata("suspesionInDefaultValue.kt")
public void testSuspesionInDefaultValue() throws Exception {
runTest("compiler/testData/diagnostics/testsWithStdLib/coroutines/suspesionInDefaultValue.kt");
@@ -561,6 +561,11 @@ public class CliTestGenerated extends AbstractCliTest {
runTest("compiler/testData/cli/jvm/suppressAllWarningsJvm.args");
}
@TestMetadata("suspensionPointInMonitor.args")
public void testSuspensionPointInMonitor() throws Exception {
runTest("compiler/testData/cli/jvm/suspensionPointInMonitor.args");
}
@TestMetadata("syntheticAccessorForPropertiesSignatureClash.args")
public void testSyntheticAccessorForPropertiesSignatureClash() throws Exception {
runTest("compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.args");
@@ -44,4 +44,12 @@ fun ModuleDescriptor.getContinuationOfTypeOrAny(kotlinType: KotlinType, isReleas
it,
arguments = listOf(kotlinType.asTypeProjection())
)
} ?: module.builtIns.nullableAnyType
} ?: module.builtIns.nullableAnyType
fun DeclarationDescriptor.isTopLevelInPackage(name: String, packageName: String): Boolean {
if (name != this.name.asString()) return false
val containingDeclaration = containingDeclaration as? PackageFragmentDescriptor ?: return false
val packageFqName = containingDeclaration.fqName.asString()
return packageName == packageFqName
}