Always add debug information for overloaded methods
To reliably retrace by PC information the method name has to be
unique. We should therefore fallback to a line-number implementation
for overloaded methods.
Bug: 37830524
Bug: 148657906
Change-Id: Iee422da03d07b4c541f1af0fd0f43b273387a581
diff --git a/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java b/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
index 0552275..7bcf1c9 100644
--- a/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
+++ b/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
@@ -328,8 +328,7 @@
Code code = method.getCode();
if (code != null) {
if (code.isDexCode() && doesContainPositions(code.asDexCode())) {
- if (appView.options().canUseDexPcAsDebugInformation()) {
- // TODO(b/148657906): Fall back if methods.size() > 1.
+ if (appView.options().canUseDexPcAsDebugInformation() && methods.size() == 1) {
optimizeDexCodePositionsForPc(method, kotlinRemapper, mappedPositions);
} else {
optimizeDexCodePositions(
diff --git a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
index 2defd19..13afb51 100644
--- a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
@@ -205,6 +205,10 @@
return addKeepRules("-keepattributes " + String.join(",", attributes));
}
+ public T addKeepAttributeLineNumberTable() {
+ return addKeepRules("-keepattributes LineNumberTable");
+ }
+
public T addKeepAllAttributes() {
return addKeepAttributes("*");
}
diff --git a/src/test/java/com/android/tools/r8/debuginfo/DexPcWithDebugInfoForOverloadedMethodsTest.java b/src/test/java/com/android/tools/r8/debuginfo/DexPcWithDebugInfoForOverloadedMethodsTest.java
new file mode 100644
index 0000000..7e969c8
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/debuginfo/DexPcWithDebugInfoForOverloadedMethodsTest.java
@@ -0,0 +1,30 @@
+// Copyright (c) 2020, 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 DexPcWithDebugInfoForOverloadedMethodsTest {
+
+ private static void inlinee(String message) {
+ if (System.currentTimeMillis() > 0) {
+ throw new RuntimeException(message);
+ }
+ }
+
+ public static void overloaded(int x) {
+ inlinee("overloaded(int)" + x);
+ }
+
+ public static void overloaded(String y) {
+ inlinee("overloaded(String)" + y);
+ }
+
+ public static void main(String[] args) {
+ if (args.length > 0) {
+ overloaded(42);
+ } else {
+ overloaded("42");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/debuginfo/DexPcWithDebugInfoForOverloadedMethodsTestRunner.java b/src/test/java/com/android/tools/r8/debuginfo/DexPcWithDebugInfoForOverloadedMethodsTestRunner.java
new file mode 100644
index 0000000..b344944
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/debuginfo/DexPcWithDebugInfoForOverloadedMethodsTestRunner.java
@@ -0,0 +1,130 @@
+// Copyright (c) 2020, 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 com.android.tools.r8.Collectors.toSingle;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isInlineFrame;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isInlineStack;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isTopOfStackTrace;
+import static junit.framework.TestCase.assertEquals;
+import static junit.framework.TestCase.assertNotNull;
+import static junit.framework.TestCase.assertNull;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
+import com.android.tools.r8.references.Reference;
+import com.android.tools.r8.retrace.RetraceMethodResult;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.FoundMethodSubject;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.Matchers.InlinePosition;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class DexPcWithDebugInfoForOverloadedMethodsTestRunner extends TestBase {
+
+ private static final Class<?> MAIN = DexPcWithDebugInfoForOverloadedMethodsTest.class;
+ private static final int MINIFIED_LINE_POSITION = 6;
+ private static final String EXPECTED = "java.lang.RuntimeException: overloaded(String)42";
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters()
+ .withDexRuntimesStartingFromIncluding(Version.V8_1_0)
+ .withApiLevelsStartingAtIncluding(AndroidApiLevel.O)
+ .build();
+ }
+
+ public DexPcWithDebugInfoForOverloadedMethodsTestRunner(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testEmittedDebugInfoForOverloads()
+ throws ExecutionException, CompilationFailedException, IOException, NoSuchMethodException {
+ testForR8(parameters.getBackend())
+ .addProgramClasses(MAIN)
+ .addKeepMainRule(MAIN)
+ .addKeepMethodRules(MAIN, "void overloaded(...)")
+ .addKeepAttributeLineNumberTable()
+ .setMinApi(parameters.getApiLevel())
+ .addOptionsModification(
+ internalOptions -> {
+ // TODO(b/37830524): Remove when activated.
+ internalOptions.enablePcDebugInfoOutput = true;
+ })
+ .run(parameters.getRuntime(), MAIN)
+ .assertFailureWithErrorThatMatches(containsString(EXPECTED))
+ .inspectOriginalStackTrace(
+ (stackTrace, inspector) -> {
+ assertEquals(MAIN.getTypeName(), stackTrace.get(0).className);
+ assertEquals("overloaded", stackTrace.get(0).methodName);
+ assertThat(stackTrace.get(0).fileName, not("Unknown Source"));
+ inspect(inspector);
+ })
+ .inspectStackTrace(
+ (stackTrace, codeInspector) -> {
+ MethodSubject throwingSubject =
+ codeInspector.clazz(MAIN).method("void", "overloaded", "java.lang.String");
+ assertThat(throwingSubject, isPresent());
+ InlinePosition inlineStack =
+ InlinePosition.stack(
+ InlinePosition.create(
+ Reference.methodFromMethod(
+ MAIN.getDeclaredMethod("inlinee", String.class)),
+ MINIFIED_LINE_POSITION,
+ 11),
+ InlinePosition.create(
+ Reference.methodFromMethod(
+ MAIN.getDeclaredMethod("overloaded", String.class)),
+ MINIFIED_LINE_POSITION,
+ 20));
+ RetraceMethodResult retraceResult =
+ throwingSubject
+ .streamInstructions()
+ .filter(InstructionSubject::isThrow)
+ .collect(toSingle())
+ .retraceLinePosition(codeInspector.retrace());
+ assertThat(retraceResult, isInlineFrame());
+ assertThat(retraceResult, isInlineStack(inlineStack));
+ assertThat(
+ retraceResult,
+ isTopOfStackTrace(
+ stackTrace,
+ ImmutableList.of(MINIFIED_LINE_POSITION, MINIFIED_LINE_POSITION)));
+ });
+ }
+
+ private void inspect(CodeInspector inspector) {
+ ClassSubject clazz = inspector.clazz(MAIN);
+ assertThat(clazz, isPresent());
+ assertEquals(3, clazz.allMethods().size());
+ for (FoundMethodSubject method : clazz.allMethods()) {
+ if (method.getOriginalName().equals("main")) {
+ assertNull(method.getMethod().getCode().asDexCode().getDebugInfo());
+ } else {
+ assertEquals("overloaded", method.getOriginalName());
+ assertNotNull(method.getMethod().getCode().asDexCode().getDebugInfo());
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/debuginfo/EnsureNoDebugInfoEmittedForPcOnlyTestRunner.java b/src/test/java/com/android/tools/r8/debuginfo/EnsureNoDebugInfoEmittedForPcOnlyTestRunner.java
index b8dd5f3..c47f16f 100644
--- a/src/test/java/com/android/tools/r8/debuginfo/EnsureNoDebugInfoEmittedForPcOnlyTestRunner.java
+++ b/src/test/java/com/android/tools/r8/debuginfo/EnsureNoDebugInfoEmittedForPcOnlyTestRunner.java
@@ -7,7 +7,7 @@
import static com.android.tools.r8.Collectors.toSingle;
import static com.android.tools.r8.utils.codeinspector.Matchers.isInlineFrame;
import static com.android.tools.r8.utils.codeinspector.Matchers.isInlineStack;
-import static com.android.tools.r8.utils.codeinspector.Matchers.isStackTrace;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isTopOfStackTrace;
import static junit.framework.TestCase.assertEquals;
import static junit.framework.TestCase.assertNull;
import static org.hamcrest.MatcherAssert.assertThat;
@@ -58,14 +58,14 @@
testForR8(parameters.getBackend())
.addProgramClasses(MAIN)
.addKeepMainRule(MAIN)
- .addKeepAttributes("LineNumberTable")
+ .addKeepAttributeLineNumberTable()
.setMinApi(parameters.getApiLevel())
.addOptionsModification(
internalOptions -> {
// TODO(b/37830524): Remove when activated.
internalOptions.enablePcDebugInfoOutput = true;
})
- .run(parameters.getRuntime(), EnsureNoDebugInfoEmittedForPcOnlyTest.class)
+ .run(parameters.getRuntime(), MAIN)
.inspectOriginalStackTrace(
(stackTrace, inspector) -> {
assertEquals(MAIN.getTypeName(), stackTrace.get(0).className);
@@ -99,7 +99,7 @@
assertThat(retraceResult, isInlineStack(inlineStack));
assertThat(
retraceResult,
- isStackTrace(
+ isTopOfStackTrace(
stackTrace,
ImmutableList.of(INLINED_DEX_PC, INLINED_DEX_PC, INLINED_DEX_PC)));
});
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java b/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
index e1d4e6e..3fb684f 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
@@ -461,13 +461,13 @@
};
}
- public static Matcher<RetraceMethodResult> isStackTrace(
+ public static Matcher<RetraceMethodResult> isTopOfStackTrace(
StackTrace stackTrace, List<Integer> minifiedPositions) {
return new TypeSafeMatcher<RetraceMethodResult>() {
@Override
protected boolean matchesSafely(RetraceMethodResult item) {
List<Element> retraceElements = item.stream().collect(Collectors.toList());
- if (retraceElements.size() != stackTrace.size()
+ if (retraceElements.size() > stackTrace.size()
|| retraceElements.size() != minifiedPositions.size()) {
return false;
}
