[Retrace] Narrowing by non-mapped position should give empty result
Fixes: 201982044
Bug: 202055473
Change-Id: Ie1018b08cc8bc8de5dec501a4d598b583d331089
diff --git a/src/main/java/com/android/tools/r8/retrace/RetraceClassResult.java b/src/main/java/com/android/tools/r8/retrace/RetraceClassResult.java
index 9f5bc71..c2bd31a 100644
--- a/src/main/java/com/android/tools/r8/retrace/RetraceClassResult.java
+++ b/src/main/java/com/android/tools/r8/retrace/RetraceClassResult.java
@@ -12,8 +12,6 @@
@Keep
public interface RetraceClassResult extends RetraceResult<RetraceClassElement> {
- boolean hasRetraceResult();
-
RetraceFieldResult lookupField(String fieldName);
RetraceFieldResult lookupField(String fieldName, TypeReference fieldType);
diff --git a/src/main/java/com/android/tools/r8/retrace/RetraceResult.java b/src/main/java/com/android/tools/r8/retrace/RetraceResult.java
index 1bc6ed6..7ff9c4e 100644
--- a/src/main/java/com/android/tools/r8/retrace/RetraceResult.java
+++ b/src/main/java/com/android/tools/r8/retrace/RetraceResult.java
@@ -31,4 +31,6 @@
default void forEach(Consumer<E> action) {
stream().forEach(action);
}
+
+ boolean isEmpty();
}
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 f354bb3..0d834a3 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
@@ -148,7 +148,7 @@
}
@Override
- public boolean hasRetraceResult() {
+ public boolean isEmpty() {
return mapper != null;
}
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/RetraceFieldResultImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/RetraceFieldResultImpl.java
index 38667c4..33abecc 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/RetraceFieldResultImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/RetraceFieldResultImpl.java
@@ -92,6 +92,11 @@
return mappings.size() > 1;
}
+ @Override
+ public boolean isEmpty() {
+ return memberNamings == null || memberNamings.isEmpty();
+ }
+
public static class ElementImpl implements RetraceFieldElement {
private final RetracedFieldReferenceImpl fieldReference;
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/RetraceFrameResultImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/RetraceFrameResultImpl.java
index d3b75bc..ee295e2 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/RetraceFrameResultImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/RetraceFrameResultImpl.java
@@ -219,6 +219,11 @@
() -> mappedRange.getOriginalLineNumber(obfuscatedPosition.getAsInt())));
}
+ @Override
+ public boolean isEmpty() {
+ return !mappedRanges.isEmpty();
+ }
+
public static class ElementImpl implements RetraceFrameElement {
private final RetracedMethodReferenceImpl methodReference;
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 10ce889..60a6967 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
@@ -65,6 +65,11 @@
}
@Override
+ public boolean isEmpty() {
+ return mappedRanges == null || mappedRanges.isEmpty();
+ }
+
+ @Override
public RetraceFrameResultImpl narrowByPosition(
RetraceStackTraceContext context, OptionalInt position) {
List<Pair<RetraceClassElementImpl, List<MappedRange>>> narrowedRanges = new ArrayList<>();
@@ -79,36 +84,43 @@
}
MappedRangesOfName mappedRangesOfElement = new MappedRangesOfName(mappedRange.getSecond());
List<MappedRange> mappedRangesForPosition = null;
- if (position.isPresent() && position.getAsInt() >= 0) {
+ boolean hasPosition = position.isPresent() && position.getAsInt() >= 0;
+ if (hasPosition) {
mappedRangesForPosition =
mappedRangesOfElement.allRangesForLine(position.getAsInt(), false);
}
if (mappedRangesForPosition == null || mappedRangesForPosition.isEmpty()) {
- mappedRangesForPosition = mappedRangesOfElement.getMappedRanges();
- } else if (stackTraceContext != null && stackTraceContext.hasRewritePosition()) {
- List<OutlineCallsiteMappingInformation> outlineCallsiteInformation =
- ListUtils.last(mappedRangesForPosition).getOutlineCallsiteInformation();
- if (!outlineCallsiteInformation.isEmpty()) {
- assert outlineCallsiteInformation.size() == 1
- : "There can only be one outline entry for a line";
- return narrowByPosition(
- stackTraceContext.buildFromThis().clearRewritePosition().build(),
- OptionalInt.of(
- outlineCallsiteInformation
- .get(0)
- .rewritePosition(stackTraceContext.getRewritePosition())));
- }
+ mappedRangesForPosition =
+ hasPosition
+ ? ListUtils.filter(
+ mappedRangesOfElement.getMappedRanges(), range -> range.minifiedRange == null)
+ : 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<>()));
+ if (mappedRangesForPosition != null && !mappedRangesForPosition.isEmpty()) {
+ if (stackTraceContext != null && stackTraceContext.hasRewritePosition()) {
+ List<OutlineCallsiteMappingInformation> outlineCallsiteInformation =
+ ListUtils.last(mappedRangesForPosition).getOutlineCallsiteInformation();
+ if (!outlineCallsiteInformation.isEmpty()) {
+ assert outlineCallsiteInformation.size() == 1
+ : "There can only be one outline entry for a line";
+ return narrowByPosition(
+ stackTraceContext.buildFromThis().clearRewritePosition().build(),
+ OptionalInt.of(
+ outlineCallsiteInformation
+ .get(0)
+ .rewritePosition(stackTraceContext.getRewritePosition())));
+ }
}
- ListUtils.last(narrowedRanges).getSecond().add(mappedRangeForPosition);
+ // 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(
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/RetraceThrownExceptionResultImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/RetraceThrownExceptionResultImpl.java
index aa2c1f0..bb49d70 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/RetraceThrownExceptionResultImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/RetraceThrownExceptionResultImpl.java
@@ -34,6 +34,11 @@
return Stream.of(createElement());
}
+ @Override
+ public boolean isEmpty() {
+ return obfuscatedReference == null;
+ }
+
private RetraceThrownExceptionElement createElement() {
return new RetraceThrownExceptionElementImpl(
this,
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/StackTraceElementProxyRetracerImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/StackTraceElementProxyRetracerImpl.java
index 5623bf2..987aead 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/StackTraceElementProxyRetracerImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/StackTraceElementProxyRetracerImpl.java
@@ -131,6 +131,25 @@
? OptionalInt.of(element.getLineNumber())
: OptionalInt.empty(),
element.getMethodName());
+ if (!frameResult.isEmpty()) {
+ return classResult.stream()
+ .map(
+ classElement ->
+ proxy
+ .builder()
+ .setTopFrame(true)
+ .joinAmbiguous(classResult.isAmbiguous())
+ .setRetracedClass(classElement.getRetracedClass())
+ .applyIf(
+ element.hasLineNumber(),
+ b -> b.setLineNumber(element.getLineNumber()))
+ .apply(
+ setSourceFileOnProxy(
+ classElement::getSourceFile,
+ classElement.getRetracedClass(),
+ classResult))
+ .build());
+ }
return frameResult.stream()
.flatMap(
frameElement -> {
@@ -230,9 +249,7 @@
retracedSourceFile.hasRetraceResult()
? retracedSourceFile.getSourceFile()
: RetraceUtils.inferSourceFile(
- classReference.getTypeName(),
- original.getSourceFile(),
- classResult.hasRetraceResult()));
+ classReference.getTypeName(), original.getSourceFile(), classResult.isEmpty()));
};
}
diff --git a/src/test/java/com/android/tools/r8/retrace/StackTraceRegularExpressionParserTests.java b/src/test/java/com/android/tools/r8/retrace/StackTraceRegularExpressionParserTests.java
index fc0aaea..b769752 100644
--- a/src/test/java/com/android/tools/r8/retrace/StackTraceRegularExpressionParserTests.java
+++ b/src/test/java/com/android/tools/r8/retrace/StackTraceRegularExpressionParserTests.java
@@ -517,12 +517,12 @@
@Override
public List<String> retracedStackTrace() {
- return ImmutableList.of("com.android.tools.r8.R8.foo(7)");
+ return ImmutableList.of("com.android.tools.r8.R8.a(42)");
}
@Override
public List<String> retraceVerboseStackTrace() {
- return ImmutableList.of("com.android.tools.r8.R8.boolean foo()(7)");
+ return ImmutableList.of("com.android.tools.r8.R8.a(42)");
}
@Override
diff --git a/src/test/java/com/android/tools/r8/retrace/api/RetraceApiOutsideLineRangeTest.java b/src/test/java/com/android/tools/r8/retrace/api/RetraceApiOutsideLineRangeTest.java
index 714719a..a3ba721 100644
--- a/src/test/java/com/android/tools/r8/retrace/api/RetraceApiOutsideLineRangeTest.java
+++ b/src/test/java/com/android/tools/r8/retrace/api/RetraceApiOutsideLineRangeTest.java
@@ -61,8 +61,7 @@
Retracer retracer =
Retracer.createDefault(
ProguardMapProducer.fromString(mapping), new DiagnosticsHandler() {});
- // TODO(b/201982044): Should be the empty set.
- retraceClassMethodAndPosition(retracer, someClassRenamed, someClassOriginal, 2, 4);
+ retraceClassMethodAndPosition(retracer, someClassRenamed, someClassOriginal, 2, 0);
retraceClassMethodAndPosition(retracer, someOtherClassRenamed, someOtherClassOriginal, 1, 1);
}
diff --git a/src/test/java/com/android/tools/r8/retrace/stacktraces/OutsideLineRangeStackTraceTest.java b/src/test/java/com/android/tools/r8/retrace/stacktraces/OutsideLineRangeStackTraceTest.java
index c6f7509..81587cf 100644
--- a/src/test/java/com/android/tools/r8/retrace/stacktraces/OutsideLineRangeStackTraceTest.java
+++ b/src/test/java/com/android/tools/r8/retrace/stacktraces/OutsideLineRangeStackTraceTest.java
@@ -34,41 +34,26 @@
public List<String> retracedStackTrace() {
return Arrays.asList(
"java.io.IOException: INVALID_SENDER",
- // TODO(b/201982044): Should not be ambiguous since b.a(:27) should be b.a(:27)
"\tat some.other.Class.method1(Class.java:42)",
"\tat some.other.Class.method1(Class.java:42)",
- "\tat some.Class.method2(Class.java:10)",
- "\tat some.Class.method2(Class.java)",
- "<OR> java.io.IOException: INVALID_SENDER",
- "\tat some.other.Class.method1(Class.java:42)",
- "\tat some.other.Class.method1(Class.java:42)",
- "\tat some.Class.method2(Class.java:27)",
+ "\tat some.Class.a(Class.java:27)",
"\tat some.Class.method2(Class.java)");
}
@Override
public List<String> retraceVerboseStackTrace() {
return Arrays.asList(
- "There are 4 ambiguous stack traces.",
+ "There are 2 ambiguous stack traces.",
"java.io.IOException: INVALID_SENDER",
"\tat some.other.Class.void method1()(Class.java:42)",
"\tat some.other.Class.void method1()(Class.java:42)",
- "\tat some.Class.void method2()(Class.java:10)",
+ "\tat some.Class.a(Class.java:27)",
+ // TODO(b/202055473): Could emit range 11-13.
"\tat some.Class.void method2()(Class.java)",
"<OR> java.io.IOException: INVALID_SENDER",
"\tat some.other.Class.void method1()(Class.java:42)",
"\tat some.other.Class.void method1()(Class.java:42)",
- "\tat some.Class.void method2()(Class.java:10)",
- "\tat some.Class.void method2()(Class.java:10)",
- "<OR> java.io.IOException: INVALID_SENDER",
- "\tat some.other.Class.void method1()(Class.java:42)",
- "\tat some.other.Class.void method1()(Class.java:42)",
- "\tat some.Class.void method2()(Class.java:27)",
- "\tat some.Class.void method2()(Class.java)",
- "<OR> java.io.IOException: INVALID_SENDER",
- "\tat some.other.Class.void method1()(Class.java:42)",
- "\tat some.other.Class.void method1()(Class.java:42)",
- "\tat some.Class.void method2()(Class.java:27)",
+ "\tat some.Class.a(Class.java:27)",
"\tat some.Class.void method2()(Class.java:10)");
}
diff --git a/third_party/retrace/binary_compatibility.tar.gz.sha1 b/third_party/retrace/binary_compatibility.tar.gz.sha1
index 67554d1..f81f5fc 100644
--- a/third_party/retrace/binary_compatibility.tar.gz.sha1
+++ b/third_party/retrace/binary_compatibility.tar.gz.sha1
@@ -1 +1 @@
-ef1d22b8680bd9bc362c017a5641524ffef4d22e
\ No newline at end of file
+239b7652dc8282715373a1871987c3919df029d6
\ No newline at end of file
