diff --git a/janino/src/main/java/org/codehaus/janino/CodeContext.java b/janino/src/main/java/org/codehaus/janino/CodeContext.java index 6b38cb289..4ee6702df 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() { + maybeGrow(); + fixUp(); + 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 { + /** + * Grow 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,10 +1222,12 @@ interface FixUp { this.relocatables.add(new Relocatable() { - @Override public boolean + @Override public void + grow() {} + + @Override public void relocate() { uvi.offset = (short) o.offset; - return true; } }); 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..e56aba177 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,56 @@ 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" + + 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) + ); + } + + private 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) {