From 367c58e73e32014e5e1a5fc8225b7e5c17de00ad Mon Sep 17 00:00:00 2001 From: Arno Unkrig Date: Sun, 8 Mar 2020 20:02:20 +0100 Subject: [PATCH] Merged pull request #114 "Grow the code for relocatables, and do fixup, and relocate". --- .../java/org/codehaus/janino/CodeContext.java | 73 ++++++++++-------- .../janino/tests/GithubIssuesTest.java | 77 +++++++++++++++++++ 2 files changed, 118 insertions(+), 32 deletions(-) diff --git a/janino/src/main/java/org/codehaus/janino/CodeContext.java b/janino/src/main/java/org/codehaus/janino/CodeContext.java index 6b38cb289..3c3080172 100644 --- a/janino/src/main/java/org/codehaus/janino/CodeContext.java +++ b/janino/src/main/java/org/codehaus/janino/CodeContext.java @@ -385,14 +385,19 @@ class LocalScope { */ public void fixUpAndRelocate() { + this.maybeGrow(); + this.fixUp(); + this.relocate(); + } - // We do this in a loop to allow relocatables to adjust the size - // of things in the byte stream. It is extremely unlikely, but possible - // that a late relocatable will grow the size of the bytecode, and require - // an earlier relocatable to switch from 32K mode to 64K mode branching - do { - this.fixUp(); - } while (!this.relocate()); + /** + * Grow the code if relocatables are required to. + */ + private void + maybeGrow() { + for (Relocatable relocatable : this.relocatables) { + relocatable.grow(); + } } /** @@ -407,20 +412,13 @@ class LocalScope { } /** - * Relocates all relocatables and aggregate their response into a single one. - * - * @return {@code true} if all of them relocated successfully, {@code false} if any of them needed to change size + * Relocates all relocatables. */ - private boolean + private void relocate() { - boolean finished = true; for (Relocatable relocatable : this.relocatables) { - - // Do not terminate earlier so that everything gets a chance to grow in the first pass changes the common - // case for this to be O(n) instead of O(n**2). - finished &= relocatable.relocate(); + relocatable.relocate(); } - return finished; } /** @@ -622,8 +620,8 @@ class Branch extends Relocatable { } } - @Override public boolean - relocate() { + @Override public void + grow() { if (this.destination.offset == Offset.UNSET) { throw new InternalCompilerException("Cannot relocate branch to unset destination offset"); } @@ -643,9 +641,17 @@ class Branch extends Relocatable { CodeContext.this.popInserter(); this.source.offset = pos; this.expanded = true; - return false; } + } + + @Override public void + relocate() { + if (this.destination.offset == Offset.UNSET) { + throw new InternalCompilerException("Cannot relocate branch to unset destination offset"); + } + int offset = this.destination.offset - this.source.offset; + @SuppressWarnings("deprecation") final int opcodeJsr = Opcode.JSR; final byte[] ba; if (!this.expanded) { //we fit in a 16-bit jump @@ -683,7 +689,6 @@ class Branch extends Relocatable { } } System.arraycopy(ba, 0, CodeContext.this.code, this.source.offset, ba.length); - return true; } private boolean expanded; //marks whether this has been expanded to account for a wide branch @@ -748,7 +753,10 @@ class OffsetBranch extends Relocatable { this.destination = destination; } - @Override public boolean + @Override public void + grow() {} + + @Override public void relocate() { if (this.source.offset == Offset.UNSET || this.destination.offset == Offset.UNSET) { throw new InternalCompilerException("Cannot relocate offset branch to unset destination offset"); @@ -761,7 +769,6 @@ class OffsetBranch extends Relocatable { (byte) offset }; System.arraycopy(ba, 0, CodeContext.this.code, this.where.offset, 4); - return true; } private final Offset where, source, destination; } @@ -1026,13 +1033,15 @@ class LineNumberOffset extends Offset { private abstract class Relocatable { + /** + * Grows the code if the relocation cannot be done without growing code. + */ + public abstract void grow(); + /** * Relocates this object. - * - * @return {@code true} if the relocation succeeded in place; {@code false} if the relocation grew the number - * of bytes required */ - public abstract boolean relocate(); + public abstract void relocate(); } /** @@ -1213,11 +1222,11 @@ interface FixUp { this.relocatables.add(new Relocatable() { - @Override public boolean - relocate() { - uvi.offset = (short) o.offset; - return true; - } + @Override public void + grow() {} + + @Override public void + relocate() { uvi.offset = (short) o.offset; } }); this.pushOperand(uvi); diff --git a/janino/src/test/java/org/codehaus/janino/tests/GithubIssuesTest.java b/janino/src/test/java/org/codehaus/janino/tests/GithubIssuesTest.java index 00c55febb..c187612ca 100644 --- a/janino/src/test/java/org/codehaus/janino/tests/GithubIssuesTest.java +++ b/janino/src/test/java/org/codehaus/janino/tests/GithubIssuesTest.java @@ -266,6 +266,83 @@ class GithubIssuesTest { ); } + @Test public void + testIssue113() throws Exception { + CompileUnit unit1 = new CompileUnit("demo.pkg3", "A$$1", ( + "" + + "package demo.pkg3;\n" + + "public class A$$1 {\n" + + " public static String main() {\n" + + " StringBuilder sb = new StringBuilder();\n" + + " short b = 1;\n" + + " for (int i = 0; i < 4; i++) {\n" + + " ;\n" + + " switch (i) {\n" + + " case 0:\n" + + " sb.append(\"A\");\n" + + " break;\n" + + " case 1:\n" + + " sb.append(\"B\");\n" + + " break;\n" + + " case 2:\n" + + " sb.append(\"C\");\n" + + " break;\n" + + " case 3:\n" + + " sb.append(\"D\");\n" + + " break;\n" + + " }\n" + + GithubIssuesTest.injectDummyLargeCodeExceedingShort() + + " }\n" + + " return sb.toString();\n" + + " }\n" + + "}\n" + )); + + ClassLoader classLoader = this.compile(Thread.currentThread().getContextClassLoader(), unit1); + + Assert.assertEquals("ABCD", classLoader.loadClass("demo.pkg3.A$$1").getMethod("main").invoke(null)); + } +// java.lang.VerifyError: (class: demo/pkg3/A$$1, method: main signature: ()Ljava/lang/String;) Illegal instruction found at offset 18 +// at java.lang.Class.getDeclaredMethods0(Native Method) +// at java.lang.Class.privateGetDeclaredMethods(Class.java:2436) +// at java.lang.Class.getMethod0(Class.java:2679) +// at java.lang.Class.getMethod(Class.java:1605) +// at org.codehaus.janino.tests.GithubIssuesTest.testIssue113(GithubIssuesTest.java:306) +// at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) +// at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) +// at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) +// at java.lang.reflect.Method.invoke(Method.java:597) +// at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50) +// at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) +// at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47) +// at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) +// at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325) +// at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78) +// at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57) +// at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290) +// at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71) +// at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288) +// at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58) +// at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268) +// at org.junit.runners.ParentRunner.run(ParentRunner.java:363) +// at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:89) +// at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41) +// at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:541) +// at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:763) +// at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:463) +// at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:209) + + + private static String + injectDummyLargeCodeExceedingShort() { + StringBuilder sb = new StringBuilder(); + sb.append("int a = -1;\n"); + for (int i = 0 ; i < Short.MAX_VALUE / 3 ; i++) { + sb.append("a = " + i + ";\n"); + } + return sb.toString(); + } + public ClassLoader compile(ClassLoader parentClassLoader, CompileUnit... compileUnits) {