Move vertically merged method from parent by inlining positions
Change-Id: Ic61e81df691d0b38343671295ed50e6eb24817bd
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 9c3131f..f80b0d8 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -31,6 +31,7 @@
import com.android.tools.r8.graph.GenericSignatureCorrectnessHelper;
import com.android.tools.r8.graph.GraphLens;
import com.android.tools.r8.graph.ProgramDefinition;
+import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.PrunedItems;
import com.android.tools.r8.graph.SubtypingInfo;
import com.android.tools.r8.graph.analysis.ClassInitializerAssertionEnablingAnalysis;
@@ -943,10 +944,7 @@
return;
}
if (code.isCfCode() || code.isDexCode()) {
- code.forEachPositionOrInlineFrame(
- position -> {
- assert position.getMethod() == originalMethod;
- });
+ assert verifyOriginalMethodInPosition(code, originalMethod, method);
} else {
assert code.isDefaultInstanceInitializerCode() || code.isThrowNullCode();
}
@@ -955,6 +953,27 @@
return true;
}
+ private static boolean verifyOriginalMethodInPosition(
+ Code code, DexMethod originalMethod, ProgramMethod context) {
+ code.forEachPosition(
+ position -> {
+ if (position.isOutlineCaller()) {
+ // Check the outlined positions for the original method
+ position
+ .getOutlinePositions()
+ .forEach(
+ (ignored, outlinePosition) -> {
+ assert outlinePosition.hasMethodInChain(originalMethod);
+ });
+ } else if (context.getDefinition().isD8R8Synthesized()) {
+ assert position.hasMethodInChain(originalMethod);
+ } else {
+ assert position.getOutermostCaller().getMethod() == originalMethod;
+ }
+ });
+ return true;
+ }
+
private AppView<AppInfoWithLiveness> runEnqueuer(
AnnotationRemover.Builder annotationRemoverBuilder,
ExecutorService executorService,
diff --git a/src/main/java/com/android/tools/r8/graph/AppliedGraphLens.java b/src/main/java/com/android/tools/r8/graph/AppliedGraphLens.java
index 1d05d43..f360949 100644
--- a/src/main/java/com/android/tools/r8/graph/AppliedGraphLens.java
+++ b/src/main/java/com/android/tools/r8/graph/AppliedGraphLens.java
@@ -62,7 +62,7 @@
// Record original method signatures.
for (DexEncodedMethod encodedMethod : clazz.methods()) {
DexMethod method = encodedMethod.getReference();
- DexMethod original = appView.graphLens().getOriginalMethodSignature(method);
+ DexMethod original = appView.graphLens().getOriginalMethodSignatureForMapping(method);
DexMethod existing = originalMethodSignatures.inverse().get(original);
if (existing == null) {
originalMethodSignatures.put(method, original);
diff --git a/src/main/java/com/android/tools/r8/graph/CfCode.java b/src/main/java/com/android/tools/r8/graph/CfCode.java
index acad41b..fff7a6e 100644
--- a/src/main/java/com/android/tools/r8/graph/CfCode.java
+++ b/src/main/java/com/android/tools/r8/graph/CfCode.java
@@ -975,7 +975,7 @@
}
@Override
- public void forEachPositionOrInlineFrame(Consumer<Position> positionConsumer) {
+ public void forEachPosition(Consumer<Position> positionConsumer) {
for (CfInstruction instruction : getInstructions()) {
if (instruction.isPosition()) {
positionConsumer.accept(instruction.asPosition().getPosition());
diff --git a/src/main/java/com/android/tools/r8/graph/Code.java b/src/main/java/com/android/tools/r8/graph/Code.java
index 11a95cd..bc0c72b 100644
--- a/src/main/java/com/android/tools/r8/graph/Code.java
+++ b/src/main/java/com/android/tools/r8/graph/Code.java
@@ -191,7 +191,7 @@
Position callerPosition, Position oldPosition, boolean isCalleeD8R8Synthesized) {
Position outermostCaller = oldPosition.getOutermostCaller();
if (!isCalleeD8R8Synthesized) {
- return oldPosition.withOutermostCallerPosition(callerPosition);
+ return removeSameMethodAndLineZero(oldPosition, callerPosition);
}
// We can replace the position since the callee was synthesized by the compiler, however, if
// the position carries special information we need to copy it.
@@ -210,7 +210,22 @@
return oldPosition.replacePosition(outermostCaller, positionBuilder.build());
}
- public void forEachPositionOrInlineFrame(Consumer<Position> positionConsumer) {
+ @Deprecated()
+ // TODO(b/261971803): When having complete control over the positions we should not need this.
+ private static Position removeSameMethodAndLineZero(
+ Position calleePosition, Position callerPosition) {
+ Position outermostCaller = calleePosition.getOutermostCaller();
+ if (outermostCaller.getLine() == 0) {
+ while (callerPosition != null
+ && outermostCaller.getMethod() == callerPosition.getMethod()
+ && callerPosition.getLine() == 0) {
+ callerPosition = callerPosition.getCallerPosition();
+ }
+ }
+ return calleePosition.withOutermostCallerPosition(callerPosition);
+ }
+
+ public void forEachPosition(Consumer<Position> positionConsumer) {
// Intentionally empty. Override where we have fully build CF or DEX code.
}
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexCode.java b/src/main/java/com/android/tools/r8/graph/DexCode.java
index 8e65622..60ce8b5 100644
--- a/src/main/java/com/android/tools/r8/graph/DexCode.java
+++ b/src/main/java/com/android/tools/r8/graph/DexCode.java
@@ -777,7 +777,7 @@
}
@Override
- public void forEachPositionOrInlineFrame(Consumer<Position> positionConsumer) {
+ public void forEachPosition(Consumer<Position> positionConsumer) {
if (getDebugInfo() == null || getDebugInfo().isPcBasedInfo()) {
return;
}
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLens.java b/src/main/java/com/android/tools/r8/graph/GraphLens.java
index 46d91ef..43558a8 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLens.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLens.java
@@ -329,6 +329,18 @@
return original;
}
+ public final DexMethod getOriginalMethodSignatureForMapping(DexMethod method) {
+ GraphLens current = this;
+ DexMethod original = method;
+ while (current.isNonIdentityLens()) {
+ NonIdentityGraphLens nonIdentityLens = current.asNonIdentityLens();
+ original = nonIdentityLens.getPreviousMethodSignatureForMapping(original);
+ current = nonIdentityLens.getPrevious();
+ }
+ assert current.isIdentityLens();
+ return original;
+ }
+
public final DexField getRenamedFieldSignature(DexField originalField) {
return getRenamedFieldSignature(originalField, null);
}
@@ -971,6 +983,15 @@
public abstract DexMethod getPreviousMethodSignature(DexMethod method);
+ /***
+ * The previous mapping for a method often coincides with the previous method signature, but it
+ * may not, for example for bridges inserted in vertically merged classes where the original
+ * signature is used for computing invoke-super but should not be used for mapping output.
+ */
+ public DexMethod getPreviousMethodSignatureForMapping(DexMethod method) {
+ return getPreviousMethodSignature(method);
+ }
+
public abstract DexMethod getNextMethodSignature(DexMethod method);
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/code/Invoke.java b/src/main/java/com/android/tools/r8/ir/code/Invoke.java
index 80bb77b..a5dcdae 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Invoke.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Invoke.java
@@ -94,11 +94,6 @@
}
public static Type fromInvokeSpecial(
- DexMethod invokedMethod, DexClassAndMethod context, AppView<?> appView) {
- return fromInvokeSpecial(invokedMethod, context, appView, appView.codeLens());
- }
-
- public static Type fromInvokeSpecial(
DexMethod invokedMethod,
DexClassAndMethod context,
AppView<?> appView,
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 c3a705e..4c809dc 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
@@ -64,6 +64,10 @@
return false;
}
+ public boolean isOutlineCaller() {
+ return false;
+ }
+
public DexMethod getOutlineCallee() {
return null;
}
@@ -131,8 +135,17 @@
assert position.isNone();
position = SourcePosition.builder().setMethod(context.getReference()).build();
}
- assert position.getOutermostCaller().method
- == appView.graphLens().getOriginalMethodSignature(context.getReference());
+ if (context.getDefinition().isD8R8Synthesized()) {
+ // Some rewritings map a synthetic method back to an original in the program. To ensure we
+ // have correct line information we have to rewrite the positions as inline position
+ // therefore we only check if the original method is present.
+ DexMethod originalMethodSignature =
+ appView.graphLens().getOriginalMethodSignature(context.getReference());
+ assert position.hasMethodInChain(originalMethodSignature);
+ } else {
+ assert position.getOutermostCaller().method
+ == appView.graphLens().getOriginalMethodSignature(context.getReference());
+ }
return position;
}
@@ -178,6 +191,21 @@
return null;
}
+ public boolean hasPositionMatching(Predicate<Position> positionPredicate) {
+ Position lastPosition = this;
+ while (lastPosition != null) {
+ if (positionPredicate.test(lastPosition)) {
+ return true;
+ }
+ lastPosition = lastPosition.getCallerPosition();
+ }
+ return false;
+ }
+
+ public boolean hasMethodInChain(DexMethod method) {
+ return hasPositionMatching(position -> position.getMethod() == method);
+ }
+
public Position withOutermostCallerPosition(Position newOutermostCallerPosition) {
return builderWithCopy()
.setCallerPosition(
@@ -566,6 +594,11 @@
}
@Override
+ public boolean isOutlineCaller() {
+ return true;
+ }
+
+ @Override
public DexMethod getOutlineCallee() {
return outlineCallee;
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
index ea0f655..7adbf67 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
@@ -258,10 +258,10 @@
}
private Position getCanonicalPositionAppendCaller(DexDebugEntry entry) {
- // If this instruction has already been inlined then this.method must be the outermost caller.
+ // If this instruction has already been inlined then the original method must be in the caller
+ // chain.
Position position = entry.getPosition();
- assert !position.hasCallerPosition()
- || position.getOutermostCaller().getMethod() == originalMethod;
+ assert position.hasMethodInChain(originalMethod);
return canonicalPositions.getCanonical(
position
.builderWithCopy()
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 657845b..c621494 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
@@ -753,7 +753,11 @@
.setGenericSignature(encodedMethod.getGenericSignature())
.setAnnotations(encodedMethod.annotations())
.setParameterAnnotations(encodedMethod.parameterAnnotationsList)
- .setCode(encodedMethod.getCode())
+ .setCode(
+ encodedMethod
+ .getCode()
+ .getCodeAsInlining(
+ callTarget, encodedMethod, appView.dexItemFactory()))
.setApiLevelForDefinition(encodedMethod.getApiLevelForDefinition())
.setApiLevelForCode(encodedMethod.getApiLevelForCode())
.build();
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java b/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java
index 8e0d7d4..f9c13e9 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/SyntheticSourceCode.java
@@ -70,6 +70,7 @@
.setLine(0)
.setMethod(originalMethod)
.setCallerPosition(callerPosition)
+ .setIsD8R8Synthesized(true)
.build();
}
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index b479808..32878da 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -65,6 +65,7 @@
import com.android.tools.r8.graph.classmerging.VerticallyMergedClasses;
import com.android.tools.r8.graph.proto.RewrittenPrototypeDescription;
import com.android.tools.r8.ir.code.Invoke.Type;
+import com.android.tools.r8.ir.code.Position.SyntheticPosition;
import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
import com.android.tools.r8.ir.optimize.MemberPoolCollection.MemberPool;
import com.android.tools.r8.ir.optimize.MethodPoolCollection;
@@ -2389,7 +2390,16 @@
.setTarget(invocationTarget)
.setInvokeType(type)
.setIsInterface(isInterface);
- return forwardSourceCodeBuilder::build;
+ return (context, callerPosition) -> {
+ SyntheticPosition caller =
+ SyntheticPosition.builder()
+ .setLine(0)
+ .setMethod(method)
+ .setIsD8R8Synthesized(true)
+ .setCallerPosition(callerPosition)
+ .build();
+ return forwardSourceCodeBuilder.build(context, caller);
+ };
}
@Override
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLens.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLens.java
index 05837f0..d0f940b 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLens.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMergerGraphLens.java
@@ -153,6 +153,12 @@
}
@Override
+ public DexMethod getPreviousMethodSignatureForMapping(DexMethod method) {
+ DexMethod orDefault = newMethodSignatures.getRepresentativeKeyOrDefault(method, method);
+ return super.getPreviousMethodSignature(orDefault);
+ }
+
+ @Override
protected Type mapInvocationType(DexMethod newMethod, DexMethod originalMethod, Type type) {
return mapVirtualInterfaceInvocationTypes(appView, newMethod, originalMethod, type);
}
diff --git a/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java b/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
index 189367d..c0a65b6 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
@@ -192,7 +192,7 @@
boolean canUseDexPc) {
DexEncodedMethod definition = method.getDefinition();
DexMethod originalMethod =
- appView.graphLens().getOriginalMethodSignature(method.getReference());
+ appView.graphLens().getOriginalMethodSignatureForMapping(method.getReference());
MethodSignature originalSignature =
MethodSignature.fromDexMethod(originalMethod, originalMethod.holder != originalType);
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/VerticalClassMergingRetraceTest.java b/src/test/java/com/android/tools/r8/naming/retrace/VerticalClassMergingRetraceTest.java
index 9e361e6..5b4dcb2 100644
--- a/src/test/java/com/android/tools/r8/naming/retrace/VerticalClassMergingRetraceTest.java
+++ b/src/test/java/com/android/tools/r8/naming/retrace/VerticalClassMergingRetraceTest.java
@@ -66,15 +66,6 @@
return retracedStackTraceLine.lineNumber > 0;
}
- private boolean filterSynthesizedBridgeMethod(StackTraceLine retracedStackTraceLine) {
- if (!haveSeenLines.add(retracedStackTraceLine)) {
- return false;
- }
- return !retracedStackTraceLine.className.contains("ResourceWrapper")
- || !retracedStackTraceLine.methodName.contains("foo")
- || retracedStackTraceLine.lineNumber != 0;
- }
-
@Test
public void testSourceFileAndLineNumberTable() throws Exception {
runTest(
diff --git a/src/test/java/com/android/tools/r8/naming/retraceproguard/VerticalClassMergingRetraceTest.java b/src/test/java/com/android/tools/r8/naming/retraceproguard/VerticalClassMergingRetraceTest.java
deleted file mode 100644
index 7e6306d..0000000
--- a/src/test/java/com/android/tools/r8/naming/retraceproguard/VerticalClassMergingRetraceTest.java
+++ /dev/null
@@ -1,166 +0,0 @@
-// 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.naming.retraceproguard;
-
-import static com.android.tools.r8.naming.retraceproguard.StackTrace.isSameExceptForFileName;
-import static com.android.tools.r8.naming.retraceproguard.StackTrace.isSameExceptForFileNameAndLineNumber;
-import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
-import static com.android.tools.r8.utils.codeinspector.Matchers.onlyIf;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assume.assumeTrue;
-
-import com.android.tools.r8.CompilationMode;
-import com.android.tools.r8.NeverInline;
-import com.android.tools.r8.R8TestBuilder;
-import com.android.tools.r8.R8TestCompileResult;
-import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.ToolHelper.DexVm.Version;
-import com.android.tools.r8.naming.retraceproguard.StackTrace.StackTraceLine;
-import com.android.tools.r8.utils.BooleanUtils;
-import com.android.tools.r8.utils.Box;
-import com.android.tools.r8.utils.codeinspector.ClassSubject;
-import com.android.tools.r8.utils.codeinspector.MethodSubject;
-import com.google.common.collect.ImmutableList;
-import java.io.IOException;
-import java.util.Collection;
-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 VerticalClassMergingRetraceTest extends RetraceTestBase {
-
- @Parameters(name = "{0}, mode: {1}, compat: {2}")
- public static Collection<Object[]> data() {
- return buildParameters(
- getTestParameters()
- .withCfRuntimes()
- // Runtimes prior to 8 will emit stack trace lines as:
- // Exception in thread "main" java.lang.NullPointerException: throw with null exception
- // at com.android.tools.r8.naming.retraceproguard.a.b(SourceFile)
- // PG do not support retracing if no line number is specified.
- .withDexRuntimesStartingFromIncluding(Version.V8_1_0)
- .withAllApiLevels()
- .build(),
- CompilationMode.values(),
- BooleanUtils.values());
- }
-
- public VerticalClassMergingRetraceTest(
- TestParameters parameters, CompilationMode mode, boolean compat) {
- super(parameters, mode, compat);
- }
-
- @Override
- public void configure(R8TestBuilder builder) {
- builder.enableInliningAnnotations();
- }
-
- @Override
- public Collection<Class<?>> getClasses() {
- return ImmutableList.of(getMainClass(), ResourceWrapper.class, TintResources.class);
- }
-
- @Override
- public Class<?> getMainClass() {
- return MainApp.class;
- }
-
- private int expectedActualStackTraceHeight() {
- // In RELEASE mode, a synthetic bridge will be added by vertical class merger.
- int height = mode == CompilationMode.RELEASE ? 3 : 2;
- if (parameters.isDexRuntime() && parameters.getDexRuntimeVersion().isDalvik()) {
- // Dalvik places a stack trace line in the bottom.
- height += 1;
- }
- return height;
- }
-
- private boolean filterSynthesizedMethod(
- StackTraceLine retracedStackTraceLine, MethodSubject syntheticMethod) {
- if (syntheticMethod.isPresent()) {
- String qualifiedMethodName =
- retracedStackTraceLine.className + "." + retracedStackTraceLine.methodName;
- return !qualifiedMethodName.equals(syntheticMethod.getOriginalName())
- || retracedStackTraceLine.lineNumber > 0;
- }
- return true;
- }
-
- @Test
- public void testSourceFileAndLineNumberTable() throws Exception {
- Box<MethodSubject> syntheticMethod = new Box<>();
- runTest(
- ImmutableList.of("-keepattributes SourceFile,LineNumberTable"),
- (StackTrace actualStackTrace, StackTrace retracedStackTrace) -> {
- // Even when SourceFile is present retrace replaces the file name in the stack trace.
- StackTrace reprocessedStackTrace =
- retracedStackTrace.filter(
- stackTraceLine -> filterSynthesizedMethod(stackTraceLine, syntheticMethod.get()));
- assertThat(
- reprocessedStackTrace.filter(this::isNotDalvikNativeStartMethod),
- isSameExceptForFileName(
- expectedStackTrace.filter(this::isNotDalvikNativeStartMethod)));
- assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
- },
- compileResult -> setSyntheticMethod(compileResult, syntheticMethod));
- }
-
- @Test
- public void testLineNumberTableOnly() throws Exception {
- assumeTrue(compat);
- assumeTrue(parameters.isDexRuntime());
- Box<MethodSubject> syntheticMethod = new Box<>();
- runTest(
- ImmutableList.of("-keepattributes LineNumberTable"),
- (StackTrace actualStackTrace, StackTrace retracedStackTrace) -> {
- StackTrace reprocessedStackTrace =
- retracedStackTrace.filter(
- stackTraceLine -> filterSynthesizedMethod(stackTraceLine, syntheticMethod.get()));
- assertThat(
- reprocessedStackTrace.filter(this::isNotDalvikNativeStartMethod),
- isSameExceptForFileName(
- expectedStackTrace.filter(this::isNotDalvikNativeStartMethod)));
- assertEquals(expectedActualStackTraceHeight(), actualStackTrace.size());
- },
- compileResult -> setSyntheticMethod(compileResult, syntheticMethod));
- }
-
- private void setSyntheticMethod(
- R8TestCompileResult compileResult, Box<MethodSubject> syntheticMethod) throws IOException {
- compileResult.inspect(
- inspector -> {
- ClassSubject tintResourcesClassSubject = inspector.clazz(TintResources.class);
- MethodSubject uniqueSyntheticMethod =
- tintResourcesClassSubject.uniqueMethodThatMatches(
- method -> method.getAccessFlags().isSynthetic());
- assertThat(uniqueSyntheticMethod, onlyIf(mode == CompilationMode.RELEASE, isPresent()));
- syntheticMethod.set(uniqueSyntheticMethod);
- });
- }
-}
-
-class ResourceWrapper {
- // Will be merged down, and represented as:
- // java.lang.String ...ResourceWrapper.foo() -> a
- @NeverInline
- String foo(boolean doThrow) {
- if (doThrow) {
- throw null;
- }
- return System.currentTimeMillis() > 0 ? "arg" : null;
- }
-}
-
-class TintResources extends ResourceWrapper {}
-
-class MainApp {
- public static void main(String[] args) {
- TintResources t = new TintResources();
- boolean doThrow = System.currentTimeMillis() > 0;
- System.out.println(t.foo(doThrow));
- }
-}
