From 8738ffb84f0e11670a11e47c6cb1e2f5758c5900 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 11 Aug 2023 23:01:04 +0200 Subject: [PATCH] K2: suppress exception when reading unsupported metadata Before the change, the compiler threw exception in the unmuted tests, because it tried to load metadata even though it had an unsupported version. Use the same approach as in K1 (see DeserializedDescriptorResolver). Now the tests are unmuted, but note that test data differs from K1. K1 does not report errors related to the class `a.A` because it loads this class as a _Java class_, so calling its constructor and methods somehow works. This behavior is questionable since the compiler surely knows that it is a Kotlin class, but with an unsupported metadata version. Trying to interpret it as a Java class may lead to subtle problems. So it's safer for now to avoid loading Kotlin classes with unsupported metadata versions in K2. #KT-60795 Fixed --- .../JvmClassFileBasedSymbolProvider.kt | 24 +++++++++++-- .../output.fir.txt | 35 +++++++++++++++++++ .../output.fir.txt | 35 +++++++++++++++++++ ...tCompileKotlinAgainstCustomBinariesTest.kt | 10 +++--- 4 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/output.fir.txt create mode 100644 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/output.fir.txt diff --git a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/deserialization/JvmClassFileBasedSymbolProvider.kt b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/deserialization/JvmClassFileBasedSymbolProvider.kt index 1bf8e7c8c3a..7c4bc2c0393 100644 --- a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/deserialization/JvmClassFileBasedSymbolProvider.kt +++ b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/deserialization/JvmClassFileBasedSymbolProvider.kt @@ -33,6 +33,7 @@ import org.jetbrains.kotlin.metadata.jvm.deserialization.JvmProtoBufUtil import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.StandardClassIds +import org.jetbrains.kotlin.protobuf.InvalidProtocolBufferException import org.jetbrains.kotlin.resolve.jvm.JvmClassName import org.jetbrains.kotlin.serialization.deserialization.IncompatibleVersionErrorData import org.jetbrains.kotlin.serialization.deserialization.builtins.BuiltInSerializerProtocol @@ -82,7 +83,9 @@ class JvmClassFileBasedSymbolProvider( val header = kotlinClass.classHeader val data = header.data ?: header.incompatibleData ?: return null val strings = header.strings ?: return null - val (nameResolver, packageProto) = JvmProtoBufUtil.readPackageDataFrom(data, strings) + val (nameResolver, packageProto) = parseProto(kotlinClass) { + JvmProtoBufUtil.readPackageDataFrom(data, strings) + } ?: return null val source = JvmPackagePartSource( kotlinClass, packageProto, nameResolver, kotlinClass.incompatibility, kotlinClass.isPreReleaseInvisible, @@ -142,7 +145,9 @@ class JvmClassFileBasedSymbolProvider( if (kotlinClass.classHeader.kind != KotlinClassHeader.Kind.CLASS || kotlinClass.classId != classId) return null val data = kotlinClass.classHeader.data ?: kotlinClass.classHeader.incompatibleData ?: return null val strings = kotlinClass.classHeader.strings ?: return null - val (nameResolver, classProto) = JvmProtoBufUtil.readClassDataFrom(data, strings) + val (nameResolver, classProto) = parseProto(kotlinClass) { + JvmProtoBufUtil.readClassDataFrom(data, strings) + } ?: return null return ClassMetadataFindResult.Metadata( nameResolver, @@ -190,4 +195,19 @@ class JvmClassFileBasedSymbolProvider( private fun String?.toPath(): Path? { return this?.let { Paths.get(it).normalize() } } + + private inline fun parseProto(klass: KotlinJvmBinaryClass, block: () -> T): T? = + try { + block() + } catch (e: Throwable) { + if (session.languageVersionSettings.getFlag(AnalysisFlags.skipMetadataVersionCheck) || + klass.classHeader.metadataVersion.isCompatible(ownMetadataVersion) + ) { + throw if (e is InvalidProtocolBufferException) + IllegalStateException("Could not read data from ${klass.location}", e) + else e + } + + null + } } diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/output.fir.txt b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/output.fir.txt new file mode 100644 index 00000000000..91d790c5ffb --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/output.fir.txt @@ -0,0 +1,35 @@ +$TMP_DIR$/library-after.jar!/META-INF/main.kotlin_module: error: module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 42.0.0, expected version is $ABI_VERSION$. +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:5:16: error: unresolved reference 'A'. +fun baz(param: A, nested: A.Nested) { + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:5:27: error: unresolved reference 'A.Nested'. +fun baz(param: A, nested: A.Nested) { + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:6:23: error: unresolved reference 'A'. + val constructor = A() + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:7:18: error: unresolved reference 'A'. + val nested = A.Nested() + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:8:22: error: unresolved reference 'getQuux'. + val quux = param.getQuux() + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:9:28: error: unresolved reference 'method'. + val methodCall = param.method() + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:10:30: error: unresolved reference 'A'. + val supertype = object : A() {} + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:12:13: error: unresolved reference 'foo'. + val x = foo() + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:13:13: error: unresolved reference 'bar'. + val y = bar + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:14:5: error: unresolved reference 'bar'. + bar = 239 + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:15:12: error: unresolved reference 'TA'. + val z: TA = "" + ^ +COMPILATION_ERROR diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/output.fir.txt b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/output.fir.txt new file mode 100644 index 00000000000..07829c4e8b3 --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/output.fir.txt @@ -0,0 +1,35 @@ +$TMP_DIR$/library-after.jar!/META-INF/main.kotlin_module: error: module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 42.0.0, expected version is $ABI_VERSION$. +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:5:16: error: unresolved reference 'A'. +fun baz(param: A, nested: A.Nested) { + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:5:27: error: unresolved reference 'A.Nested'. +fun baz(param: A, nested: A.Nested) { + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:6:23: error: unresolved reference 'A'. + val constructor = A() + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:7:18: error: unresolved reference 'A'. + val nested = A.Nested() + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:8:22: error: unresolved reference 'getQuux'. + val quux = param.getQuux() + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:9:28: error: unresolved reference 'method'. + val methodCall = param.method() + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:10:30: error: unresolved reference 'A'. + val supertype = object : A() {} + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:12:13: error: unresolved reference 'foo'. + val x = foo() + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:13:13: error: unresolved reference 'bar'. + val y = bar + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:14:5: error: unresolved reference 'bar'. + bar = 239 + ^ +compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:15:12: error: unresolved reference 'TA'. + val z: TA = "" + ^ +COMPILATION_ERROR diff --git a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/AbstractCompileKotlinAgainstCustomBinariesTest.kt b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/AbstractCompileKotlinAgainstCustomBinariesTest.kt index 012e63deb86..d9132f56774 100644 --- a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/AbstractCompileKotlinAgainstCustomBinariesTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/AbstractCompileKotlinAgainstCustomBinariesTest.kt @@ -286,8 +286,11 @@ abstract class AbstractCompileKotlinAgainstCustomBinariesTest : AbstractKotlinCo doTestKotlinLibraryWithWrongMetadataVersion("library", null) } - // KT-60795 K2: missing INCOMPATIBLE_CLASS and corresponding CLI error - fun testWrongMetadataVersionBadMetadata() = muteForK2 { + // This test compiles a library with a "future" metadata version, then intentionally inserts some gibberish to the metadata, and tries + // to compile something against this library. It emulates the scenario when a future Kotlin version has a completely different metadata + // format -- so different that reading it as if it's the current (protobuf-based) format would most likely result in an exception. + // Expected result is that the compiler does NOT try to read it, and instead reports incompatible version & unresolved reference errors. + fun testWrongMetadataVersionBadMetadata() { doTestKotlinLibraryWithWrongMetadataVersion("library", { name, value -> if (JvmAnnotationNames.METADATA_DATA_FIELD_NAME == name) { @Suppress("UNCHECKED_CAST") @@ -299,8 +302,7 @@ abstract class AbstractCompileKotlinAgainstCustomBinariesTest : AbstractKotlinCo }) } - // KT-60795 K2: missing INCOMPATIBLE_CLASS and corresponding CLI error - fun testWrongMetadataVersionBadMetadata2() = muteForK2 { + fun testWrongMetadataVersionBadMetadata2() { doTestKotlinLibraryWithWrongMetadataVersion("library", { name, _ -> if (JvmAnnotationNames.METADATA_STRINGS_FIELD_NAME == name) arrayOf() else null })