Fix a few debug-info related issues.
Store the debug position associated with a move-exception in the move-exception instruction.
This ensures that the line information when ART breaks at catch handlers will be accurate.
Calculate live-at-block-entry locals prior to its entry instruction.
R=ager
Change-Id: Ic994ab73973457a36f7578dee214dd4d1749a2db
diff --git a/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java b/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
index c468613..72a43f7 100644
--- a/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
+++ b/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
@@ -9,6 +9,7 @@
import com.android.tools.r8.ir.code.DebugLocalsChange;
import com.android.tools.r8.ir.code.DebugPosition;
import com.android.tools.r8.ir.code.Instruction;
+import com.android.tools.r8.ir.code.MoveException;
import it.unimi.dsi.fastutil.ints.Int2ReferenceAVLTreeMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap.Entry;
@@ -51,12 +52,6 @@
private DexString emittedFile = null;
private Int2ReferenceMap<DebugLocalInfo> emittedLocals;
- // If lastMoveInstructionPc != NO_PC_INFO, then the last pc-advancing instruction was a
- // move-exception at lastMoveInstructionPc. This is needed to maintain the art/dx specific
- // behaviour that the move-exception pc is associated with the catch-declaration line.
- // See debug.ExceptionTest.testStepOnCatch().
- private int lastMoveInstructionPc = NO_PC_INFO;
-
// Emitted events.
private final List<DexDebugEvent> events = new ArrayList<>();
@@ -81,23 +76,21 @@
}
assert pendingLocals != null;
- // If this is a position emit and exit as it always emits events.
if (instruction.isDebugPosition()) {
emitDebugPosition(pc, instruction.asDebugPosition());
- return;
- }
-
- if (instruction.isArgument()) {
+ } else if (instruction.isMoveException()) {
+ MoveException move = instruction.asMoveException();
+ if (move.getPosition() != null) {
+ emitDebugPosition(pc, move.getPosition());
+ }
+ } else if (instruction.isArgument()) {
startArgument(instruction.asArgument());
} else if (instruction.isDebugLocalsChange()) {
updateLocals(instruction.asDebugLocalsChange());
} else if (instruction.getBlock().exit() == instruction) {
- // If this is the end of the block clear out the pending state and exit.
+ // If this is the end of the block clear out the pending state.
pendingLocals = null;
pendingLocalChanges = false;
- return;
- } else if (instruction.isMoveException()) {
- lastMoveInstructionPc = pc;
} else {
// For non-exit / pc-advancing instructions emit any pending changes.
emitLocalChanges(pc);
@@ -191,18 +184,16 @@
}
private void emitDebugPosition(int pc, int line, DexString file) {
- int emitPc = lastMoveInstructionPc != NO_PC_INFO ? lastMoveInstructionPc : pc;
- lastMoveInstructionPc = NO_PC_INFO;
// The position requires a pc change event and possible events for line, file and local changes.
// Verify that we do not ever produce two subsequent positions at the same pc.
- assert emittedPc != emitPc;
+ assert emittedPc != pc;
if (startLine == NO_LINE_INFO) {
assert emittedLine == NO_LINE_INFO;
startLine = line;
emittedLine = line;
}
- emitAdvancementEvents(emittedPc, emittedLine, emittedFile, emitPc, line, file, events, factory);
- emittedPc = emitPc;
+ emitAdvancementEvents(emittedPc, emittedLine, emittedFile, pc, line, file, events, factory);
+ emittedPc = pc;
emittedLine = line;
emittedFile = file;
if (localsChanged()) {
@@ -215,11 +206,9 @@
private void emitLocalChanges(int pc) {
// If pc advanced since the locals changed and locals indeed have changed, emit the changes.
if (localsChanged()) {
- int emitPc = lastMoveInstructionPc != NO_PC_INFO ? lastMoveInstructionPc : pc;
- lastMoveInstructionPc = NO_PC_INFO;
emitAdvancementEvents(
- emittedPc, emittedLine, emittedFile, emitPc, emittedLine, emittedFile, events, factory);
- emittedPc = emitPc;
+ emittedPc, emittedLine, emittedFile, pc, emittedLine, emittedFile, events, factory);
+ emittedPc = pc;
emitLocalChangeEvents(emittedLocals, pendingLocals, lastKnownLocals, events, factory);
pendingLocalChanges = false;
assert localsEqual(emittedLocals, pendingLocals);
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
index 290a1a2..16bc790 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
@@ -1120,11 +1120,13 @@
List<BasicBlock> predecessors = this.getPredecessors();
boolean hasMoveException = entry().isMoveException();
MoveException move = null;
+ DebugPosition position = null;
if (hasMoveException) {
// Remove the move-exception instruction.
move = entry().asMoveException();
+ position = move.getPosition();
assert move.getPreviousLocalValue() == null;
- this.getInstructions().remove(0);
+ getInstructions().remove(0);
}
// Create new predecessor blocks.
List<BasicBlock> newPredecessors = new ArrayList<>();
@@ -1140,7 +1142,11 @@
Value value = new Value(
valueNumberGenerator.next(), MoveType.OBJECT, move.getDebugInfo());
values.add(value);
- newBlock.add(new MoveException(value));
+ MoveException newMove = new MoveException(value);
+ newBlock.add(newMove);
+ if (position != null) {
+ newMove.setPosition(new DebugPosition(position.line, position.file));
+ }
}
newBlock.add(new Goto());
newBlock.close(null);
diff --git a/src/main/java/com/android/tools/r8/ir/code/MoveException.java b/src/main/java/com/android/tools/r8/ir/code/MoveException.java
index 2691eed..554e71f 100644
--- a/src/main/java/com/android/tools/r8/ir/code/MoveException.java
+++ b/src/main/java/com/android/tools/r8/ir/code/MoveException.java
@@ -9,6 +9,8 @@
public class MoveException extends Instruction {
+ private DebugPosition position = null;
+
public MoveException(Value dest) {
super(dest);
}
@@ -17,6 +19,14 @@
return outValue;
}
+ public DebugPosition getPosition() {
+ return position;
+ }
+
+ public void setPosition(DebugPosition position) {
+ this.position = position;
+ }
+
@Override
public void buildDex(DexBuilder builder) {
int dest = builder.allocatedRegister(dest(), getNumber());
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index ba63d80..1b9fef5 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -1977,6 +1977,16 @@
if (currentDebugPosition != null) {
DebugPosition position = currentDebugPosition;
currentDebugPosition = null;
+ if (!currentBlock.getInstructions().isEmpty()) {
+ MoveException move = currentBlock.getInstructions().getLast().asMoveException();
+ if (move != null && move.getPosition() == null) {
+ // Set the position on the move-exception instruction.
+ // ART/DX associates the move-exception pc with the catch-declaration line.
+ // See debug.ExceptionTest.testStepOnCatch().
+ move.setPosition(position);
+ return;
+ }
+ }
addInstruction(position);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java b/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
index 0bd7188..1e45d52 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
@@ -28,7 +28,6 @@
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.HashMultiset;
-import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Multiset;
import com.google.common.collect.Multiset.Entry;
import com.google.common.collect.Multisets;
@@ -288,7 +287,7 @@
}
for (BasicBlock block : blocks) {
- boolean blockEntry = true;
+ block.setLocalsAtEntry(new Int2ReferenceOpenHashMap<>(currentLocals));
ListIterator<Instruction> instructionIterator = block.listIterator();
while (instructionIterator.hasNext()) {
Instruction instruction = instructionIterator.next();
@@ -307,8 +306,8 @@
}
}
while (nextStartingRange != null && nextStartingRange.start <= index) {
- // If the full range is between the two debug positions ignore it.
- if (nextStartingRange.end > index) {
+ // If the range is live at this index open it.
+ if (index < nextStartingRange.end) {
openRanges.add(nextStartingRange);
assert !currentLocals.containsKey(nextStartingRange.register);
currentLocals.put(nextStartingRange.register, nextStartingRange.local);
@@ -317,10 +316,7 @@
}
nextStartingRange = rangeIterator.hasNext() ? rangeIterator.next() : null;
}
- if (blockEntry) {
- blockEntry = false;
- block.setLocalsAtEntry(new Int2ReferenceOpenHashMap<>(currentLocals));
- } else if (localsChanged && shouldEmitChangesAtInstruction(instruction)) {
+ if (localsChanged && shouldEmitChangesAtInstruction(instruction)) {
DebugLocalsChange change = createLocalsChange(ending, starting);
if (change != null) {
if (instruction.isDebugPosition() || instruction.isJumpInstruction()) {
@@ -368,18 +364,6 @@
|| (instruction.isGoto() && instruction.asGoto().getTarget() == code.getNormalExitBlock());
}
- private boolean verifyLocalsEqual(
- ImmutableMap<Integer, DebugLocalInfo> a, Map<Integer, DebugLocalInfo> b) {
- int size = 0;
- for (Map.Entry<Integer, DebugLocalInfo> entry : b.entrySet()) {
- if (entry.getValue() != null) {
- assert a.get(entry.getKey()) == entry.getValue();
- ++size;
- }
- }
- return a.size() == size;
- }
-
private void clearState() {
liveAtEntrySets = null;
liveIntervals = null;
