From 830d2f660342a6eeeb801ae1620465e621876e68 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 7 Dec 2016 21:30:26 +0300 Subject: [PATCH] Do not fail when deserializing incompatible metadata Catch all exceptions when deserializing metadata with an incompatible version to prevent the compiler from failing on discovering incompatible classes on the classpath. Note that this is not the perfect solution: any invariant may be broken in the incompatible metadata and it may result in a later exception --- .../library/a.kt | 13 +++++ .../output.txt | 13 +++++ .../wrongMetadataVersionBadMetadata/source.kt | 16 ++++++ .../library/a.kt | 13 +++++ .../output.txt | 13 +++++ .../source.kt | 16 ++++++ .../kotlin/cli/WrongBytecodeVersionTest.kt | 22 ++++++++ ...ompileKotlinAgainstCustomBinariesTest.java | 52 +++++++++++++++++-- .../kotlin/DeserializedDescriptorResolver.kt | 26 ++++++---- 9 files changed, 172 insertions(+), 12 deletions(-) create mode 100644 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/library/a.kt create mode 100644 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/output.txt create mode 100644 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt create mode 100644 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/library/a.kt create mode 100644 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/output.txt create mode 100644 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/library/a.kt b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/library/a.kt new file mode 100644 index 00000000000..cf2b6f3503d --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/library/a.kt @@ -0,0 +1,13 @@ +package a + +open class A { + class Nested + + fun method() {} + + val quux = "" +} + +fun foo() {} +var bar = 42 +typealias TA = String diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/output.txt b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/output.txt new file mode 100644 index 00000000000..166662be83b --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/output.txt @@ -0,0 +1,13 @@ +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 \ No newline at end of file diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt new file mode 100644 index 00000000000..6832020556e --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt @@ -0,0 +1,16 @@ +package usage + +import a.* + +fun baz(param: A, nested: A.Nested) { + val constructor = A() + val nested = A.Nested() + val quux = param.getQuux() + val methodCall = param.method() + val supertype = object : A() {} + + val x = foo() + val y = bar + bar = 239 + val z: TA = "" +} diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/library/a.kt b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/library/a.kt new file mode 100644 index 00000000000..cf2b6f3503d --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/library/a.kt @@ -0,0 +1,13 @@ +package a + +open class A { + class Nested + + fun method() {} + + val quux = "" +} + +fun foo() {} +var bar = 42 +typealias TA = String diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/output.txt b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/output.txt new file mode 100644 index 00000000000..cf93c1bfe55 --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/output.txt @@ -0,0 +1,13 @@ +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 \ No newline at end of file diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt new file mode 100644 index 00000000000..6832020556e --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt @@ -0,0 +1,16 @@ +package usage + +import a.* + +fun baz(param: A, nested: A.Nested) { + val constructor = A() + val nested = A.Nested() + val quux = param.getQuux() + val methodCall = param.method() + val supertype = object : A() {} + + val x = foo() + val y = bar + bar = 239 + val z: TA = "" +} diff --git a/compiler/tests/org/jetbrains/kotlin/cli/WrongBytecodeVersionTest.kt b/compiler/tests/org/jetbrains/kotlin/cli/WrongBytecodeVersionTest.kt index 1e0fba1159e..3ec10b5fe7b 100644 --- a/compiler/tests/org/jetbrains/kotlin/cli/WrongBytecodeVersionTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/cli/WrongBytecodeVersionTest.kt @@ -74,6 +74,28 @@ class WrongBytecodeVersionTest : KtUsefulTestCase() { override fun visit(name: String, value: Any) { super.visit(name, transform(name, value) ?: value) } + + override fun visitArray(name: String): AnnotationVisitor { + val entries = arrayListOf() + val arrayVisitor = { super.visitArray(name) } + return object : AnnotationVisitor(Opcodes.ASM5) { + override fun visit(name: String?, value: Any) { + entries.add(value as String) + } + + override fun visitEnd() { + @Suppress("UNCHECKED_CAST") + val result = transform(name, entries.toTypedArray()) as Array? ?: entries.toTypedArray() + if (result.isEmpty()) return + with(arrayVisitor()) { + for (value in result) { + visit(null, value) + } + visitEnd() + } + } + } + } } } return superVisitor diff --git a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.java b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.java index 032eaea819a..9d4c4c6c306 100644 --- a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.java +++ b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.java @@ -19,11 +19,13 @@ package org.jetbrains.kotlin.jvm.compiler; import com.google.common.collect.Iterables; import com.intellij.openapi.util.Ref; import com.intellij.openapi.util.io.FileUtil; +import com.intellij.util.ArrayUtil; import kotlin.Pair; import kotlin.collections.SetsKt; import kotlin.io.FilesKt; import kotlin.jvm.functions.Function2; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.analyzer.AnalysisResult; import org.jetbrains.kotlin.cli.AbstractCliTest; import org.jetbrains.kotlin.cli.WrongBytecodeVersionTest; @@ -250,7 +252,11 @@ public class CompileKotlinAgainstCustomBinariesTest extends TestCaseWithTmpdir { KotlinTestUtils.assertEqualsToFile(new File(getTestDataDirectory(), "output.txt"), normalizeOutput(output)); } - private void doTestKotlinLibraryWithWrongMetadataVersion(@NotNull String libraryName, @NotNull String... additionalOptions) throws Exception { + private void doTestKotlinLibraryWithWrongMetadataVersion( + @NotNull String libraryName, + @Nullable final Function2 additionalTransformation, + @NotNull String... additionalOptions + ) throws Exception { final int[] version = new JvmMetadataVersion(42, 0, 0).toArray(); File library = transformJar(compileLibrary(libraryName), new Function2() { @Override @@ -258,6 +264,10 @@ public class CompileKotlinAgainstCustomBinariesTest extends TestCaseWithTmpdir { return WrongBytecodeVersionTest.Companion.transformMetadataInClassFile(bytes, new Function2() { @Override public Object invoke(String name, Object value) { + if (additionalTransformation != null) { + Object result = additionalTransformation.invoke(name, value); + if (result != null) return result; + } return JvmAnnotationNames.METADATA_VERSION_FIELD_NAME.equals(name) ? version : null; } }); @@ -417,13 +427,49 @@ public class CompileKotlinAgainstCustomBinariesTest extends TestCaseWithTmpdir { */ public void testWrongMetadataVersion() throws Exception { - doTestKotlinLibraryWithWrongMetadataVersion("library"); + doTestKotlinLibraryWithWrongMetadataVersion("library", null); + } + + public void testWrongMetadataVersionBadMetadata() throws Exception { + doTestKotlinLibraryWithWrongMetadataVersion( + "library", + new Function2() { + @Override + public Object invoke(String name, Object value) { + if (JvmAnnotationNames.METADATA_DATA_FIELD_NAME.equals(name)) { + String[] strings = (String[]) value; + for (int i = 0; i < strings.length; i++) { + byte[] bytes = strings[i].getBytes(); + for (int j = 0; j < bytes.length; j++) bytes[j] ^= 42; + strings[i] = new String(bytes); + } + return strings; + } + return null; + } + } + ); + } + + public void testWrongMetadataVersionBadMetadata2() throws Exception { + doTestKotlinLibraryWithWrongMetadataVersion( + "library", + new Function2() { + @Override + public Object invoke(String name, Object value) { + if (JvmAnnotationNames.METADATA_STRINGS_FIELD_NAME.equals(name)) { + return ArrayUtil.EMPTY_STRING_ARRAY; + } + return null; + } + } + ); } /* // TODO: refactor and uncomment public void testWrongMetadataVersionSkipVersionCheck() throws Exception { - doTestKotlinLibraryWithWrongMetadataVersion("library", "-Xskip-metadata-version-check"); + doTestKotlinLibraryWithWrongMetadataVersion("library", null, "-Xskip-metadata-version-check"); } */ diff --git a/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/kotlin/DeserializedDescriptorResolver.kt b/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/kotlin/DeserializedDescriptorResolver.kt index 987aaa35923..812228ebb48 100644 --- a/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/kotlin/DeserializedDescriptorResolver.kt +++ b/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/kotlin/DeserializedDescriptorResolver.kt @@ -28,7 +28,6 @@ import org.jetbrains.kotlin.serialization.deserialization.IncompatibleVersionErr import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedPackageMemberScope import org.jetbrains.kotlin.serialization.jvm.JvmProtoBufUtil import org.jetbrains.kotlin.utils.addToStdlib.check -import org.jetbrains.kotlin.utils.sure import javax.inject.Inject class DeserializedDescriptorResolver { @@ -47,10 +46,10 @@ class DeserializedDescriptorResolver { internal fun readClassData(kotlinClass: KotlinJvmBinaryClass): ClassDataWithSource? { val data = readData(kotlinClass, KOTLIN_CLASS) ?: return null - val strings = kotlinClass.classHeader.strings.sure { "String table not found in $kotlinClass" } + val strings = kotlinClass.classHeader.strings ?: return null val classData = parseProto(kotlinClass) { JvmProtoBufUtil.readClassDataFrom(data, strings) - } + } ?: return null val sourceElement = KotlinJvmBinarySourceElement( kotlinClass, kotlinClass.incompatibility, @@ -61,10 +60,10 @@ class DeserializedDescriptorResolver { fun createKotlinPackagePartScope(descriptor: PackageFragmentDescriptor, kotlinClass: KotlinJvmBinaryClass): MemberScope? { val data = readData(kotlinClass, KOTLIN_FILE_FACADE_OR_MULTIFILE_CLASS_PART) ?: return null - val strings = kotlinClass.classHeader.strings.sure { "String table not found in $kotlinClass" } + val strings = kotlinClass.classHeader.strings ?: return null val (nameResolver, packageProto) = parseProto(kotlinClass) { JvmProtoBufUtil.readPackageDataFrom(data, strings) - } + } ?: return null val source = JvmPackagePartSource( kotlinClass, kotlinClass.incompatibility, @@ -87,12 +86,21 @@ class DeserializedDescriptorResolver { return (header.data ?: header.incompatibleData)?.check { header.kind in expectedKinds } } - private inline fun parseProto(klass: KotlinJvmBinaryClass, block: () -> T): T { + private inline fun parseProto(klass: KotlinJvmBinaryClass, block: () -> T): T? { try { - return block() + try { + return block() + } + catch (e: InvalidProtocolBufferException) { + throw IllegalStateException("Could not read data from ${klass.location}", e) + } } - catch (e: InvalidProtocolBufferException) { - throw IllegalStateException("Could not read data from ${klass.location}", e) + catch (e: Throwable) { + if (!klass.classHeader.metadataVersion.isCompatible()) { + // TODO: log.warn + return null + } + throw e } }