Fix handling of debug information for common return block
Change-Id: Idf3f930c8dbd16347edbf1ce6c5bd845129c4d27
diff --git a/src/main/java/com/android/tools/r8/ir/code/Instruction.java b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
index 52fff08..1cc21a3 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Instruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
@@ -124,6 +124,12 @@
debugValues.clear();
}
+ public void moveDebugValue(Value value, Instruction target) {
+ assert debugValues.contains(value);
+ value.replaceDebugUser(this, target);
+ debugValues.remove(value);
+ }
+
/**
* Returns the basic block containing this instruction.
*/
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index 5eeda96..32159c1 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -79,6 +79,7 @@
import com.android.tools.r8.ir.code.ValueNumberGenerator;
import com.android.tools.r8.ir.code.Xor;
import com.android.tools.r8.utils.InternalOptions;
+import com.google.common.collect.Sets;
import it.unimi.dsi.fastutil.ints.Int2ReferenceAVLTreeMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceSortedMap;
@@ -1789,6 +1790,26 @@
assert returnType == MoveType.fromDexType(method.method.proto.returnType);
}
closeCurrentBlock();
+
+ // Collect the debug values which are live on all returns.
+ Set<Value> debugValuesForReturn = Sets.newIdentityHashSet();
+ for (Value value : exitBlocks.get(0).exit().getDebugValues()) {
+ boolean include = true;
+ for (int i = 1; i < exitBlocks.size() && include; i++) {
+ include = exitBlocks.get(i).exit().getDebugValues().contains(value);
+ }
+ if (include) {
+ debugValuesForReturn.add(value);
+ }
+ }
+
+ // Move all these debug values to the new return.
+ for (Value value : debugValuesForReturn) {
+ for (BasicBlock block : exitBlocks) {
+ block.exit().moveDebugValue(value, normalExitBlock.exit());
+ }
+ }
+
// Replace each return instruction with a goto to the new exit block.
List<Value> operands = new ArrayList<>();
for (BasicBlock block : exitBlocks) {
diff --git a/src/test/debugTestResources/Locals.java b/src/test/debugTestResources/Locals.java
index d42dd61..890cb85 100644
--- a/src/test/debugTestResources/Locals.java
+++ b/src/test/debugTestResources/Locals.java
@@ -244,6 +244,24 @@
return sum + x + y;
}
+ public static int argumentLiveAtReturn(int x) {
+ switch (x) {
+ case 0:
+ return 0;
+ case 1:
+ return 0;
+ case 2:
+ return 0;
+ case 100:
+ return 1;
+ case 101:
+ return 1;
+ case 102:
+ return 1;
+ }
+ return -1;
+ }
+
public static void main(String[] args) {
noLocals();
unusedLocals();
@@ -259,5 +277,6 @@
stepNonEmptyForLoopBody(3);
tempInCase(42);
localSwap(1, 2);
+ argumentLiveAtReturn(-1);
}
}
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 6340728..b2b77bc 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
@@ -973,7 +973,7 @@
.filter(m -> methodSignature.equals(m.methodSignature)).collect(
Collectors.toList());
}
- Assert.assertFalse("No method found", methodInfos.isEmpty());
+ Assert.assertFalse("No method named " + methodName + " found", methodInfos.isEmpty());
// There must be only one matching method
Assert.assertEquals("More than 1 method found: please specify a signature", 1,
methodInfos.size());
diff --git a/src/test/java/com/android/tools/r8/debug/LocalsTest.java b/src/test/java/com/android/tools/r8/debug/LocalsTest.java
index 02fb694..6f73f02c 100644
--- a/src/test/java/com/android/tools/r8/debug/LocalsTest.java
+++ b/src/test/java/com/android/tools/r8/debug/LocalsTest.java
@@ -470,4 +470,18 @@
checkNoLocal("t"),
run());
}
+
+ @Test
+ public void argumentLiveAtReturn() throws Throwable {
+ runDebugTest(
+ "Locals",
+ breakpoint("Locals", "argumentLiveAtReturn"),
+ run(),
+ checkLine(SOURCE_FILE, 248),
+ stepOver(),
+ checkLine(SOURCE_FILE, 262),
+ checkLocal("x", Value.createInt(-1)),
+ checkNoLocal("t"),
+ run());
+ }
}
diff --git a/src/test/java/com/android/tools/r8/debuginfo/DebugInfoInspector.java b/src/test/java/com/android/tools/r8/debuginfo/DebugInfoInspector.java
index ad6f925..31872ed 100644
--- a/src/test/java/com/android/tools/r8/debuginfo/DebugInfoInspector.java
+++ b/src/test/java/com/android/tools/r8/debuginfo/DebugInfoInspector.java
@@ -18,6 +18,7 @@
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.StringUtils;
import java.io.IOException;
import java.util.HashSet;
import java.util.List;
@@ -102,7 +103,8 @@
DexDebugEntry previousEntry = null;
for (DexDebugEntry entry : entries) {
if (previousEntry != null) {
- assertTrue("More than one entry defined for PC " + entry.address,
+ assertTrue(
+ "More than one entry defined for PC " + StringUtils.hexString(entry.address, 2),
entry.address > previousEntry.address);
}
previousEntry = entry;
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 2782c91..5ffd815 100644
--- a/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java
+++ b/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java
@@ -336,4 +336,105 @@
info.checkLineHasExactLocals(8, "param", "int", "x", "int");
info.checkLineHasExactLocals(9, "param", "int");
}
+
+ @Test
+ public void argumentLiveAtReturn() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+ JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
+
+ /*
+ This is the original Java source code.
+
+ public static int argumentLiveAtReturn(int x) { // Line 1
+ switch (x) {
+ case 0:
+ return 0;
+ case 1:
+ return 0;
+ case 2:
+ return 0;
+ case 100:
+ return 1;
+ case 101:
+ return 1;
+ case 102:
+ return 1;
+ }
+ return -1;
+ }
+ */
+ MethodSignature foo = clazz.addStaticMethod("argumentLiveAtReturn", ImmutableList.of("I"), "I",
+ ".limit stack 2",
+ ".limit locals 1",
+ ".var 0 is x I from L0 to L8",
+ "L0:",
+ ".line 2",
+ " iload 0",
+ "lookupswitch",
+ " 0: L1",
+ " 1: L2",
+ " 2: L3",
+ " 100: L4",
+ " 101: L5",
+ " 102: L6",
+ " default: L7",
+ "L1:",
+ ".line 4",
+ " iconst_0",
+ " ireturn",
+ "L2:",
+ ".line 6",
+ " iconst_0",
+ " ireturn",
+ "L3:",
+ ".line 8",
+ " iconst_0",
+ " ireturn",
+ "L4:",
+ ".line 10",
+ " iconst_1",
+ " ireturn",
+ "L5:",
+ ".line 12",
+ " iconst_1",
+ " ireturn",
+ "L6:",
+ ".line 14",
+ " iconst_1",
+ " ireturn",
+ "L7:",
+ ".line 16",
+ " iconst_m1",
+ " ireturn",
+ "L8:"
+ );
+
+ clazz.addMainMethod(
+ ".limit stack 2",
+ ".limit locals 1",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " ldc -1",
+ " invokestatic Test/argumentLiveAtReturn(I)I",
+ " invokevirtual java/io/PrintStream/print(I)V",
+ " return");
+
+ String expected = "-1";
+ String javaResult = runOnJava(builder, clazz.name);
+ assertEquals(expected, javaResult);
+
+ AndroidApp jasminApp = builder.build();
+ AndroidApp d8App = ToolHelper.runD8(jasminApp);
+ String artResult = runOnArt(d8App, clazz.name);
+ assertEquals(expected, artResult);
+ DebugInfoInspector info = new DebugInfoInspector(d8App, clazz.name, foo);
+ info.checkStartLine(2);
+ info.checkLineHasExactLocals(2, "x", "int");
+ info.checkLineHasExactLocals(4, "x", "int");
+ info.checkLineHasExactLocals(6, "x", "int");
+ info.checkLineHasExactLocals(8, "x", "int");
+ info.checkLineHasExactLocals(10, "x", "int");
+ info.checkLineHasExactLocals(12, "x", "int");
+ info.checkLineHasExactLocals(14, "x", "int");
+ info.checkLineHasExactLocals(16, "x", "int");
+ }
}
