Make DEX/JVM stepping behaviour even less different
Fixes: b/495501447
Change-Id: I7168a1b0e011be2acfc363b4786b2bb28942ab0e
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 4ff74e3..cb07299 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
@@ -182,7 +182,7 @@
} else {
throw new Unreachable("Unexpected result type " + outType());
}
- builder.add(this, instruction, moveResult);
+ builder.addInvokeAndMoveResult(this, instruction, moveResult);
} else {
builder.add(this, instruction);
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
index ff6589b..3aa130b 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
@@ -62,6 +62,7 @@
import com.android.tools.r8.ir.code.InstructionList;
import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.IntSwitch;
+import com.android.tools.r8.ir.code.Invoke;
import com.android.tools.r8.ir.code.JumpInstruction;
import com.android.tools.r8.ir.code.Move;
import com.android.tools.r8.ir.code.NewArrayFilledData;
@@ -129,6 +130,9 @@
// Keeps track of the previous non-fallthrough info added to the dex builder.
private Info previousNonFallthroughInfo;
+ // Keeps track of a move-result instruction that should be emitted at the following DebugPosition.
+ private DexInstruction pendingMoveResult;
+
// The number of ingoing and outgoing argument registers for the code.
private int inRegisterCount = 0;
private int outRegisterCount = 0;
@@ -181,6 +185,7 @@
instructionToInfo = new Info[instructionNumberToIndex(ir.numberRemainingInstructions())];
inRegisterCount = 0;
outRegisterCount = 0;
+ pendingMoveResult = null;
nextBlock = null;
}
@@ -336,6 +341,7 @@
TryInfo tryInfo = computeTryInfo(dexInstructions);
// Return the dex code.
+ assert pendingMoveResult == null;
DexCode code =
new DexCode(
registerAllocator.registersUsed(),
@@ -355,7 +361,8 @@
BasicBlock previousBlock, BasicBlock currentBlock) {
return previousBlock.exit().isGoto()
&& currentBlock.getPredecessors().size() == 1
- && currentBlock.getPredecessors().get(0) == previousBlock;
+ && currentBlock.getPredecessors().get(0) == previousBlock
+ && !previousBlock.hasCatchSuccessor(currentBlock);
}
@SuppressWarnings("ReferenceEquality")
@@ -750,10 +757,57 @@
add(ir, new FixedSizeInfo(ir, new DexNop()));
}
+ private Instruction skipDebugLocalChangeNotStartingNewLocals(Instruction next) {
+ while (next != null && next.isDebugLocalsChange()) {
+ if (!next.asDebugLocalsChange().getStarting().isEmpty()) {
+ return null;
+ }
+ next = next.getNext();
+ }
+ return next;
+ }
+
+ @SuppressWarnings("ReferenceEquality")
+ private boolean hasDebugPositionForMoveResult(Invoke invoke) {
+ if (isBuildingForComparison()
+ || !options.debug
+ || options.disableAdditionalDebuggerSupport
+ || !options.ensureJvmCompatibleStepOutBehavior
+ || invoke.outValue().hasLocalInfo()) {
+ return false;
+ }
+ BasicBlock currentBlock = invoke.getBlock();
+ Instruction next = skipDebugLocalChangeNotStartingNewLocals(invoke.getNext());
+ if (next != null
+ && next.isGoto()
+ && next.asGoto().getTarget() == nextBlock
+ && isTrivialFallthroughTarget(currentBlock, nextBlock)) {
+ currentBlock = nextBlock;
+ next = skipDebugLocalChangeNotStartingNewLocals(currentBlock.entry());
+ }
+ return next != null && next.isDebugPosition();
+ }
+
+ public void addInvokeAndMoveResult(
+ Invoke invoke, DexInstruction dexInvoke, DexInstruction dexMoveResult) {
+ if (hasDebugPositionForMoveResult(invoke)) {
+ assert pendingMoveResult == null;
+ pendingMoveResult = dexMoveResult;
+ add(invoke, dexInvoke);
+ } else {
+ add(invoke, dexInvoke, dexMoveResult);
+ }
+ }
+
public void addDebugPosition(DebugPosition position) {
- // Remaining debug positions always require we emit an actual nop instruction.
- // See removeRedundantDebugPositions.
- addNop(position);
+ if (pendingMoveResult != null) {
+ add(position, new FixedSizeInfo(position, pendingMoveResult));
+ pendingMoveResult = null;
+ } else {
+ // Remaining debug positions always require we emit an actual nop instruction.
+ // See removeRedundantDebugPositions.
+ addNop(position);
+ }
}
public void add(Instruction instr, DexInstruction dex) {
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 00bc67a..2dd1b74 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
@@ -169,9 +169,18 @@
public void buildInstruction(
IRBuilder builder, int instructionIndex, boolean firstBlockInstruction) {
updateCurrentCatchHandlers(instructionIndex, builder.appView.dexItemFactory());
- updateDebugPosition(instructionIndex, builder);
currentDexInstruction = code.instructions[instructionIndex];
- currentDexInstruction.buildIR(builder);
+ if (isMoveResult(currentDexInstruction)) {
+ // In DEX, a move-result instruction can have its own debug position entry (see
+ // hasDebugPositionForMoveResult in DexBuilder). In IR, move-result is merged into the
+ // preceding invoke instruction, so we must build the IR for move-result before emitting
+ // the debug position instruction.
+ currentDexInstruction.buildIR(builder);
+ updateDebugPosition(instructionIndex, builder);
+ } else {
+ updateDebugPosition(instructionIndex, builder);
+ currentDexInstruction.buildIR(builder);
+ }
}
@Override
diff --git a/src/test/java8/debug/com/android/tools/r8/debug/StepOutOfKotlinFunctionWhereKotlincAddedNopTest.java b/src/test/java8/debug/com/android/tools/r8/debug/StepOutOfKotlinFunctionWhereKotlincAddedNopTest.java
index c147a28..56570a1 100644
--- a/src/test/java8/debug/com/android/tools/r8/debug/StepOutOfKotlinFunctionWhereKotlincAddedNopTest.java
+++ b/src/test/java8/debug/com/android/tools/r8/debug/StepOutOfKotlinFunctionWhereKotlincAddedNopTest.java
@@ -50,11 +50,14 @@
.writeToZip();
}
- @Test
- public void test() throws Throwable {
+ private void runTest(
+ byte[] classFileData,
+ int expectedStepOutLine,
+ int expectedStepOutLineWithoutAdditionalDebuggerSupport)
+ throws Throwable {
runDebugTest(
testForRuntime(parameters)
- .addProgramClassFileData(dump())
+ .addProgramClassFileData(classFileData)
.debugConfig(parameters.getRuntime())
.addPaths(
parameters.isCfRuntime()
@@ -65,46 +68,59 @@
run(),
checkLine("B495501447.kt", 1),
stepOut(INTELLIJ_FILTER),
- checkLine("B495501447.kt", parameters.isCfRuntime() ? 4 : 5),
+ checkLine("B495501447.kt", expectedStepOutLine),
run());
+ if (parameters.isDexRuntime()) {
+ Path dex =
+ testForD8(parameters.getBackend())
+ .addProgramClassFileData(classFileData)
+ .setMinApi(parameters)
+ .compile()
+ .writeToZip();
+ runDebugTest(
+ testForD8(parameters.getBackend())
+ .addProgramFiles(dex)
+ .setMinApi(parameters)
+ .addOptionsModification(options -> options.passthroughDexCode = false)
+ .debugConfig(parameters.getRuntime())
+ .addPaths(kotlinStdlibDex),
+ "B495501447Kt",
+ breakpoint("B495501447Kt", "foo"),
+ run(),
+ checkLine("B495501447.kt", 1),
+ stepOut(INTELLIJ_FILTER),
+ checkLine("B495501447.kt", expectedStepOutLine),
+ run());
+ runDebugTest(
+ testForD8(parameters.getBackend())
+ .addProgramClassFileData(classFileData)
+ .setMinApi(parameters)
+ .addOptionsModification(options -> options.disableAdditionalDebuggerSupport = true)
+ .debugConfig(parameters.getRuntime())
+ .addPaths(kotlinStdlibDex),
+ "B495501447Kt",
+ breakpoint("B495501447Kt", "foo"),
+ run(),
+ checkLine("B495501447.kt", 1),
+ stepOut(INTELLIJ_FILTER),
+ checkLine("B495501447.kt", expectedStepOutLineWithoutAdditionalDebuggerSupport),
+ run());
+ }
+ }
+
+ @Test
+ public void test() throws Throwable {
+ runTest(dump(), 4, 5);
}
@Test
public void testTryCatch() throws Throwable {
- runDebugTest(
- testForRuntime(parameters)
- .addProgramClassFileData(dumpTryCatch())
- .debugConfig(parameters.getRuntime())
- .addPaths(
- parameters.isCfRuntime()
- ? KOTLINC_2_4_20.getCompiler().getKotlinStdlibJar()
- : kotlinStdlibDex),
- "B495501447Kt",
- breakpoint("B495501447Kt", "foo"),
- run(),
- checkLine("B495501447.kt", 1),
- stepOut(INTELLIJ_FILTER),
- checkLine("B495501447.kt", parameters.isCfRuntime() ? 5 : 6),
- run());
+ runTest(dumpTryCatch(), 5, 6);
}
@Test
public void testAssignToLocal() throws Throwable {
- runDebugTest(
- testForRuntime(parameters)
- .addProgramClassFileData(dumpAssignToLocal())
- .debugConfig(parameters.getRuntime())
- .addPaths(
- parameters.isCfRuntime()
- ? KOTLINC_2_4_20.getCompiler().getKotlinStdlibJar()
- : kotlinStdlibDex),
- "B495501447Kt",
- breakpoint("B495501447Kt", "foo"),
- run(),
- checkLine("B495501447.kt", 1),
- stepOut(INTELLIJ_FILTER),
- checkLine("B495501447.kt", 6),
- run());
+ runTest(dumpAssignToLocal(), 6, 6);
}
/*
