Annotate all instructions with position information.
This addresses several outstanding issues: blocks can now be reordered without
issue, if positions are present all instructions will be annotated, in
particular move-exceptions for synchornized blocks. This change requires that
all IR tranformations properly transfer position information for each
instruction.
Bug: 65618023
Bug: 65567013
Bug: 65474153
Change-Id: I9ded6003c2b349e738f50e7be58f35536bc5b08b
diff --git a/src/test/debugTestResources/FinallyBlock.java b/src/test/debugTestResources/FinallyBlock.java
new file mode 100644
index 0000000..4b30391
--- /dev/null
+++ b/src/test/debugTestResources/FinallyBlock.java
@@ -0,0 +1,37 @@
+// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+public class FinallyBlock {
+
+ public static int finallyBlock(Throwable obj) throws Throwable {
+ int x = 21;
+ try {
+ if (obj != null) {
+ throw obj;
+ }
+ } catch (AssertionError e) {
+ x = e.getMessage().length() + 1;
+ } catch (RuntimeException e) {
+ x = e.getMessage().length() + 2;
+ } finally {
+ x *= 2;
+ }
+ return x;
+ }
+
+ private static int callFinallyBlock(Throwable obj) {
+ try {
+ return finallyBlock(obj);
+ } catch (Throwable e) {
+ return -1;
+ }
+ }
+
+ public static void main(String[] args) throws Throwable {
+ System.out.println(callFinallyBlock(null));
+ System.out.println(callFinallyBlock(new AssertionError("assert error")));
+ System.out.println(callFinallyBlock(new RuntimeException("runtime error")));
+ System.out.println(callFinallyBlock(new Throwable("throwable")));
+ }
+}
diff --git a/src/test/debugTestResources/SharedCode.java b/src/test/debugTestResources/SharedCode.java
new file mode 100644
index 0000000..f88fdbd
--- /dev/null
+++ b/src/test/debugTestResources/SharedCode.java
@@ -0,0 +1,25 @@
+// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+public class SharedCode {
+
+ public static int sharedIf(int x) {
+ if (x == 0) {
+ doit(1); doit(2); doit(1); doit(2);
+ } else {
+ doit(1); doit(2); doit(1); doit(2);
+ }
+ return x;
+ }
+
+
+ public static void doit(int x) {
+ // nothing to do really.
+ }
+
+ public static void main(String[] args) {
+ System.out.println(sharedIf(0));
+ System.out.println(sharedIf(1));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/debug/BlockReorderingTest.java b/src/test/java/com/android/tools/r8/debug/BlockReorderingTest.java
index 6f6b7d0..7081921 100644
--- a/src/test/java/com/android/tools/r8/debug/BlockReorderingTest.java
+++ b/src/test/java/com/android/tools/r8/debug/BlockReorderingTest.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.debug;
import org.junit.BeforeClass;
-import org.junit.Ignore;
import org.junit.Test;
/**
@@ -23,7 +22,6 @@
}
@Test
- @Ignore("b/65618023")
public void testConditionalReturn() throws Throwable {
final String method = "conditionalReturn";
runDebugTest(CLASS,
@@ -39,7 +37,6 @@
}
@Test
- @Ignore("b/65618023")
public void testInvertConditionalReturn() throws Throwable {
final String method = "invertConditionalReturn";
runDebugTest(CLASS,
@@ -55,7 +52,6 @@
}
@Test
- @Ignore("b/65618023")
public void testFallthroughReturn() throws Throwable {
final String method = "fallthroughReturn";
runDebugTest(CLASS,
diff --git a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
index ec32bd3..6686635 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
@@ -11,6 +11,8 @@
import com.android.tools.r8.ToolHelper.ArtCommandBuilder;
import com.android.tools.r8.ToolHelper.DexVm;
import com.android.tools.r8.ToolHelper.DexVm.Version;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.shaking.ProguardConfiguration;
import com.android.tools.r8.shaking.ProguardRuleParserException;
import com.android.tools.r8.utils.AndroidApiLevel;
@@ -88,9 +90,16 @@
ART
}
+ enum DexCompilerKind {
+ DX,
+ D8
+ }
+
// Set to JAVA to run tests with java
private static final RuntimeKind RUNTIME_KIND = RuntimeKind.ART;
+ private static final DexCompilerKind DEX_COMPILER_KIND = DexCompilerKind.D8;
+
// Set to true to enable verbose logs
private static final boolean DEBUG_TESTS = false;
@@ -123,35 +132,63 @@
Consumer<InternalOptions> optionsConsumer,
Consumer<ProguardConfiguration.Builder> pgConsumer)
throws Exception {
- // Convert jar to dex with d8 with debug info
- jdwpDexD8 = compileToDex(null, JDWP_JAR);
// TODO(zerny): supply a set of compilers to run with.
- debuggeeDexD8 = compileToDex(optionsConsumer, DEBUGGEE_JAR);
+ // Compile the debuggee code first so potential compilation errors or debugging breakpoints hit
+ // here first.
+ debuggeeDexD8 = compileToDex(DEBUGGEE_JAR, optionsConsumer);
debuggeeDexR8 = compileToDexViaR8(optionsConsumer, pgConsumer, DEBUGGEE_JAR);
- debuggeeJava8DexD8 = compileToDex(options -> {
+ debuggeeJava8DexD8 = compileToDex(DEBUGGEE_JAVA8_JAR, options -> {
// Enable desugaring for preN runtimes
options.interfaceMethodDesugaring = OffOrAuto.Auto;
if (optionsConsumer != null) {
optionsConsumer.accept(options);
}
- }, DEBUGGEE_JAVA8_JAR);
- debuggeeKotlinDexD8 = compileToDex(optionsConsumer, DEBUGGEE_KOTLIN_JAR);
+ });
+ debuggeeKotlinDexD8 = compileToDex(DEBUGGEE_KOTLIN_JAR, optionsConsumer);
+ // Convert jar to dex with d8 with debug info
+ jdwpDexD8 = compileToDex(JDWP_JAR, null);
}
- static Path compileToDex(Consumer<InternalOptions> optionsConsumer, Path jarToCompile)
+ protected static Path compileToDex(Path jarToCompile, Consumer<InternalOptions> optionsConsumer)
+ throws IOException, CompilationException {
+ return compileToDex(DEX_COMPILER_KIND, jarToCompile, optionsConsumer);
+ }
+
+ static Path compileToDex(
+ DexCompilerKind compiler, Path jarToCompile, Consumer<InternalOptions> optionsConsumer)
throws IOException, CompilationException {
int minSdk = ToolHelper.getMinApiLevelForDexVm(ToolHelper.getDexVm());
assert jarToCompile.toFile().exists();
Path dexOutputDir = temp.newFolder().toPath();
- ToolHelper.runD8(
- D8Command.builder()
- .addProgramFiles(jarToCompile)
- .setOutputPath(dexOutputDir)
- .setMinApiLevel(minSdk)
- .setMode(CompilationMode.DEBUG)
- .addLibraryFiles(Paths.get(ToolHelper.getAndroidJar(minSdk)))
- .build(),
- optionsConsumer);
+ switch (compiler) {
+ case D8:
+ {
+ ToolHelper.runD8(
+ D8Command.builder()
+ .addProgramFiles(jarToCompile)
+ .setOutputPath(dexOutputDir)
+ .setMinApiLevel(minSdk)
+ .setMode(CompilationMode.DEBUG)
+ .addLibraryFiles(Paths.get(ToolHelper.getAndroidJar(minSdk)))
+ .build(),
+ optionsConsumer);
+ break;
+ }
+ case DX:
+ {
+ ProcessResult result =
+ ToolHelper.runDX(
+ new String[] {
+ "--output=" + dexOutputDir,
+ "--min-sdk-version=" + minSdk,
+ jarToCompile.toString()
+ });
+ Assert.assertEquals(result.stderr, 0, result.exitCode);
+ break;
+ }
+ default:
+ throw new Unreachable();
+ }
return dexOutputDir.resolve("classes.dex");
}
diff --git a/src/test/java/com/android/tools/r8/debug/FinallyBlockTest.java b/src/test/java/com/android/tools/r8/debug/FinallyBlockTest.java
new file mode 100644
index 0000000..83a0486
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/debug/FinallyBlockTest.java
@@ -0,0 +1,79 @@
+// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.debug;
+
+import org.junit.Test;
+
+/** Test single stepping behaviour of synchronized blocks. */
+public class FinallyBlockTest extends DebugTestBase {
+
+ public static final String CLASS = "FinallyBlock";
+ public static final String FILE = "FinallyBlock.java";
+
+ @Test
+ public void testEmptyBlock() throws Throwable {
+ final String method = "finallyBlock";
+ runDebugTest(CLASS,
+ breakpoint(CLASS, method),
+ run(),
+ checkLine(FILE, 8),
+ stepOver(),
+ checkLine(FILE, 10),
+ stepOver(),
+ checkLine(FILE, 18),
+ stepOver(),
+ checkLine(FILE, 19),
+ stepOver(),
+ checkLine(FILE, 20),
+ stepOver(),
+ checkLine(FILE, 25), // return in callFinallyBlock
+ run(),
+ checkLine(FILE, 8),
+ stepOver(),
+ checkLine(FILE, 10),
+ stepOver(),
+ checkLine(FILE, 11),
+ stepOver(),
+ checkLine(FILE, 13), // catch AE
+ stepOver(),
+ checkLine(FILE, 14),
+ stepOver(),
+ checkLine(FILE, 18),
+ stepOver(),
+ checkLine(FILE, 19),
+ stepOver(),
+ checkLine(FILE, 20),
+ stepOver(),
+ checkLine(FILE, 25), // return in callFinallyBlock
+ run(),
+ checkLine(FILE, 8),
+ stepOver(),
+ checkLine(FILE, 10),
+ stepOver(),
+ checkLine(FILE, 11),
+ stepOver(),
+ checkLine(FILE, 15), // catch RE
+ stepOver(),
+ checkLine(FILE, 16),
+ stepOver(),
+ checkLine(FILE, 18),
+ stepOver(),
+ checkLine(FILE, 19),
+ stepOver(),
+ checkLine(FILE, 20),
+ stepOver(),
+ checkLine(FILE, 25), // return in callFinallyBlock
+ run(),
+ checkLine(FILE, 8),
+ stepOver(),
+ checkLine(FILE, 10),
+ stepOver(),
+ checkLine(FILE, 11), // throw without catch
+ stepOver(),
+ checkLine(FILE, 18), // finally
+ stepOver(),
+ checkLine(FILE, 26), // catch in callFinallyBlock
+ run());
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/debug/JasminDebugTest.java b/src/test/java/com/android/tools/r8/debug/JasminDebugTest.java
index 1b29472..2b0084d 100644
--- a/src/test/java/com/android/tools/r8/debug/JasminDebugTest.java
+++ b/src/test/java/com/android/tools/r8/debug/JasminDebugTest.java
@@ -30,23 +30,20 @@
final String className = "UselessCheckCast";
final String sourcefile = className + ".j";
final String methodName = "test";
- runDebugTest(getExtraPaths(getBuilderForUselessCheckcast(className, methodName)),
+ List<Path> paths = getExtraPaths(getBuilderForUselessCheckcast(className, methodName));
+ runDebugTest(paths,
className,
breakpoint(className, methodName),
run(),
- checkLine(sourcefile, 8),
+ checkLine(sourcefile, 1),
stepOver(),
- checkLine(sourcefile, 9),
- stepOver(),
- checkLine(sourcefile, 10),
- stepOver(),
- checkLine(sourcefile, 12),
+ checkLine(sourcefile, 2),
checkLocal("local"),
stepOver(),
- checkLine(sourcefile, 14),
+ checkLine(sourcefile, 3),
checkNoLocal("local"),
stepOver(),
- checkLine(sourcefile, 15),
+ checkLine(sourcefile, 4),
run());
}
@@ -58,13 +55,17 @@
".limit stack 1",
".limit locals 3",
".var 1 is local Ljava/lang/Object; from Label1 to Label2",
+ ".line 1",
" aload 0",
" dup",
" astore 1",
" Label1:",
+ ".line 2",
" checkcast " + testClassName,
" Label2:",
+ ".line 3",
" checkcast " + testClassName,
+ ".line 4",
"return");
clazz.addMainMethod(
@@ -84,14 +85,14 @@
for (ClassBuilder clazz : classes) {
ClassFile file = new ClassFile();
- file.readJasmin(new StringReader(clazz.toString()), clazz.name, true);
+ file.readJasmin(new StringReader(clazz.toString()), clazz.name, false);
Path path = out.toPath().resolve(clazz.name + ".class");
Files.createDirectories(path.getParent());
file.write(new FileOutputStream(path.toFile()));
if (isRunningJava()) {
extraPaths.add(path);
} else {
- extraPaths.add(compileToDex(null, path));
+ extraPaths.add(compileToDex(path, null));
}
}
diff --git a/src/test/java/com/android/tools/r8/debug/SharedCodeTest.java b/src/test/java/com/android/tools/r8/debug/SharedCodeTest.java
new file mode 100644
index 0000000..d4149e8
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/debug/SharedCodeTest.java
@@ -0,0 +1,36 @@
+// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.debug;
+
+import org.junit.Test;
+
+public class SharedCodeTest extends DebugTestBase {
+
+ public static final String CLASS = "SharedCode";
+ public static final String FILE = "SharedCode.java";
+
+ @Test
+ public void testSharedIf() throws Throwable {
+ final String methodName = "sharedIf";
+ runDebugTest(CLASS,
+ breakpoint(CLASS, methodName),
+ run(),
+ checkMethod(CLASS, methodName),
+ checkLine(FILE, 8),
+ stepOver(),
+ checkLine(FILE, 9),
+ stepOver(),
+ checkLine(FILE, 13),
+ run(),
+ checkMethod(CLASS, methodName),
+ checkLine(FILE, 8),
+ stepOver(),
+ checkLine(FILE, 11),
+ stepOver(),
+ checkLine(FILE, 13),
+ run());
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/debug/SynchronizedBlockTest.java b/src/test/java/com/android/tools/r8/debug/SynchronizedBlockTest.java
index 42f0174..fa6aa47 100644
--- a/src/test/java/com/android/tools/r8/debug/SynchronizedBlockTest.java
+++ b/src/test/java/com/android/tools/r8/debug/SynchronizedBlockTest.java
@@ -3,7 +3,6 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.debug;
-import org.junit.Ignore;
import org.junit.Test;
/**
@@ -71,7 +70,6 @@
}
@Test
- @Ignore("b/65567013")
public void testThrowingBlock() throws Throwable {
final String method = "throwingBlock";
runDebugTest(CLASS,
@@ -79,22 +77,27 @@
run(),
checkLine(FILE, 25),
checkLocal("obj"),
+ checkNoLocal("x"),
stepOver(),
checkLine(FILE, 26),
checkLocal("obj"),
checkLocal("x"),
+ checkNoLocal("y"),
stepOver(),
checkLine(FILE, 27),
checkLocal("obj"),
checkLocal("x"),
+ checkNoLocal("y"),
stepOver(),
checkLine(FILE, 28), // synchronized block end
checkLocal("obj"),
checkLocal("x"),
+ checkNoLocal("y"),
stepOver(),
checkLine(FILE, 31), // catch handler
checkLocal("obj"),
checkNoLocal("x"),
+ checkNoLocal("y"),
stepOver(),
run());
}
@@ -153,7 +156,6 @@
}
@Test
- @Ignore("b/65567013")
public void testNestedThrowingBlock() throws Throwable {
final String method = "nestedThrowingBlock";
runDebugTest(CLASS,
diff --git a/src/test/java/com/android/tools/r8/ir/SplitBlockTest.java b/src/test/java/com/android/tools/r8/ir/SplitBlockTest.java
index c911d37..7351206 100644
--- a/src/test/java/com/android/tools/r8/ir/SplitBlockTest.java
+++ b/src/test/java/com/android/tools/r8/ir/SplitBlockTest.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.MoveType;
import com.android.tools.r8.ir.code.NumericType;
+import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.code.ValueNumberGenerator;
import com.android.tools.r8.smali.SmaliTestBase;
@@ -373,6 +374,8 @@
iterator.previous();
iterator.add(constInstruction);
iterator.add(addInstruction);
+ addInstruction.setPosition(Position.none());
+ constInstruction.setPosition(Position.none());
}
// Run code and check result (code in the test object is updated).
String result = test.run();
diff --git a/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java b/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
index 6eebf6b..6969c2d 100644
--- a/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
+++ b/src/test/java/com/android/tools/r8/ir/regalloc/IdenticalAfterRegisterAllocationTest.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.ir.code.ConstType;
import com.android.tools.r8.ir.code.MoveType;
import com.android.tools.r8.ir.code.NumericType;
+import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.Value;
import org.junit.Test;
@@ -54,7 +55,9 @@
ConstNumber const2 = new ConstNumber(ConstType.INT, value2, 2);
Value value3 = new Value(2, MoveType.SINGLE, null);
Add add0 = new Add(NumericType.INT, value3, value0, value1);
+ add0.setPosition(Position.none());
Add add1 = new Add(NumericType.INT, value3, value0, value2);
+ add1.setPosition(Position.none());
value0.computeNeedsRegister();
assertTrue(value0.needsRegister());
value1.computeNeedsRegister();
diff --git a/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java b/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java
index 8c93a8a..2511d79 100644
--- a/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java
+++ b/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java
@@ -196,21 +196,23 @@
" default: CaseDefault",
"Case1:",
+ ".line 3",
" ldc 42",
" istore 1",
"LabelYStart:",
- ".line 3",
+ ".line 4",
" invokestatic Test/ensureLine()V",
" goto AfterSwitch",
"CaseDefault:",
+ ".line 5",
" ldc -42",
" istore 1",
- ".line 4",
+ ".line 6",
" invokestatic Test/ensureLine()V",
"AfterSwitch:",
- ".line 5",
+ ".line 7",
" iload 1",
" ireturn",
"LabelYEnd:",
@@ -246,9 +248,9 @@
info.checkStartLine(1);
info.checkLineHasExactLocals(1, "param", "int");
info.checkLineHasExactLocals(2, "param", "int", "x", "int");
- info.checkLineHasExactLocals(3, "param", "int", "y", "int");
info.checkLineHasExactLocals(4, "param", "int", "y", "int");
- info.checkLineHasExactLocals(5, "param", "int", "y", "int");
+ info.checkLineHasExactLocals(6, "param", "int", "y", "int");
+ info.checkLineHasExactLocals(7, "param", "int", "y", "int");
}
@Test
diff --git a/src/test/java/com/android/tools/r8/jasmin/JasminTestBase.java b/src/test/java/com/android/tools/r8/jasmin/JasminTestBase.java
index ad625cc..c609701 100644
--- a/src/test/java/com/android/tools/r8/jasmin/JasminTestBase.java
+++ b/src/test/java/com/android/tools/r8/jasmin/JasminTestBase.java
@@ -52,7 +52,7 @@
File out = temp.newFolder("classes");
for (ClassBuilder clazz : builder.getClasses()) {
ClassFile file = new ClassFile();
- file.readJasmin(new StringReader(clazz.toString()), clazz.name, true);
+ file.readJasmin(new StringReader(clazz.toString()), clazz.name, false);
Path path = out.toPath().resolve(clazz.name + ".class");
Files.createDirectories(path.getParent());
file.write(new FileOutputStream(path.toFile()));
@@ -110,7 +110,7 @@
private ProcessResult runDx(JasminBuilder builder, File classes, Path dex) throws Exception {
for (ClassBuilder clazz : builder.getClasses()) {
ClassFile file = new ClassFile();
- file.readJasmin(new StringReader(clazz.toString()), clazz.name, true);
+ file.readJasmin(new StringReader(clazz.toString()), clazz.name, false);
file.write(new FileOutputStream(classes.toPath().resolve(clazz.name + ".class").toFile()));
}
List<String> args = new ArrayList<>();
diff --git a/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java b/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
index 15cd3b6..c289c8e 100644
--- a/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
+++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexListTests.java
@@ -37,8 +37,8 @@
import com.android.tools.r8.graph.DexTypeList;
import com.android.tools.r8.graph.DirectMappedDexApplication;
import com.android.tools.r8.ir.code.CatchHandlers;
-import com.android.tools.r8.ir.code.DebugPosition;
import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.conversion.IRBuilder;
import com.android.tools.r8.ir.conversion.SourceCode;
import com.android.tools.r8.ir.regalloc.LinearScanRegisterAllocator;
@@ -593,7 +593,7 @@
code);
IRCode ir = code.buildIR(method, options);
RegisterAllocator allocator = new LinearScanRegisterAllocator(ir, options);
- method.setCode(ir, allocator, factory);
+ method.setCode(ir, allocator, factory, options);
directMethods[i] = method;
}
builder.addProgramClass(
@@ -705,11 +705,16 @@
}
@Override
- public DebugPosition getDebugPositionAtOffset(int offset) {
+ public Position getDebugPositionAtOffset(int offset) {
throw new Unreachable();
}
@Override
+ public Position getCurrentPosition() {
+ return Position.none();
+ }
+
+ @Override
public boolean verifyRegister(int register) {
throw new Unreachable();
}
