From da69e3db7cf9f69d6125217a7c17aa5d3da7c9e1 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 7 Oct 2020 20:29:35 +0200 Subject: [PATCH] kotlinx-metadata-jvm: report error on using metadata version < 1.4 in writers The main reason for this change is that the current API for class version requirements (`KmClass.versionRequirements`) makes it impossible to support pre-1.4 metadata where this was stored incorrectly for nested classes: with the "version requirement table" in the outer class, and indexes into that table in nested classes. See KT-41011. Other than this aspect, metadata of classes is basically the same in pre-1.4 and 1.4, which means that if some kotlinx-metadata-jvm users really need to serialize metadata of an earlier version and they don't care about the version requirements issue, they can just use these new bytes but write the earlier version (e.g. 1.1) to the class file. Everything will work the same, except for the possible version requirements issue mentioned above. Note that metadata version 1.4 is still supported for `KotlinModuleMetadata.Writer` though. #KT-41011 Fixed --- .../versionSpecificBehavior.kt | 12 ++++------ libraries/kotlinx-metadata/jvm/ChangeLog.md | 5 +++- .../kotlinx-metadata/jvm/build.gradle.kts | 10 ++++---- .../metadata/jvm/KotlinClassMetadata.kt | 23 ++++++++++++++----- .../metadata/test/MetadataSmokeTest.kt | 15 +++++++++++- 5 files changed, 45 insertions(+), 20 deletions(-) diff --git a/core/metadata/src/org/jetbrains/kotlin/metadata/deserialization/versionSpecificBehavior.kt b/core/metadata/src/org/jetbrains/kotlin/metadata/deserialization/versionSpecificBehavior.kt index af0273527cd..3ac6b424082 100644 --- a/core/metadata/src/org/jetbrains/kotlin/metadata/deserialization/versionSpecificBehavior.kt +++ b/core/metadata/src/org/jetbrains/kotlin/metadata/deserialization/versionSpecificBehavior.kt @@ -12,14 +12,12 @@ package org.jetbrains.kotlin.metadata.deserialization // fix the bug in serialization when the binary version advances to the value supported in the first bug fix. /** - * Before Kotlin 1.4, version requirements for nested classes were deserialized incorrectly: the version requirement table was loaded from - * the outermost class and passed to the nested classes and their members, even though indices of their version requirements were pointing - * to the other table stored in the nested class (which was not read by deserialization). See KT-25120 for more information + * Before metadata version 1.4, version requirements for nested classes were deserialized incorrectly: the version requirement table was + * loaded from the outermost class and passed to the nested classes and their members, even though indices of their version requirements + * were pointing to the other table stored in the nested class (which was not read by deserialization). See KT-25120 for more information. */ fun isVersionRequirementTableWrittenCorrectly(version: BinaryVersion): Boolean = isKotlin1Dot4OrLater(version) -fun isKotlin1Dot4OrLater(version: BinaryVersion): Boolean { - // All metadata versions (JVM, JS, common) will be advanced to 1.4.0 in Kotlin 1.4 - return version.major == 1 && version.minor >= 4 -} +fun isKotlin1Dot4OrLater(version: BinaryVersion): Boolean = + version.major == 1 && version.minor >= 4 diff --git a/libraries/kotlinx-metadata/jvm/ChangeLog.md b/libraries/kotlinx-metadata/jvm/ChangeLog.md index 52f17bfc5c8..1dd23732bf4 100644 --- a/libraries/kotlinx-metadata/jvm/ChangeLog.md +++ b/libraries/kotlinx-metadata/jvm/ChangeLog.md @@ -1,7 +1,10 @@ # kotlinx-metadata-jvm -## 0.1.1 +## 0.2.0 +- ['KT-41011`](https://youtrack.jetbrains.com/issue/KT-41011) Using KotlinClassMetadata.Class.Writer with metadata version < 1.4 will write incorrect version requirement table + - Breaking change: `KotlinClassMetadata.*.Writer.write` throws exception on `metadataVersion` earlier than 1.4.0. + Note: metadata of version 1.4 is readable by Kotlin compiler/reflection of versions 1.3 and later. - Breaking change: `KotlinClassMetadata.*.Writer.write` no longer accept `bytecodeVersion`. - [`KT-42429`](https://youtrack.jetbrains.com/issue/KT-42429) Wrong interpretation of Flag.Constructor.IS_PRIMARY - Breaking change: `Flag.Constructor.IS_PRIMARY` is deprecated, use `Flag.Constructor.IS_SECONDARY` instead diff --git a/libraries/kotlinx-metadata/jvm/build.gradle.kts b/libraries/kotlinx-metadata/jvm/build.gradle.kts index 87e72e3798c..b7fa1993e17 100644 --- a/libraries/kotlinx-metadata/jvm/build.gradle.kts +++ b/libraries/kotlinx-metadata/jvm/build.gradle.kts @@ -1,5 +1,4 @@ import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar -import org.jetbrains.kotlin.gradle.tasks.KotlinCompile description = "Kotlin JVM metadata manipulation library" @@ -37,15 +36,16 @@ configurations.getByName("compileOnly").extendsFrom(shadows) configurations.getByName("testCompile").extendsFrom(shadows) dependencies { - compile(kotlinStdlib()) + api(kotlinStdlib()) shadows(project(":kotlinx-metadata")) shadows(project(":core:metadata")) shadows(project(":core:metadata.jvm")) shadows(protobufLite()) - testCompile(commonDep("junit:junit")) - testCompile(intellijDep()) { includeJars("asm-all", rootProject = rootProject) } + testImplementation(project(":kotlin-test:kotlin-test-junit")) + testImplementation(commonDep("junit:junit")) + testImplementation(intellijDep()) { includeJars("asm-all", rootProject = rootProject) } testCompileOnly(project(":kotlin-reflect-api")) - testRuntime(project(":kotlin-reflect")) + testRuntimeOnly(project(":kotlin-reflect")) } if (deployVersion != null) { diff --git a/libraries/kotlinx-metadata/jvm/src/kotlinx/metadata/jvm/KotlinClassMetadata.kt b/libraries/kotlinx-metadata/jvm/src/kotlinx/metadata/jvm/KotlinClassMetadata.kt index 9ad54554626..0c270c18690 100644 --- a/libraries/kotlinx-metadata/jvm/src/kotlinx/metadata/jvm/KotlinClassMetadata.kt +++ b/libraries/kotlinx-metadata/jvm/src/kotlinx/metadata/jvm/KotlinClassMetadata.kt @@ -57,7 +57,7 @@ sealed class KotlinClassMetadata(val header: KotlinClassHeader) { * Returns the metadata of the class that was written with this writer. * * @param metadataVersion metadata version to be written to the metadata (see [KotlinClassHeader.metadataVersion]), - * [KotlinClassHeader.COMPATIBLE_METADATA_VERSION] by default + * [KotlinClassHeader.COMPATIBLE_METADATA_VERSION] by default. Cannot be less (lexicographically) than `[1, 4]` * @param extraInt the value of the class-level flags to be written to the metadata (see [KotlinClassHeader.extraInt]), * 0 by default */ @@ -66,6 +66,7 @@ sealed class KotlinClassMetadata(val header: KotlinClassHeader) { metadataVersion: IntArray = KotlinClassHeader.COMPATIBLE_METADATA_VERSION, extraInt: Int = 0 ): Class { + checkMetadataVersion(metadataVersion) val (d1, d2) = writeProtoBufData(t.build(), c) val metadata = KotlinClassHeader( KotlinClassHeader.CLASS_KIND, metadataVersion, KotlinClassHeader.COMPATIBLE_BYTECODE_VERSION, @@ -110,7 +111,7 @@ sealed class KotlinClassMetadata(val header: KotlinClassHeader) { * Returns the metadata of the file facade that was written with this writer. * * @param metadataVersion metadata version to be written to the metadata (see [KotlinClassHeader.metadataVersion]), - * [KotlinClassHeader.COMPATIBLE_METADATA_VERSION] by default + * [KotlinClassHeader.COMPATIBLE_METADATA_VERSION] by default. Cannot be less (lexicographically) than `[1, 4]` * @param extraInt the value of the class-level flags to be written to the metadata (see [KotlinClassHeader.extraInt]), * 0 by default */ @@ -119,6 +120,7 @@ sealed class KotlinClassMetadata(val header: KotlinClassHeader) { metadataVersion: IntArray = KotlinClassHeader.COMPATIBLE_METADATA_VERSION, extraInt: Int = 0 ): FileFacade { + checkMetadataVersion(metadataVersion) val (d1, d2) = writeProtoBufData(t.build(), c) val metadata = KotlinClassHeader( KotlinClassHeader.FILE_FACADE_KIND, metadataVersion, KotlinClassHeader.COMPATIBLE_BYTECODE_VERSION, @@ -181,7 +183,7 @@ sealed class KotlinClassMetadata(val header: KotlinClassHeader) { * Returns the metadata of the synthetic class that was written with this writer. * * @param metadataVersion metadata version to be written to the metadata (see [KotlinClassHeader.metadataVersion]), - * [KotlinClassHeader.COMPATIBLE_METADATA_VERSION] by default + * [KotlinClassHeader.COMPATIBLE_METADATA_VERSION] by default. Cannot be less (lexicographically) than `[1, 4]` * @param extraInt the value of the class-level flags to be written to the metadata (see [KotlinClassHeader.extraInt]), * 0 by default */ @@ -190,6 +192,7 @@ sealed class KotlinClassMetadata(val header: KotlinClassHeader) { metadataVersion: IntArray = KotlinClassHeader.COMPATIBLE_METADATA_VERSION, extraInt: Int = 0 ): SyntheticClass { + checkMetadataVersion(metadataVersion) val proto = t?.build() val (d1, d2) = if (proto != null) writeProtoBufData(proto, c) @@ -223,7 +226,7 @@ sealed class KotlinClassMetadata(val header: KotlinClassHeader) { * * @param partClassNames JVM internal names of the part classes which this multi-file class combines * @param metadataVersion metadata version to be written to the metadata (see [KotlinClassHeader.metadataVersion]), - * [KotlinClassHeader.COMPATIBLE_METADATA_VERSION] by default + * [KotlinClassHeader.COMPATIBLE_METADATA_VERSION] by default. Cannot be less (lexicographically) than `[1, 4]` * @param extraInt the value of the class-level flags to be written to the metadata (see [KotlinClassHeader.extraInt]), * 0 by default */ @@ -233,6 +236,7 @@ sealed class KotlinClassMetadata(val header: KotlinClassHeader) { metadataVersion: IntArray = KotlinClassHeader.COMPATIBLE_METADATA_VERSION, extraInt: Int = 0 ): MultiFileClassFacade { + checkMetadataVersion(metadataVersion) val metadata = KotlinClassHeader( KotlinClassHeader.MULTI_FILE_CLASS_FACADE_KIND, metadataVersion, KotlinClassHeader.COMPATIBLE_BYTECODE_VERSION, partClassNames.toTypedArray(), null, null, null, extraInt @@ -286,7 +290,7 @@ sealed class KotlinClassMetadata(val header: KotlinClassHeader) { * * @param facadeClassName JVM internal name of the corresponding multi-file class facade * @param metadataVersion metadata version to be written to the metadata (see [KotlinClassHeader.metadataVersion]), - * [KotlinClassHeader.COMPATIBLE_METADATA_VERSION] by default + * [KotlinClassHeader.COMPATIBLE_METADATA_VERSION] by default. Cannot be less (lexicographically) than `[1, 4]` * @param extraInt the value of the class-level flags to be written to the metadata (see [KotlinClassHeader.extraInt]), * 0 by default */ @@ -296,6 +300,7 @@ sealed class KotlinClassMetadata(val header: KotlinClassHeader) { metadataVersion: IntArray = KotlinClassHeader.COMPATIBLE_METADATA_VERSION, extraInt: Int = 0 ): MultiFileClassPart { + checkMetadataVersion(metadataVersion) val (d1, d2) = writeProtoBufData(t.build(), c) val metadata = KotlinClassHeader( KotlinClassHeader.MULTI_FILE_CLASS_PART_KIND, metadataVersion, KotlinClassHeader.COMPATIBLE_BYTECODE_VERSION, @@ -324,7 +329,6 @@ sealed class KotlinClassMetadata(val header: KotlinClassHeader) { */ @JvmStatic fun read(header: KotlinClassHeader): KotlinClassMetadata? { - // We only support metadata of version 1.1.* (this is Kotlin from 1.0 until today) if (!JvmMetadataVersion( header.metadataVersion, (header.extraInt and (1 shl 3)/* see JvmAnnotationNames.METADATA_STRICT_VERSION_SEMANTICS_FLAG */) != 0 @@ -346,5 +350,12 @@ sealed class KotlinClassMetadata(val header: KotlinClassHeader) { throw InconsistentKotlinMetadataException("Exception occurred when reading Kotlin metadata", e) } } + + private fun checkMetadataVersion(version: IntArray) { + require(version.size >= 2 && version[0] >= 1 && (version[0] > 1 || version[1] >= 4)) { + "This version of kotlinx-metadata-jvm doesn't support writing Kotlin metadata of version earlier than 1.4. " + + "Please change the version from ${version.toList()} to at least [1, 4]." + } + } } } diff --git a/libraries/kotlinx-metadata/jvm/test/kotlinx/metadata/test/MetadataSmokeTest.kt b/libraries/kotlinx-metadata/jvm/test/kotlinx/metadata/test/MetadataSmokeTest.kt index cdef452f3e5..ba58029cd31 100644 --- a/libraries/kotlinx-metadata/jvm/test/kotlinx/metadata/test/MetadataSmokeTest.kt +++ b/libraries/kotlinx-metadata/jvm/test/kotlinx/metadata/test/MetadataSmokeTest.kt @@ -14,6 +14,7 @@ import org.junit.Test import java.net.URLClassLoader import kotlin.coroutines.CoroutineContext import kotlin.reflect.full.primaryConstructor +import kotlin.test.assertFailsWith class MetadataSmokeTest { private fun Class<*>.readMetadata(): KotlinClassHeader { @@ -172,7 +173,7 @@ class MetadataSmokeTest { @Test fun unstableParameterNames() { - @Suppress("unused") + @Suppress("unused", "UNUSED_PARAMETER") class Test(a: String, b: Int, c: Boolean) { fun foo(a: String, b: Int, c: Boolean) = Unit } @@ -203,4 +204,16 @@ class MetadataSmokeTest { classWithUnstableParameterNames.constructors.forEach { assertTrue(Flag.Constructor.HAS_NON_STABLE_PARAMETER_NAMES(it.flags)) } classWithUnstableParameterNames.functions.forEach { assertTrue(Flag.Function.HAS_NON_STABLE_PARAMETER_NAMES(it.flags)) } } + + @Test + fun metadataVersionEarlierThan1_4() { + val mv = intArrayOf(1, 3) + assertFailsWith { KotlinClassMetadata.Class.Writer().write(mv) } + assertFailsWith { KotlinClassMetadata.FileFacade.Writer().write(mv) } + assertFailsWith { KotlinClassMetadata.MultiFileClassFacade.Writer().write(listOf("A"), mv) } + assertFailsWith { KotlinClassMetadata.MultiFileClassPart.Writer().write("A", mv) } + assertFailsWith { KotlinClassMetadata.SyntheticClass.Writer().write(mv) } + + KotlinModuleMetadata.Writer().write(mv) + } }