Merge "Do not replace assumevalue target twice."
diff --git a/src/main/java/com/android/tools/r8/graph/DexCode.java b/src/main/java/com/android/tools/r8/graph/DexCode.java
index dc3ae37..a2edd92 100644
--- a/src/main/java/com/android/tools/r8/graph/DexCode.java
+++ b/src/main/java/com/android/tools/r8/graph/DexCode.java
@@ -175,6 +175,9 @@
public String toString(DexEncodedMethod method, ClassNameMapper naming) {
StringBuilder builder = new StringBuilder();
+ if (method != null) {
+ builder.append(method.toSourceString()).append("\n");
+ }
builder.append("registers: ").append(registerSize);
builder.append(", inputs: ").append(incomingRegisterSize);
builder.append(", outputs: ").append(outgoingRegisterSize).append("\n");
diff --git a/src/main/java/com/android/tools/r8/graph/DexDebugEntryBuilder.java b/src/main/java/com/android/tools/r8/graph/DexDebugEntryBuilder.java
index 39ec7d0..7bfa534 100644
--- a/src/main/java/com/android/tools/r8/graph/DexDebugEntryBuilder.java
+++ b/src/main/java/com/android/tools/r8/graph/DexDebugEntryBuilder.java
@@ -5,6 +5,8 @@
import com.android.tools.r8.ir.code.MoveType;
import com.google.common.collect.ImmutableMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceArrayMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -44,6 +46,7 @@
private boolean prologueEnd = false;
private boolean epilogueBegin = false;
private final Map<Integer, LocalEntry> locals = new HashMap<>();
+ private final Int2ReferenceMap<DebugLocalInfo> arguments = new Int2ReferenceArrayMap<>();
// Delayed construction of an entry. Is finalized once locals information has been collected.
private DexDebugEntry pending = null;
@@ -65,7 +68,7 @@
if (!method.accessFlags.isStatic()) {
DexString name = factory.thisName;
DexType type = method.method.getHolder();
- startLocal(argumentRegister, name, type, null);
+ startArgument(argumentRegister, name, type);
argumentRegister += MoveType.fromDexType(type).requiredRegisters();
}
DexType[] types = method.method.proto.parameters.values;
@@ -73,9 +76,9 @@
for (int i = 0; i < types.length; i++) {
// If null, the parameter has a parameterized type and the local is introduced in the stream.
if (names[i] != null) {
- startLocal(argumentRegister, names[i], types[i], null);
- argumentRegister += MoveType.fromDexType(types[i]).requiredRegisters();
+ startArgument(argumentRegister, names[i], types[i]);
}
+ argumentRegister += MoveType.fromDexType(types[i]).requiredRegisters();
}
currentLine = info.startLine;
for (DexDebugEvent event : info.events) {
@@ -83,6 +86,10 @@
}
}
+ public Int2ReferenceMap<DebugLocalInfo> getArguments() {
+ return arguments;
+ }
+
public void setFile(DexString file) {
currentFile = file;
}
@@ -104,6 +111,12 @@
epilogueBegin = true;
}
+ public void startArgument(int register, DexString name, DexType type) {
+ DebugLocalInfo argument = canonicalize(name, type, null);
+ arguments.put(register, argument);
+ getEntry(register).set(argument);
+ }
+
public void startLocal(int register, DexString name, DexType type, DexString signature) {
getEntry(register).set(canonicalize(name, type, signature));
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java
index dd1df27..968e560 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlockInstructionIterator.java
@@ -349,6 +349,8 @@
List<Value> arguments = inlinee.collectArguments();
assert invoke.inValues().size() == arguments.size();
for (int i = 0; i < invoke.inValues().size(); i++) {
+ // TODO(zerny): Support inlining in --debug mode.
+ assert arguments.get(i).getDebugInfo() == null;
if ((i == 0) && (downcast != null)) {
Value invokeValue = invoke.inValues().get(0);
Value receiverValue = arguments.get(0);
diff --git a/src/main/java/com/android/tools/r8/ir/code/Phi.java b/src/main/java/com/android/tools/r8/ir/code/Phi.java
index 1abdb1d..7e05dc9 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Phi.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Phi.java
@@ -231,8 +231,11 @@
StringBuilder builder = new StringBuilder();
builder.append("v");
builder.append(number);
+ if (getLocalInfo() != null) {
+ builder.append("(").append(getLocalInfo()).append(")");
+ }
builder.append(" <- phi");
- StringUtils.append(builder, ListUtils.map(operands, (Value operand) -> "v" + operand.number));
+ StringUtils.append(builder, ListUtils.map(operands, Value::toString));
return builder.toString();
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index b1cdcbc..ddc83f8 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -509,7 +509,7 @@
Instruction current = iterator.next();
if (current.isInvokeMethod()) {
InvokeMethod invoke = current.asInvokeMethod();
- if (invoke.outValue() != null) {
+ if (invoke.outValue() != null && invoke.outValue().getLocalInfo() == null) {
DexEncodedMethod target = invoke.computeSingleTarget(appInfo.withSubtyping());
// We have a set of library classes with optimization information - consider those
// as well.
diff --git a/src/test/debugTestResources/Locals.java b/src/test/debugTestResources/Locals.java
index 3ad6596..3ab5d7b 100644
--- a/src/test/debugTestResources/Locals.java
+++ b/src/test/debugTestResources/Locals.java
@@ -188,6 +188,29 @@
}
}
+ public static int stepEmptyForLoopBody1(int n) {
+ int i;
+ for (i = 0; i < n; i++) ;
+ return i;
+ }
+
+ public static int stepEmptyForLoopBody2(int n) {
+ int i;
+ for (i = 0; i < n; i++) {
+ // has a line but still empty...
+ }
+ return i;
+ }
+
+ public static int stepNonEmptyForLoopBody(int n) {
+ int i;
+ for (i = 0; i < n; i++)
+ nop();
+ return i;
+ }
+
+ public static void nop() {}
+
public static void main(String[] args) {
noLocals();
unusedLocals();
@@ -198,6 +221,9 @@
reverseRange(1,2,3,4,5,6,7);
new Locals().lotsOfArrayLength();
new Locals().foo(21);
+ stepEmptyForLoopBody1(3);
+ stepEmptyForLoopBody2(3);
+ stepNonEmptyForLoopBody(3);
}
}
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index 1683557..6d6412b 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -640,18 +640,36 @@
Consumer<ArtCommandBuilder> extras,
DexVm version)
throws IOException {
- // Run art on original.
- for (String file : files1) {
- assertTrue("file1 " + file + " must exists", Files.exists(Paths.get(file)));
+ return checkArtOutputIdentical(
+ version,
+ mainClass,
+ extras,
+ ImmutableList.of(ListUtils.map(files1, Paths::get), ListUtils.map(files2, Paths::get)));
+ }
+
+ public static String checkArtOutputIdentical(
+ DexVm version,
+ String mainClass,
+ Consumer<ArtCommandBuilder> extras,
+ Collection<Collection<Path>> programs)
+ throws IOException {
+ for (Collection<Path> program : programs) {
+ for (Path path : program) {
+ assertTrue("File " + path + " must exist", Files.exists(path));
+ }
}
- String output1 = ToolHelper.runArtNoVerificationErrors(files1, mainClass, extras, version);
- // Run art on R8 processed version.
- for (String file : files2) {
- assertTrue("file2 " + file + " must exists", Files.exists(Paths.get(file)));
+ String output = null;
+ for (Collection<Path> program : programs) {
+ String result =
+ ToolHelper.runArtNoVerificationErrors(
+ ListUtils.map(program, Path::toString), mainClass, extras, version);
+ if (output != null) {
+ assertEquals(output, result);
+ } else {
+ output = result;
+ }
}
- String output2 = ToolHelper.runArtNoVerificationErrors(files2, mainClass, extras, version);
- assertEquals(output1, output2);
- return output1;
+ return output;
}
public static void runDex2Oat(Path file, Path outFile) throws IOException {
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 6a5f738..57a1ce8 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
@@ -623,15 +623,24 @@
));
}
+ private void failNoLocal(String localName) {
+ Assert.fail(
+ "line " + getLineNumber() + ": Expected local '" + localName + "' not present");
+ }
+
public void checkLocal(String localName) {
Optional<Variable> localVar = JUnit3Wrapper
.getVariableAt(mirror, getLocation(), localName);
- Assert.assertTrue("No local '" + localName + "'", localVar.isPresent());
+ if (!localVar.isPresent()) {
+ failNoLocal(localName);
+ }
}
public void checkLocal(String localName, Value expectedValue) {
Optional<Variable> localVar = getVariableAt(mirror, getLocation(), localName);
- Assert.assertTrue("No local '" + localName + "'", localVar.isPresent());
+ if (!localVar.isPresent()) {
+ failNoLocal(localName);
+ }
// Get value
CommandPacket commandPacket = new CommandPacket(
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 1fd4bc9..16d1a60 100644
--- a/src/test/java/com/android/tools/r8/debug/LocalsTest.java
+++ b/src/test/java/com/android/tools/r8/debug/LocalsTest.java
@@ -348,4 +348,79 @@
run());
}
+ @Test
+ public void testStepEmptyForLoopBody1() throws Throwable {
+ runDebugTest(
+ "Locals",
+ breakpoint("Locals", "stepEmptyForLoopBody1"),
+ run(),
+ checkLocal("n", Value.createInt(3)),
+ checkNoLocal("i"),
+ stepOver(),
+ checkLocal("n", Value.createInt(3)),
+ checkLocal("i", Value.createInt(3)),
+ run());
+ }
+
+ @Test
+ public void testStepEmptyForLoopBody2() throws Throwable {
+ runDebugTest(
+ "Locals",
+ breakpoint("Locals", "stepEmptyForLoopBody2"),
+ run(),
+ checkLocal("n", Value.createInt(3)),
+ checkNoLocal("i"),
+ stepOver(),
+ checkLocal("n", Value.createInt(3)),
+ checkLocal("i", Value.createInt(3)),
+ run());
+ }
+
+ @Test
+ public void testStepNonEmptyForLoopBody() throws Throwable {
+ final int LOOP_HEADER_LINE = 207;
+ final int LOOP_BODY_LINE = 208;
+ final int RETURN_LINE = 209;
+ final Value N = Value.createInt(3);
+ final Value I0 = Value.createInt(0);
+ final Value I1 = Value.createInt(1);
+ final Value I2 = Value.createInt(2);
+ final Value I3 = Value.createInt(3);
+ runDebugTest(
+ "Locals",
+ breakpoint("Locals", "stepNonEmptyForLoopBody"),
+ run(),
+ checkLine(SOURCE_FILE, LOOP_HEADER_LINE),
+ checkLocal("n", N),
+ checkNoLocal("i"),
+ stepOver(),
+ checkLine(SOURCE_FILE, LOOP_BODY_LINE),
+ checkLocal("n", N),
+ checkLocal("i", I0),
+ stepOver(),
+ checkLine(SOURCE_FILE, LOOP_HEADER_LINE),
+ checkLocal("n", N),
+ checkLocal("i", I0),
+ stepOver(),
+ checkLine(SOURCE_FILE, LOOP_BODY_LINE),
+ checkLocal("n", N),
+ checkLocal("i", I1),
+ stepOver(),
+ checkLine(SOURCE_FILE, LOOP_HEADER_LINE),
+ checkLocal("n", N),
+ checkLocal("i", I1),
+ stepOver(),
+ checkLine(SOURCE_FILE, LOOP_BODY_LINE),
+ checkLocal("n", N),
+ checkLocal("i", I2),
+ stepOver(),
+ checkLine(SOURCE_FILE, LOOP_HEADER_LINE),
+ checkLocal("n", N),
+ checkLocal("i", I2),
+ stepOver(),
+ checkLine(SOURCE_FILE, RETURN_LINE),
+ checkLocal("n", N),
+ checkLocal("i", I3),
+ run());
+ }
}
diff --git a/src/test/java/com/android/tools/r8/debuginfo/ConditionalLocalTest.java b/src/test/java/com/android/tools/r8/debuginfo/ConditionalLocalTest.java
new file mode 100644
index 0000000..5a83545
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/debuginfo/ConditionalLocalTest.java
@@ -0,0 +1,21 @@
+// 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.debuginfo;
+
+public class ConditionalLocalTest {
+
+ public void foo(int x) {
+ if (x % 2 != 0) {
+ Integer obj = new Integer(x + x);
+ long l = obj.longValue();
+ x = (int) l;
+ System.out.print(obj);
+ }
+ return;
+ }
+
+ public static void main(String[] args) {
+ new ConditionalLocalTest().foo(21);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/debuginfo/ConditionalLocalTestRunner.java b/src/test/java/com/android/tools/r8/debuginfo/ConditionalLocalTestRunner.java
new file mode 100644
index 0000000..c793f3e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/debuginfo/ConditionalLocalTestRunner.java
@@ -0,0 +1,41 @@
+// 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.debuginfo;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.utils.AndroidApp;
+import org.junit.Test;
+
+public class ConditionalLocalTestRunner extends DebugInfoTestBase {
+
+ @Test
+ public void testConditionalLocal() throws Exception {
+ Class clazz = ConditionalLocalTest.class;
+
+ AndroidApp d8App = compileWithD8(clazz);
+ AndroidApp dxApp = getDxCompiledSources();
+
+ String expected = "42";
+ assertEquals(expected, runOnJava(clazz));
+ assertEquals(expected, runOnArt(d8App, clazz.getCanonicalName()));
+ assertEquals(expected, runOnArt(dxApp, clazz.getCanonicalName()));
+
+ checkConditonalLocal(inspectMethod(d8App, clazz, "void", "foo", "int"));
+ checkConditonalLocal(inspectMethod(dxApp, clazz, "void", "foo", "int"));
+ }
+
+ private void checkConditonalLocal(DebugInfoInspector info) {
+ String self = ConditionalLocalTest.class.getCanonicalName();
+ String Integer = "java.lang.Integer";
+ info.checkStartLine(6);
+ info.checkLineHasExactLocals(6, "this", self, "x", "int");
+ info.checkLineHasExactLocals(7, "this", self, "x", "int");
+ info.checkLineHasExactLocals(8, "this", self, "x", "int", "obj", Integer);
+ info.checkLineHasExactLocals(9, "this", self, "x", "int", "obj", Integer, "l", "long");
+ info.checkLineHasExactLocals(10, "this", self, "x", "int", "obj", Integer, "l", "long");
+ info.checkNoLine(11);
+ info.checkLineHasExactLocals(12, "this", self, "x", "int");
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/debuginfo/LocalsAtThrowTestRunner.java b/src/test/java/com/android/tools/r8/debuginfo/LocalsAtThrowTestRunner.java
index 8df6f79..8a5b77d 100644
--- a/src/test/java/com/android/tools/r8/debuginfo/LocalsAtThrowTestRunner.java
+++ b/src/test/java/com/android/tools/r8/debuginfo/LocalsAtThrowTestRunner.java
@@ -22,11 +22,11 @@
assertEquals(expected, runOnArt(d8App, clazz.getCanonicalName()));
assertEquals(expected, runOnArt(dxApp, clazz.getCanonicalName()));
- checkBackBranchToSelf(inspectMethod(d8App, clazz, "int", "localsAtThrow", "int"));
- checkBackBranchToSelf(inspectMethod(dxApp, clazz, "int", "localsAtThrow", "int"));
+ checkLocalsAtThrow(inspectMethod(d8App, clazz, "int", "localsAtThrow", "int"));
+ checkLocalsAtThrow(inspectMethod(dxApp, clazz, "int", "localsAtThrow", "int"));
}
- private void checkBackBranchToSelf(DebugInfoInspector info) {
+ private void checkLocalsAtThrow(DebugInfoInspector info) {
info.checkStartLine(9);
info.checkLineHasExactLocals(9, "x", "int");
info.checkLineHasExactLocals(10, "x", "int", "a", "int");
diff --git a/src/test/java/com/android/tools/r8/jasmin/JumboStringTests.java b/src/test/java/com/android/tools/r8/jasmin/JumboStringTests.java
new file mode 100644
index 0000000..d0ade31
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/jasmin/JumboStringTests.java
@@ -0,0 +1,106 @@
+// 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.jasmin;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.debuginfo.DebugInfoInspector;
+import com.android.tools.r8.dex.Constants;
+import com.android.tools.r8.naming.MemberNaming.MethodSignature;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.DexInspector;
+import com.google.common.collect.ImmutableList;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map.Entry;
+import org.junit.Ignore;
+import org.junit.Test;
+
+public class JumboStringTests extends JasminTestBase {
+
+ // String constants are split into several class files to ensure both the constant-pool and
+ // instruction count are below the class-file limits.
+ private static int CLASSES_COUNT = 10;
+ private static int MIN_STRING_COUNT = Constants.FIRST_JUMBO_INDEX + 1;
+ private static int EXTRA_STRINGS_PER_CLASSES_COUNT = MIN_STRING_COUNT % CLASSES_COUNT;
+ private static int STRINGS_PER_CLASSES_COUNT =
+ EXTRA_STRINGS_PER_CLASSES_COUNT + MIN_STRING_COUNT / CLASSES_COUNT;
+
+ @Test
+ @Ignore("b/35701208")
+ public void test() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+ LinkedHashMap<String, MethodSignature> classes = new LinkedHashMap<>(CLASSES_COUNT);
+ for (int i = 0; i < CLASSES_COUNT; i++) {
+ JasminBuilder.ClassBuilder clazz = builder.addClass("Test" + i);
+ List<String> lines = new ArrayList<>(STRINGS_PER_CLASSES_COUNT + 100);
+ lines.addAll(
+ ImmutableList.of(
+ ".limit locals 3",
+ ".limit stack 4",
+ ".var 0 is this LTest; from L0 to L2",
+ ".var 1 is i I from L0 to L2",
+ ".var 2 is strings [Ljava/lang/String; from L1 to L2",
+ "L0:",
+ ".line 1",
+ " ldc " + STRINGS_PER_CLASSES_COUNT,
+ " anewarray java/lang/String",
+ " astore 2",
+ "L1:",
+ ".line 2"));
+ for (int j = 0; j < STRINGS_PER_CLASSES_COUNT; j++) {
+ lines.add(" aload 2");
+ lines.add(" ldc " + j);
+ lines.add(" ldc \"string" + i + "_" + j + "\"");
+ lines.add(" aastore");
+ }
+ lines.addAll(
+ ImmutableList.of(
+ "L2:",
+ " .line 3",
+ " aload 2",
+ " iload 1",
+ " aaload",
+ " checkcast java/lang/String",
+ " areturn"));
+ MethodSignature foo =
+ clazz.addVirtualMethod(
+ "foo", ImmutableList.of("I"), "Ljava/lang/String;", lines.toArray(new String[0]));
+ classes.put(clazz.name, foo);
+ }
+
+ JasminBuilder.ClassBuilder clazz = builder.addClass("Test");
+ clazz.addMainMethod(
+ ".limit stack 3",
+ ".limit locals 1",
+ " new Test0",
+ " dup",
+ " invokespecial Test0/<init>()V",
+ " ldc 42",
+ " invokevirtual Test0/foo(I)Ljava/lang/String;",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " swap",
+ " invokevirtual java/io/PrintStream/print(Ljava/lang/String;)V",
+ " return");
+
+ String expected = "string0_42";
+ assertEquals(expected, runOnJava(builder, clazz.name));
+
+ AndroidApp jasminApp = builder.build();
+ AndroidApp d8App = ToolHelper.runD8(jasminApp);
+ assertEquals(expected, runOnArt(d8App, clazz.name));
+
+ DexInspector inspector = new DexInspector(d8App);
+ for (Entry<String, MethodSignature> entry : classes.entrySet()) {
+ DebugInfoInspector info = new DebugInfoInspector(inspector, entry.getKey(), entry.getValue());
+ info.checkStartLine(1);
+ // If jumbo-string processing fails to keep debug info, some methods will have lost 'i' here.
+ info.checkLineHasExactLocals(1, "this", entry.getKey(), "i", "int");
+ info.checkLineHasExactLocals(
+ 2, "this", entry.getKey(), "i", "int", "strings", "java.lang.String[]");
+ }
+ }
+}
