From ea660863ac94119e9b18323b66e9ce6d383f4cce Mon Sep 17 00:00:00 2001 From: Arno Unkrig Date: Mon, 16 Mar 2020 13:06:48 +0100 Subject: [PATCH] Issue #113: Kindly ask to maintain 3.0.x bugfix versions in parallel (or one time release for 3.0.16) Backported PR #114 (the fix for issue #113) into the "3.0.x" branch. --- .../java/org/codehaus/janino/CodeContext.java | 62 +++++++++++-------- .../janino/tests/GithubIssuesTest.java | 54 ++++++++++++++++ 2 files changed, 89 insertions(+), 27 deletions(-) diff --git a/janino/src/main/java/org/codehaus/janino/CodeContext.java b/janino/src/main/java/org/codehaus/janino/CodeContext.java index 443e6d8cb..d667e2f3a 100644 --- a/janino/src/main/java/org/codehaus/janino/CodeContext.java +++ b/janino/src/main/java/org/codehaus/janino/CodeContext.java @@ -803,14 +803,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(); + } } /** @@ -825,20 +830,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; } /** @@ -1102,8 +1100,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"); } @@ -1125,8 +1123,15 @@ 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; final byte[] ba; if (!this.expanded) { @@ -1165,7 +1170,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 @@ -1224,7 +1228,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"); @@ -1237,7 +1244,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; } @@ -1410,13 +1416,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(); } /** 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 c5cbaccd8..89d130830 100644 --- a/janino/src/test/java/org/codehaus/janino/tests/GithubIssuesTest.java +++ b/janino/src/test/java/org/codehaus/janino/tests/GithubIssuesTest.java @@ -263,4 +263,58 @@ class ExpressionCompiler extends ExpressionEvaluator { Assert.assertEquals(3, top.getLineNumber()); } } + + @Test public void + testIssue113() throws Exception { + CompilationUnit cu = (CompilationUnit) new Parser(new Scanner( + "issue113", // This will appear in stack traces as "file name". + new StringReader( + "" + + "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" + ) + )).parseAbstractCompilationUnit(); + + // Compile the code into a ClassLoader. + SimpleCompiler sc = new SimpleCompiler(); + sc.setDebuggingInformation(true, true, true); + sc.cook(cu); + ClassLoader cl = sc.getClassLoader(); + + Assert.assertEquals("ABCD", cl.loadClass("demo.pkg3.A$$1").getMethod("main").invoke(null)); + } + + 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(); + } }