Emit local changes accurately at all instruction points.
Bug: 63243012
Change-Id: I874308618da6366b79b68e5b8eb0c0e1b08aa447
diff --git a/src/main/java/com/android/tools/r8/graph/DexCode.java b/src/main/java/com/android/tools/r8/graph/DexCode.java
index 9b62842..3535bfa 100644
--- a/src/main/java/com/android/tools/r8/graph/DexCode.java
+++ b/src/main/java/com/android/tools/r8/graph/DexCode.java
@@ -184,14 +184,12 @@
}
int instructionNumber = 0;
for (Instruction insn : instructions) {
- StringUtils.appendLeftPadded(builder, Integer.toString(instructionNumber++), 5);
- builder.append(": ")
- .append(insn.toString(naming))
- .append('\n');
while (debugInfo != null && debugInfo.address == insn.getOffset()) {
builder.append(" ").append(debugInfo).append("\n");
debugInfo = debugInfoIterator.hasNext() ? debugInfoIterator.next() : null;
}
+ StringUtils.appendLeftPadded(builder, Integer.toString(instructionNumber++), 5);
+ builder.append(": ").append(insn.toString(naming)).append('\n');
}
if (debugInfoIterator.hasNext()) {
throw new Unreachable("Could not print all debug information.");
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 2342b3b..c468613 100644
--- a/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
+++ b/src/main/java/com/android/tools/r8/graph/DexDebugEventBuilder.java
@@ -5,14 +5,18 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.graph.DexDebugEvent.StartLocal;
+import com.android.tools.r8.ir.code.Argument;
+import com.android.tools.r8.ir.code.DebugLocalsChange;
import com.android.tools.r8.ir.code.DebugPosition;
-import com.google.common.collect.ImmutableMap;
+import com.android.tools.r8.ir.code.Instruction;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceAVLTreeMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap.Entry;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMaps;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceOpenHashMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceSortedMap;
import java.util.ArrayList;
-import java.util.HashMap;
import java.util.List;
-import java.util.Map;
-import java.util.SortedSet;
-import java.util.TreeSet;
/**
* Builder for constructing a list of debug events suitable for DexDebugInfo.
@@ -25,135 +29,226 @@
private static final int NO_PC_INFO = -1;
private static final int NO_LINE_INFO = -1;
- private static class PositionState {
- int pc = NO_PC_INFO;
- int line = NO_LINE_INFO;
- DexString file = null;
- ImmutableMap<Integer, DebugLocalInfo> locals = null;
- }
-
- private final DexMethod method;
-
- private final DexItemFactory dexItemFactory;
-
- // Previous and current position info to delay emitting position changes.
- private final PositionState previous;
- private final PositionState current;
+ private final DexEncodedMethod method;
+ private final DexItemFactory factory;
// In order list of non-this argument locals.
- private int lastArgumentRegister = -1;
- private final List<DebugLocalInfo> arguments;
-
- // Mapping from register to local for currently open/visible locals.
- private final Map<Integer, DebugLocalInfo> openLocals = new HashMap<>();
+ private ArrayList<DebugLocalInfo> arguments;
// Mapping from register to the last known local in that register (See DBG_RESTART_LOCAL).
- private final Map<Integer, DebugLocalInfo> lastKnownLocals = new HashMap<>();
+ private Int2ReferenceMap<DebugLocalInfo> lastKnownLocals;
- // Flushed events.
+ // Mapping from register to local for currently open/visible locals.
+ private Int2ReferenceMap<DebugLocalInfo> pendingLocals = null;
+
+ // Conservative pending-state of locals to avoid some equality checks on locals.
+ // pendingLocalChanges == true ==> localsEqual(emittedLocals, pendingLocals).
+ private boolean pendingLocalChanges = false;
+
+ // State of pc, line, file and locals in the emitted event stream.
+ private int emittedPc = NO_PC_INFO;
+ private int emittedLine = NO_LINE_INFO;
+ 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<>();
+ // Initial known line for the method.
private int startLine = NO_LINE_INFO;
- public DexDebugEventBuilder(DexMethod method, DexItemFactory dexItemFactory) {
+ public DexDebugEventBuilder(DexEncodedMethod method, DexItemFactory factory) {
this.method = method;
- this.dexItemFactory = dexItemFactory;
- arguments = new ArrayList<>(method.proto.parameters.values.length);
- current = new PositionState();
- previous = new PositionState();
+ this.factory = factory;
}
- public void startArgument(int register, DebugLocalInfo local, boolean isThis) {
- // Verify that arguments are started in order.
- assert register > lastArgumentRegister;
- lastArgumentRegister = register;
- // If this is an actual argument record it for header information.
- if (!isThis) {
- arguments.add(local);
- }
- // If the argument does not have a parametrized type, implicitly open it.
- if (local != null && local.signature == null) {
- openLocals.put(register, local);
- lastKnownLocals.put(register, local);
- }
+ // Public method for the DebugStripper.
+ public void setPosition(int pc, int line) {
+ emitDebugPosition(pc, line, null);
}
- /** Emits a positions entry if the position has changed and associates any local changes. */
- public void setPosition(int pc, DebugPosition position) {
- setPosition(pc, position.line, position.file, position.getLocals());
- }
-
- public void setPosition(
- int pc, int line, DexString file, ImmutableMap<Integer, DebugLocalInfo> locals) {
- // If we have a pending position and the next differs from it flush the pending one.
- if (previous.pc != current.pc && positionChanged(current, pc, line, file)) {
- flushCurrentPosition();
+ /** Add events at pc for instruction. */
+ public void add(int pc, Instruction instruction) {
+ // Initialize locals state on block entry.
+ if (instruction.getBlock().entry() == instruction) {
+ updateBlockEntry(instruction);
}
- current.pc = pc;
- current.line = line;
- current.file = file;
- current.locals = locals;
- }
+ assert pendingLocals != null;
- private void flushCurrentPosition() {
- // If this is the first emitted possition, initialize previous state: start-line is forced to be
- // the first actual line, in-effect, causing the first position to be a zero-delta line change.
- if (startLine == NO_LINE_INFO) {
- assert events.isEmpty();
- assert previous.pc == NO_PC_INFO;
- assert previous.line == NO_LINE_INFO;
- startLine = current.line;
- previous.line = current.line;
- previous.pc = 0;
+ // If this is a position emit and exit as it always emits events.
+ if (instruction.isDebugPosition()) {
+ emitDebugPosition(pc, instruction.asDebugPosition());
+ return;
}
- // Emit position change (which might result in additional advancement events).
- emitAdvancementEvents();
- // Emit local changes for new current position (they relate to the already emitted position).
- // Locals are either defined on all positions or on none.
- assert current.locals != null || previous.locals == null;
- if (current.locals != null) {
- emitLocalChanges();
+
+ 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.
+ pendingLocals = null;
+ pendingLocalChanges = false;
+ return;
+ } else if (instruction.isMoveException()) {
+ lastMoveInstructionPc = pc;
+ } else {
+ // For non-exit / pc-advancing instructions emit any pending changes.
+ emitLocalChanges(pc);
}
}
/** Build the resulting DexDebugInfo object. */
public DexDebugInfo build() {
- if (previous.pc != current.pc) {
- flushCurrentPosition();
- }
+ assert pendingLocals == null;
+ assert !pendingLocalChanges;
if (startLine == NO_LINE_INFO) {
return null;
}
- DexString[] params = new DexString[method.proto.parameters.values.length];
- assert arguments.isEmpty() || params.length == arguments.size();
- for (int i = 0; i < arguments.size(); i++) {
- DebugLocalInfo local = arguments.get(i);
- params[i] = (local == null || local.signature != null) ? null : local.name;
+ DexString[] params = new DexString[method.method.proto.parameters.values.length];
+ if (arguments != null) {
+ assert params.length == arguments.size();
+ for (int i = 0; i < arguments.size(); i++) {
+ DebugLocalInfo local = arguments.get(i);
+ params[i] = (local == null || local.signature != null) ? null : local.name;
+ }
}
return new DexDebugInfo(startLine, params, events.toArray(new DexDebugEvent[events.size()]));
}
- private static boolean positionChanged(
- PositionState current, int nextPc, int nextLine, DexString nextFile) {
- return nextPc != current.pc && (nextLine != current.line || nextFile != current.file);
+ private void updateBlockEntry(Instruction instruction) {
+ assert pendingLocals == null;
+ assert !pendingLocalChanges;
+ Int2ReferenceMap<DebugLocalInfo> locals = instruction.getBlock().getLocalsAtEntry();
+ if (locals == null) {
+ pendingLocals = Int2ReferenceMaps.emptyMap();
+ } else {
+ pendingLocals = new Int2ReferenceOpenHashMap<>(locals);
+ pendingLocalChanges = true;
+ }
+ if (emittedLocals == null) {
+ initialize(locals);
+ }
}
- private void emitAdvancementEvents() {
- int pcDelta = current.pc - previous.pc;
- int lineDelta = current.line - previous.line;
+ private void initialize(Int2ReferenceMap<DebugLocalInfo> locals) {
+ assert arguments == null;
+ assert emittedLocals == null;
+ assert lastKnownLocals == null;
+ assert startLine == NO_LINE_INFO;
+ if (locals == null) {
+ emittedLocals = Int2ReferenceMaps.emptyMap();
+ lastKnownLocals = Int2ReferenceMaps.emptyMap();
+ return;
+ }
+ // Implicitly open all unparameterized arguments.
+ emittedLocals = new Int2ReferenceOpenHashMap<>();
+ for (Entry<DebugLocalInfo> entry : locals.int2ReferenceEntrySet()) {
+ if (entry.getValue().signature == null) {
+ emittedLocals.put(entry.getIntKey(), entry.getValue());
+ }
+ }
+ lastKnownLocals = new Int2ReferenceOpenHashMap<>(emittedLocals);
+ }
+
+ private void startArgument(Argument argument) {
+ if (arguments == null) {
+ arguments = new ArrayList<>(method.method.proto.parameters.values.length);
+ }
+ if (!argument.outValue().isThis()) {
+ arguments.add(argument.getLocalInfo());
+ }
+ }
+
+ private void updateLocals(DebugLocalsChange change) {
+ pendingLocalChanges = true;
+ for (Entry<DebugLocalInfo> end : change.getEnding().int2ReferenceEntrySet()) {
+ assert pendingLocals.get(end.getIntKey()) == end.getValue();
+ pendingLocals.remove(end.getIntKey());
+ }
+ for (Entry<DebugLocalInfo> start : change.getStarting().int2ReferenceEntrySet()) {
+ assert !pendingLocals.containsKey(start.getIntKey());
+ pendingLocals.put(start.getIntKey(), start.getValue());
+ }
+ }
+
+ private boolean localsChanged() {
+ if (!pendingLocalChanges) {
+ return false;
+ }
+ pendingLocalChanges = !localsEqual(emittedLocals, pendingLocals);
+ return pendingLocalChanges;
+ }
+
+ private void emitDebugPosition(int pc, DebugPosition position) {
+ emitDebugPosition(pc, position.line, position.file);
+ }
+
+ 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;
+ 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;
+ emittedLine = line;
+ emittedFile = file;
+ if (localsChanged()) {
+ emitLocalChangeEvents(emittedLocals, pendingLocals, lastKnownLocals, events, factory);
+ assert localsEqual(emittedLocals, pendingLocals);
+ }
+ pendingLocalChanges = false;
+ }
+
+ 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;
+ emitLocalChangeEvents(emittedLocals, pendingLocals, lastKnownLocals, events, factory);
+ pendingLocalChanges = false;
+ assert localsEqual(emittedLocals, pendingLocals);
+ }
+ }
+
+ private static void emitAdvancementEvents(
+ int previousPc,
+ int previousLine,
+ DexString previousFile,
+ int nextPc,
+ int nextLine,
+ DexString nextFile,
+ List<DexDebugEvent> events,
+ DexItemFactory factory) {
+ int pcDelta = previousPc == NO_PC_INFO ? nextPc : nextPc - previousPc;
+ int lineDelta = nextLine == NO_LINE_INFO ? 0 : nextLine - previousLine;
assert pcDelta >= 0;
- if (current.file != previous.file) {
- assert current.file == null || !current.file.equals(previous.file);
- events.add(dexItemFactory.createSetFile(current.file));
+ if (nextFile != previousFile) {
+ events.add(factory.createSetFile(nextFile));
}
if (lineDelta < Constants.DBG_LINE_BASE
|| lineDelta - Constants.DBG_LINE_BASE >= Constants.DBG_LINE_RANGE) {
- events.add(dexItemFactory.createAdvanceLine(lineDelta));
+ events.add(factory.createAdvanceLine(lineDelta));
// TODO(herhut): To be super clever, encode only the part that is above limit.
lineDelta = 0;
}
if (pcDelta >= Constants.DBG_ADDRESS_RANGE) {
- events.add(dexItemFactory.createAdvancePC(pcDelta));
+ events.add(factory.createAdvancePC(pcDelta));
pcDelta = 0;
}
// TODO(herhut): Maybe only write this one if needed (would differ from DEX).
@@ -161,37 +256,65 @@
0x0a + (lineDelta - Constants.DBG_LINE_BASE) + Constants.DBG_LINE_RANGE * pcDelta;
assert specialOpcode >= 0x0a;
assert specialOpcode <= 0xff;
- events.add(dexItemFactory.createDefault(specialOpcode));
- previous.pc = current.pc;
- previous.line = current.line;
- previous.file = current.file;
+ events.add(factory.createDefault(specialOpcode));
}
- private void emitLocalChanges() {
- if (previous.locals == current.locals) {
- return;
- }
- SortedSet<Integer> currentRegisters = new TreeSet<>(openLocals.keySet());
- SortedSet<Integer> positionRegisters = new TreeSet<>(current.locals.keySet());
- for (Integer register : currentRegisters) {
- if (!positionRegisters.contains(register)) {
- events.add(dexItemFactory.createEndLocal(register));
- openLocals.remove(register);
+ public static void emitLocalChangeEvents(
+ Int2ReferenceMap<DebugLocalInfo> previousLocals,
+ Int2ReferenceMap<DebugLocalInfo> nextLocals,
+ Int2ReferenceMap<DebugLocalInfo> lastKnownLocals,
+ List<DexDebugEvent> events,
+ DexItemFactory factory) {
+ Int2ReferenceSortedMap<DebugLocalInfo> ending = new Int2ReferenceAVLTreeMap<>();
+ Int2ReferenceSortedMap<DebugLocalInfo> starting = new Int2ReferenceAVLTreeMap<>();
+ for (Entry<DebugLocalInfo> entry : previousLocals.int2ReferenceEntrySet()) {
+ int register = entry.getIntKey();
+ DebugLocalInfo local = entry.getValue();
+ if (nextLocals.get(register) != local) {
+ ending.put(register, local);
}
}
- for (Integer register : positionRegisters) {
- DebugLocalInfo positionLocal = current.locals.get(register);
- DebugLocalInfo currentLocal = openLocals.get(register);
- if (currentLocal != positionLocal) {
- openLocals.put(register, positionLocal);
- if (currentLocal == null && lastKnownLocals.get(register) == positionLocal) {
- events.add(dexItemFactory.createRestartLocal(register));
- } else {
- events.add(new StartLocal(register, positionLocal));
- lastKnownLocals.put(register, positionLocal);
- }
+ for (Entry<DebugLocalInfo> entry : nextLocals.int2ReferenceEntrySet()) {
+ int register = entry.getIntKey();
+ DebugLocalInfo local = entry.getValue();
+ if (previousLocals.get(register) != local) {
+ starting.put(register, local);
}
}
- previous.locals = current.locals;
+ assert !ending.isEmpty() || !starting.isEmpty();
+ for (Entry<DebugLocalInfo> end : ending.int2ReferenceEntrySet()) {
+ int register = end.getIntKey();
+ if (!starting.containsKey(register)) {
+ previousLocals.remove(register);
+ events.add(factory.createEndLocal(register));
+ }
+ }
+ for (Entry<DebugLocalInfo> start : starting.int2ReferenceEntrySet()) {
+ int register = start.getIntKey();
+ DebugLocalInfo local = start.getValue();
+ previousLocals.put(register, local);
+ if (lastKnownLocals.get(register) == local) {
+ events.add(factory.createRestartLocal(register));
+ } else {
+ events.add(new StartLocal(register, local));
+ lastKnownLocals.put(register, local);
+ }
+ }
+ }
+
+ private static boolean localsEqual(
+ Int2ReferenceMap<DebugLocalInfo> locals1, Int2ReferenceMap<DebugLocalInfo> locals2) {
+ if (locals1 == locals2) {
+ return true;
+ }
+ if (locals1.size() != locals2.size()) {
+ return false;
+ }
+ for (Int2ReferenceMap.Entry<DebugLocalInfo> entry : locals1.int2ReferenceEntrySet()) {
+ if (locals2.get(entry.getIntKey()) != entry.getValue()) {
+ return false;
+ }
+ }
+ return true;
}
}
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 b9508b5..46149e7 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
@@ -4,13 +4,16 @@
package com.android.tools.r8.ir.code;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.conversion.DexBuilder;
import com.android.tools.r8.ir.conversion.IRBuilder;
import com.android.tools.r8.utils.CfgPrinter;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.StringUtils.BraceType;
import com.google.common.collect.ImmutableList;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -31,6 +34,16 @@
*/
public class BasicBlock {
+ private Int2ReferenceMap<DebugLocalInfo> localsAtEntry;
+
+ public void setLocalsAtEntry(Int2ReferenceMap<DebugLocalInfo> localsAtEntry) {
+ this.localsAtEntry = localsAtEntry;
+ }
+
+ public Int2ReferenceMap<DebugLocalInfo> getLocalsAtEntry() {
+ return localsAtEntry;
+ }
+
public enum ThrowingInfo {
NO_THROW, CAN_THROW
}
@@ -770,6 +783,11 @@
} else {
builder.append("no phis\n");
}
+ if (localsAtEntry != null) {
+ builder.append("locals: ");
+ StringUtils.append(builder, localsAtEntry.int2ReferenceEntrySet(), ", ", BraceType.NONE);
+ builder.append("\n");
+ }
for (Instruction instruction : instructions) {
StringUtils.appendLeftPadded(builder, Integer.toString(instruction.getNumber()), 6);
builder.append(": ");
diff --git a/src/main/java/com/android/tools/r8/ir/code/ConstNumber.java b/src/main/java/com/android/tools/r8/ir/code/ConstNumber.java
index 6c5fbb7..0da5b43 100644
--- a/src/main/java/com/android/tools/r8/ir/code/ConstNumber.java
+++ b/src/main/java/com/android/tools/r8/ir/code/ConstNumber.java
@@ -90,7 +90,7 @@
@Override
public void buildDex(DexBuilder builder) {
if (!dest().needsRegister()) {
- builder.addFallThrough(this);
+ builder.addNop(this);
return;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/DebugLocalsChange.java b/src/main/java/com/android/tools/r8/ir/code/DebugLocalsChange.java
new file mode 100644
index 0000000..a08b044
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/code/DebugLocalsChange.java
@@ -0,0 +1,84 @@
+// Copyright (c) 2017, 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.ir.code;
+
+import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.graph.DebugLocalInfo;
+import com.android.tools.r8.ir.conversion.DexBuilder;
+import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.StringUtils;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
+
+public class DebugLocalsChange extends Instruction {
+
+ private final Int2ReferenceMap<DebugLocalInfo> ending;
+ private final Int2ReferenceMap<DebugLocalInfo> starting;
+
+ public DebugLocalsChange(
+ Int2ReferenceMap<DebugLocalInfo> ending, Int2ReferenceMap<DebugLocalInfo> starting) {
+ super(null);
+ this.ending = ending;
+ this.starting = starting;
+ }
+
+ public Int2ReferenceMap<DebugLocalInfo> getEnding() {
+ return ending;
+ }
+
+ public Int2ReferenceMap<DebugLocalInfo> getStarting() {
+ return starting;
+ }
+
+ @Override
+ public boolean isDebugLocalsChange() {
+ return true;
+ }
+
+ @Override
+ public DebugLocalsChange asDebugLocalsChange() {
+ return this;
+ }
+
+ @Override
+ public void buildDex(DexBuilder builder) {
+ builder.addNop(this);
+ }
+
+ @Override
+ public boolean identicalNonValueParts(Instruction other) {
+ assert other.isDebugLocalsChange();
+ return false;
+ }
+
+ @Override
+ public int compareNonValueParts(Instruction other) {
+ assert other.isDebugLocalsChange();
+ return 0;
+ }
+
+ @Override
+ public int maxInValueRegister() {
+ throw new Unreachable();
+ }
+
+ @Override
+ public int maxOutValueRegister() {
+ throw new Unreachable();
+ }
+
+ @Override
+ public boolean canBeDeadCode(IRCode code, InternalOptions options) {
+ return false;
+ }
+
+ @Override
+ public String toString() {
+ StringBuilder builder = new StringBuilder(super.toString());
+ builder.append("ending: ");
+ StringUtils.append(builder, ending.int2ReferenceEntrySet());
+ builder.append(", starting: ");
+ StringUtils.append(builder, starting.int2ReferenceEntrySet());
+ return builder.toString();
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/ir/code/DebugPosition.java b/src/main/java/com/android/tools/r8/ir/code/DebugPosition.java
index e7917c6..ee6b65f 100644
--- a/src/main/java/com/android/tools/r8/ir/code/DebugPosition.java
+++ b/src/main/java/com/android/tools/r8/ir/code/DebugPosition.java
@@ -4,18 +4,14 @@
package com.android.tools.r8.ir.code;
import com.android.tools.r8.errors.Unreachable;
-import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.ir.conversion.DexBuilder;
import com.android.tools.r8.utils.InternalOptions;
-import com.android.tools.r8.utils.StringUtils;
-import com.google.common.collect.ImmutableMap;
public class DebugPosition extends Instruction {
public final int line;
public final DexString file;
- private ImmutableMap<Integer, DebugLocalInfo> locals;
public DebugPosition(int line, DexString file) {
super(null);
@@ -72,18 +68,6 @@
builder.append(file).append(":");
}
builder.append(line);
- if (locals != null && !locals.isEmpty()) {
- builder.append(", locals: ");
- StringUtils.append(builder, locals.values());
- }
return builder.toString();
}
-
- public void setLocals(ImmutableMap<Integer, DebugLocalInfo> locals) {
- this.locals = locals;
- }
-
- public ImmutableMap<Integer, DebugLocalInfo> getLocals() {
- return locals;
- }
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Instruction.java b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
index 0c4697d..083100b 100644
--- a/src/main/java/com/android/tools/r8/ir/code/Instruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
@@ -713,6 +713,7 @@
public boolean isDebugInstruction() {
return isDebugPosition()
+ || isDebugLocalsChange()
|| isDebugLocalWrite()
|| isDebugLocalUninitialized();
}
@@ -725,6 +726,14 @@
return null;
}
+ public boolean isDebugLocalsChange() {
+ return false;
+ }
+
+ public DebugLocalsChange asDebugLocalsChange() {
+ return null;
+ }
+
public boolean isDebugLocalUninitialized() {
return false;
}
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 fc1eab5..de19fdd 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
@@ -59,6 +59,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import java.util.ArrayList;
+import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
@@ -192,27 +193,16 @@
} while (!ifsNeedingRewrite.isEmpty());
// Build instructions.
- DexDebugEventBuilder debugEventBuilder =
- new DexDebugEventBuilder(ir.method.method, dexItemFactory);
+ DexDebugEventBuilder debugEventBuilder = new DexDebugEventBuilder(ir.method, dexItemFactory);
List<Instruction> dexInstructions = new ArrayList<>(numberOfInstructions);
int instructionOffset = 0;
InstructionIterator instructionIterator = ir.instructionIterator();
- int lastMoveExceptionOffset = -1;
while (instructionIterator.hasNext()) {
com.android.tools.r8.ir.code.Instruction ir = instructionIterator.next();
Info info = getInfo(ir);
int previousInstructionCount = dexInstructions.size();
info.addInstructions(this, dexInstructions);
-
- if (ir.isArgument()) {
- int register = registerAllocator.getRegisterForValue(ir.outValue(), ir.getNumber());
- debugEventBuilder.startArgument(register, ir.getLocalInfo(), ir.outValue().isThis());
- } else if (ir.isDebugPosition()) {
- int pc = lastMoveExceptionOffset >= 0 ? lastMoveExceptionOffset : instructionOffset;
- debugEventBuilder.setPosition(pc, ir.asDebugPosition());
- }
- lastMoveExceptionOffset = ir.isMoveException() ? instructionOffset : -1;
-
+ debugEventBuilder.add(instructionOffset, ir);
if (previousInstructionCount < dexInstructions.size()) {
while (previousInstructionCount < dexInstructions.size()) {
Instruction instruction = dexInstructions.get(previousInstructionCount++);
@@ -332,13 +322,8 @@
public void addGoto(com.android.tools.r8.ir.code.Goto jump) {
if (jump.getTarget() != nextBlock) {
add(jump, new GotoInfo(jump));
- return;
- }
- List<com.android.tools.r8.ir.code.Instruction> instructions = jump.getBlock().getInstructions();
- if (instructions.size() > 1) {
- addFallThroughOrNop(jump, instructions.get(instructions.size() - 2), nextBlock.entry());
} else {
- addFallThrough(jump);
+ addNop(jump);
}
}
@@ -351,30 +336,41 @@
add(move, new MoveInfo(move));
}
- public void addFallThrough(com.android.tools.r8.ir.code.Instruction instruction) {
+ public void addNop(com.android.tools.r8.ir.code.Instruction instruction) {
add(instruction, new FallThroughInfo(instruction));
}
- private void addFallThroughOrNop(
- com.android.tools.r8.ir.code.Instruction key,
- com.android.tools.r8.ir.code.Instruction instruction,
- com.android.tools.r8.ir.code.Instruction nextInstruction) {
- if (nextInstruction != null
- && instruction.isDebugPosition()
- && nextInstruction.isDebugPosition()) {
- add(key, new FixedSizeInfo(key, new Nop()));
- } else {
- addFallThrough(key);
- }
+ private boolean isNopInstruction(com.android.tools.r8.ir.code.Instruction instruction) {
+ return instruction.isDebugLocalsChange()
+ || (instruction.isConstNumber() && !instruction.outValue().needsRegister());
}
public void addDebugPosition(DebugPosition position) {
BasicBlock block = position.getBlock();
- int nextIndex = block.getInstructions().indexOf(position) + 1;
- List<com.android.tools.r8.ir.code.Instruction> instructions = block.getInstructions();
- com.android.tools.r8.ir.code.Instruction nextInstruction =
- nextIndex < instructions.size() ? instructions.get(nextIndex) : null;
- addFallThroughOrNop(position, position, nextInstruction);
+ int blockIndex = ir.blocks.indexOf(block);
+ Iterator<com.android.tools.r8.ir.code.Instruction> iterator =
+ block.listIterator(1 + block.getInstructions().indexOf(position));
+
+ com.android.tools.r8.ir.code.Instruction next = null;
+ while (next == null) {
+ next = iterator.next();
+ while (isNopInstruction(next)) {
+ next = iterator.next();
+ }
+ if (next.isGoto()) {
+ ++blockIndex;
+ BasicBlock nextBlock = blockIndex < ir.blocks.size() ? ir.blocks.get(blockIndex) : null;
+ if (next.asGoto().getTarget() == nextBlock) {
+ iterator = nextBlock.iterator();
+ next = null;
+ }
+ }
+ }
+ if (next.isDebugPosition()) {
+ add(position, new FixedSizeInfo(position, new Nop()));
+ } else {
+ addNop(position);
+ }
}
public void add(com.android.tools.r8.ir.code.Instruction ir, Instruction dex) {
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 eb0aa77..13f2d29 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
@@ -9,7 +9,7 @@
import com.android.tools.r8.ir.code.Add;
import com.android.tools.r8.ir.code.And;
import com.android.tools.r8.ir.code.BasicBlock;
-import com.android.tools.r8.ir.code.DebugPosition;
+import com.android.tools.r8.ir.code.DebugLocalsChange;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InstructionListIterator;
@@ -32,8 +32,12 @@
import com.google.common.collect.Multiset;
import com.google.common.collect.Multiset.Entry;
import com.google.common.collect.Multisets;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceOpenHashMap;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntIterator;
+import it.unimi.dsi.fastutil.ints.IntSet;
import java.util.ArrayList;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Iterator;
@@ -268,10 +272,8 @@
ranges.sort(LocalRange::compareTo);
// For each debug position compute the set of live locals.
- // The previously known locals is used to share locals when unchanged.
boolean localsChanged = false;
- ImmutableMap<Integer, DebugLocalInfo> previousLocals = ImmutableMap.of();
- Map<Integer, DebugLocalInfo> currentLocals = new HashMap<>();
+ Int2ReferenceMap<DebugLocalInfo> currentLocals = new Int2ReferenceOpenHashMap<>();
LinkedList<LocalRange> openRanges = new LinkedList<>();
Iterator<LocalRange> rangeIterator = ranges.iterator();
@@ -282,54 +284,89 @@
currentLocals.put(nextStartingRange.register, nextStartingRange.local);
openRanges.add(nextStartingRange);
nextStartingRange = rangeIterator.hasNext() ? rangeIterator.next() : null;
- localsChanged = true;
}
for (BasicBlock block : blocks) {
- Iterator<Instruction> instructionIterator = block.getInstructions().iterator();
+ boolean blockEntry = true;
+ ListIterator<Instruction> instructionIterator = block.listIterator();
while (instructionIterator.hasNext()) {
Instruction instruction = instructionIterator.next();
- if (!instruction.isDebugPosition()) {
- continue;
- }
int index = instruction.getNumber();
ListIterator<LocalRange> it = openRanges.listIterator(0);
+ Int2ReferenceMap<DebugLocalInfo> ending = new Int2ReferenceOpenHashMap<>();
+ Int2ReferenceMap<DebugLocalInfo> starting = new Int2ReferenceOpenHashMap<>();
while (it.hasNext()) {
LocalRange openRange = it.next();
if (openRange.end <= index) {
it.remove();
assert currentLocals.get(openRange.register) == openRange.local;
- currentLocals.put(openRange.register, null);
+ currentLocals.remove(openRange.register);
localsChanged = true;
+ ending.put(openRange.register, openRange.local);
}
}
while (nextStartingRange != null && nextStartingRange.start <= index) {
// If the full range is between the two debug positions ignore it.
if (nextStartingRange.end > index) {
openRanges.add(nextStartingRange);
- assert currentLocals.get(nextStartingRange.register) == null;
+ assert !currentLocals.containsKey(nextStartingRange.register);
currentLocals.put(nextStartingRange.register, nextStartingRange.local);
+ starting.put(nextStartingRange.register, nextStartingRange.local);
localsChanged = true;
}
nextStartingRange = rangeIterator.hasNext() ? rangeIterator.next() : null;
}
- DebugPosition position = instruction.asDebugPosition();
- if (localsChanged) {
- ImmutableMap.Builder<Integer, DebugLocalInfo> locals = ImmutableMap.builder();
- for (Map.Entry<Integer, DebugLocalInfo> entry : currentLocals.entrySet()) {
- if (entry.getValue() != null) {
- locals.put(entry);
+ if (blockEntry) {
+ blockEntry = false;
+ block.setLocalsAtEntry(new Int2ReferenceOpenHashMap<>(currentLocals));
+ } else if (localsChanged && shouldEmitChangesAtInstruction(instruction)) {
+ DebugLocalsChange change = createLocalsChange(ending, starting);
+ if (change != null) {
+ if (instruction.isDebugPosition() || instruction.isJumpInstruction()) {
+ instructionIterator.previous();
+ instructionIterator.add(new DebugLocalsChange(ending, starting));
+ instructionIterator.next();
+ } else {
+ instructionIterator.add(new DebugLocalsChange(ending, starting));
}
}
- localsChanged = false;
- previousLocals = locals.build();
}
- assert verifyLocalsEqual(previousLocals, currentLocals);
- position.setLocals(previousLocals);
+ localsChanged = false;
}
}
}
+ private DebugLocalsChange createLocalsChange(
+ Int2ReferenceMap<DebugLocalInfo> ending, Int2ReferenceMap<DebugLocalInfo> starting) {
+ if (ending.isEmpty() || starting.isEmpty()) {
+ return new DebugLocalsChange(ending, starting);
+ }
+ IntSet unneeded = new IntArraySet(Math.min(ending.size(), starting.size()));
+ for (Int2ReferenceMap.Entry<DebugLocalInfo> entry : ending.int2ReferenceEntrySet()) {
+ if (starting.get(entry.getIntKey()) == entry.getValue()) {
+ unneeded.add(entry.getIntKey());
+ }
+ }
+ if (unneeded.size() == ending.size() && unneeded.size() == starting.size()) {
+ return null;
+ }
+ IntIterator iterator = unneeded.iterator();
+ while (iterator.hasNext()) {
+ int key = iterator.nextInt();
+ ending.remove(key);
+ starting.remove(key);
+ }
+ return new DebugLocalsChange(ending, starting);
+ }
+
+ private boolean shouldEmitChangesAtInstruction(Instruction instruction) {
+ BasicBlock block = instruction.getBlock();
+ // We emit local changes on all non-exit instructions or, since we have only a singe return
+ // block, any exits directly targeting that.
+ return instruction != block.exit()
+ || (instruction.isGoto() && instruction.asGoto().getTarget() == code.getNormalExitBlock());
+ }
+
private boolean verifyLocalsEqual(
ImmutableMap<Integer, DebugLocalInfo> a, Map<Integer, DebugLocalInfo> b) {
int size = 0;
diff --git a/src/main/java/com/android/tools/r8/optimize/DebugStripper.java b/src/main/java/com/android/tools/r8/optimize/DebugStripper.java
index 9d39d29..3f83606 100644
--- a/src/main/java/com/android/tools/r8/optimize/DebugStripper.java
+++ b/src/main/java/com/android/tools/r8/optimize/DebugStripper.java
@@ -3,14 +3,12 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.optimize;
-import com.android.tools.r8.graph.DebugLocalInfo;
import com.android.tools.r8.graph.DexCode;
import com.android.tools.r8.graph.DexDebugEntry;
import com.android.tools.r8.graph.DexDebugEventBuilder;
import com.android.tools.r8.graph.DexDebugInfo;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexItemFactory;
-import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.naming.ClassNameMapper;
@@ -19,7 +17,6 @@
import com.android.tools.r8.naming.MemberNaming.Range;
import com.android.tools.r8.naming.MemberNaming.Signature;
import com.android.tools.r8.utils.InternalOptions;
-import com.google.common.collect.ImmutableMap;
import it.unimi.dsi.fastutil.objects.Reference2IntMap;
import it.unimi.dsi.fastutil.objects.Reference2IntOpenHashMap;
import java.util.List;
@@ -65,8 +62,8 @@
}
}
- private NumberedDebugInfo processDebugInfo(DexMethod method, DexDebugInfo info,
- MemberNaming naming, int startLine) {
+ private NumberedDebugInfo processDebugInfo(
+ DexEncodedMethod method, DexDebugInfo info, MemberNaming naming, int startLine) {
if (info == null || naming == null) {
return new NumberedDebugInfo(0, null);
}
@@ -75,13 +72,8 @@
// that pertains to a different method body.
Range currentRange = naming.topLevelRange;
DexDebugEventBuilder builder = new DexDebugEventBuilder(method, dexItemFactory);
- // Always start with a no-op bytecode to make sure that the start-line is manifested by
- // the Dalvik VM and the event based processing in R8. This also avoids empty bytecode
- // sequences.
- int entryCount = 1;
- DexString file = null;
- ImmutableMap<Integer, DebugLocalInfo> locals = null;
- builder.setPosition(0, startLine, file, locals);
+ // Below we insert the start-line at pc-0 except if another entry already defines pc-0.
+ int entryCount = 0;
for (DexDebugEntry entry : info.computeEntries()) {
boolean addEntry = false;
// We are in a range, check whether we have left it.
@@ -98,13 +90,21 @@
}
}
if (addEntry) {
+ if (entryCount == 0 && entry.address > 0) {
+ ++entryCount;
+ builder.setPosition(0, startLine);
+ }
int line = options.skipDebugLineNumberOpt
? entry.line
: startLine + ranges.indexOf(currentRange) + 1;
- builder.setPosition(entry.address, line, file, locals);
+ builder.setPosition(entry.address, line);
++entryCount;
}
}
+ if (entryCount == 0) {
+ ++entryCount;
+ builder.setPosition(0, startLine);
+ }
return new NumberedDebugInfo(entryCount, builder.build());
}
@@ -133,8 +133,8 @@
}
}
- NumberedDebugInfo numberedInfo = processDebugInfo(
- encodedMethod.method, originalInfo, naming, startLine);
+ NumberedDebugInfo numberedInfo =
+ processDebugInfo(encodedMethod, originalInfo, naming, startLine);
DexDebugInfo newInfo = numberedInfo.info;
if (!options.skipDebugLineNumberOpt) {
// Fix up the line information.
diff --git a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
index d10357c..a272eca 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
@@ -73,7 +73,7 @@
ART
}
- // Set to true to run tests with java
+ // Set to JAVA to run tests with java
private static final RuntimeKind RUNTIME_KIND = RuntimeKind.ART;
// Set to true to enable verbose logs
@@ -280,6 +280,13 @@
return inspect(t -> t.checkLocal(localName, expectedValue));
}
+ protected final JUnit3Wrapper.Command checkNoLocal(String localName) {
+ return inspect(t -> {
+ List<String> localNames = t.getLocalNames();
+ Assert.assertFalse("Unexpected local: " + localName, localNames.contains(localName));
+ });
+ }
+
protected final JUnit3Wrapper.Command checkNoLocal() {
return inspect(t -> {
List<String> localNames = t.getLocalNames();
diff --git a/src/test/java/com/android/tools/r8/debug/ExceptionTest.java b/src/test/java/com/android/tools/r8/debug/ExceptionTest.java
index 3d4202b..7ca9eae 100644
--- a/src/test/java/com/android/tools/r8/debug/ExceptionTest.java
+++ b/src/test/java/com/android/tools/r8/debug/ExceptionTest.java
@@ -14,22 +14,31 @@
@Test
public void testStepOnCatch() throws Throwable {
- int catchLine;
if (isRunningJava()) {
// Java jumps to first instruction of the catch handler, matching the source code.
- catchLine = 11;
+ runDebugTest("Exceptions",
+ breakpoint("Exceptions", "catchException"),
+ run(),
+ checkLine(SOURCE_FILE, 9), // line of the method call throwing the exception
+ stepOver(),
+ checkLine(SOURCE_FILE, 11), // first line in the catch handler
+ checkLocal("e"),
+ run());
} else {
// ART/Dalvik jumps to 'move-exception' which initializes the local variable with the pending
// exception. Thus it is "attached" to the line declaring the exception in the catch handler.
- catchLine = 10;
+ runDebugTest("Exceptions",
+ breakpoint("Exceptions", "catchException"),
+ run(),
+ checkLine(SOURCE_FILE, 9), // line of the method call throwing the exception
+ stepOver(),
+ checkLine(SOURCE_FILE, 10), // line of the catch declaration
+ checkNoLocal("e"),
+ stepOver(),
+ checkLine(SOURCE_FILE, 11), // first line in the catch handler
+ checkLocal("e"),
+ run());
}
- runDebugTest("Exceptions",
- breakpoint("Exceptions", "catchException"),
- run(),
- checkLine(SOURCE_FILE, 9), // line of the method call throwing the exception
- stepOver(),
- checkLine(SOURCE_FILE, catchLine), // line of the catch declaration
- run());
}
}
diff --git a/src/test/java/com/android/tools/r8/debug/LocalsTest.java b/src/test/java/com/android/tools/r8/debug/LocalsTest.java
index 810ec25..6a6a20f 100644
--- a/src/test/java/com/android/tools/r8/debug/LocalsTest.java
+++ b/src/test/java/com/android/tools/r8/debug/LocalsTest.java
@@ -285,7 +285,7 @@
run());
}
- @Test(expected = Throwable.class)
+ @Test
public void testInvokeRangeLong() throws Throwable {
final int initialValueOfX = 21;
final long expectedValueOfL = (long) initialValueOfX * 2;
diff --git a/src/test/java/com/android/tools/r8/debuginfo/DebugInfoInspector.java b/src/test/java/com/android/tools/r8/debuginfo/DebugInfoInspector.java
index 368a8fc..ad6f925 100644
--- a/src/test/java/com/android/tools/r8/debuginfo/DebugInfoInspector.java
+++ b/src/test/java/com/android/tools/r8/debuginfo/DebugInfoInspector.java
@@ -111,8 +111,10 @@
private int checkLines(int line, Consumer<DexDebugEntry> check) {
int found = 0;
- for (DexDebugEntry entry : entries) {
- if (entry.line == line) {
+ for (int i = 0; i < entries.size(); i++) {
+ DexDebugEntry entry = entries.get(i);
+ // Matches each entry at 'line' that is not a zero-line increment.
+ if (entry.line == line && (i == 0 || entries.get(i - 1).line != line)) {
found++;
check.accept(entry);
}
diff --git a/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java b/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java
index d53b6d6..72bbaed 100644
--- a/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java
+++ b/src/test/java/com/android/tools/r8/jasmin/DebugLocalTests.java
@@ -161,12 +161,7 @@
info.checkStartLine(1);
info.checkLineHasExactLocals(1, "this", "Test");
info.checkLineHasExactLocals(2, "this", "Test", "x", "int");
- // TODO(zerny): Reintroduce this check once we compute exact local changes. b/63243012
- // Without the exact local changes between the last debug position and the return we fail to
- // end the local scope of 'x'.
- if (false) {
- info.checkLineHasExactLocals(3, "this", "Test");
- }
+ info.checkLineHasExactLocals(3, "this", "Test");
String artResult = runOnArt(d8App, clazz.name);
assertEquals(expected, artResult);
