[Retrace] Use the same procedure for looking up frames
This CL removes the custom building of mapped frames such that
ClassResult.lookupFrame is equal to Retrace.retraceFrame.
Bug: 201397823
Change-Id: Ifb8184936873a84d9121e3fd9c5850baa8e3c1b5
diff --git a/src/main/java/com/android/tools/r8/retrace/RetraceMethodResult.java b/src/main/java/com/android/tools/r8/retrace/RetraceMethodResult.java
index 805443c..f06fad3 100644
--- a/src/main/java/com/android/tools/r8/retrace/RetraceMethodResult.java
+++ b/src/main/java/com/android/tools/r8/retrace/RetraceMethodResult.java
@@ -5,9 +5,10 @@
package com.android.tools.r8.retrace;
import com.android.tools.r8.Keep;
+import java.util.OptionalInt;
@Keep
public interface RetraceMethodResult extends RetraceResult<RetraceMethodElement> {
- RetraceFrameResult narrowByPosition(RetraceStackTraceContext context, int position);
+ RetraceFrameResult narrowByPosition(RetraceStackTraceContext context, OptionalInt position);
}
diff --git a/src/main/java/com/android/tools/r8/retrace/Retracer.java b/src/main/java/com/android/tools/r8/retrace/Retracer.java
index 34d413b..293ac8e 100644
--- a/src/main/java/com/android/tools/r8/retrace/Retracer.java
+++ b/src/main/java/com/android/tools/r8/retrace/Retracer.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.references.MethodReference;
import com.android.tools.r8.references.TypeReference;
import com.android.tools.r8.retrace.internal.RetracerImpl;
+import java.util.OptionalInt;
/** This is the main api interface for retrace. */
@Keep
@@ -20,10 +21,10 @@
RetraceMethodResult retraceMethod(MethodReference methodReference);
- RetraceFrameResult retraceFrame(MethodReference methodReference, int position);
+ RetraceFrameResult retraceFrame(MethodReference methodReference, OptionalInt position);
RetraceFrameResult retraceFrame(
- MethodReference methodReference, int position, RetraceStackTraceContext context);
+ MethodReference methodReference, OptionalInt position, RetraceStackTraceContext context);
RetraceFieldResult retraceField(FieldReference fieldReference);
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/RetraceClassResultImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/RetraceClassResultImpl.java
index f94035b..f354bb3 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/RetraceClassResultImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/RetraceClassResultImpl.java
@@ -6,7 +6,6 @@
import com.android.tools.r8.naming.ClassNamingForNameMapper;
-import com.android.tools.r8.naming.ClassNamingForNameMapper.MappedRange;
import com.android.tools.r8.naming.ClassNamingForNameMapper.MappedRangesOfName;
import com.android.tools.r8.naming.MemberNaming;
import com.android.tools.r8.naming.mappinginformation.MappingInformation;
@@ -20,7 +19,6 @@
import com.android.tools.r8.retrace.RetraceStackTraceContext;
import com.android.tools.r8.retrace.RetraceUnknownJsonMappingInformationResult;
import com.android.tools.r8.retrace.RetracedSourceFile;
-import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.Pair;
import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
@@ -138,62 +136,17 @@
Reference.method(obfuscatedReference, methodName, formalTypes, returnType)));
}
+ private RetraceFrameResultImpl lookupFrame(
+ RetraceStackTraceContext context, OptionalInt position, MethodDefinition definition) {
+ return lookupMethod(definition).narrowByPosition(context, position);
+ }
+
@Override
public RetraceThrownExceptionResultImpl lookupThrownException(RetraceStackTraceContext context) {
return new RetraceThrownExceptionResultImpl(
(RetraceStackTraceContextImpl) context, obfuscatedReference, mapper);
}
- private RetraceFrameResultImpl lookupFrame(
- RetraceStackTraceContext context, OptionalInt position, MethodDefinition definition) {
- List<Pair<RetraceClassElementImpl, List<MappedRange>>> mappings = new ArrayList<>();
- internalStream()
- .forEach(
- element -> {
- getMappedRangesForFrame(element, definition, position)
- .forEach(
- mappedRanges -> {
- mappings.add(new Pair<>(element, mappedRanges));
- });
- });
- return new RetraceFrameResultImpl(
- this, mappings, definition, position, retracer, (RetraceStackTraceContextImpl) context);
- }
-
- private List<List<MappedRange>> getMappedRangesForFrame(
- RetraceClassElementImpl element, MethodDefinition definition, OptionalInt position) {
- List<List<MappedRange>> overloadedRanges = new ArrayList<>();
- if (mapper == null) {
- overloadedRanges.add(null);
- return overloadedRanges;
- }
- assert element.mapper != null;
- MappedRangesOfName mappedRanges = mapper.mappedRangesByRenamedName.get(definition.getName());
- if (mappedRanges == null || mappedRanges.getMappedRanges().isEmpty()) {
- overloadedRanges.add(null);
- return overloadedRanges;
- }
- List<MappedRange> mappedRangesForPosition = null;
- if (position.isPresent() && position.getAsInt() >= 0) {
- mappedRangesForPosition = mappedRanges.allRangesForLine(position.getAsInt(), false);
- }
- if (mappedRangesForPosition == null || mappedRangesForPosition.isEmpty()) {
- mappedRangesForPosition = mappedRanges.getMappedRanges();
- }
- assert mappedRangesForPosition != null && !mappedRangesForPosition.isEmpty();
- // Mapped ranges can have references to overloaded signatures. We distinguish those by looking
- // at the cardinal mapping range.
- for (MappedRange mappedRange : mappedRangesForPosition) {
- if (overloadedRanges.isEmpty()
- || mappedRange.originalRange == null
- || !mappedRange.originalRange.isCardinal) {
- overloadedRanges.add(new ArrayList<>());
- }
- ListUtils.last(overloadedRanges).add(mappedRange);
- }
- return overloadedRanges;
- }
-
@Override
public boolean hasRetraceResult() {
return mapper != null;
@@ -366,23 +319,7 @@
private RetraceFrameResultImpl lookupFrame(
RetraceStackTraceContext context, OptionalInt position, MethodDefinition definition) {
- MethodDefinition methodDefinition =
- MethodDefinition.create(classReference.getClassReference(), definition.getName());
- ImmutableList.Builder<Pair<RetraceClassElementImpl, List<MappedRange>>> builder =
- ImmutableList.builder();
- classResult
- .getMappedRangesForFrame(this, methodDefinition, position)
- .forEach(
- mappedRanges -> {
- builder.add(new Pair<>(this, mappedRanges));
- });
- return new RetraceFrameResultImpl(
- classResult,
- builder.build(),
- methodDefinition,
- position,
- classResult.retracer,
- (RetraceStackTraceContextImpl) context);
+ return classResult.lookupFrame(context, position, definition);
}
}
}
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/RetraceMethodResultImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/RetraceMethodResultImpl.java
index 978a933..178d359 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/RetraceMethodResultImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/RetraceMethodResultImpl.java
@@ -14,8 +14,8 @@
import com.android.tools.r8.retrace.RetracedMethodReference;
import com.android.tools.r8.retrace.RetracedSourceFile;
import com.android.tools.r8.retrace.internal.RetraceClassResultImpl.RetraceClassElementImpl;
+import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.Pair;
-import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.List;
import java.util.OptionalInt;
@@ -62,38 +62,40 @@
}
@Override
- public RetraceFrameResultImpl narrowByPosition(RetraceStackTraceContext context, int position) {
+ public RetraceFrameResultImpl narrowByPosition(
+ RetraceStackTraceContext context, OptionalInt position) {
List<Pair<RetraceClassElementImpl, List<MappedRange>>> narrowedRanges = new ArrayList<>();
- List<Pair<RetraceClassElementImpl, List<MappedRange>>> noMappingRanges = new ArrayList<>();
for (Pair<RetraceClassElementImpl, List<MappedRange>> mappedRange : mappedRanges) {
if (mappedRange.getSecond() == null) {
- noMappingRanges.add(new Pair<>(mappedRange.getFirst(), null));
+ narrowedRanges.add(new Pair<>(mappedRange.getFirst(), null));
continue;
}
- List<MappedRange> ranges =
- new MappedRangesOfName(mappedRange.getSecond()).allRangesForLine(position, false);
- boolean hasAddedRanges = false;
- if (!ranges.isEmpty()) {
- narrowedRanges.add(new Pair<>(mappedRange.getFirst(), ranges));
- hasAddedRanges = true;
- } else {
- narrowedRanges = new ArrayList<>();
- for (MappedRange mapped : mappedRange.getSecond()) {
- if (mapped.minifiedRange == null) {
- narrowedRanges.add(new Pair<>(mappedRange.getFirst(), ImmutableList.of(mapped)));
- hasAddedRanges = true;
- }
- }
+ MappedRangesOfName mappedRangesOfElement = new MappedRangesOfName(mappedRange.getSecond());
+ List<MappedRange> mappedRangesForPosition = null;
+ if (position.isPresent() && position.getAsInt() >= 0) {
+ mappedRangesForPosition =
+ mappedRangesOfElement.allRangesForLine(position.getAsInt(), false);
}
- if (!hasAddedRanges) {
- narrowedRanges.add(new Pair<>(mappedRange.getFirst(), null));
+ if (mappedRangesForPosition == null || mappedRangesForPosition.isEmpty()) {
+ mappedRangesForPosition = mappedRangesOfElement.getMappedRanges();
+ }
+ assert mappedRangesForPosition != null && !mappedRangesForPosition.isEmpty();
+ // Mapped ranges can have references to overloaded signatures. We distinguish those by looking
+ // at the cardinal mapping range.
+ for (MappedRange mappedRangeForPosition : mappedRangesForPosition) {
+ if (narrowedRanges.isEmpty()
+ || mappedRangeForPosition.originalRange == null
+ || !mappedRangeForPosition.originalRange.isCardinal) {
+ narrowedRanges.add(new Pair<>(mappedRange.getFirst(), new ArrayList<>()));
+ }
+ ListUtils.last(narrowedRanges).getSecond().add(mappedRangeForPosition);
}
}
return new RetraceFrameResultImpl(
classResult,
- narrowedRanges.isEmpty() ? noMappingRanges : narrowedRanges,
+ narrowedRanges,
methodDefinition,
- OptionalInt.of(position),
+ position,
retracer,
(RetraceStackTraceContextImpl) context);
}
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/RetracerImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/RetracerImpl.java
index 9aadbb3..563de26 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/RetracerImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/RetracerImpl.java
@@ -16,6 +16,7 @@
import com.android.tools.r8.retrace.RetraceStackTraceContext;
import com.android.tools.r8.retrace.Retracer;
import java.io.BufferedReader;
+import java.util.OptionalInt;
/** A default implementation for the retrace api using the ClassNameMapper defined in R8. */
public class RetracerImpl implements Retracer {
@@ -62,13 +63,13 @@
}
@Override
- public RetraceFrameResult retraceFrame(MethodReference methodReference, int position) {
+ public RetraceFrameResult retraceFrame(MethodReference methodReference, OptionalInt position) {
return retraceFrame(methodReference, position, RetraceStackTraceContext.empty());
}
@Override
public RetraceFrameResult retraceFrame(
- MethodReference methodReference, int position, RetraceStackTraceContext context) {
+ MethodReference methodReference, OptionalInt position, RetraceStackTraceContext context) {
return retraceClass(methodReference.getHolderClass())
.lookupMethod(methodReference.getMethodName())
.narrowByPosition(context, position);
diff --git a/src/test/java/com/android/tools/r8/retrace/api/RetraceApiSingleFrameTest.java b/src/test/java/com/android/tools/r8/retrace/api/RetraceApiSingleFrameTest.java
new file mode 100644
index 0000000..bec1a7f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/retrace/api/RetraceApiSingleFrameTest.java
@@ -0,0 +1,75 @@
+// Copyright (c) 2021, 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.retrace.api;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.DiagnosticsHandler;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.references.ClassReference;
+import com.android.tools.r8.references.Reference;
+import com.android.tools.r8.retrace.ProguardMapProducer;
+import com.android.tools.r8.retrace.RetraceFrameElement;
+import com.android.tools.r8.retrace.RetraceStackTraceContext;
+import com.android.tools.r8.retrace.RetracedMethodReference;
+import com.android.tools.r8.retrace.Retracer;
+import java.util.List;
+import java.util.OptionalInt;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class RetraceApiSingleFrameTest extends RetraceApiTestBase {
+
+ public RetraceApiSingleFrameTest(TestParameters parameters) {
+ super(parameters);
+ }
+
+ @Override
+ protected Class<? extends RetraceApiBinaryTest> binaryTestClass() {
+ return ApiTest.class;
+ }
+
+ public static class ApiTest implements RetraceApiBinaryTest {
+
+ private final ClassReference originalClass = Reference.classFromTypeName("some.Class");
+ private final ClassReference renamedClass = Reference.classFromTypeName("a");
+
+ private final String mapping =
+ originalClass.getTypeName()
+ + " -> "
+ + renamedClass.getTypeName()
+ + ":\n"
+ + " int strawberry(int):99:99 -> a\n";
+
+ @Test
+ public void testRetracingFrameEqualToNarrow() {
+ Retracer retracer =
+ Retracer.createDefault(
+ ProguardMapProducer.fromString(mapping), new DiagnosticsHandler() {});
+ checkResults(
+ retracer
+ .retraceFrame(
+ Reference.methodFromDescriptor(renamedClass, "a", "()I"), OptionalInt.empty())
+ .stream()
+ .collect(Collectors.toList()));
+ checkResults(
+ retracer
+ .retraceClass(renamedClass)
+ .lookupFrame(RetraceStackTraceContext.empty(), OptionalInt.empty(), "a")
+ .stream()
+ .collect(Collectors.toList()));
+ }
+
+ private void checkResults(List<RetraceFrameElement> elements) {
+ assertEquals(1, elements.size());
+ RetracedMethodReference topFrame = elements.get(0).getTopFrame();
+ assertEquals("strawberry", topFrame.getMethodName());
+ assertEquals(99, topFrame.getOriginalPositionOrDefault(-1));
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
index 4e2b27d..cf9ce0a 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/InstructionSubject.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.retrace.RetraceMethodResult;
import com.android.tools.r8.retrace.RetraceStackTraceContext;
import com.android.tools.r8.retrace.Retracer;
+import java.util.OptionalInt;
public interface InstructionSubject {
@@ -163,11 +164,13 @@
}
default RetraceFrameResult retraceLinePosition(Retracer retracer) {
- return retrace(retracer).narrowByPosition(RetraceStackTraceContext.empty(), getLineNumber());
+ return retrace(retracer)
+ .narrowByPosition(RetraceStackTraceContext.empty(), OptionalInt.of(getLineNumber()));
}
default RetraceFrameResult retracePcPosition(Retracer retracer, MethodSubject methodSubject) {
return retrace(retracer)
- .narrowByPosition(RetraceStackTraceContext.empty(), getOffset(methodSubject).offset);
+ .narrowByPosition(
+ RetraceStackTraceContext.empty(), OptionalInt.of(getOffset(methodSubject).offset));
}
}
diff --git a/third_party/retrace/binary_compatibility.tar.gz.sha1 b/third_party/retrace/binary_compatibility.tar.gz.sha1
index 6b09100..1295509 100644
--- a/third_party/retrace/binary_compatibility.tar.gz.sha1
+++ b/third_party/retrace/binary_compatibility.tar.gz.sha1
@@ -1 +1 @@
-e7ea0fbb97ccfb2faa89b58d60f8b54f5bca0130
\ No newline at end of file
+e69bc2aa67c9ef4d89849a5554dd16383813a9ea
\ No newline at end of file
