Account for preamble position when reprocessing ir
Bug: b/263457890
Change-Id: I4721c611e6f5475d2cf18092b53520c12d8469c1
diff --git a/src/main/java/com/android/tools/r8/graph/CfCode.java b/src/main/java/com/android/tools/r8/graph/CfCode.java
index fff7a6e..82e4b48 100644
--- a/src/main/java/com/android/tools/r8/graph/CfCode.java
+++ b/src/main/java/com/android/tools/r8/graph/CfCode.java
@@ -399,6 +399,23 @@
return true;
}
+ public Position getPreamblePosition() {
+ Position preamble = null;
+ for (CfInstruction instruction : instructions) {
+ if (instruction.isLabel()) {
+ continue;
+ }
+ if (instruction.isPosition()) {
+ Position candidate = instruction.asPosition().getPosition();
+ if (candidate.getLine() == 0) {
+ preamble = candidate;
+ }
+ }
+ break;
+ }
+ return preamble;
+ }
+
private static class PrunePreambleMethodVisitor extends MethodVisitor {
private final AppView<?> appView;
diff --git a/src/main/java/com/android/tools/r8/graph/Code.java b/src/main/java/com/android/tools/r8/graph/Code.java
index bc0c72b..c70e226 100644
--- a/src/main/java/com/android/tools/r8/graph/Code.java
+++ b/src/main/java/com/android/tools/r8/graph/Code.java
@@ -187,7 +187,7 @@
throw new Unreachable();
}
- public Position newInlineePosition(
+ public static Position newInlineePosition(
Position callerPosition, Position oldPosition, boolean isCalleeD8R8Synthesized) {
Position outermostCaller = oldPosition.getOutermostCaller();
if (!isCalleeD8R8Synthesized) {
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 5dcc091..c7e251e 100644
--- a/src/main/java/com/android/tools/r8/graph/DexCode.java
+++ b/src/main/java/com/android/tools/r8/graph/DexCode.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph;
+import static com.android.tools.r8.graph.DexDebugEventBuilder.addDefaultEventWithAdvancePcIfNecessary;
+import static com.android.tools.r8.utils.DexDebugUtils.computePreamblePosition;
import static com.android.tools.r8.utils.DexDebugUtils.verifySetPositionFramesFollowedByDefaultEvent;
import com.android.tools.r8.dex.CodeToKeep;
@@ -15,6 +17,7 @@
import com.android.tools.r8.dex.code.DexReturnVoid;
import com.android.tools.r8.dex.code.DexSwitchPayload;
import com.android.tools.r8.graph.DexCode.TryHandler.TypeAddrPair;
+import com.android.tools.r8.graph.DexDebugEvent.AdvanceLine;
import com.android.tools.r8.graph.DexDebugEvent.Default;
import com.android.tools.r8.graph.DexDebugEvent.SetPositionFrame;
import com.android.tools.r8.graph.DexDebugEvent.StartLocal;
@@ -35,7 +38,7 @@
import com.android.tools.r8.ir.conversion.MethodConversionOptions.ThrowingMethodConversionOptions;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.ArrayUtils;
-import com.android.tools.r8.utils.IntBox;
+import com.android.tools.r8.utils.DexDebugUtils.PositionInfo;
import com.android.tools.r8.utils.RetracerForCodePrinting;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.structural.Equatable;
@@ -47,11 +50,13 @@
import com.google.common.base.Strings;
import it.unimi.dsi.fastutil.ints.Int2IntMap;
import java.nio.ShortBuffer;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Consumer;
@@ -297,7 +302,8 @@
private DexDebugInfo debugInfoAsInlining(
DexMethod caller, DexMethod callee, boolean isCalleeD8R8Synthesized, DexItemFactory factory) {
- Position callerPosition = SyntheticPosition.builder().setLine(0).setMethod(caller).build();
+ Position callerPosition =
+ SyntheticPosition.builder().setLine(0).setMethod(caller).setIsD8R8Synthesized(true).build();
EventBasedDebugInfo eventBasedInfo = DexDebugInfo.convertToEventBased(this, factory);
if (eventBasedInfo == null) {
// If the method has no debug info we generate a preamble position to denote the inlining.
@@ -318,55 +324,78 @@
factory.createPositionFrame(preamblePosition), factory.zeroChangeDefaultEvent
});
}
- // The inline position should match the first actual callee position, so either its actual line
- // at first instruction or it is a synthetic preamble.
- int lineAtPcZero = findLineAtPcZero(callee, eventBasedInfo);
- PositionBuilder<?, ?> frameBuilder =
- lineAtPcZero == -1
- ? SyntheticPosition.builder().setLine(0)
- : SourcePosition.builder().setLine(lineAtPcZero);
+ // At this point we know we had existing debug information:
+ // 1) There is an already existing SET_POSITION_FRAME before a default event and the default
+ // event sets a position for PC 0
+ // => Nothing to do except append caller.
+ // 2) There is no SET_POSITION_FRAME before a default event and a default event covers PC 0.
+ // => Insert a SET_POSITION_FRAME
+ // 3) There is a SET_POSITION_FRAME and no default event setting a position for PC 0.
+ // => Insert a default event and potentially advance line.
+ // 4) There is no SET_POSITION_FRAME and no default event setting a position for PC 0..
+ // => Insert a SET_POSITION_FRAME and a default event and potentially advance line.
+ PositionInfo positionInfo = computePreamblePosition(callee, eventBasedInfo);
DexDebugEvent[] oldEvents = eventBasedInfo.events;
- DexDebugEvent[] newEvents = new DexDebugEvent[oldEvents.length + 1];
- int i = 0;
- newEvents[i++] =
- new SetPositionFrame(
- isCalleeD8R8Synthesized
- ? callerPosition
- : frameBuilder.setMethod(callee).setCallerPosition(callerPosition).build());
+ boolean adjustStartPosition =
+ !positionInfo.hasLinePositionAtPcZero() && debugInfo.getStartLine() > 0;
+ List<DexDebugEvent> newEvents =
+ new ArrayList<>(
+ oldEvents.length
+ + (positionInfo.hasFramePosition() ? 0 : 1)
+ + (positionInfo.hasLinePositionAtPcZero() ? 0 : 1)
+ + (adjustStartPosition ? 1 : 0)); // Potentially an advance line.
+ if (!positionInfo.hasFramePosition()) {
+ PositionBuilder<?, ?> calleePositionBuilder =
+ isCalleeD8R8Synthesized ? SyntheticPosition.builder() : SourcePosition.builder();
+ newEvents.add(
+ factory.createPositionFrame(
+ newInlineePosition(
+ callerPosition,
+ calleePositionBuilder
+ .setLine(
+ positionInfo.hasLinePositionAtPcZero()
+ ? positionInfo.getLinePositionAtPcZero()
+ : 0)
+ .setMethod(callee)
+ .setIsD8R8Synthesized(isCalleeD8R8Synthesized)
+ .build(),
+ isCalleeD8R8Synthesized)));
+ }
+ if (!positionInfo.hasLinePositionAtPcZero()) {
+ newEvents.add(factory.zeroChangeDefaultEvent);
+ }
for (DexDebugEvent event : oldEvents) {
- if (event instanceof SetPositionFrame) {
- SetPositionFrame oldFrame = (SetPositionFrame) event;
+ if (event.isAdvanceLine() && adjustStartPosition) {
+ AdvanceLine advanceLine = event.asAdvanceLine();
+ newEvents.add(factory.createAdvanceLine(debugInfo.getStartLine() + advanceLine.delta));
+ adjustStartPosition = false;
+ } else if (event.isDefaultEvent() && adjustStartPosition) {
+ Default oldDefaultEvent = event.asDefaultEvent();
+ addDefaultEventWithAdvancePcIfNecessary(
+ oldDefaultEvent.getLineDelta() + debugInfo.getStartLine(),
+ oldDefaultEvent.getPCDelta(),
+ newEvents,
+ factory);
+ adjustStartPosition = false;
+ } else if (event.isPositionFrame()) {
+ SetPositionFrame oldFrame = event.asSetPositionFrame();
assert oldFrame.getPosition() != null;
- newEvents[i++] =
+ newEvents.add(
new SetPositionFrame(
newInlineePosition(
- callerPosition, oldFrame.getPosition(), isCalleeD8R8Synthesized));
+ callerPosition, oldFrame.getPosition(), isCalleeD8R8Synthesized)));
} else {
- newEvents[i++] = event;
+ newEvents.add(event);
}
}
- return new EventBasedDebugInfo(eventBasedInfo.startLine, eventBasedInfo.parameters, newEvents);
- }
-
- private static int findLineAtPcZero(DexMethod method, EventBasedDebugInfo debugInfo) {
- IntBox lineAtPcZero = new IntBox(-1);
- DexDebugPositionState visitor =
- new DexDebugPositionState(debugInfo.startLine, method) {
- @Override
- public void visit(Default defaultEvent) {
- super.visit(defaultEvent);
- if (getCurrentPc() == 0) {
- lineAtPcZero.set(getCurrentLine());
- }
- }
- };
- for (DexDebugEvent event : debugInfo.events) {
- event.accept(visitor);
- if (visitor.getCurrentPc() > 0) {
- break;
- }
+ if (adjustStartPosition) {
+ // This only happens if we have no default event and the debug start line is > 0.
+ newEvents.add(factory.createAdvanceLine(debugInfo.getStartLine()));
}
- return lineAtPcZero.get();
+ return new EventBasedDebugInfo(
+ positionInfo.hasLinePositionAtPcZero() ? eventBasedInfo.getStartLine() : 0,
+ eventBasedInfo.parameters,
+ newEvents.toArray(DexDebugEvent.EMPTY_ARRAY));
}
public static int getLargestPrefix(DexItemFactory factory, DexString name) {
diff --git a/src/main/java/com/android/tools/r8/graph/DexDebugEvent.java b/src/main/java/com/android/tools/r8/graph/DexDebugEvent.java
index cd157e3..737df40 100644
--- a/src/main/java/com/android/tools/r8/graph/DexDebugEvent.java
+++ b/src/main/java/com/android/tools/r8/graph/DexDebugEvent.java
@@ -97,10 +97,22 @@
return false;
}
+ public boolean isAdvanceLine() {
+ return false;
+ }
+
public SetPositionFrame asSetPositionFrame() {
return null;
}
+ public Default asDefaultEvent() {
+ return null;
+ }
+
+ public AdvanceLine asAdvanceLine() {
+ return null;
+ }
+
public static class AdvancePC extends DexDebugEvent {
public final int delta;
@@ -267,6 +279,16 @@
}
@Override
+ public boolean isAdvanceLine() {
+ return true;
+ }
+
+ @Override
+ public AdvanceLine asAdvanceLine() {
+ return this;
+ }
+
+ @Override
public void internalWriteOn(
DebugBytecodeWriter writer, ObjectToOffsetMapping mapping, GraphLens graphLens) {
writer.putByte(Constants.DBG_ADVANCE_LINE);
@@ -648,6 +670,11 @@
}
@Override
+ public Default asDefaultEvent() {
+ return this;
+ }
+
+ @Override
boolean isWritableEvent() {
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/CanonicalPositions.java b/src/main/java/com/android/tools/r8/ir/code/CanonicalPositions.java
index 85467b7..bde37fd 100644
--- a/src/main/java/com/android/tools/r8/ir/code/CanonicalPositions.java
+++ b/src/main/java/com/android/tools/r8/ir/code/CanonicalPositions.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.ir.code;
+import com.android.tools.r8.graph.Code;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.ir.code.Position.SourcePosition;
import com.android.tools.r8.ir.code.Position.SyntheticPosition;
@@ -28,26 +29,23 @@
Position callerPosition,
int expectedPositionsCount,
DexMethod method,
- boolean methodIsSynthesized) {
+ boolean methodIsSynthesized,
+ Position preamblePosition) {
canonicalPositions =
new HashMap<>(1 + (callerPosition == null ? 0 : 1) + expectedPositionsCount);
+ if (preamblePosition == null) {
+ preamblePosition = SyntheticPosition.builder().setLine(0).setMethod(method).build();
+ }
if (callerPosition != null) {
this.callerPosition = getCanonical(callerPosition);
isCompilerSynthesizedInlinee = methodIsSynthesized;
- preamblePosition =
- methodIsSynthesized
- ? callerPosition
- : getCanonical(
- SourcePosition.builder()
- .setLine(0)
- .setMethod(method)
- .setCallerPosition(callerPosition)
- .build());
+ this.preamblePosition =
+ getCanonical(
+ Code.newInlineePosition(callerPosition, preamblePosition, methodIsSynthesized));
} else {
this.callerPosition = null;
isCompilerSynthesizedInlinee = false;
- preamblePosition =
- getCanonical(SyntheticPosition.builder().setLine(0).setMethod(method).build());
+ this.preamblePosition = getCanonical(preamblePosition);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
index bffd1fe..60d5807 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
@@ -252,7 +252,8 @@
callerPosition,
cfPositionCount,
originalMethod,
- method.getDefinition().isD8R8Synthesized());
+ method.getDefinition().isD8R8Synthesized(),
+ code.getPreamblePosition());
internalOutputMode = appView.options().getInternalOutputMode();
needsGeneratedMethodSynchronization =
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 3242779..229fcfc 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
@@ -44,6 +44,7 @@
import com.android.tools.r8.ir.code.CanonicalPositions;
import com.android.tools.r8.ir.code.CatchHandlers;
import com.android.tools.r8.ir.code.Position;
+import com.android.tools.r8.utils.DexDebugUtils;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
@@ -94,7 +95,8 @@
callerPosition,
debugEntries == null ? 0 : debugEntries.size(),
originalMethod,
- method.getDefinition().isD8R8Synthesized());
+ method.getDefinition().isD8R8Synthesized(),
+ DexDebugUtils.computePreamblePosition(originalMethod, info).getFramePosition());
}
@Override
diff --git a/src/main/java/com/android/tools/r8/utils/DexDebugUtils.java b/src/main/java/com/android/tools/r8/utils/DexDebugUtils.java
index 1dc89fd..adfda77 100644
--- a/src/main/java/com/android/tools/r8/utils/DexDebugUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/DexDebugUtils.java
@@ -5,7 +5,13 @@
package com.android.tools.r8.utils;
import com.android.tools.r8.graph.DexDebugEvent;
+import com.android.tools.r8.graph.DexDebugEvent.SetPositionFrame;
import com.android.tools.r8.graph.DexDebugInfo;
+import com.android.tools.r8.graph.DexDebugInfo.EventBasedDebugInfo;
+import com.android.tools.r8.graph.DexDebugPositionState;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.ir.code.Position;
+import com.android.tools.r8.utils.DexDebugUtils.PositionInfo.PositionInfoBuilder;
import java.util.List;
public class DexDebugUtils {
@@ -29,4 +35,81 @@
}
return true;
}
+
+ public static PositionInfo computePreamblePosition(
+ DexMethod method, EventBasedDebugInfo debugInfo) {
+ if (debugInfo == null) {
+ return PositionInfo.builder().build();
+ }
+ Box<Position> existingPositionFrame = new Box<>();
+ DexDebugPositionState visitor =
+ new DexDebugPositionState(debugInfo.startLine, method) {
+ @Override
+ public void visit(SetPositionFrame setPositionFrame) {
+ super.visit(setPositionFrame);
+ existingPositionFrame.set(setPositionFrame.getPosition());
+ }
+ };
+ PositionInfoBuilder builder = PositionInfo.builder();
+ for (DexDebugEvent event : debugInfo.events) {
+ event.accept(visitor);
+ if (visitor.getCurrentPc() > 0) {
+ break;
+ }
+ if (event.isDefaultEvent()) {
+ builder.setLinePositionAtPcZero(visitor.getCurrentLine());
+ builder.setFramePosition(existingPositionFrame.get());
+ }
+ }
+ return builder.build();
+ }
+
+ public static class PositionInfo {
+
+ private final Position framePosition;
+ private final int linePositionAtPcZero;
+
+ private PositionInfo(Position framePosition, int linePositionAtPcZero) {
+ this.framePosition = framePosition;
+ this.linePositionAtPcZero = linePositionAtPcZero;
+ }
+
+ public boolean hasFramePosition() {
+ return framePosition != null;
+ }
+
+ public boolean hasLinePositionAtPcZero() {
+ return linePositionAtPcZero > -1;
+ }
+
+ public Position getFramePosition() {
+ return framePosition;
+ }
+
+ public int getLinePositionAtPcZero() {
+ return linePositionAtPcZero;
+ }
+
+ public static PositionInfoBuilder builder() {
+ return new PositionInfoBuilder();
+ }
+
+ public static class PositionInfoBuilder {
+
+ private Position framePosition;
+ private int linePositionAtPcZero = -1;
+
+ public void setFramePosition(Position position) {
+ this.framePosition = position;
+ }
+
+ public void setLinePositionAtPcZero(int linePositionAtPcZero) {
+ this.linePositionAtPcZero = linePositionAtPcZero;
+ }
+
+ public PositionInfo build() {
+ return new PositionInfo(framePosition, linePositionAtPcZero);
+ }
+ }
+ }
}
diff --git a/src/main/java/com/android/tools/r8/utils/positions/DexPositionToNoPcMappedRangeMapper.java b/src/main/java/com/android/tools/r8/utils/positions/DexPositionToNoPcMappedRangeMapper.java
index a660a37..a6a2a06 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/DexPositionToNoPcMappedRangeMapper.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/DexPositionToNoPcMappedRangeMapper.java
@@ -87,9 +87,6 @@
private final List<MappedPosition> mappedPositions;
private final PositionRemapper positionRemapper;
private final List<DexDebugEvent> processedEvents;
- private final DexItemFactory factory;
-
- private final DexMethod startMethod;
// Keep track of what PC has been emitted.
private int emittedPc = 0;
@@ -109,8 +106,6 @@
this.mappedPositions = mappedPositions;
this.positionRemapper = positionRemapper;
this.processedEvents = processedEvents;
- this.factory = factory;
- this.startMethod = method;
}
// Force the current PC to emitted.
@@ -124,9 +119,6 @@
// A default event denotes a line table entry and must always be emitted. Remap its line.
@Override
public void visit(Default defaultEvent) {
- if (hasPreamblePosition(defaultEvent)) {
- emitPreamblePosition();
- }
super.visit(defaultEvent);
assert getCurrentLine() >= 0;
Position position = getPosition();
@@ -139,13 +131,6 @@
emittedPc = getCurrentPc();
}
- private boolean hasPreamblePosition(Default defaultEvent) {
- return getCurrentPc() == 0
- && defaultEvent.getPCDelta() > 0
- && currentPosition != null
- && currentPosition.getLine() != getCurrentLine();
- }
-
// Non-materializing events use super, ie, AdvancePC, AdvanceLine and SetInlineFrame.
// Materializing events are just amended to the stream.
@@ -184,17 +169,6 @@
flushPc();
processedEvents.add(restartLocal);
}
-
- public void emitPreamblePosition() {
- if (currentPosition == null || positionEventEmitter.didEmitLineEvents()) {
- return;
- }
- Position mappedPosition =
- PositionUtils.remapAndAdd(currentPosition, positionRemapper, mappedPositions);
- processedEvents.add(factory.createPositionFrame(mappedPosition));
- currentPosition = null;
- currentMethod = startMethod;
- }
}
private final AppView<?> appView;
@@ -206,7 +180,7 @@
}
public List<MappedPosition> optimizeDexCodePositions(
- ProgramMethod method, PositionRemapper positionRemapper, boolean hasOverloads) {
+ ProgramMethod method, PositionRemapper positionRemapper) {
List<MappedPosition> mappedPositions = new ArrayList<>();
// Do the actual processing for each method.
DexApplication application = appView.appInfo().app();
@@ -237,11 +211,6 @@
event.accept(visitor);
}
- // We still need to emit a preamble if we did not materialize any other instructions.
- if (mappedPositions.isEmpty()) {
- visitor.emitPreamblePosition();
- }
-
EventBasedDebugInfo optimizedDebugInfo =
new EventBasedDebugInfo(
positionEventEmitter.didEmitLineEvents() ? positionEventEmitter.getStartLine() : 0,
diff --git a/src/main/java/com/android/tools/r8/utils/positions/PositionToMappedRangeMapper.java b/src/main/java/com/android/tools/r8/utils/positions/PositionToMappedRangeMapper.java
index 914b174..2ada4e3 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/PositionToMappedRangeMapper.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/PositionToMappedRangeMapper.java
@@ -57,7 +57,7 @@
int pcEncodingCutoff) {
return canUseDexPc
? pcMapper.optimizeDexCodePositionsForPc(method, positionRemapper, pcEncodingCutoff)
- : noPcMapper.optimizeDexCodePositions(method, positionRemapper, hasOverloads);
+ : noPcMapper.optimizeDexCodePositions(method, positionRemapper);
}
@Override
