Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue #113: Grow the code for relocatables, and do fixup, and relocate #114

Closed
wants to merge 1 commit into from
Closed

Issue #113: Grow the code for relocatables, and do fixup, and relocate #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
69 changes: 40 additions & 29 deletions janino/src/main/java/org/codehaus/janino/CodeContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}

/**
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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");
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
Expand All @@ -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;
}
Expand Down Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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;
}
});

Expand Down
50 changes: 50 additions & 0 deletions janino/src/test/java/org/codehaus/janino/tests/GithubIssuesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Expand Down