Merge "Support for keepattributes LineNumberTable and LocalVariableTable."
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 400e5ba..0f0b5e5 100644
--- a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.graph;
import static org.objectweb.asm.ClassReader.SKIP_CODE;
+import static org.objectweb.asm.ClassReader.SKIP_DEBUG;
import static org.objectweb.asm.ClassReader.SKIP_FRAMES;
import static org.objectweb.asm.Opcodes.ACC_DEPRECATED;
import static org.objectweb.asm.Opcodes.ASM6;
@@ -27,6 +28,7 @@
import com.android.tools.r8.graph.DexValue.DexValueString;
import com.android.tools.r8.graph.DexValue.DexValueType;
import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.shaking.ProguardKeepAttributes;
import com.android.tools.r8.utils.FieldSignatureEquivalence;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.MethodSignatureEquivalence;
@@ -93,9 +95,18 @@
input.reset();
ClassReader reader = new ClassReader(input);
+
+ 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.
+ ProguardKeepAttributes keep = application.options.proguardConfiguration.getKeepAttributes();
+ if (!keep.sourceFile && !keep.sourceDebugExtension) {
+ parsingOptions |= SKIP_DEBUG;
+ }
reader.accept(
new CreateDexClassVisitor(origin, classKind, reader.b, application, classConsumer),
- SKIP_FRAMES | SKIP_CODE);
+ parsingOptions);
}
private static int cleanAccessFlags(int access) {
diff --git a/src/main/java/com/android/tools/r8/graph/JarCode.java b/src/main/java/com/android/tools/r8/graph/JarCode.java
index b978408..0668d05 100644
--- a/src/main/java/com/android/tools/r8/graph/JarCode.java
+++ b/src/main/java/com/android/tools/r8/graph/JarCode.java
@@ -17,6 +17,7 @@
import com.android.tools.r8.naming.ClassNameMapper;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.ProguardKeepAttributes;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
import java.io.PrintWriter;
@@ -163,7 +164,7 @@
InternalOptions options,
ValueNumberGenerator generator,
Position callerPosition) {
- if (!options.debug || options.testing.removeLocalsTable) {
+ if (!options.debug || !options.proguardConfiguration.getKeepAttributes().localVariableTable) {
node.localVariables.clear();
}
JarSourceCode source =
@@ -241,9 +242,16 @@
}
private void parseCode(ReparseContext context, boolean useJsrInliner) {
+ // If the keep attributes do not specify keeping LocalVariableTable, LocalVariableTypeTable or
+ // LineNumberTable, then we can skip parsing all the debug related attributes during code read.
+ int parsingOptions = ClassReader.SKIP_FRAMES;
+ ProguardKeepAttributes keep = application.options.proguardConfiguration.getKeepAttributes();
+ if (!keep.localVariableTable && !keep.localVariableTypeTable && !keep.lineNumberTable) {
+ parsingOptions |= ClassReader.SKIP_DEBUG;
+ }
SecondVisitor classVisitor = new SecondVisitor(createCodeLocator(context), useJsrInliner);
try {
- new ClassReader(context.classCache).accept(classVisitor, ClassReader.SKIP_FRAMES);
+ new ClassReader(context.classCache).accept(classVisitor, parsingOptions);
} catch (Exception exception) {
throw new CompilationError(
"Unable to parse method `" + method.toSourceString() + "`", exception);
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardKeepAttributes.java b/src/main/java/com/android/tools/r8/shaking/ProguardKeepAttributes.java
index fe31b14..49f99dc 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardKeepAttributes.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardKeepAttributes.java
@@ -16,6 +16,9 @@
public static final String ENCLOSING_METHOD = "EnclosingMethod";
public static final String SIGNATURE = "Signature";
public static final String EXCEPTIONS = "Exceptions";
+ public static final String LINE_NUMBER_TABLE = "LineNumberTable";
+ public static final String LOCAL_VARIABLE_TABLE = "LocalVariableTable";
+ public static final String LOCAL_VARIABLE_TYPE_TABLE = "LocalVariableTypeTable";
public static final String SOURCE_DEBUG_EXTENSION = "SourceDebugExtension";
public static final String RUNTIME_VISIBLE_ANNOTATIONS = "RuntimeVisibleAnnotations";
public static final String RUNTIME_INVISIBLE_ANNOTATIONS = "RuntimeInvisibleAnnotations";
@@ -36,6 +39,9 @@
public boolean enclosingMethod = false;
public boolean signature = false;
public boolean exceptions = false;
+ public boolean lineNumberTable = false;
+ public boolean localVariableTable = false;
+ public boolean localVariableTypeTable = false;
public boolean sourceDebugExtension = false;
public boolean runtimeVisibleAnnotations = false;
public boolean runtimeInvisibleAnnotations = false;
@@ -108,6 +114,9 @@
innerClasses = update(innerClasses, INNER_CLASSES, patterns);
enclosingMethod = update(enclosingMethod, ENCLOSING_METHOD, patterns);
signature = update(signature, SIGNATURE, patterns);
+ lineNumberTable = update(lineNumberTable, LINE_NUMBER_TABLE, patterns);
+ localVariableTable = update(localVariableTable, LOCAL_VARIABLE_TABLE, patterns);
+ localVariableTypeTable = update(localVariableTypeTable, LOCAL_VARIABLE_TYPE_TABLE, patterns);
exceptions = update(exceptions, EXCEPTIONS, patterns);
sourceDebugExtension = update(sourceDebugExtension, SOURCE_DEBUG_EXTENSION, patterns);
runtimeVisibleAnnotations = update(runtimeVisibleAnnotations, RUNTIME_VISIBLE_ANNOTATIONS,
@@ -146,6 +155,18 @@
throw new CompilationError("Attribute Signature requires InnerClasses attribute. Check "
+ "-keepattributes directive.");
}
+ if (forceProguardCompatibility && localVariableTable && !lineNumberTable) {
+ // If locals are kept, assume line numbers should be kept too.
+ lineNumberTable = true;
+ compatibility.addKeepAttributePatterns(
+ ImmutableList.of(ProguardKeepAttributes.LINE_NUMBER_TABLE));
+ }
+ if (localVariableTable && !lineNumberTable) {
+ throw new CompilationError(
+ "Attribute " + LOCAL_VARIABLE_TABLE
+ + " requires " + LINE_NUMBER_TABLE
+ + ". Check -keepattributes directive.");
+ }
}
@Override
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index e0e4227..93b379d 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -462,7 +462,6 @@
public boolean nondeterministicCycleElimination = false;
public Set<Inliner.Reason> validInliningReasons = null;
public boolean suppressExperimentalCfBackendWarning = false;
- public boolean removeLocalsTable = false;
}
public boolean canUseInvokePolymorphicOnVarHandle() {
diff --git a/src/test/examples/classmerging/keep-rules.txt b/src/test/examples/classmerging/keep-rules.txt
index 97be613..5e70808 100644
--- a/src/test/examples/classmerging/keep-rules.txt
+++ b/src/test/examples/classmerging/keep-rules.txt
@@ -2,6 +2,9 @@
# 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.
+# Keep line numbers to ensure method mappings in the map file.
+-keepattributes LineNumberTable
+
# Keep the application entry point. Get rid of everything that is not
# reachable from there.
-keep public class classmerging.Test {
@@ -57,6 +60,3 @@
}
-printmapping
-
-# TODO(herhut): Consider supporting merging of inner-class attributes.
-# -keepattributes *
\ No newline at end of file
diff --git a/src/test/examples/shaking1/keep-rules.txt b/src/test/examples/shaking1/keep-rules.txt
index 66cf1c6..82786d5 100644
--- a/src/test/examples/shaking1/keep-rules.txt
+++ b/src/test/examples/shaking1/keep-rules.txt
@@ -2,6 +2,8 @@
# 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.
+-keepattributes LineNumberTable
+
# Keep the application entry point. Get rid of everything that is not
# reachable from there.
-keep public class shaking1.Shaking {
diff --git a/src/test/java/com/android/tools/r8/debug/ArraySimplificationLineNumberTestRunner.java b/src/test/java/com/android/tools/r8/debug/ArraySimplificationLineNumberTestRunner.java
index 308e8d7..e0b953a 100644
--- a/src/test/java/com/android/tools/r8/debug/ArraySimplificationLineNumberTestRunner.java
+++ b/src/test/java/com/android/tools/r8/debug/ArraySimplificationLineNumberTestRunner.java
@@ -27,7 +27,7 @@
DebugTestConfig d8NoLocals = new D8DebugTestConfig().compileAndAdd(
temp,
Collections.singletonList(ToolHelper.getClassFileForTestClass(CLASS)),
- options -> options.testing.removeLocalsTable = true);
+ options -> options.proguardConfiguration.getKeepAttributes().localVariableTable = false);
new DebugStreamComparator()
.add("CF", streamDebugTest(cf, NAME, NO_FILTER))
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
index caa697b..e65ebca 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
@@ -40,6 +40,7 @@
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.FieldAccessInstructionSubject;
@@ -429,10 +430,11 @@
}
private String getProguardConfig(String main) {
- return keepMainProguardConfiguration(main)
- + "\n"
- + "-dontobfuscate\n"
- + "-allowaccessmodification";
+ return StringUtils.joinLines(
+ keepMainProguardConfiguration(main),
+ "-dontobfuscate",
+ "-allowaccessmodification",
+ "-keepattributes LineNumberTable");
}
private void configure(InternalOptions options) {
diff --git a/src/test/java/com/android/tools/r8/naming/WarnReflectiveAccessTest.java b/src/test/java/com/android/tools/r8/naming/WarnReflectiveAccessTest.java
index 9ccc94b..5127a59 100644
--- a/src/test/java/com/android/tools/r8/naming/WarnReflectiveAccessTest.java
+++ b/src/test/java/com/android/tools/r8/naming/WarnReflectiveAccessTest.java
@@ -112,6 +112,7 @@
+ " <methods>;"
+ "}",
"-printmapping",
+ "-keepattributes LineNumberTable",
reflectionRules),
Origin.unknown())
.setOutput(out, outputMode(backend));
diff --git a/src/test/java/com/android/tools/r8/shaking/KeepAttributesTest.java b/src/test/java/com/android/tools/r8/shaking/KeepAttributesTest.java
new file mode 100644
index 0000000..275d2e9
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/KeepAttributesTest.java
@@ -0,0 +1,122 @@
+// Copyright (c) 2018, 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.shaking;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.DexIndexedConsumer;
+import com.android.tools.r8.R8;
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.debuginfo.DebugInfoInspector;
+import com.android.tools.r8.naming.MemberNaming.MethodSignature;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.shaking.forceproguardcompatibility.keepattributes.TestKeepAttributes;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.google.common.collect.ImmutableList;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import org.junit.Test;
+
+public class KeepAttributesTest extends TestBase {
+
+ public static final Class CLASS = TestKeepAttributes.class;
+
+ @Test
+ public void discardAllAttributes()
+ throws CompilationFailedException, IOException, ExecutionException {
+ List<String> keepRules = ImmutableList.of(
+ "-keep class ** { *; }"
+ );
+ CodeInspector inspector = compile(keepRules);
+ DebugInfoInspector debugInfo = debugInfoForMain(inspector);
+ checkLineNumbers(false, debugInfo);
+ checkLocals(false, debugInfo);
+ }
+
+ @Test
+ public void keepLineNumberTable()
+ throws CompilationFailedException, IOException, ExecutionException {
+ List<String> keepRules = ImmutableList.of(
+ "-keep class ** { *; }",
+ "-keepattributes " + ProguardKeepAttributes.LINE_NUMBER_TABLE
+ );
+ CodeInspector inspector = compile(keepRules);
+ DebugInfoInspector debugInfo = debugInfoForMain(inspector);
+ checkLineNumbers(true, debugInfo);
+ checkLocals(false, debugInfo);
+ }
+
+ @Test
+ public void keepLineNumberTableAndLocalVariableTable()
+ throws CompilationFailedException, IOException, ExecutionException {
+ List<String> keepRules = ImmutableList.of(
+ "-keep class ** { *; }",
+ "-keepattributes "
+ + ProguardKeepAttributes.LINE_NUMBER_TABLE
+ + ", "
+ + ProguardKeepAttributes.LOCAL_VARIABLE_TABLE
+ );
+ CodeInspector inspector = compile(keepRules);
+ DebugInfoInspector debugInfo = debugInfoForMain(inspector);
+ checkLineNumbers(true, debugInfo);
+ checkLocals(true, debugInfo);
+ }
+
+ @Test
+ public void keepLocalVariableTable() throws IOException, ExecutionException {
+ List<String> keepRules = ImmutableList.of(
+ "-keep class ** { *; }",
+ "-keepattributes " + ProguardKeepAttributes.LOCAL_VARIABLE_TABLE
+ );
+ // Compiling with a keep rule for locals but no line results in an error in R8.
+ try {
+ compile(keepRules);
+ } catch (CompilationFailedException e) {
+ assertTrue(e.getCause().getMessage().contains(ProguardKeepAttributes.LOCAL_VARIABLE_TABLE));
+ assertTrue(e.getCause().getMessage().contains(ProguardKeepAttributes.LINE_NUMBER_TABLE));
+ return;
+ }
+ fail("Expected error");
+ }
+
+ private CodeInspector compile(List<String> keepRules)
+ throws CompilationFailedException, IOException, ExecutionException {
+ Path dexOut = temp.getRoot().toPath().resolve("dex.zip");
+ R8.run(R8Command.builder()
+ .setMode(CompilationMode.DEBUG)
+ .addProgramFiles(ToolHelper.getClassFileForTestClass(CLASS))
+ .addLibraryFiles(ToolHelper.getDefaultAndroidJar())
+ .addProguardConfiguration(keepRules, Origin.unknown())
+ .setProgramConsumer(new DexIndexedConsumer.ArchiveConsumer(dexOut))
+ .build());
+ ToolHelper.runArtRaw(dexOut.toString(), CLASS.getCanonicalName());
+ return new CodeInspector(dexOut);
+ }
+
+ private DebugInfoInspector debugInfoForMain(CodeInspector inspector) {
+ return new DebugInfoInspector(
+ inspector,
+ CLASS.getCanonicalName(),
+ new MethodSignature("main", "void", Collections.singleton("java.lang.String[]")));
+ }
+
+ private void checkLineNumbers(boolean expected, DebugInfoInspector debugInfo) {
+ assertEquals("Expected " + (expected ? "line entries" : "no line entries"),
+ expected, debugInfo.getEntries().stream().anyMatch(e -> e.lineEntry));
+ }
+
+ private void checkLocals(boolean expected, DebugInfoInspector debugInfo) {
+ assertEquals("Expected " + (expected ? "locals" : "no locals"),
+ expected, debugInfo.getEntries().stream().anyMatch(e -> !e.locals.isEmpty()));
+ }
+}
