From 82a02ea9a51f7f262111fa6b30177edfac66b116 Mon Sep 17 00:00:00 2001 From: Alon Albert Date: Thu, 1 Aug 2024 09:00:59 -0700 Subject: [PATCH] Fix Line Table Parsing in ByteCodeParser The existing code assumes that the line table comes right after the code section but this is not always the case. For example, if there are try-catch blocks in the code, the exception table will come before the line table. --- .../explorer/bytecode/ByteCodeParser.kt | 110 +++++++++++------- .../explorer/bytecode/ByteCodeParserTest.kt | 34 +++--- .../testData/Issue_45-Bytecode.expected | 47 ++++++++ .../testData/TryCatch-Bytecode.expected | 23 ++++ src/jvmTest/kotlin/testData/TryCatch.javap | 45 +++++++ src/jvmTest/kotlin/testData/TryCatch.kt | 15 +++ 6 files changed, 217 insertions(+), 57 deletions(-) create mode 100644 src/jvmTest/kotlin/testData/Issue_45-Bytecode.expected create mode 100644 src/jvmTest/kotlin/testData/TryCatch-Bytecode.expected create mode 100644 src/jvmTest/kotlin/testData/TryCatch.javap create mode 100644 src/jvmTest/kotlin/testData/TryCatch.kt diff --git a/src/jvmMain/kotlin/dev/romainguy/kotlin/explorer/bytecode/ByteCodeParser.kt b/src/jvmMain/kotlin/dev/romainguy/kotlin/explorer/bytecode/ByteCodeParser.kt index 42c1569e..d6913af5 100644 --- a/src/jvmMain/kotlin/dev/romainguy/kotlin/explorer/bytecode/ByteCodeParser.kt +++ b/src/jvmMain/kotlin/dev/romainguy/kotlin/explorer/bytecode/ByteCodeParser.kt @@ -83,60 +83,82 @@ class ByteCodeParser { } } - private fun PeekingIterator.readClass(classHeader: String): Class { - val methods = buildList { - while (hasNext()) { - val line = peek() - when { - line == "}" -> break - MethodRegex.matches(line) -> add(readMethod()) - else -> next() - } - } - if (next() != "}") { - throw IllegalStateException("Expected '}' but got '${peek()}'") +} + +private fun PeekingIterator.readClass(classHeader: String): Class { + val methods = buildList { + while (hasNext()) { + val line = peek() + when { + line == "}" -> break + MethodRegex.matches(line) -> add(readMethod()) + else -> next() } } - return Class(classHeader, methods) + if (next() != "}") { + throw IllegalStateException("Expected '}' but got '${peek()}'") + } } + return Class(classHeader, methods) +} - private fun PeekingIterator.readMethod(): Method { - val match = MethodRegex.matchEntire(next()) - ?: throw IllegalStateException("Expected method but got '${peek()}'") - val header = match.getValue("header") - if (next().trim() != "Code:") { - throw IllegalStateException("Expected 'Code:' but got '${peek()}'") - } - val instructions = readInstructions() - val lineNumbers = readLineNumbers() - return Method(header, InstructionSet(ISA.ByteCode, instructions.withLineNumbers(lineNumbers))) +private fun PeekingIterator.readMethod(): Method { + val match = MethodRegex.matchEntire(next()) + ?: throw IllegalStateException("Expected method but got '${peek()}'") + val header = match.getValue("header") + if (next().trim() != "Code:") { + throw IllegalStateException("Expected 'Code:' but got '${peek()}'") } + val instructions = readInstructions() + val lineNumbers = readLineNumbers() - private fun PeekingIterator.readInstructions(): List { - return buildList { - while (hasNext()) { - val line = next().trim() - val match = InstructionRegex.matchEntire(line) ?: break - val address = match.getValue("address") - val code = match.getValue("code").replace(CommentRegex, " //") - val jumpAddress = JumpRegex.matchEntire(code)?.getValue("address")?.toInt() ?: -1 - val (op, operands) = codeToOpAndOperands(code) - add(Instruction(address.toInt(), address, op, operands, jumpAddress)) - } - } - } + return Method(header, InstructionSet(ISA.ByteCode, instructions.withLineNumbers(lineNumbers))) +} - private fun PeekingIterator.readLineNumbers(): IntIntMap { - val map = mutableIntIntMapOf() + +private fun PeekingIterator.readInstructions(): List { + return buildList { while (hasNext()) { - val line = next().trim() - if (!line.startsWith("line")) { - break - } - val (lineNumber, address) = line.substringAfter(' ').split(": ", limit = 2) - map.put(address.toInt(), lineNumber.toInt()) + val line = peek().trim() + val match = InstructionRegex.matchEntire(line) ?: break + next() + val address = match.getValue("address") + val code = match.getValue("code").replace(CommentRegex, " //") + val jumpAddress = JumpRegex.matchEntire(code)?.getValue("address")?.toInt() ?: -1 + val (op, operands) = codeToOpAndOperands(code) + add(Instruction(address.toInt(), address, op, operands, jumpAddress)) } + } +} + +private fun PeekingIterator.readLineNumbers(): IntIntMap { + val map = mutableIntIntMapOf() + val found = skipToLineNumberTable() + if (!found) { return map } + next() + while (hasNext()) { + val line = peek().trim() + if (!line.startsWith("line")) { + break + } + next() + val (lineNumber, address) = line.substringAfter(' ').split(": ", limit = 2) + map.put(address.toInt(), lineNumber.toInt()) + } + return map +} + +private fun PeekingIterator.skipToLineNumberTable(): Boolean { + while (hasNext()) { + val line = peek().trim() + when (line) { + "LineNumberTable:" -> return true + "", "}" -> return false + } + next() + } + return false } diff --git a/src/jvmTest/kotlin/dev/romainguy/kotlin/explorer/bytecode/ByteCodeParserTest.kt b/src/jvmTest/kotlin/dev/romainguy/kotlin/explorer/bytecode/ByteCodeParserTest.kt index 8e286de8..320eb0c9 100644 --- a/src/jvmTest/kotlin/dev/romainguy/kotlin/explorer/bytecode/ByteCodeParserTest.kt +++ b/src/jvmTest/kotlin/dev/romainguy/kotlin/explorer/bytecode/ByteCodeParserTest.kt @@ -17,11 +17,14 @@ package dev.romainguy.kotlin.explorer.bytecode import com.google.common.truth.Truth.assertThat +import dev.romainguy.kotlin.explorer.code.Code import dev.romainguy.kotlin.explorer.testing.Builder import dev.romainguy.kotlin.explorer.testing.parseSuccess import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder +import java.nio.file.Path +import kotlin.io.path.readText class ByteCodeParserTest { @get:Rule @@ -34,18 +37,23 @@ class ByteCodeParserTest { fun issue_45() { val content = byteCodeParser.parseSuccess(builder.generateByteCode("Issue_45.kt")) - assertThat(content.classes.map { it.header }).containsExactly( - "final class testData.Issue_45Kt\$main$1", - "public final class testData.Issue_45Kt", - ) - assertThat(content.classes.flatMap { it.methods }.map { it.header }).containsExactly( - "testData.Issue_45Kt\$main\$1()", - "public final void invoke()", - "public java.lang.Object invoke()", - "public static final void main()", - "public static final void f1(kotlin.jvm.functions.Function0)", - "public static final void f2()", - "public static void main(java.lang.String[])", - ) + val text = Code.fromClasses(content.classes).text + + assertThat(text).isEqualTo(loadTestDataFile("Issue_45-Bytecode.expected")) + } + + @Test + fun tryCatch() { + val content = byteCodeParser.parseSuccess(builder.generateByteCode("TryCatch.kt")) + + val text = Code.fromClasses(content.classes).text + + assertThat(text).isEqualTo(loadTestDataFile("TryCatch-Bytecode.expected")) } } + +fun loadTestDataFile(path: String): String { + val cwd = Path.of(System.getProperty("user.dir")) + val testData = cwd.resolve("src/jvmTest/kotlin/testData") + return testData.resolve(path).readText() +} diff --git a/src/jvmTest/kotlin/testData/Issue_45-Bytecode.expected b/src/jvmTest/kotlin/testData/Issue_45-Bytecode.expected new file mode 100644 index 00000000..c9a72f03 --- /dev/null +++ b/src/jvmTest/kotlin/testData/Issue_45-Bytecode.expected @@ -0,0 +1,47 @@ +final class testData.Issue_45Kt$main$1 + testData.Issue_45Kt$main$1() + -- 4 instructions + 0: aload_0 + 1: iconst_0 + 2: invokespecial #12 // Method kotlin/jvm/internal/Lambda."":(I)V + 5: return + + public final void invoke() + -- 2 instructions + 5: 0: invokestatic #20 // Method testData/Issue_45Kt.f2:()V + 6: 3: return + + public java.lang.Object invoke() + -- 4 instructions + 4: 0: aload_0 + 1: invokevirtual #23 // Method invoke:()V + 4: getstatic #29 // Field kotlin/Unit.INSTANCE:Lkotlin/Unit; + 7: areturn + +public final class testData.Issue_45Kt + public static final void main() + -- 4 instructions + 4: 0: getstatic #12 // Field testData/Issue_45Kt$main$1.INSTANCE:LtestData/Issue_45Kt$main$1; + 3: checkcast #14 // class kotlin/jvm/functions/Function0 + 6: invokestatic #18 // Method f1:(Lkotlin/jvm/functions/Function0;)V + 7: 9: return + + public static final void f1(kotlin.jvm.functions.Function0) + -- 4 instructions + 9: 0: aload_0 + 1: invokeinterface #24, 1 // InterfaceMethod kotlin/jvm/functions/Function0.invoke:()Ljava/lang/Object; + 6: pop + 7: return + + public static final void f2() + -- 5 instructions + 11: 0: ldc #29 // String Hi + 2: getstatic #35 // Field java/lang/System.out:Ljava/io/PrintStream; + 5: swap + 6: invokevirtual #41 // Method java/io/PrintStream.println:(Ljava/lang/Object;)V + 12: 9: return + + public static void main(java.lang.String[]) + -- 2 instructions + 0: invokestatic #44 // Method main:()V + 3: return diff --git a/src/jvmTest/kotlin/testData/TryCatch-Bytecode.expected b/src/jvmTest/kotlin/testData/TryCatch-Bytecode.expected new file mode 100644 index 00000000..f1800e1e --- /dev/null +++ b/src/jvmTest/kotlin/testData/TryCatch-Bytecode.expected @@ -0,0 +1,23 @@ +public final class testData.TryCatchKt + public static final void main() + -- 9 instructions + 4: 0: aconst_null + 1: astore_0 + 5: 2: nop + 6: 3: invokestatic #11 // Method foo:()V + 6: goto 16 + 8: 9: astore_1 + 9: 10: getstatic #17 // Field java/lang/System.out:Ljava/io/PrintStream; + 13: invokevirtual #22 // Method java/io/PrintStream.println:()V + 11: 16: return + + public static final void foo() + -- 3 instructions + 14: 0: getstatic #17 // Field java/lang/System.out:Ljava/io/PrintStream; + 3: invokevirtual #22 // Method java/io/PrintStream.println:()V + 15: 6: return + + public static void main(java.lang.String[]) + -- 2 instructions + 0: invokestatic #29 // Method main:()V + 3: return diff --git a/src/jvmTest/kotlin/testData/TryCatch.javap b/src/jvmTest/kotlin/testData/TryCatch.javap new file mode 100644 index 00000000..b0f16b47 --- /dev/null +++ b/src/jvmTest/kotlin/testData/TryCatch.javap @@ -0,0 +1,45 @@ +Compiled from "TryCatch.kt" +public final class testData.TryCatchKt { + public static final void main(); + Code: + 0: aconst_null + 1: astore_0 + 2: nop + 3: invokestatic #11 // Method foo:()V + 6: goto 16 + 9: astore_1 + 10: getstatic #17 // Field java/lang/System.out:Ljava/io/PrintStream; + 13: invokevirtual #22 // Method java/io/PrintStream.println:()V + 16: return + Exception table: + from to target type + 2 6 9 Class java/lang/Exception + LineNumberTable: + line 4: 0 + line 5: 2 + line 6: 3 + line 8: 9 + line 9: 10 + line 11: 16 + LocalVariableTable: + Start Length Slot Name Signature + 10 6 1 e Ljava/lang/Exception; + 2 15 0 a Ljava/lang/Void; + + public static final void foo(); + Code: + 0: getstatic #17 // Field java/lang/System.out:Ljava/io/PrintStream; + 3: invokevirtual #22 // Method java/io/PrintStream.println:()V + 6: return + LineNumberTable: + line 14: 0 + line 15: 6 + + public static void main(java.lang.String[]); + Code: + 0: invokestatic #29 // Method main:()V + 3: return + LocalVariableTable: + Start Length Slot Name Signature + 0 4 0 args [Ljava/lang/String; +} \ No newline at end of file diff --git a/src/jvmTest/kotlin/testData/TryCatch.kt b/src/jvmTest/kotlin/testData/TryCatch.kt new file mode 100644 index 00000000..39577939 --- /dev/null +++ b/src/jvmTest/kotlin/testData/TryCatch.kt @@ -0,0 +1,15 @@ +package testData + +fun main() { + val a = null + try { + foo() + } + catch (e: Exception) { + println() + } +} + +fun foo() { + println() +}