Reland "Extend kotlin inline function retracing with callee positions"
This reverts commit ae88d453bfe4e9101fa8e837e8e11641d132a0ea.
Bug: 185358363
Change-Id: I60e67381a920a09de2153de7e506e5fa91564893
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinSourceDebugExtensionParser.java b/src/main/java/com/android/tools/r8/kotlin/KotlinSourceDebugExtensionParser.java
index 256ee1e..065ff92 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinSourceDebugExtensionParser.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinSourceDebugExtensionParser.java
@@ -25,6 +25,7 @@
private static final String SMAP_IDENTIFIER = "SMAP";
private static final String SMAP_SECTION_START = "*S";
private static final String SMAP_SECTION_KOTLIN_START = SMAP_SECTION_START + " Kotlin";
+ private static final String SMAP_SECTION_KOTLIN_DEBUG_START = SMAP_SECTION_START + " KotlinDebug";
private static final String SMAP_FILES_IDENTIFIER = "*F";
private static final String SMAP_LINES_IDENTIFIER = "*L";
private static final String SMAP_END_IDENTIFIER = "*E";
@@ -84,13 +85,13 @@
readUntil(terminator::equals, linesInBlock, callback);
}
- void readUntil(
+ String readUntil(
Predicate<String> terminator,
int linesInBlock,
ThrowingConsumer<List<String>, KotlinSourceDebugExtensionParserException> callback)
throws IOException, KotlinSourceDebugExtensionParserException {
if (terminator.test(readLine)) {
- return;
+ return readLine;
}
List<String> readStrings = new ArrayList<>();
readStrings.add(readNextLine());
@@ -106,10 +107,11 @@
}
readStrings.add(readNextLine());
}
- if (readStrings.size() > 0 && !terminator.test(readStrings.get(0))) {
+ if (!readStrings.isEmpty() && !terminator.test(readStrings.get(0))) {
throw new KotlinSourceDebugExtensionParserException(
"Block size does not match linesInBlock = " + linesInBlock);
}
+ return readStrings.isEmpty() ? null : readStrings.get(0);
}
@Override
@@ -132,14 +134,17 @@
// + <file_id_i> <file_name_i>
// <path_i>
// *L
- // <from>#<file>,<to>:<debug-line-position>
+ // <from>#<file>,<range>:<debug-line-position>
// <from>#<file>:<debug-line-position>
// *E <-- This is an error in versions prior to kotlin 1.5 and is not present in kotlin 1.5.
// *S KotlinDebug
- // ***
+ // + <file_id_i> <file_name_i>
+ // <path_i>
+ // *L
+ // <from>#<file>,<range>:<debug-line-position>
+ // <from>#<file>:<debug-line-position>
// *E
try (BufferedStringReader reader = new BufferedStringReader(annotationData)) {
- ResultBuilder builder = new ResultBuilder();
// Check for SMAP
if (!reader.readExpectedLine(SMAP_IDENTIFIER)) {
return null;
@@ -147,42 +152,67 @@
if (reader.readUntil(SMAP_SECTION_KOTLIN_START).isEOF()) {
return null;
}
- // At this point we should be parsing a kotlin source debug extension, so we will throw if we
- // read an unexpected line.
- reader.readExpectedLineOrThrow(SMAP_FILES_IDENTIFIER);
- // Iterate over the files section with the format:
- // + <file_number_i> <file_name_i>
- // <file_path_i>
- reader.readUntil(
- SMAP_LINES_IDENTIFIER, 2, block -> addFileToBuilder(block.get(0), block.get(1), builder));
- // Ensure that we read *L.
- if (reader.isEOF()) {
- throw new KotlinSourceDebugExtensionParserException(
- "Unexpected EOF - no debug line positions");
+ StratumBuilder inlineePositions = new StratumBuilder();
+ StratumBuilder calleePositions = new StratumBuilder();
+
+ String terminatedLine = parseStratumContents(reader, inlineePositions);
+
+ // Read callee positions
+ if (terminatedLine.equals(SMAP_END_IDENTIFIER)) {
+ String nextLine = reader.readNextLine();
+ // If we read to the end then there is not KotlinDebug section which translates to no need
+ // for mapping the resulting positions obtained from the inlineePositions.
+ if (reader.isEOF()) {
+ assert nextLine == null;
+ return new Result(inlineePositions.segmentTree, calleePositions.segmentTree);
+ }
+ if (!nextLine.equals(SMAP_SECTION_KOTLIN_DEBUG_START)) {
+ return null;
+ }
+ } else if (!terminatedLine.equals(SMAP_SECTION_KOTLIN_DEBUG_START)) {
+ return null;
}
- // Iterate over the debug line number positions:
- // <from>#<file>,<range>:<debug-line-position>
- // or
- // <from>#<file>:<debug-line-position>
- reader.readUntil(
- line -> line.equals(SMAP_END_IDENTIFIER) || line.startsWith(SMAP_SECTION_START),
- 1,
- block -> addDebugEntryToBuilder(block.get(0), builder));
+ parseStratumContents(reader, calleePositions);
// Ensure that we read the end section and it is terminated.
if (reader.isEOF() && !reader.readLine.equals(SMAP_END_IDENTIFIER)) {
throw new KotlinSourceDebugExtensionParserException(
"Unexpected EOF when parsing SMAP debug entries");
}
-
- return builder.build();
+ return new Result(inlineePositions.segmentTree, calleePositions.segmentTree);
} catch (IOException | KotlinSourceDebugExtensionParserException e) {
return null;
}
}
- private static void addFileToBuilder(String entryLine, String filePath, ResultBuilder builder)
+ private static String parseStratumContents(BufferedStringReader reader, StratumBuilder builder)
+ throws KotlinSourceDebugExtensionParserException, IOException {
+ // At this point we should be parsing a kotlin source debug extension, so we will throw if we
+ // read an unexpected line.
+ reader.readExpectedLineOrThrow(SMAP_FILES_IDENTIFIER);
+ // Iterate over the files section with the format:
+ // + <file_number_i> <file_name_i>
+ // <file_path_i>
+ reader.readUntil(
+ SMAP_LINES_IDENTIFIER, 2, block -> addFileToBuilder(block.get(0), block.get(1), builder));
+
+ // Ensure that we read *L.
+ if (reader.isEOF()) {
+ throw new KotlinSourceDebugExtensionParserException(
+ "Unexpected EOF - no debug line positions");
+ }
+ // Iterate over the debug line number positions:
+ // <from>#<file>,<range>:<debug-line-position>
+ // or
+ // <from>#<file>:<debug-line-position>
+ return reader.readUntil(
+ line -> line.equals(SMAP_END_IDENTIFIER) || line.startsWith(SMAP_SECTION_START),
+ 1,
+ block -> addDebugEntryToBuilder(block.get(0), builder));
+ }
+
+ private static void addFileToBuilder(String entryLine, String filePath, StratumBuilder builder)
throws KotlinSourceDebugExtensionParserException {
// + <file_number_i> <file_name_i>
// <file_path_i>
@@ -221,22 +251,30 @@
return number;
}
- private static void addDebugEntryToBuilder(String debugEntry, ResultBuilder builder)
+ private static void addDebugEntryToBuilder(String debugEntry, StratumBuilder builder)
throws KotlinSourceDebugExtensionParserException {
- // <from>#<file>,<size>:<debug-line-position>
+ // <from>#<file>,<size>:<debug-line-position>,<size>
// or
// <from>#<file>:<debug-line-position>
// All positions should define intervals for mappings.
try {
+ int size = 1;
int targetSplit = debugEntry.indexOf(':');
- int target = Integer.parseInt(debugEntry.substring(targetSplit + 1));
+ int targetRangeStart = targetSplit + 1;
+ int targetRangeSeparator = debugEntry.indexOf(',', targetSplit);
+ int target;
+ if (targetRangeSeparator > -1) {
+ target = Integer.parseInt(debugEntry.substring(targetRangeStart, targetRangeSeparator));
+ size = Integer.parseInt(debugEntry.substring(targetRangeSeparator + 1));
+ } else {
+ target = Integer.parseInt(debugEntry.substring(targetRangeStart));
+ }
String original = debugEntry.substring(0, targetSplit);
int fileIndexSplit = original.indexOf('#');
int originalStart = Integer.parseInt(original.substring(0, fileIndexSplit));
// The range may have a different end than start.
String fileAndEndRange = original.substring(fileIndexSplit + 1);
int endRangeCharPosition = fileAndEndRange.indexOf(',');
- int size = 1;
if (endRangeCharPosition > -1) {
// The file should be at least one number wide.
assert endRangeCharPosition > 0;
@@ -260,29 +298,31 @@
public static class Result {
- private final SegmentTree<Position> segmentTree;
+ private final SegmentTree<Position> inlineePositions;
+ private final SegmentTree<Position> calleePositions;
- public Result(SegmentTree<Position> segmentTree) {
- this.segmentTree = segmentTree;
+ public Result(SegmentTree<Position> inlineePositions, SegmentTree<Position> calleePositions) {
+ this.inlineePositions = inlineePositions;
+ this.calleePositions = calleePositions;
}
- public Map.Entry<Integer, Position> lookup(int point) {
- return segmentTree.findEntry(point);
+ public Map.Entry<Integer, Position> lookupInlinedPosition(int point) {
+ return inlineePositions.findEntry(point);
}
- public int size() {
- return segmentTree.size();
+ public Map.Entry<Integer, Position> lookupCalleePosition(int point) {
+ return calleePositions.findEntry(point);
+ }
+
+ public int inlinePositionsCount() {
+ return inlineePositions.size();
}
}
- public static class ResultBuilder {
+ public static class StratumBuilder {
SegmentTree<Position> segmentTree = new SegmentTree<>(false);
Map<Integer, Source> files = new HashMap<>();
-
- public Result build() {
- return new Result(segmentTree);
- }
}
public static class Source {
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 da825d1..ec1b7d0 100644
--- a/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
+++ b/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
@@ -60,6 +60,7 @@
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Objects;
import java.util.function.Function;
import java.util.function.Supplier;
@@ -143,17 +144,17 @@
if (parsedData == null) {
return baseRemapper.createRemappedPosition(position);
}
- Map.Entry<Integer, KotlinSourceDebugExtensionParser.Position> currentPosition =
- parsedData.lookup(line);
- if (currentPosition == null) {
+ Map.Entry<Integer, KotlinSourceDebugExtensionParser.Position> inlinedPosition =
+ parsedData.lookupInlinedPosition(line);
+ if (inlinedPosition == null) {
return baseRemapper.createRemappedPosition(position);
}
- int delta = line - currentPosition.getKey();
- int originalPosition = currentPosition.getValue().getRange().from + delta;
+ int inlineeLineDelta = line - inlinedPosition.getKey();
+ int originalInlineeLine = inlinedPosition.getValue().getRange().from + inlineeLineDelta;
try {
- String binaryName = currentPosition.getValue().getSource().getPath();
+ String binaryName = inlinedPosition.getValue().getSource().getPath();
String nameAndDescriptor =
- lineToMethodMapper.lookupNameAndDescriptor(binaryName, originalPosition);
+ lineToMethodMapper.lookupNameAndDescriptor(binaryName, originalInlineeLine);
if (nameAndDescriptor == null) {
return baseRemapper.createRemappedPosition(position);
}
@@ -173,8 +174,20 @@
factory.createString(returnTypeDescriptor),
argumentDexStringDescriptors);
if (!inlinee.equals(position.method)) {
+ // We have an inline from a different method than the current position.
+ Entry<Integer, KotlinSourceDebugExtensionParser.Position> calleePosition =
+ parsedData.lookupCalleePosition(line);
+ if (calleePosition != null) {
+ // Take the first line as the callee position
+ position =
+ new Position(
+ calleePosition.getValue().getRange().from,
+ position.file,
+ position.method,
+ position.callerPosition);
+ }
return baseRemapper.createRemappedPosition(
- new Position(originalPosition, null, inlinee, position));
+ new Position(originalInlineeLine, null, inlinee, position));
}
// This is the same position, so we should really not mark this as an inline position. Fall
// through to the default case.
diff --git a/src/test/java/com/android/tools/r8/kotlin/KotlinSourceDebugExtensionParserTest.java b/src/test/java/com/android/tools/r8/kotlin/KotlinSourceDebugExtensionParserTest.java
index 336e9a8..6ba334d 100644
--- a/src/test/java/com/android/tools/r8/kotlin/KotlinSourceDebugExtensionParserTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/KotlinSourceDebugExtensionParserTest.java
@@ -53,9 +53,9 @@
"*E");
Result result = KotlinSourceDebugExtensionParser.parse(annotationData);
assertNotNull(result);
- assertEquals(1, result.size());
- assertEquals(1, (int) result.lookup(1).getKey());
- Position position = result.lookup(1).getValue();
+ assertEquals(1, result.inlinePositionsCount());
+ assertEquals(1, (int) result.lookupInlinedPosition(1).getKey());
+ Position position = result.lookupInlinedPosition(1).getValue();
assertEquals("EnumSwitch.kt", position.getSource().getFileName());
assertEquals("enumswitch/EnumSwitchKt", position.getSource().getPath());
assertEquals(1, position.getRange().from);
@@ -94,21 +94,21 @@
"*E");
Result result = KotlinSourceDebugExtensionParser.parse(annotationData);
assertNotNull(result);
- assertEquals(3, result.size());
- assertEquals(1, (int) result.lookup(1).getKey());
- assertEquals(23, (int) result.lookup(23).getKey());
- assertEquals(24, (int) result.lookup(24).getKey());
+ assertEquals(3, result.inlinePositionsCount());
+ assertEquals(1, (int) result.lookupInlinedPosition(1).getKey());
+ assertEquals(23, (int) result.lookupInlinedPosition(23).getKey());
+ assertEquals(24, (int) result.lookupInlinedPosition(24).getKey());
// Check that files are correctly parsed.
- Position pos1 = result.lookup(1).getValue();
+ Position pos1 = result.lookupInlinedPosition(1).getValue();
assertEquals("Main.kt", pos1.getSource().getFileName());
assertEquals("retrace/MainKt", pos1.getSource().getPath());
- Position pos2 = result.lookup(23).getValue();
+ Position pos2 = result.lookupInlinedPosition(23).getValue();
assertEquals("InlineFunction.kt", pos2.getSource().getFileName());
assertEquals("retrace/InlineFunctionKt", pos2.getSource().getPath());
- Position pos3 = result.lookup(24).getValue();
+ Position pos3 = result.lookupInlinedPosition(24).getValue();
assertEquals("InlineFunction.kt", pos3.getSource().getFileName());
assertEquals("retrace/InlineFunction", pos3.getSource().getPath());
@@ -315,8 +315,8 @@
"*E");
Result parsedResult = KotlinSourceDebugExtensionParser.parse(annotationData);
assertNotNull(parsedResult);
- assertEquals(24, (int) parsedResult.lookup(25).getKey());
- Position value = parsedResult.lookup(25).getValue();
+ assertEquals(24, (int) parsedResult.lookupInlinedPosition(25).getKey());
+ Position value = parsedResult.lookupInlinedPosition(25).getValue();
assertEquals(12, value.getRange().from);
assertEquals(13, value.getRange().to);
}
diff --git a/src/test/java/com/android/tools/r8/retrace/KotlinInlineFunctionInSameFileRetraceTests.java b/src/test/java/com/android/tools/r8/retrace/KotlinInlineFunctionInSameFileRetraceTests.java
index 24fad30..e0eb6ca 100644
--- a/src/test/java/com/android/tools/r8/retrace/KotlinInlineFunctionInSameFileRetraceTests.java
+++ b/src/test/java/com/android/tools/r8/retrace/KotlinInlineFunctionInSameFileRetraceTests.java
@@ -74,7 +74,6 @@
}
private int getObfuscatedLinePosition() {
- // TODO(b/185358363): This should go away when we correctly retrace.
return kotlinc.is(KotlinCompilerVersion.KOTLINC_1_3_72) ? 43 : 32;
}
@@ -122,10 +121,7 @@
8,
FILENAME_INLINE),
LinePosition.create(
- mainSubject.asFoundMethodSubject(),
- 1,
- getObfuscatedLinePosition(),
- FILENAME_INLINE));
+ mainSubject.asFoundMethodSubject(), 1, 21, FILENAME_INLINE));
checkInlineInformation(stackTrace, codeInspector, mainSubject, inlineStack);
});
}
diff --git a/src/test/java/com/android/tools/r8/retrace/KotlinInlineFunctionRetraceTest.java b/src/test/java/com/android/tools/r8/retrace/KotlinInlineFunctionRetraceTest.java
index ed842eb..032a370 100644
--- a/src/test/java/com/android/tools/r8/retrace/KotlinInlineFunctionRetraceTest.java
+++ b/src/test/java/com/android/tools/r8/retrace/KotlinInlineFunctionRetraceTest.java
@@ -122,7 +122,7 @@
LinePosition.stack(
LinePosition.create(
inlineExceptionStatic(kotlinInspector), 2, 8, FILENAME_INLINE_STATIC),
- LinePosition.create(mainSubject.asFoundMethodSubject(), 2, 15, mainFileName));
+ LinePosition.create(mainSubject.asFoundMethodSubject(), 2, 9, mainFileName));
checkInlineInformation(stackTrace, codeInspector, mainSubject, inlineStack);
});
}
@@ -155,7 +155,7 @@
2,
15,
FILENAME_INLINE_INSTANCE),
- LinePosition.create(mainSubject.asFoundMethodSubject(), 2, 13, mainFileName));
+ LinePosition.create(mainSubject.asFoundMethodSubject(), 2, 7, mainFileName));
checkInlineInformation(stackTrace, codeInspector, mainSubject, inlineStack);
});
}
@@ -187,7 +187,7 @@
inlineExceptionStatic(kotlinInspector), 3, 8, FILENAME_INLINE_STATIC),
// TODO(b/146399675): There should be a nested frame on
// retrace.NestedInlineFunctionKt.nestedInline(line 10).
- LinePosition.create(mainSubject.asFoundMethodSubject(), 3, 19, mainFileName));
+ LinePosition.create(mainSubject.asFoundMethodSubject(), 3, 10, mainFileName));
checkInlineInformation(stackTrace, codeInspector, mainSubject, inlineStack);
});
}
@@ -218,8 +218,8 @@
LinePosition.create(
inlineExceptionStatic(kotlinInspector), 2, 8, FILENAME_INLINE_STATIC),
// TODO(b/146399675): There should be a nested frame on
- // retrace.NestedInlineFunctionKt.nestedInlineOnFirstLine(line 15).
- LinePosition.create(mainSubject.asFoundMethodSubject(), 2, 20, mainFileName));
+ // retrace.NestedInlineFunctionKt.nestedInline(line 10).
+ LinePosition.create(mainSubject.asFoundMethodSubject(), 2, 10, mainFileName));
checkInlineInformation(stackTrace, codeInspector, mainSubject, inlineStack);
});
}
