From 96c45c74e826ec89549a76eb1aab91e5c2527f4a Mon Sep 17 00:00:00 2001 From: Dmitrii Gridin Date: Thu, 21 Sep 2023 19:47:45 +0200 Subject: [PATCH] [LL FIR] improve contract violation checks Add check into LLFirModuleLazyDeclarationResolver to die fast and avoid StackOverflowError for jumping phases ^KT-56551 ^KT-57802 ^KT-57811 --- .../api/fir/file/builder/LLFirLockProvider.kt | 15 +++++++++++++ .../LLFirLazyResolveContractChecker.kt | 2 +- .../LLFirModuleLazyDeclarationResolver.kt | 7 +++++- ...CompilerTestFE10TestdataTestGenerated.java | 6 +++++ ...sticCompilerFE10TestDataTestGenerated.java | 6 +++++ ...eeOldFrontendDiagnosticsTestGenerated.java | 6 +++++ ...siOldFrontendDiagnosticsTestGenerated.java | 6 +++++ .../modifiers/const/kotlinJavaCycle.fail | 9 ++++++++ .../tests/modifiers/const/kotlinJavaCycle.kt | 22 +++++++++++++++++++ .../test/runners/DiagnosticTestGenerated.java | 6 +++++ 10 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.fail create mode 100644 compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.kt diff --git a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/file/builder/LLFirLockProvider.kt b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/file/builder/LLFirLockProvider.kt index 816f33c7f93..8e46f0d123f 100644 --- a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/file/builder/LLFirLockProvider.kt +++ b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/file/builder/LLFirLockProvider.kt @@ -54,6 +54,21 @@ internal class LLFirLockProvider(private val checker: LLFirLazyResolveContractCh } } + /** + * A contract violation check to be sure that we won't request a violated phase later. + * This is useful to catch a contract violation for jumping phases because they may encounter infinite recursion. + * + * Example: we have cycle between phases 'implicit type (1) -> body (2) -> implicit type (3)` and + * we can get [StackOverflowError] because regular phases checks can't catch such case + * because will check only implicit type -> implicit type resolution due to + * sequent resolution requests + * + * @see org.jetbrains.kotlin.analysis.low.level.api.fir.lazy.resolve.LLFirModuleLazyDeclarationResolver + */ + fun checkContractViolations(toPhase: FirResolvePhase) { + checker.checkIfCanLazyResolveToPhase(toPhase, isJumpingPhase = true) + } + /** * Locks an a [FirElementWithResolveState] to resolve from `phase - 1` to [phase] and * then updates the [FirElementWithResolveState.resolveState] to a [phase]. diff --git a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/lazy/resolve/LLFirLazyResolveContractChecker.kt b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/lazy/resolve/LLFirLazyResolveContractChecker.kt index 5a78e9decb4..90b4140d082 100644 --- a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/lazy/resolve/LLFirLazyResolveContractChecker.kt +++ b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/lazy/resolve/LLFirLazyResolveContractChecker.kt @@ -23,7 +23,7 @@ internal class LLFirLazyResolveContractChecker { } } - private fun checkIfCanLazyResolveToPhase(requestedPhase: FirResolvePhase, isJumpingPhase: Boolean) { + internal fun checkIfCanLazyResolveToPhase(requestedPhase: FirResolvePhase, isJumpingPhase: Boolean) { val currentPhase = currentTransformerPhase.get() ?: return if (requestedPhase > currentPhase || !isJumpingPhase && requestedPhase == currentPhase) { diff --git a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/lazy/resolve/LLFirModuleLazyDeclarationResolver.kt b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/lazy/resolve/LLFirModuleLazyDeclarationResolver.kt index ffa0488ace5..98de89bbf33 100644 --- a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/lazy/resolve/LLFirModuleLazyDeclarationResolver.kt +++ b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/lazy/resolve/LLFirModuleLazyDeclarationResolver.kt @@ -184,6 +184,11 @@ internal class LLFirModuleLazyDeclarationResolver(val moduleComponents: LLFirMod var currentPhase = getMinResolvePhase(targets).coerceAtLeast(FirResolvePhase.IMPORTS) if (currentPhase >= toPhase) return + val lockProvider = moduleComponents.globalResolveComponents.lockProvider + + // to catch a contract violation for jumping phases + lockProvider.checkContractViolations(toPhase) + while (currentPhase < toPhase) { currentPhase = currentPhase.next checkCanceled() @@ -193,7 +198,7 @@ internal class LLFirModuleLazyDeclarationResolver(val moduleComponents: LLFirMod phase = currentPhase, target = target, scopeSession = scopeSession, - lockProvider = moduleComponents.globalResolveComponents.lockProvider, + lockProvider = lockProvider, towerDataContextCollector = towerDataContextCollector, ) } diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java index b82818e8e07..69829638e5f 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java @@ -22601,6 +22601,12 @@ public class DiagnosticCompilerTestFE10TestdataTestGenerated extends AbstractDia runTest("compiler/testData/diagnostics/tests/modifiers/const/kCallable_before.kt"); } + @Test + @TestMetadata("kotlinJavaCycle.kt") + public void testKotlinJavaCycle() throws Exception { + runTest("compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.kt"); + } + @Test @TestMetadata("kt12248.kt") public void testKt12248() throws Exception { diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java index 6007f15a91b..04419647646 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java @@ -22601,6 +22601,12 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated runTest("compiler/testData/diagnostics/tests/modifiers/const/kCallable_before.kt"); } + @Test + @TestMetadata("kotlinJavaCycle.kt") + public void testKotlinJavaCycle() throws Exception { + runTest("compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.kt"); + } + @Test @TestMetadata("kt12248.kt") public void testKt12248() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java index ba278b11677..933c035e9cf 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java @@ -22601,6 +22601,12 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir runTest("compiler/testData/diagnostics/tests/modifiers/const/kCallable_before.kt"); } + @Test + @TestMetadata("kotlinJavaCycle.kt") + public void testKotlinJavaCycle() throws Exception { + runTest("compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.kt"); + } + @Test @TestMetadata("kt12248.kt") public void testKt12248() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java index 5d4993f52d0..51b7ebd5ec9 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java @@ -22607,6 +22607,12 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia runTest("compiler/testData/diagnostics/tests/modifiers/const/kCallable_before.kt"); } + @Test + @TestMetadata("kotlinJavaCycle.kt") + public void testKotlinJavaCycle() throws Exception { + runTest("compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.kt"); + } + @Test @TestMetadata("kt12248.kt") public void testKt12248() throws Exception { diff --git a/compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.fail b/compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.fail new file mode 100644 index 00000000000..ba3e8b4f2bc --- /dev/null +++ b/compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.fail @@ -0,0 +1,9 @@ +Suppressed: org.jetbrains.kotlin.util.ReenteringLazyValueComputationException + at org.jetbrains.kotlin.types.DeferredType.lambda$static$0(DeferredType.java:35) + at org.jetbrains.kotlin.storage.LockBasedStorageManager$3.recursionDetected(LockBasedStorageManager.java:192) + at org.jetbrains.kotlin.storage.LockBasedStorageManager$LockBasedLazyValue.invoke(LockBasedStorageManager.java:393) + at org.jetbrains.kotlin.storage.LockBasedStorageManager$LockBasedNotNullLazyValue.invoke(LockBasedStorageManager.java:527) + at org.jetbrains.kotlin.types.DeferredType.getDelegate(DeferredType.java:108) + at org.jetbrains.kotlin.types.WrappedType.getAttributes(KotlinType.kt:135) + at org.jetbrains.kotlin.types.KotlinType.getAnnotations(KotlinType.kt:57) + at org.jetbrains.kotlin.types.checker.ClassicTypeSystemContext$DefaultImpls.hasAnnotation(ClassicTypeSystemContext.kt:753) \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.kt b/compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.kt new file mode 100644 index 00000000000..a4883802b04 --- /dev/null +++ b/compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.kt @@ -0,0 +1,22 @@ +// IGNORE_REVERSED_RESOLVE +// IGNORE_CONTRACT_VIOLATIONS +// FIR_IDENTICAL +// FILE: Bar.java + +public class Bar { + public static final int BAR = Foo.FOO + 1; +} + +// FILE: Test.kt + +class Foo { + companion object { + const val FOO = Baz.BAZ + 1 + } +} + +class Baz { + companion object { + const val BAZ = Bar.BAR + 1 + } +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index 5b6108669b5..c526dc4be19 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -22607,6 +22607,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/modifiers/const/kCallable_before.kt"); } + @Test + @TestMetadata("kotlinJavaCycle.kt") + public void testKotlinJavaCycle() throws Exception { + runTest("compiler/testData/diagnostics/tests/modifiers/const/kotlinJavaCycle.kt"); + } + @Test @TestMetadata("kt12248.kt") public void testKt12248() throws Exception {