Always preserve source file information.
Bug: b/214018111
Change-Id: Icd1155686188ebf7fa0d7000742bbcbaeb9315c1
diff --git a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
index d9158fc..8cb3ed0 100644
--- a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
@@ -36,7 +36,6 @@
import com.android.tools.r8.graph.GenericSignature.MethodTypeSignature;
import com.android.tools.r8.jar.CfApplicationWriter;
import com.android.tools.r8.origin.Origin;
-import com.android.tools.r8.shaking.ProguardKeepAttributes;
import com.android.tools.r8.synthesis.SyntheticMarker;
import com.android.tools.r8.utils.AsmUtils;
import com.android.tools.r8.utils.DescriptorUtils;
@@ -116,16 +115,6 @@
ClassReader reader = new ClassReader(bytes);
int parsingOptions = SKIP_FRAMES | SKIP_CODE;
-
- // If the source-file and source-debug-extension attributes are not kept we can skip all debug
- // related attributes when parsing the class structure.
- if (application.options.getProguardConfiguration() != null) {
- ProguardKeepAttributes keep =
- application.options.getProguardConfiguration().getKeepAttributes();
- if (!keep.sourceFile && !keep.sourceDebugExtension && !keep.methodParameters) {
- parsingOptions |= SKIP_DEBUG;
- }
- }
if (classKind != ClassKind.PROGRAM) {
parsingOptions |= SKIP_DEBUG;
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java b/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java
index 3d2be09..5264b18 100644
--- a/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergerTest.java
@@ -9,7 +9,6 @@
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue;
@@ -117,6 +116,14 @@
"classmerging.SuperClass"
);
+ private static void assertAbsentOrIdentity(ClassNameMapper mapper, String className) {
+ // The mapping should be absent or identity.
+ ClassNamingForNameMapper classNaming = mapper.getClassNaming(className);
+ if (classNaming != null) {
+ assertEquals(classNaming.originalName, classNaming.renamedName);
+ }
+ }
+
@Test
public void testClassesHaveNotBeenMerged() throws Throwable {
runR8(DONT_OPTIMIZE, null);
@@ -526,7 +533,7 @@
ClassNameMapper mappingWithClassMerging =
ClassNameMapper.mapperFromString(compileResultWithClassMerging.getProguardMap());
- assertNull(mappingWithClassMerging.getClassNaming("classmerging.ProguardFieldMapTest$A"));
+ assertAbsentOrIdentity(mappingWithClassMerging, "classmerging.ProguardFieldMapTest$A");
ClassNamingForNameMapper mappingsForClassBWithClassMerging =
mappingWithClassMerging.getClassNaming("classmerging.ProguardFieldMapTest$B");
@@ -607,7 +614,7 @@
ClassNameMapper mappingWithClassMerging =
ClassNameMapper.mapperFromString(compileResultWithClassMerging.getProguardMap());
- assertNull(mappingWithClassMerging.getClassNaming("classmerging.ProguardMethodMapTest$A"));
+ assertAbsentOrIdentity(mappingWithClassMerging, "classmerging.ProguardMethodMapTest$A");
ClassNamingForNameMapper mappingsForClassBWithClassMerging =
mappingWithClassMerging.getClassNaming("classmerging.ProguardMethodMapTest$B");
diff --git a/src/test/java/com/android/tools/r8/debuginfo/NoKeepSourceFileAttributeTest.java b/src/test/java/com/android/tools/r8/debuginfo/NoKeepSourceFileAttributeTest.java
new file mode 100644
index 0000000..9a68a09
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/debuginfo/NoKeepSourceFileAttributeTest.java
@@ -0,0 +1,107 @@
+// Copyright (c) 2023, 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 static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.naming.retrace.StackTrace.StackTraceLine;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class NoKeepSourceFileAttributeTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDexRuntimesAndAllApiLevels().build();
+ }
+
+ public NoKeepSourceFileAttributeTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ public boolean isRuntimeWithPcAsLineNumberSupport() {
+ return parameters.isDexRuntime()
+ && parameters
+ .getRuntime()
+ .maxSupportedApiLevel()
+ .isGreaterThanOrEqualTo(apiLevelWithPcAsLineNumberSupport());
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(NoKeepSourceFileAttributeTest.class)
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertFailureWithErrorThatThrows(RuntimeException.class)
+ .inspectOriginalStackTrace(
+ stacktrace -> {
+ List<StackTraceLine> stackTraceLines = stacktrace.getStackTraceLines();
+ assertEquals(1, stackTraceLines.size());
+ StackTraceLine stackTraceLine = stackTraceLines.get(0);
+ if (!isRuntimeWithPcAsLineNumberSupport()) {
+ assertEquals("SourceFile", stackTraceLine.fileName);
+ } else {
+ // VMs with native PC support and no debug info print "Unknown Source".
+ // TODO(b/146565491): This will need a check for new VMs once fixed.
+ assertEquals("Unknown Source", stackTraceLine.fileName);
+ }
+ })
+ .inspectStackTrace(
+ stacktrace -> {
+ List<StackTraceLine> stackTraceLines = stacktrace.getStackTraceLines();
+ assertEquals(1, stackTraceLines.size());
+ StackTraceLine stackTraceLine = stackTraceLines.get(0);
+ assertEquals("NoKeepSourceFileAttributeTest.java", stackTraceLine.fileName);
+ });
+ }
+
+ @Test
+ public void testNoOpt() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(NoKeepSourceFileAttributeTest.class)
+ .addKeepMainRule(TestClass.class)
+ // Keeping lines and not obfuscating or optimizing will retain original lines.
+ .addDontObfuscate()
+ .addDontOptimize()
+ .addKeepAttributeLineNumberTable()
+ .setMinApi(parameters)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertFailureWithErrorThatThrows(RuntimeException.class)
+ .inspectOriginalStackTrace(
+ stacktrace -> {
+ List<StackTraceLine> stackTraceLines = stacktrace.getStackTraceLines();
+ assertEquals(1, stackTraceLines.size());
+ StackTraceLine stackTraceLine = stackTraceLines.get(0);
+ assertEquals("SourceFile", stackTraceLine.fileName);
+ // The non-optimizing/obfuscating build will retain the original line info.
+ assertTrue(stackTraceLine.lineNumber > 50);
+ })
+ .inspectStackTrace(
+ stacktrace -> {
+ List<StackTraceLine> stackTraceLines = stacktrace.getStackTraceLines();
+ assertEquals(1, stackTraceLines.size());
+ StackTraceLine stackTraceLine = stackTraceLines.get(0);
+ assertEquals("NoKeepSourceFileAttributeTest.java", stackTraceLine.fileName);
+ assertTrue(stackTraceLine.lineNumber > 50);
+ });
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ throw new RuntimeException("My Exception!");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/IdentityMappingFileTest.java b/src/test/java/com/android/tools/r8/naming/IdentityMappingFileTest.java
index 21ece10..6bffb78 100644
--- a/src/test/java/com/android/tools/r8/naming/IdentityMappingFileTest.java
+++ b/src/test/java/com/android/tools/r8/naming/IdentityMappingFileTest.java
@@ -4,9 +4,9 @@
package com.android.tools.r8.naming;
import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.DiagnosticsHandler;
@@ -51,11 +51,20 @@
assertThat(mapping, containsString("# {\"id\":\"com.android.tools.r8.mapping\",\"version\":"));
assertThat(mapping, containsString("# pg_map_id: "));
assertThat(mapping, containsString("# pg_map_hash: SHA-256 "));
- // Check the mapping is the identity, e.g., only comments are defined.
- // Note, this could change if the mapping is ever changed to be complete, in which case the
- // mapping will have actual identity mappings.
+ // Check the mapping is the identity, e.g., only comments and identity entries are defined.
for (String line : StringUtils.splitLines(mapping)) {
- assertThat(line, startsWith("#"));
+ if (line.startsWith("#")) {
+ continue;
+ }
+ String[] parts = line.split(" -> ");
+ if (parts.length == 2 && line.endsWith(":")) {
+ String left = parts[0];
+ String right = parts[1];
+ if (left.equals(right.substring(0, right.length() - 1))) {
+ continue;
+ }
+ }
+ fail("Expected comment or identity, got: " + line);
}
}
