Add support for -keepparameternames
Bug: 132549918
Change-Id: I699bc767247fb6ab69f9e7aee78e490638dbf130
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 5c876f1..d9fbabd 100644
--- a/src/main/java/com/android/tools/r8/graph/JarCode.java
+++ b/src/main/java/com/android/tools/r8/graph/JarCode.java
@@ -18,13 +18,16 @@
import com.android.tools.r8.naming.ClassNameMapper;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.ProguardConfiguration;
import com.android.tools.r8.shaking.ProguardKeepAttributes;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.OffOrAuto;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceArrayMap;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.Iterator;
+import java.util.Map;
import java.util.function.BiFunction;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
@@ -34,6 +37,7 @@
import org.objectweb.asm.tree.AbstractInsnNode;
import org.objectweb.asm.tree.LabelNode;
import org.objectweb.asm.tree.LineNumberNode;
+import org.objectweb.asm.tree.LocalVariableNode;
import org.objectweb.asm.tree.MethodNode;
import org.objectweb.asm.tree.TryCatchBlockNode;
import org.objectweb.asm.util.Textifier;
@@ -148,9 +152,37 @@
Position callerPosition) {
triggerDelayedParsingIfNeccessary();
if (!keepLocals(encodedMethod, appView.options())) {
+ // If the locals are not kept, we might still need a bit of locals information to satisfy
+ // -keepparameternames for R8.
+ Map<Integer, DebugLocalInfo> parameterInfo = IRCode.NO_PARAMETER_INFO;
+ if (appView.options().hasProguardConfiguration()
+ && appView.options().getProguardConfiguration().isKeepParameterNames()) {
+ // Collect the number of local slots used for arguments.
+ int localsForParameters = encodedMethod.isStatic() ? 0 : 1;
+ for (DexType type : encodedMethod.method.proto.parameters.values) {
+ localsForParameters += type.isLongType() || type.isDoubleType() ? 2 : 1;
+ }
+ // Collect the first piece of local variable information for each argument local slot,
+ // assuming that that does actually describe the parameter (name, type and possibly
+ // signature).
+ DexItemFactory factory = appView.options().itemFactory;
+ parameterInfo = new Int2ReferenceArrayMap<>(localsForParameters);
+ for (Object o : node.localVariables) {
+ LocalVariableNode node = (LocalVariableNode) o;
+ if (node.index < localsForParameters && !parameterInfo.containsKey(node.index)) {
+ parameterInfo.put(
+ node.index,
+ new DebugLocalInfo(
+ factory.createString(node.name),
+ factory.createType(factory.createString(node.desc)),
+ node.signature == null ? null : factory.createString(node.signature)));
+ }
+ }
+ }
// We strip locals here because we will not be able to recover from invalid info.
node.localVariables.clear();
- return internalBuild(context, encodedMethod, appView, generator, callerPosition);
+ return internalBuild(
+ context, encodedMethod, appView, generator, callerPosition, parameterInfo);
} else {
return internalBuildWithLocals(context, encodedMethod, appView, generator, callerPosition);
}
@@ -163,11 +195,13 @@
ValueNumberGenerator generator,
Position callerPosition) {
try {
- return internalBuild(context, encodedMethod, appView, generator, callerPosition);
+ return internalBuild(
+ context, encodedMethod, appView, generator, callerPosition, IRCode.NO_PARAMETER_INFO);
} catch (InvalidDebugInfoException e) {
appView.options().warningInvalidDebugInfo(encodedMethod, origin, e);
node.localVariables.clear();
- return internalBuild(context, encodedMethod, appView, generator, callerPosition);
+ return internalBuild(
+ context, encodedMethod, appView, generator, callerPosition, IRCode.NO_PARAMETER_INFO);
}
}
@@ -186,7 +220,8 @@
DexEncodedMethod encodedMethod,
AppView<?> appView,
ValueNumberGenerator generator,
- Position callerPosition) {
+ Position callerPosition,
+ Map<Integer, DebugLocalInfo> parameterInfo) {
assert node.localVariables.isEmpty() || keepLocals(encodedMethod, appView.options());
JarSourceCode source =
new JarSourceCode(
@@ -196,7 +231,7 @@
appView.graphLense().getOriginalMethodSignature(encodedMethod.method),
callerPosition);
IRBuilder builder = new IRBuilder(encodedMethod, appView, source, origin, generator);
- return builder.build(context);
+ return builder.build(context, parameterInfo);
}
@Override
@@ -286,17 +321,19 @@
}
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.
- // If the method is reachability sensitive we have to include debug information in order
- // to get locals information which we need to extend the live ranges of locals for their
- // entire scope.
+ // If -keepparameternames is not specified and the keep attributes do not specify keeping
+ // either of LocalVariableTable, LocalVariableTypeTable or LineNumberTable, then we can skip
+ // parsing all the debug related attributes during code read. If the method is reachability
+ // sensitive we have to include debug information in order to get locals information which we
+ // need to extend the live ranges of locals for their entire scope.
int parsingOptions = ClassReader.SKIP_FRAMES;
- if (application.options.getProguardConfiguration() != null) {
+ ProguardConfiguration configuration = application.options.getProguardConfiguration();
+ if (configuration != null && !configuration.isKeepParameterNames()) {
ProguardKeepAttributes keep =
application.options.getProguardConfiguration().getKeepAttributes();
- if (!keep.localVariableTable
+ if (!application.options.getProguardConfiguration().isKeepParameterNames()
+ && !keep.localVariableTable
&& !keep.localVariableTypeTable
&& !keep.lineNumberTable
&& !reachabilitySensitive) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCode.java b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
index 9e045d9..3e1fed3 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCode.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
@@ -21,6 +21,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceArrayMap;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
@@ -89,6 +90,8 @@
// When numbering instructions we number instructions only with even numbers. This allows us to
// use odd instruction numbers for the insertion of moves during spilling.
public static final int INSTRUCTION_NUMBER_DELTA = 2;
+ public static final Map<Integer, DebugLocalInfo> NO_PARAMETER_INFO =
+ new Int2ReferenceArrayMap<>(0);
public final DexEncodedMethod method;
@@ -112,6 +115,8 @@
public final Origin origin;
+ public final Map<Integer, DebugLocalInfo> parameterInfo;
+
public IRCode(
InternalOptions options,
DexEncodedMethod method,
@@ -120,7 +125,8 @@
boolean hasDebugPositions,
boolean hasMonitorInstruction,
boolean hasConstString,
- Origin origin) {
+ Origin origin,
+ Map<Integer, DebugLocalInfo> parameterInfo) {
assert options != null;
this.options = options;
this.method = method;
@@ -130,8 +136,10 @@
this.hasMonitorInstruction = hasMonitorInstruction;
this.hasConstString = hasConstString;
this.origin = origin;
+ this.parameterInfo = parameterInfo;
// TODO(zerny): Remove or update this property now that all instructions have positions.
allThrowingInstructionsHavePositions = computeAllThrowingInstructionsHavePositions();
+ assert parameterInfo != null;
}
public void copyMetadataFromInlinee(IRCode inlinee) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
index bfc8352..ffe8806 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
@@ -268,6 +268,7 @@
CatchHandlers<BasicBlock> tryCatchHandlers = CatchHandlers.EMPTY_BASIC_BLOCK;
boolean previousFallthrough = false;
+ boolean firstBlock = true;
do {
CatchHandlers<BasicBlock> handlers = block.getCatchHandlers();
if (!tryCatchHandlers.equals(handlers)) {
@@ -309,6 +310,11 @@
assert !block.exit().isReturn() || stackHeightTracker.isEmpty();
+ if (firstBlock) {
+ addParameterNamesIfRequired(block, code.parameterInfo);
+ firstBlock = false;
+ }
+
block = nextBlock;
previousFallthrough = fallthrough;
} while (block != null);
@@ -625,6 +631,24 @@
}
}
+ private void addParameterNamesIfRequired(
+ BasicBlock block, Map<Integer, DebugLocalInfo> parameterInfo) {
+ // Don't add this information if the code already have full debug information.
+ if (appView.options().debug) {
+ return;
+ }
+
+ if (code.parameterInfo != IRCode.NO_PARAMETER_INFO) {
+ for (Map.Entry<Integer, DebugLocalInfo> entries : parameterInfo.entrySet()) {
+ LocalVariableInfo localVariableInfo =
+ new LocalVariableInfo(entries.getKey(), entries.getValue(), getLabel(block));
+ CfLabel endLabel = ensureLabel();
+ localVariableInfo.setEnd(endLabel);
+ localVariablesTable.add(localVariableInfo);
+ }
+ }
+ }
+
// Callbacks
public CfLabel getLabel(BasicBlock target) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index b287d0a..c3c2e8c 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -495,6 +495,19 @@
* @return The list of basic blocks. First block is the main entry.
*/
public IRCode build(DexEncodedMethod context) {
+ return build(context, IRCode.NO_PARAMETER_INFO);
+ }
+
+ /**
+ * Build the high-level IR in SSA form.
+ *
+ * @param context Under what context this IRCode is built. Either the current method or caller.
+ * @param parameterInfo Parameter information to include in the output. Pass <code>
+ * IRCode.NO_PARAMETER_INFO</code> if this is not relevant. This information is only used if
+ * the generated code does not contain any debug information.
+ * @return The list of basic blocks. First block is the main entry.
+ */
+ public IRCode build(DexEncodedMethod context, Map<Integer, DebugLocalInfo> parameterInfo) {
assert source != null;
source.setUp();
@@ -595,7 +608,8 @@
hasDebugPositions,
hasMonitorInstruction,
hasConstString,
- origin);
+ origin,
+ parameterInfo);
// Verify critical edges are split so we have a place to insert phi moves if necessary.
assert ir.verifySplitCriticalEdges();
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 ea6b9cb..7e41a90 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -72,6 +72,10 @@
public final DexItemFactory itemFactory;
+ public boolean hasProguardConfiguration() {
+ return proguardConfiguration != null;
+ }
+
public ProguardConfiguration getProguardConfiguration() {
return proguardConfiguration;
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/ConstantRemovalTest.java b/src/test/java/com/android/tools/r8/ir/optimize/ConstantRemovalTest.java
index d32606d..3284be0 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/ConstantRemovalTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/ConstantRemovalTest.java
@@ -141,7 +141,8 @@
false,
false,
false,
- Origin.unknown());
+ Origin.unknown(),
+ IRCode.NO_PARAMETER_INFO);
PeepholeOptimizer.optimize(code, new MockLinearScanRegisterAllocator(appView, code));
// Check that all four constant number instructions remain.
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/TrivialGotoEliminationTest.java b/src/test/java/com/android/tools/r8/ir/optimize/TrivialGotoEliminationTest.java
index c3e6823..40ac512 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/TrivialGotoEliminationTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/TrivialGotoEliminationTest.java
@@ -81,7 +81,8 @@
false,
false,
false,
- Origin.unknown());
+ Origin.unknown(),
+ IRCode.NO_PARAMETER_INFO);
CodeRewriter.collapseTrivialGotos(null, code);
assertTrue(code.entryBlock().isTrivialGoto());
assertTrue(blocks.contains(block0));
@@ -168,7 +169,8 @@
false,
false,
false,
- Origin.unknown());
+ Origin.unknown(),
+ IRCode.NO_PARAMETER_INFO);
CodeRewriter.collapseTrivialGotos(null, code);
assertTrue(block0.getInstructions().get(1).isIf());
assertEquals(block1, block0.getInstructions().get(1).asIf().fallthroughBlock());
diff --git a/src/test/java/com/android/tools/r8/shaking/keepparameternames/KeepParameterNamesTest.java b/src/test/java/com/android/tools/r8/shaking/keepparameternames/KeepParameterNamesTest.java
new file mode 100644
index 0000000..2818650
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/keepparameternames/KeepParameterNamesTest.java
@@ -0,0 +1,214 @@
+// Copyright (c) 2019, 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.keepparameternames;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestShrinkerBuilder;
+import com.android.tools.r8.utils.BooleanUtils;
+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.LocalVariableTable;
+import com.android.tools.r8.utils.codeinspector.LocalVariableTable.LocalVariableTableEntry;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.android.tools.r8.utils.codeinspector.TypeSubject;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class KeepParameterNamesTest extends TestBase {
+
+ private final TestParameters parameters;
+ private final boolean keepParameterNames;
+
+ @Parameterized.Parameters(name = "{0}, keepparameternames {1}")
+ public static Collection<Object[]> data() {
+ return buildParameters(getTestParameters().withCfRuntimes().build(), BooleanUtils.values());
+ }
+
+ public KeepParameterNamesTest(TestParameters parameters, boolean keepParameterNames) {
+ this.parameters = parameters;
+ this.keepParameterNames = keepParameterNames;
+ }
+
+ private void checkLocalVariable(
+ LocalVariableTableEntry localVariable,
+ int index,
+ String name,
+ TypeSubject type,
+ String signature) {
+ assertEquals(index, localVariable.index);
+ assertEquals(name, localVariable.name);
+ assertEquals(type, localVariable.type);
+ assertEquals(signature, localVariable.signature);
+ }
+
+ private void checkLocalVariableTable(CodeInspector inspector) {
+ ClassSubject classSubject = inspector.clazz(Api.class);
+ assertThat(classSubject, isPresent());
+
+ MethodSubject method;
+ method = classSubject.uniqueMethodWithName("apiNoArgs");
+ assertThat(method, isPresent());
+ assertEquals(keepParameterNames, method.hasLocalVariableTable());
+ if (keepParameterNames) {
+ LocalVariableTable localVariableTable = method.getLocalVariableTable();
+ assertEquals(1, localVariableTable.size());
+ assertEquals("this", localVariableTable.get(0).name);
+ assertTrue(localVariableTable.get(0).type.is(classSubject));
+ } else {
+ method.getLocalVariableTable().isEmpty();
+ }
+
+ method = classSubject.uniqueMethodWithName("apiNoArgsStatic");
+ assertThat(method, isPresent());
+ assertFalse(method.hasLocalVariableTable());
+
+ method = classSubject.uniqueMethodWithName("api1");
+ assertThat(method, isPresent());
+
+ assertEquals(keepParameterNames, method.hasLocalVariableTable());
+ if (keepParameterNames) {
+ LocalVariableTable localVariableTable = method.getLocalVariableTable();
+ assertEquals(3, localVariableTable.size());
+ checkLocalVariable(
+ localVariableTable.get(0),
+ 0,
+ "this",
+ classSubject.asFoundClassSubject().asTypeSybject(),
+ null);
+ checkLocalVariable(
+ localVariableTable.get(1), 1, "parameter1", inspector.getTypeSubject("int"), null);
+ checkLocalVariable(
+ localVariableTable.get(2),
+ 2,
+ "parameter2",
+ inspector.getTypeSubject("java.lang.String"),
+ null);
+ } else {
+ method.getLocalVariableTable().isEmpty();
+ }
+
+ method = classSubject.uniqueMethodWithName("api2");
+ assertThat(method, isPresent());
+ assertEquals(keepParameterNames, method.hasLocalVariableTable());
+ if (keepParameterNames) {
+ LocalVariableTable localVariableTable = method.getLocalVariableTable();
+ assertEquals(3, localVariableTable.size());
+ checkLocalVariable(
+ localVariableTable.get(0),
+ 0,
+ "this",
+ classSubject.asFoundClassSubject().asTypeSybject(),
+ null);
+ checkLocalVariable(
+ localVariableTable.get(1), 1, "parameter1", inspector.getTypeSubject("long"), null);
+ checkLocalVariable(
+ localVariableTable.get(2), 3, "parameter2", inspector.getTypeSubject("double"), null);
+ } else {
+ method.getLocalVariableTable().isEmpty();
+ }
+
+ method = classSubject.uniqueMethodWithName("api3");
+ assertThat(method, isPresent());
+ assertEquals(keepParameterNames, method.hasLocalVariableTable());
+ if (keepParameterNames) {
+ LocalVariableTable localVariableTable = method.getLocalVariableTable();
+ assertEquals(3, localVariableTable.size());
+ checkLocalVariable(
+ localVariableTable.get(0),
+ 0,
+ "this",
+ classSubject.asFoundClassSubject().asTypeSybject(),
+ null);
+ checkLocalVariable(
+ localVariableTable.get(1),
+ 1,
+ "parameter1",
+ inspector.getTypeSubject("java.util.List"),
+ "Ljava/util/List<Ljava/lang/String;>;");
+ checkLocalVariable(
+ localVariableTable.get(2),
+ 2,
+ "parameter2",
+ inspector.getTypeSubject("java.util.Map"),
+ "Ljava/util/Map<Ljava/lang/String;Ljava/lang/Object;>;");
+ } else {
+ method.getLocalVariableTable().isEmpty();
+ }
+ }
+
+ @Test
+ public void test() throws Exception {
+ String expectedOutput =
+ StringUtils.lines(
+ "In Api.apiNoArgs",
+ "In Api.apiNoArgsStatic",
+ "In Api.api1",
+ "In Api.api2",
+ "In Api.api3");
+ testForR8(parameters.getBackend())
+ .addInnerClasses(KeepParameterNamesTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addKeepRules("-keep class " + Api.class.getTypeName() + "{ api*(...); }")
+ .apply(this::configureKeepParameterNames)
+ .compile()
+ .disassemble()
+ .inspect(this::checkLocalVariableTable)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(expectedOutput);
+ }
+
+ private void configureKeepParameterNames(TestShrinkerBuilder builder) {
+ if (keepParameterNames) {
+ builder.addKeepRules("-keepparameternames");
+ }
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ Api api = new Api();
+ api.apiNoArgs();
+ Api.apiNoArgsStatic();
+ api.api1(0, "Hello, world!");
+ api.api2(0, 0.0d);
+ api.api3(null, null);
+ }
+ }
+
+ static class Api {
+ void apiNoArgs() {
+ System.out.println("In Api.apiNoArgs");
+ }
+
+ static void apiNoArgsStatic() {
+ System.out.println("In Api.apiNoArgsStatic");
+ }
+
+ void api1(int parameter1, String parameter2) {
+ System.out.println("In Api.api1");
+ }
+
+ void api2(long parameter1, double parameter2) {
+ System.out.println("In Api.api2");
+ }
+
+ void api3(List<String> parameter1, Map<String, Object> parameter2) {
+ System.out.println("In Api.api3");
+ }
+ }
+}
