Do not apply the graph lens to positions.
The positions are always in their original position.
Bug: 140851070
Change-Id: I79ec34f4aeaf88f818ef58778aead9f24cc41a6e
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index bb772d8..17ee9af 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -6,6 +6,8 @@
import static com.android.tools.r8.R8Command.USAGE_MESSAGE;
import static com.android.tools.r8.utils.ExceptionUtils.unwrapExecutionException;
+import com.android.tools.r8.cf.code.CfInstruction;
+import com.android.tools.r8.cf.code.CfPosition;
import com.android.tools.r8.dex.ApplicationReader;
import com.android.tools.r8.dex.ApplicationWriter;
import com.android.tools.r8.dex.Marker;
@@ -16,10 +18,15 @@
import com.android.tools.r8.graph.AppServices;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.AppliedGraphLens;
+import com.android.tools.r8.graph.CfCode;
+import com.android.tools.r8.graph.Code;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexCode;
+import com.android.tools.r8.graph.DexDebugEvent;
import com.android.tools.r8.graph.DexDefinition;
import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.DexType;
@@ -694,7 +701,8 @@
executorService,
timing)
.withEnumValueInfoMaps(enumValueInfoMapCollection));
-
+ // Rerunning the enqueuer should not give rise to any method rewritings.
+ assert enqueuer.buildGraphLense(appView) == appView.graphLense();
appView.withGeneratedMessageLiteBuilderShrinker(
shrinker ->
shrinker.rewriteDeadBuilderReferencesFromDynamicMethods(
@@ -810,6 +818,8 @@
new KotlinMetadataRewriter(appView, namingLens).run(executorService);
timing.end();
+ assert verifyMovedMethodsHaveOriginalMethodPosition(appView, application);
+
timing.begin("Line number remapping");
// When line number optimization is turned off the identity mapping for line numbers is
// used. We still run the line number optimizer to collect line numbers and inline frame
@@ -893,6 +903,58 @@
}
}
+ private static boolean verifyMovedMethodsHaveOriginalMethodPosition(
+ AppView<?> appView, DirectMappedDexApplication application) {
+ application
+ .classes()
+ .forEach(
+ clazz -> {
+ clazz.forEachProgramMethod(
+ method -> {
+ DexMethod originalMethod =
+ appView.graphLense().getOriginalMethodSignature(method.getReference());
+ if (originalMethod != method.getReference()) {
+ DexMethod originalMethod2 =
+ appView.graphLense().getOriginalMethodSignature(method.getReference());
+ appView.graphLense().getOriginalMethodSignature(method.getReference());
+ DexEncodedMethod definition = method.getDefinition();
+ Code code = definition.getCode();
+ if (code == null) {
+ return;
+ }
+ if (code.isCfCode()) {
+ assert verifyOriginalMethodInPosition(code.asCfCode(), originalMethod);
+ } else {
+ assert code.isDexCode();
+ assert verifyOriginalMethodInDebugInfo(code.asDexCode(), originalMethod);
+ }
+ }
+ });
+ });
+ return true;
+ }
+
+ private static boolean verifyOriginalMethodInPosition(CfCode code, DexMethod originalMethod) {
+ for (CfInstruction instruction : code.instructions) {
+ if (!instruction.isPosition()) {
+ continue;
+ }
+ CfPosition position = instruction.asPosition();
+ assert position.getPosition().getOutermostCaller().method == originalMethod;
+ }
+ return true;
+ }
+
+ private static boolean verifyOriginalMethodInDebugInfo(DexCode code, DexMethod originalMethod) {
+ if (code.getDebugInfo() == null) {
+ return true;
+ }
+ for (DexDebugEvent event : code.getDebugInfo().events) {
+ assert !event.isSetInlineFrame() || event.asSetInlineFrame().hasOuterPosition(originalMethod);
+ }
+ return true;
+ }
+
private AppView<AppInfoWithLiveness> runEnqueuer(
AnnotationRemover.Builder annotationRemoverBuilder,
ExecutorService executorService,
@@ -917,6 +979,7 @@
options.getProguardConfiguration().getDontWarnPatterns(),
executorService,
timing));
+ appView.setGraphLense(enqueuer.buildGraphLense(appView));
if (InternalOptions.assertionsEnabled()) {
// Register the dead proto types. These are needed to verify that no new missing types are
// reported and that no dead proto types are referenced in the generated application.
diff --git a/src/main/java/com/android/tools/r8/graph/DexDebugEvent.java b/src/main/java/com/android/tools/r8/graph/DexDebugEvent.java
index f790131..059f7f6 100644
--- a/src/main/java/com/android/tools/r8/graph/DexDebugEvent.java
+++ b/src/main/java/com/android/tools/r8/graph/DexDebugEvent.java
@@ -38,6 +38,14 @@
public abstract void accept(DexDebugEventVisitor visitor);
+ public boolean isSetInlineFrame() {
+ return false;
+ }
+
+ public SetInlineFrame asSetInlineFrame() {
+ return null;
+ }
+
public static class AdvancePC extends DexDebugEvent {
public final int delta;
@@ -421,6 +429,21 @@
SetInlineFrame o = (SetInlineFrame) other;
return callee == o.callee && Objects.equals(caller, o.caller);
}
+
+ @Override
+ public boolean isSetInlineFrame() {
+ return true;
+ }
+
+ @Override
+ public SetInlineFrame asSetInlineFrame() {
+ return this;
+ }
+
+ public boolean hasOuterPosition(DexMethod method) {
+ return (caller == null && callee == method)
+ || (caller != null && caller.getOutermostCaller().method == method);
+ }
}
public static class Default extends DexDebugEvent {
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 5cbe06a..2269517 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -36,6 +36,7 @@
import com.android.tools.r8.dex.MixedSectionCollection;
import com.android.tools.r8.errors.InternalCompilerError;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.graph.DexDebugEvent.SetInlineFrame;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Invoke;
import com.android.tools.r8.ir.code.ValueType;
@@ -858,6 +859,36 @@
}
}
+ public static void setOriginalMethodPosition(Code code, DexMethod originalMethod) {
+ if (code.isDexCode()) {
+ DexCode dexCode = code.asDexCode();
+ DexDebugInfo debugInfo = dexCode.getDebugInfo();
+ if (debugInfo == null) {
+ return;
+ }
+ for (DexDebugEvent event : debugInfo.events) {
+ if (event.isSetInlineFrame() && event.asSetInlineFrame().hasOuterPosition(originalMethod)) {
+ return;
+ }
+ }
+ DexDebugEvent[] newEvents = new DexDebugEvent[debugInfo.events.length + 1];
+ newEvents[0] = new SetInlineFrame(originalMethod, null);
+ System.arraycopy(debugInfo.events, 0, newEvents, 1, debugInfo.events.length);
+ dexCode.setDebugInfo(new DexDebugInfo(debugInfo.startLine, debugInfo.parameters, newEvents));
+ } else {
+ assert code.isCfCode();
+ CfCode cfCode = code.asCfCode();
+ for (CfInstruction instruction : cfCode.instructions) {
+ if (instruction.isPosition()) {
+ assert instruction.asPosition().getPosition().getOutermostCaller().method
+ == originalMethod;
+ return;
+ }
+ }
+ assert false : "The original position should be present in the CF code.";
+ }
+ }
+
private DexEncodedMethod toMethodThatLogsErrorDexCode(DexItemFactory itemFactory) {
checkIfObsolete();
Signature signature = MethodSignature.fromDexMethod(method);
diff --git a/src/main/java/com/android/tools/r8/ir/code/Position.java b/src/main/java/com/android/tools/r8/ir/code/Position.java
index 6ed94b4..d11bf38 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Position.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Position.java
@@ -85,9 +85,8 @@
assert position.isNone();
position = Position.noneWithMethod(context.getReference(), null);
}
- assert position.callerPosition == null
- || position.getOutermostCaller().method
- == appView.graphLense().getOriginalMethodSignature(context.getReference());
+ assert position.getOutermostCaller().method
+ == appView.graphLense().getOriginalMethodSignature(context.getReference());
return position;
}
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 00eb737..9055230 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
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.conversion;
+import static com.android.tools.r8.ir.conversion.CfSourceUtils.ensureLabel;
+
import com.android.tools.r8.cf.CfRegisterAllocator;
import com.android.tools.r8.cf.LoadStoreHelper;
import com.android.tools.r8.cf.TypeVerificationHelper;
@@ -322,7 +324,7 @@
} while (block != null);
// TODO(mkroghj) Move computation of stack-height to CF instructions.
if (!openLocalVariables.isEmpty()) {
- CfLabel endLabel = ensureLabel();
+ CfLabel endLabel = ensureLabel(instructions);
for (LocalVariableInfo info : openLocalVariables.values()) {
info.setEnd(endLabel);
localVariablesTable.add(info);
@@ -460,7 +462,7 @@
}
} else {
if (instruction.isNewInstance()) {
- newInstanceLabels.put(instruction.asNewInstance(), ensureLabel());
+ newInstanceLabels.put(instruction.asNewInstance(), ensureLabel(instructions));
}
updatePositionAndLocals(instruction);
instruction.buildCf(this);
@@ -481,7 +483,7 @@
if (!didLocalsChange && !didPositionChange) {
return;
}
- CfLabel label = ensureLabel();
+ CfLabel label = ensureLabel(instructions);
if (didLocalsChange) {
updateLocals(label);
}
@@ -526,20 +528,6 @@
return pendingLocalChanges;
}
- private CfLabel ensureLabel() {
- CfInstruction last = getLastInstruction();
- if (last instanceof CfLabel) {
- return (CfLabel) last;
- }
- CfLabel label = new CfLabel();
- add(label);
- return label;
- }
-
- private CfInstruction getLastInstruction() {
- return instructions.isEmpty() ? null : instructions.get(instructions.size() - 1);
- }
-
private void addFrame(BasicBlock block) {
List<TypeInfo> stack = registerAllocator.getTypesAtBlockEntry(block).stack;
List<FrameType> stackTypes;
@@ -570,7 +558,7 @@
// CfCode.
boolean didLocalsChange = localsChanged();
if (didLocalsChange) {
- CfLabel label = ensureLabel();
+ CfLabel label = ensureLabel(instructions);
updateLocals(label);
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceUtils.java b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceUtils.java
new file mode 100644
index 0000000..9c3a2f1
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceUtils.java
@@ -0,0 +1,26 @@
+// 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.ir.conversion;
+
+import com.android.tools.r8.cf.code.CfInstruction;
+import com.android.tools.r8.cf.code.CfLabel;
+import java.util.List;
+
+public class CfSourceUtils {
+
+ public static CfLabel ensureLabel(List<CfInstruction> instructions) {
+ CfInstruction last = getLastInstruction(instructions);
+ if (last != null && last.isLabel()) {
+ return last.asLabel();
+ }
+ CfLabel label = new CfLabel();
+ instructions.add(label);
+ return label;
+ }
+
+ private static CfInstruction getLastInstruction(List<CfInstruction> instructions) {
+ return instructions.isEmpty() ? null : instructions.get(instructions.size() - 1);
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
index a277a71..2b8ce66 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/InterfaceProcessor.java
@@ -4,6 +4,8 @@
package com.android.tools.r8.ir.desugar;
+import static com.android.tools.r8.graph.DexEncodedMethod.setOriginalMethodPosition;
+
import com.android.tools.r8.cf.code.CfInstruction;
import com.android.tools.r8.cf.code.CfInvoke;
import com.android.tools.r8.code.Instruction;
@@ -94,6 +96,12 @@
newFlags.promoteToStatic();
DexEncodedMethod.setDebugInfoWithFakeThisParameter(
code, companionMethod.getArity(), appView);
+ if (!appView.options().isDesugaredLibraryCompilation()) {
+ setOriginalMethodPosition(
+ code, appView.graphLense().getOriginalMethodSignature(virtual.method));
+ } else {
+ assert appView.graphLense().isIdentityLense();
+ }
DexEncodedMethod implMethod =
new DexEncodedMethod(
companionMethod,
@@ -128,13 +136,18 @@
if (originalFlags.isPrivate()) {
newFlags.promoteToPublic();
}
-
DexMethod oldMethod = direct.method;
if (isStaticMethod(direct)) {
assert originalFlags.isPrivate() || originalFlags.isPublic()
: "Static interface method " + direct.toSourceString() + " is expected to "
+ "either be public or private in " + iface.origin;
DexMethod companionMethod = rewriter.staticAsMethodOfCompanionClass(oldMethod);
+ if (!appView.options().isDesugaredLibraryCompilation()) {
+ setOriginalMethodPosition(
+ direct.getCode(), appView.graphLense().getOriginalMethodSignature(oldMethod));
+ } else {
+ assert appView.graphLense().isIdentityLense();
+ }
DexEncodedMethod implMethod =
new DexEncodedMethod(
companionMethod,
@@ -162,6 +175,12 @@
}
DexEncodedMethod.setDebugInfoWithFakeThisParameter(
code, companionMethod.getArity(), appView);
+ if (!appView.options().isDesugaredLibraryCompilation()) {
+ setOriginalMethodPosition(
+ code, appView.graphLense().getOriginalMethodSignature(oldMethod));
+ } else {
+ assert appView.graphLense().isIdentityLense();
+ }
DexEncodedMethod implMethod =
new DexEncodedMethod(
companionMethod,
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
index d203a12..e40e0e4 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaClass.java
@@ -670,8 +670,7 @@
encodedMethod -> {
assert encodedMethod.isDirectMethod();
// We need to create a new method with the same code to be able to safely relax
- // its
- // accessibility and make it virtual.
+ // its accessibility and make it virtual.
MethodAccessFlags newAccessFlags = encodedMethod.accessFlags.copy();
newAccessFlags.unsetPrivate();
newAccessFlags.setPublic();
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
index 8a7b983..fb66a2a 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
@@ -7,10 +7,13 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexApplication.Builder;
import com.android.tools.r8.graph.DexCallSite;
+import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.GraphLense;
+import com.android.tools.r8.graph.GraphLense.NestedGraphLense;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.analysis.type.Nullability;
import com.android.tools.r8.ir.analysis.type.TypeAnalysis;
@@ -29,6 +32,7 @@
import com.android.tools.r8.utils.collections.SortedProgramMethodSet;
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import java.util.ArrayList;
@@ -342,4 +346,35 @@
public Map<DexType, LambdaClass> getKnownLambdaClasses() {
return knownLambdaClasses;
}
+
+ public GraphLense buildMappingLense(AppView<?> appView) {
+ if (originalMethodSignatures.isEmpty()) {
+ return appView.graphLense();
+ }
+ return new LambdaRewriterLense(
+ originalMethodSignatures, appView.graphLense(), appView.dexItemFactory());
+ }
+
+ static class LambdaRewriterLense extends NestedGraphLense {
+
+ public LambdaRewriterLense(
+ BiMap<DexMethod, DexMethod> originalMethodSignatures,
+ GraphLense graphLense,
+ DexItemFactory factory) {
+ super(
+ ImmutableMap.of(),
+ ImmutableMap.of(),
+ ImmutableMap.of(),
+ null,
+ originalMethodSignatures,
+ graphLense,
+ factory);
+ }
+
+ @Override
+ protected boolean isLegitimateToHaveEmptyMappings() {
+ return true;
+ }
+ }
}
+
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index e033e78..5b53c1f 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -57,6 +57,7 @@
import com.android.tools.r8.graph.FieldAccessInfoCollectionImpl;
import com.android.tools.r8.graph.FieldAccessInfoImpl;
import com.android.tools.r8.graph.FieldResolutionResult;
+import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.InnerClassAttribute;
import com.android.tools.r8.graph.LookupLambdaTarget;
import com.android.tools.r8.graph.LookupTarget;
@@ -2715,6 +2716,12 @@
return appInfoWithLiveness;
}
+ public GraphLense buildGraphLense(AppView<?> appView) {
+ return lambdaRewriter != null
+ ? lambdaRewriter.buildMappingLense(appView)
+ : appView.graphLense();
+ }
+
private void keepClassWithRules(DexProgramClass clazz, Set<ProguardKeepRuleBase> rules) {
keepInfo.joinClass(clazz, info -> applyKeepRules(rules, info));
}
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 552cc23..6907805 100644
--- a/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
+++ b/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
@@ -378,12 +378,8 @@
signatures.put(originalMethod, originalSignature);
Function<DexMethod, MethodSignature> getOriginalMethodSignature =
m -> {
- DexMethod original = appView.graphLense().getOriginalMethodSignature(m);
return signatures.computeIfAbsent(
- original,
- key ->
- MethodSignature.fromDexMethod(
- original, original.holder != clazz.getType()));
+ m, key -> MethodSignature.fromDexMethod(m, m.holder != clazz.getType()));
};
MemberNaming memberNaming = new MemberNaming(originalSignature, obfuscatedName);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentsCollisionMappingTest.java b/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentsCollisionMappingTest.java
index 5e7a17d..30f5c93 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentsCollisionMappingTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentsCollisionMappingTest.java
@@ -9,11 +9,11 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import com.android.tools.r8.CompilationMode;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.R8TestRunResult;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
@@ -28,20 +28,25 @@
public class UnusedArgumentsCollisionMappingTest extends TestBase {
private final TestParameters parameters;
+ private final CompilationMode compilationMode;
- @Parameters(name = "{0}")
- public static TestParametersCollection data() {
- return getTestParameters().withAllRuntimesAndApiLevels().build();
+ @Parameters(name = "{0}, compilation mode: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withAllRuntimesAndApiLevels().build(), CompilationMode.values());
}
- public UnusedArgumentsCollisionMappingTest(TestParameters parameters) {
+ public UnusedArgumentsCollisionMappingTest(
+ TestParameters parameters, CompilationMode compilationMode) {
this.parameters = parameters;
+ this.compilationMode = compilationMode;
}
@Test
public void testR8() throws Exception {
R8TestRunResult runResult =
testForR8(parameters.getBackend())
+ .setMode(compilationMode)
.addProgramClasses(Main.class)
.setMinApi(parameters.getApiLevel())
.addKeepMainRule(Main.class)
@@ -50,14 +55,13 @@
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("test with unused", "test with used: foo")
.inspect(this::verifyArgumentsRemoved);
- // TODO(b/140851070): Duplicate entries for different methods.
assertEquals(
- 2,
+ 1,
StringUtils.splitLines(runResult.proguardMap()).stream()
.filter(line -> line.contains("void test(java.lang.String,java.lang.String)"))
.count());
assertEquals(
- 0,
+ 1,
StringUtils.splitLines(runResult.proguardMap()).stream()
.filter(line -> line.contains("void test(java.lang.String)"))
.count());
