Change non-kept SourceFile from empty string to SourceFile
This change will re-use the already existing string SourceFile and not
add a new empty string. For DEX this possibly saves a single empty
string but in CF this will save an empty-string per class file.
Bug: 129362602
Bug: 147571342
Change-Id: I71809cb8c14654e33551559d2cf562bbd745b01b
diff --git a/src/main/java/com/android/tools/r8/naming/SourceFileRewriter.java b/src/main/java/com/android/tools/r8/naming/SourceFileRewriter.java
index 8f4330c..cfc3878 100644
--- a/src/main/java/com/android/tools/r8/naming/SourceFileRewriter.java
+++ b/src/main/java/com/android/tools/r8/naming/SourceFileRewriter.java
@@ -29,16 +29,21 @@
public void run() {
ProguardConfiguration proguardConfiguration = appView.options().getProguardConfiguration();
String renameSourceFile = proguardConfiguration.getRenameSourceFileAttribute();
+ boolean hasRenameSourceFileAttribute = renameSourceFile != null;
// Return early if a user wants to keep the current source file attribute as-is.
- if (renameSourceFile == null && proguardConfiguration.getKeepAttributes().sourceFile) {
+ if (!hasRenameSourceFileAttribute
+ && proguardConfiguration.getKeepAttributes().sourceFile
+ && appView.options().forceProguardCompatibility) {
return;
}
- // Now, the user wants either to remove source file attribute or to rename it.
- DexString dexRenameSourceFile =
- renameSourceFile == null
- ? appView.dexItemFactory().createString("")
- : appView.dexItemFactory().createString(renameSourceFile);
+ // Now, the user wants either to remove source file attribute or to rename it for non-kept
+ // classes.
+ DexString dexRenameSourceFile = getSourceFileRenaming(proguardConfiguration);
for (DexClass clazz : appView.appInfo().classes()) {
+ if (!hasRenameSourceFileAttribute
+ && appView.rootSet().mayNotBeMinified(clazz.type, appView)) {
+ continue;
+ }
clazz.sourceFile = dexRenameSourceFile;
clazz.forEachMethod(encodedMethod -> {
// Abstract methods do not have code_item.
@@ -66,4 +71,23 @@
});
}
}
+
+ private DexString getSourceFileRenaming(ProguardConfiguration proguardConfiguration) {
+ // If we should not be keeping the source file, null it out.
+ if (!proguardConfiguration.getKeepAttributes().sourceFile) {
+ return null;
+ }
+
+ String renamedSourceFileAttribute = proguardConfiguration.getRenameSourceFileAttribute();
+ if (renamedSourceFileAttribute != null) {
+ return appView.dexItemFactory().createString(renamedSourceFileAttribute);
+ }
+
+ // Otherwise, take the smallest size depending on platform. We cannot use NULL since the jvm
+ // and art will write at foo.bar.baz(Unknown Source) without a line-number. Newer version of ART
+ // will report the DEX PC.
+ return appView
+ .dexItemFactory()
+ .createString(appView.options().isGeneratingClassFiles() ? "SourceFile" : "");
+ }
}
diff --git a/src/test/java/com/android/tools/r8/R8TestRunResult.java b/src/test/java/com/android/tools/r8/R8TestRunResult.java
index 2ec28b8..9fe91d1 100644
--- a/src/test/java/com/android/tools/r8/R8TestRunResult.java
+++ b/src/test/java/com/android/tools/r8/R8TestRunResult.java
@@ -59,6 +59,16 @@
return new CodeInspector(app, proguardMap);
}
+ @Override
+ public <E extends Throwable> R8TestRunResult inspectFailure(
+ ThrowingConsumer<CodeInspector, E> consumer) throws IOException, ExecutionException, E {
+ assertFailure();
+ assertNotNull(app);
+ CodeInspector codeInspector = new CodeInspector(app, proguardMap);
+ consumer.accept(codeInspector);
+ return self();
+ }
+
public <E extends Throwable> R8TestRunResult inspectOriginalStackTrace(
ThrowingConsumer<StackTrace, E> consumer) throws E {
consumer.accept(getOriginalStackTrace());
diff --git a/src/test/java/com/android/tools/r8/TestRunResult.java b/src/test/java/com/android/tools/r8/TestRunResult.java
index 46bf0a1..bf3bb9d 100644
--- a/src/test/java/com/android/tools/r8/TestRunResult.java
+++ b/src/test/java/com/android/tools/r8/TestRunResult.java
@@ -142,6 +142,15 @@
return self();
}
+ public <E extends Throwable> RR inspectFailure(ThrowingConsumer<CodeInspector, E> consumer)
+ throws IOException, ExecutionException, E {
+ assertFailure();
+ assertNotNull(app);
+ CodeInspector inspector = new CodeInspector(app);
+ consumer.accept(inspector);
+ return self();
+ }
+
public <E extends Throwable> RR inspectStackTrace(ThrowingConsumer<StackTrace, E> consumer)
throws E {
consumer.accept(getStackTrace());
diff --git a/src/test/java/com/android/tools/r8/naming/RenameSourceFileRetraceTest.java b/src/test/java/com/android/tools/r8/naming/RenameSourceFileRetraceTest.java
new file mode 100644
index 0000000..324df58
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/RenameSourceFileRetraceTest.java
@@ -0,0 +1,141 @@
+// 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.naming;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.containsLinePositions;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static junit.framework.TestCase.assertEquals;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertNull;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.R8TestBuilder;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.naming.retrace.StackTrace;
+import com.android.tools.r8.naming.testclasses.ClassToBeMinified;
+import com.android.tools.r8.naming.testclasses.Main;
+import com.android.tools.r8.references.MethodReference;
+import com.android.tools.r8.references.Reference;
+import com.android.tools.r8.shaking.ProguardKeepAttributes;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.Matchers.LinePosition;
+import java.io.IOException;
+import java.util.List;
+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 RenameSourceFileRetraceTest extends TestBase {
+
+ private static final String FILENAME_RENAME = "FOO";
+ private static final String FILENAME_MAIN = "Main.java";
+ private static final String FILENAME_CLASS_TO_BE_MINIFIED = "ClassToBeMinified.java";
+
+ private final TestParameters parameters;
+ private final boolean isCompat;
+ private final boolean keepSourceFile;
+
+ @Parameters(name = "{0}, is compat: {1}, keep source file attribute: {2}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withAllRuntimesAndApiLevels().build(),
+ BooleanUtils.values(),
+ BooleanUtils.values());
+ }
+
+ public RenameSourceFileRetraceTest(
+ TestParameters parameters, Boolean isCompat, Boolean keepSourceFile) {
+ this.parameters = parameters;
+ this.isCompat = isCompat;
+ this.keepSourceFile = keepSourceFile;
+ }
+
+ @Test
+ public void testR8()
+ throws ExecutionException, CompilationFailedException, IOException, NoSuchMethodException {
+ R8TestBuilder<? extends R8TestBuilder<?>> r8TestBuilder =
+ isCompat ? testForR8Compat(parameters.getBackend()) : testForR8(parameters.getBackend());
+ if (keepSourceFile) {
+ r8TestBuilder.addKeepAttributes(ProguardKeepAttributes.SOURCE_FILE);
+ }
+ String minifiedFileName =
+ (keepSourceFile && isCompat)
+ ? FILENAME_CLASS_TO_BE_MINIFIED
+ : (parameters.getBackend() == Backend.CF ? "SourceFile" : "");
+ r8TestBuilder
+ .addProgramClasses(ClassToBeMinified.class, Main.class)
+ .addKeepAttributes(ProguardKeepAttributes.LINE_NUMBER_TABLE)
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters.getApiLevel())
+ .enableInliningAnnotations()
+ .run(parameters.getRuntime(), Main.class)
+ .assertFailureWithErrorThatMatches(containsString("ClassToBeMinified.foo()"))
+ .inspectFailure(inspector -> inspectOutput(inspector, minifiedFileName, FILENAME_MAIN))
+ .inspectStackTrace(stackTrace -> inspectStackTrace(stackTrace, FILENAME_MAIN));
+ }
+
+ @Test
+ public void testRenameSourceFileR8()
+ throws ExecutionException, CompilationFailedException, IOException, NoSuchMethodException {
+ R8TestBuilder<? extends R8TestBuilder<?>> r8TestBuilder =
+ isCompat ? testForR8Compat(parameters.getBackend()) : testForR8(parameters.getBackend());
+ if (keepSourceFile) {
+ r8TestBuilder.addKeepAttributes(ProguardKeepAttributes.SOURCE_FILE);
+ }
+ r8TestBuilder
+ .addProgramClasses(ClassToBeMinified.class, Main.class)
+ .addKeepAttributes(ProguardKeepAttributes.LINE_NUMBER_TABLE)
+ .addKeepRules("-renamesourcefileattribute " + FILENAME_RENAME)
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters.getApiLevel())
+ .enableInliningAnnotations()
+ .run(parameters.getRuntime(), Main.class)
+ .assertFailureWithErrorThatMatches(containsString("ClassToBeMinified.foo()"))
+ .inspectFailure(inspector -> inspectOutput(inspector, FILENAME_RENAME, FILENAME_RENAME))
+ .inspectStackTrace(stackTrace -> inspectStackTrace(stackTrace, FILENAME_RENAME));
+ }
+
+ private void inspectOutput(
+ CodeInspector inspector, String classToBeMinifiedFilename, String mainClassFilename) {
+ ClassSubject mainSubject = inspector.clazz(Main.class);
+ assertThat(mainSubject, isPresent());
+ DexClass mainClazz = mainSubject.getDexClass();
+ ClassSubject classToBeMinifiedSubject = inspector.clazz(ClassToBeMinified.class);
+ assertThat(classToBeMinifiedSubject, isPresent());
+ DexClass minifiedClazz = classToBeMinifiedSubject.getDexClass();
+ if (keepSourceFile) {
+ assertEquals(mainClassFilename, mainClazz.sourceFile.toString());
+ assertEquals(classToBeMinifiedFilename, minifiedClazz.sourceFile.toString());
+ } else {
+ assertNull(mainClazz.sourceFile);
+ assertNull(minifiedClazz.sourceFile);
+ }
+ }
+
+ private void inspectStackTrace(StackTrace stackTrace, String mainFileName)
+ throws NoSuchMethodException {
+ if (!keepSourceFile) {
+ return;
+ }
+ assertEquals(2, stackTrace.getStackTraceLines().size());
+ MethodReference classToBeMinifiedFoo =
+ Reference.methodFromMethod(ClassToBeMinified.class.getDeclaredMethod("foo"));
+ MethodReference mainMain =
+ Reference.methodFromMethod(Main.class.getDeclaredMethod("main", String[].class));
+ LinePosition expectedStack =
+ LinePosition.stack(
+ LinePosition.create(classToBeMinifiedFoo, 1, 13, FILENAME_CLASS_TO_BE_MINIFIED),
+ LinePosition.create(mainMain, 1, 10, mainFileName));
+ assertThat(stackTrace, containsLinePositions(expectedStack));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/testclasses/ClassToBeMinified.java b/src/test/java/com/android/tools/r8/naming/testclasses/ClassToBeMinified.java
new file mode 100644
index 0000000..da6f9c0
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/testclasses/ClassToBeMinified.java
@@ -0,0 +1,15 @@
+// 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.naming.testclasses;
+
+import com.android.tools.r8.NeverInline;
+
+public class ClassToBeMinified {
+
+ @NeverInline
+ public static void foo() {
+ throw new RuntimeException("ClassToBeMinified.foo()");
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/testclasses/Main.java b/src/test/java/com/android/tools/r8/naming/testclasses/Main.java
new file mode 100644
index 0000000..1e37d4c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/testclasses/Main.java
@@ -0,0 +1,12 @@
+// 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.naming.testclasses;
+
+public class Main {
+
+ public static void main(String[] args) {
+ ClassToBeMinified.foo();
+ }
+}
