Only use JSRInlinerAdapter if necessary
Since JSR and RET instructions are not emitted by modern Java compilers,
they will almost never be encountered in practice. Try parsing input
classfiles without JSRInlinerAdapter; if JSR or RET is encountered,
restart parsing the problematic class with the JSRInlinerAdapter.
In JarCode, this is implemented by iterating over the instructions,
looking for JSR and RET explicitly.
In LazyCfCode, a private exception class is thrown to restart processing
the class as soon as JSR or RET is encountered.
JumpSubroutineTests: Add testing of CF frontend.
CfStackInstruction: Enable building IR for the SWAP instruction,
since this is encountered in JumpSubroutineTests.
Change-Id: Ic6865d1e11ecb10b1388dded3b90fcfca4f18d7d
diff --git a/src/main/java/com/android/tools/r8/cf/code/CfStackInstruction.java b/src/main/java/com/android/tools/r8/cf/code/CfStackInstruction.java
index 733f2d9..a30c6e6 100644
--- a/src/main/java/com/android/tools/r8/cf/code/CfStackInstruction.java
+++ b/src/main/java/com/android/tools/r8/cf/code/CfStackInstruction.java
@@ -216,9 +216,7 @@
// Current stack: ..., value1, value2, value1copy
state.pop();
// Output stack: ..., value1, value2
- throw new Unimplemented(
- "Building IR for CfStackInstruction " + opcode + " not supported");
- // break;
+ break;
}
}
}
diff --git a/src/main/java/com/android/tools/r8/graph/JarCode.java b/src/main/java/com/android/tools/r8/graph/JarCode.java
index 6c414ac..009bb61 100644
--- a/src/main/java/com/android/tools/r8/graph/JarCode.java
+++ b/src/main/java/com/android/tools/r8/graph/JarCode.java
@@ -186,15 +186,44 @@
}
private void triggerDelayedParsingIfNeccessary() {
- if (context != null) {
- // The SecondVistor is in charge of setting the context to null.
- DexProgramClass owner = context.owner;
- new ClassReader(context.classCache).accept(new SecondVisitor(context, application),
- ClassReader.SKIP_FRAMES);
- assert verifyNoReparseContext(owner);
+ if (this.context != null) {
+ // The SecondVisitor is in charge of setting this.context to null.
+ ReparseContext context = this.context;
+ parseCode(context, false);
+ if (hasJsr(context)) {
+ System.out.println("JarCode: JSR encountered; reparse using JSRInlinerAdapter");
+ parseCode(context, true);
+ assert !hasJsr(context);
+ }
+ assert verifyNoReparseContext(context.owner);
}
}
+ private void parseCode(ReparseContext context, boolean useJsrInliner) {
+ SecondVisitor classVisitor = new SecondVisitor(context, application, useJsrInliner);
+ new ClassReader(context.classCache).accept(classVisitor, ClassReader.SKIP_FRAMES);
+ }
+
+ private boolean hasJsr(ReparseContext context) {
+ for (Code code : context.codeList) {
+ if (hasJsr(code.asJarCode().node)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private boolean hasJsr(MethodNode node) {
+ Iterator<AbstractInsnNode> it = node.instructions.iterator();
+ while (it.hasNext()) {
+ int opcode = it.next().getOpcode();
+ if (opcode == Opcodes.JSR || opcode == Opcodes.RET) {
+ return true;
+ }
+ }
+ return false;
+ }
+
/**
* Fills the MethodNodes of all the methods in the class and removes the ReparseContext.
*/
@@ -202,18 +231,24 @@
private final ReparseContext context;
private final JarApplicationReader application;
+ private final boolean useJsrInliner;
private int methodIndex = 0;
- public SecondVisitor(ReparseContext context, JarApplicationReader application) {
+ public SecondVisitor(
+ ReparseContext context, JarApplicationReader application, boolean useJsrInliner) {
super(Opcodes.ASM6);
this.context = context;
this.application = application;
+ this.useJsrInliner = useJsrInliner;
}
@Override
public MethodVisitor visitMethod(int access, String name, String desc, String signature,
String[] exceptions) {
- MethodNode node = new JSRInlinerAdapter(null, access, name, desc, signature, exceptions);
+ MethodNode node =
+ useJsrInliner
+ ? new JSRInlinerAdapter(null, access, name, desc, signature, exceptions)
+ : new MethodNode(Opcodes.ASM6, access, name, desc, signature, exceptions);
JarCode code = null;
MethodAccessFlags flags = JarClassFileReader.createMethodAccessFlags(name, access);
if (!flags.isAbstract() && !flags.isNative()) {
diff --git a/src/main/java/com/android/tools/r8/graph/LazyCfCode.java b/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
index 740ee27..8ec4c0a 100644
--- a/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
+++ b/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
@@ -79,6 +79,12 @@
public class LazyCfCode extends Code {
+ private static class JsrEncountered extends RuntimeException {
+ public JsrEncountered(String s) {
+ super(s);
+ }
+ }
+
public LazyCfCode(
DexMethod method, Origin origin, ReparseContext context, JarApplicationReader application) {
@@ -108,17 +114,34 @@
@Override
public CfCode asCfCode() {
if (code == null) {
+ ReparseContext context = this.context;
assert context != null;
- // The SecondVistor is in charge of setting the context to null.
- DexProgramClass owner = context.owner;
- ClassReader classReader = new ClassReader(context.classCache);
- classReader.accept(new ClassCodeVisitor(context, application), ClassReader.EXPAND_FRAMES);
- assert verifyNoReparseContext(owner);
+ // The ClassCodeVisitor is in charge of setting this.context to null.
+ try {
+ parseCode(context, false);
+ } catch (JsrEncountered e) {
+ System.out.println("LazyCfCode: JSR encountered; reparse using JSRInlinerAdapter");
+ for (Code code : context.codeList) {
+ code.asLazyCfCode().code = null;
+ code.asLazyCfCode().context = context;
+ }
+ try {
+ parseCode(context, true);
+ } catch (JsrEncountered e1) {
+ throw new Unreachable(e1);
+ }
+ }
+ assert verifyNoReparseContext(context.owner);
}
assert code != null;
return code;
}
+ public void parseCode(ReparseContext context, boolean useJsrInliner) {
+ ClassCodeVisitor classVisitor = new ClassCodeVisitor(context, application, useJsrInliner);
+ new ClassReader(context.classCache).accept(classVisitor, ClassReader.EXPAND_FRAMES);
+ }
+
private void setCode(CfCode code) {
assert this.code == null;
assert this.context != null;
@@ -189,11 +212,14 @@
private final ReparseContext context;
private final JarApplicationReader application;
private int methodIndex = 0;
+ private boolean usrJsrInliner;
- ClassCodeVisitor(ReparseContext context, JarApplicationReader application) {
+ ClassCodeVisitor(
+ ReparseContext context, JarApplicationReader application, boolean useJsrInliner) {
super(Opcodes.ASM6);
this.context = context;
this.application = application;
+ this.usrJsrInliner = useJsrInliner;
}
@Override
@@ -205,6 +231,9 @@
DexMethod method = application.getMethod(context.owner.type, name, desc);
assert code.method == method;
MethodCodeVisitor methodVisitor = new MethodCodeVisitor(application, code);
+ if (!usrJsrInliner) {
+ return methodVisitor;
+ }
return new JSRInlinerAdapter(methodVisitor, access, name, desc, signature, exceptions);
}
return null;
@@ -581,7 +610,7 @@
type = ValueType.OBJECT;
break;
case Opcodes.RET:
- throw new Unreachable("RET should be handled by the ASM jsr inliner");
+ throw new JsrEncountered("RET should be handled by the ASM jsr inliner");
default:
throw new Unreachable("Unexpected VarInsn opcode: " + opcode);
}
@@ -670,7 +699,7 @@
instructions.add(new CfIf(type, ValueType.OBJECT, target));
break;
case Opcodes.JSR:
- throw new Unreachable("JSR should be handled by the ASM jsr inliner");
+ throw new JsrEncountered("JSR should be handled by the ASM jsr inliner");
default:
throw new Unreachable("Unexpected JumpInsn opcode: " + opcode);
}
diff --git a/src/test/java/com/android/tools/r8/jasmin/JumpSubroutineTests.java b/src/test/java/com/android/tools/r8/jasmin/JumpSubroutineTests.java
index 5e41a25..ec18fac 100644
--- a/src/test/java/com/android/tools/r8/jasmin/JumpSubroutineTests.java
+++ b/src/test/java/com/android/tools/r8/jasmin/JumpSubroutineTests.java
@@ -8,11 +8,15 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
+import com.android.tools.r8.OutputMode;
+import com.android.tools.r8.R8Command;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.jasmin.JasminBuilder.ClassFileVersion;
import com.android.tools.r8.utils.AndroidApp;
import com.google.common.collect.ImmutableList;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import org.junit.Test;
public class JumpSubroutineTests extends JasminTestBase {
@@ -22,10 +26,28 @@
assertEquals(expected, javaResult);
String artResult = runOnArtD8(builder, main);
assertEquals(expected, artResult);
+ String cfFrontendResult = runOnJavaR8CfFrontend(builder, main);
+ assertEquals(expected, cfFrontendResult);
String dxArtResult = runOnArtDx(builder, main);
assertEquals(expected, dxArtResult);
}
+ private String runOnJavaR8CfFrontend(JasminBuilder builder, String main) throws Exception {
+ Path inputJar = temp.getRoot().toPath().resolve("input.jar");
+ Path outputJar = temp.getRoot().toPath().resolve("output.jar");
+ builder.writeJar(inputJar, null);
+ ToolHelper.runR8(
+ R8Command.builder()
+ .addProgramFiles(inputJar)
+ .setOutput(outputJar, OutputMode.ClassFile)
+ .addLibraryFiles(Paths.get(ToolHelper.JAVA_8_RUNTIME))
+ .build(),
+ options -> options.enableCfFrontend = true);
+ ProcessResult processResult = ToolHelper.runJava(outputJar, main);
+ assertEquals(0, processResult.exitCode);
+ return processResult.stdout;
+ }
+
private void expectDxFailure(JasminBuilder builder) throws Exception {
// This expects this dx failure:
// Uncaught translation error: com.android.dex.util.ExceptionWithContext: returning from
@@ -387,6 +409,8 @@
assertEquals(expected, javaResult);
String artResult = runOnArtD8(builder, clazz.name);
assertEquals(expected, artResult);
+ String cfFrontendResult = runOnJavaR8CfFrontend(builder, clazz.name);
+ assertEquals(expected, cfFrontendResult);
// This fails with dx.
expectDxFailure(builder);
}
@@ -1344,6 +1368,8 @@
String artResult = runOnArtD8(builder, clazz.name);
// The ASM jsr inliner does not get the control-flow dependent ret right in his case.
assertNotEquals(expected, artResult);
+ String cfFrontendResult = runOnJavaR8CfFrontend(builder, clazz.name);
+ assertNotEquals(expected, cfFrontendResult);
// This fails with dx.
expectDxFailure(builder);
}
@@ -1392,6 +1418,8 @@
String artResult = runOnArtD8(builder, clazz.name);
// The ASM jsr inliner does not get the control-flow dependent ret right in his case.
assertNotEquals(expected, artResult);
+ String cfFrontendResult = runOnJavaR8CfFrontend(builder, clazz.name);
+ assertNotEquals(expected, cfFrontendResult);
// This fails with dx.
expectDxFailure(builder);
}
@@ -1441,6 +1469,8 @@
String artResult = runOnArtD8(builder, clazz.name);
// The ASM jsr inliner does not get the control-flow dependent ret right in his case.
assertNotEquals(expected, artResult);
+ String cfFrontendResult = runOnJavaR8CfFrontend(builder, clazz.name);
+ assertNotEquals(expected, cfFrontendResult);
// This fails with dx.
expectDxFailure(builder);
}
