Support skipping of class loader frames in debug tests
The default debug filtering of IntelliJ skips frames in class loader.
This CL adds the same capability to our debug test infrastructure.
We used to skip frames by repeating the same step command. But to
skip class loading, we actually need to step out of the current frame
so that the whole class loading code has executed. In order to achieve
this, this CL refactors how step filtering is implemented. Now the
StepFilter instance is reponsible for adding extra step commands into
the commands queue when it is necessary. By extension, the 'step
until' capability is also a StepFilter instance now.
Since we can skip class loader frames now, this CL also enables the
tests that were skipped for Dalvik due to class loader frames:
* com.android.tools.r8.debug.InterfaceMethodTest#testDefaultMethod
* com.android.tools.r8.debug.MinificationTest#testBreakInMainClass
Bug: 67225390
Change-Id: I112161be31cc99ddcf1eeaa1b6a63a97a311b4d6
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 35760ef..de8c673 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
@@ -25,6 +25,7 @@
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.OffOrAuto;
+import com.google.common.collect.ImmutableList;
import it.unimi.dsi.fastutil.longs.LongArrayList;
import it.unimi.dsi.fastutil.longs.LongList;
import java.io.File;
@@ -434,14 +435,39 @@
private JUnit3Wrapper.Command step(StepKind stepKind, StepLevel stepLevel,
StepFilter stepFilter) {
- return new JUnit3Wrapper.Command.StepCommand(stepKind.jdwpValue, stepLevel.jdwpValue,
- stepFilter, state -> true);
+ return new JUnit3Wrapper.Command.StepCommand(stepKind, stepLevel, stepFilter);
}
protected JUnit3Wrapper.Command stepUntil(StepKind stepKind, StepLevel stepLevel,
Function<JUnit3Wrapper.DebuggeeState, Boolean> stepUntil) {
- return new JUnit3Wrapper.Command.StepCommand(stepKind.jdwpValue, stepLevel.jdwpValue, NO_FILTER,
- stepUntil);
+ return stepUntil(stepKind, stepLevel, stepUntil, DEFAULT_FILTER);
+ }
+
+ protected JUnit3Wrapper.Command stepUntil(StepKind stepKind, StepLevel stepLevel,
+ Function<JUnit3Wrapper.DebuggeeState, Boolean> stepUntil, StepFilter stepFilter) {
+ // We create an extension to the given step filter which will also check whether we need to
+ // step again according to the given stepUntil function.
+ StepFilter stepUntilFilter = new StepFilter() {
+ @Override
+ public List<String> getExcludedClasses() {
+ return stepFilter.getExcludedClasses();
+ }
+
+ @Override
+ public boolean skipLocation(JUnit3Wrapper.DebuggeeState debuggeeState, JUnit3Wrapper wrapper,
+ JUnit3Wrapper.Command.StepCommand stepCommand) {
+ if (stepFilter.skipLocation(debuggeeState, wrapper, stepCommand)) {
+ return true;
+ }
+ if (stepUntil.apply(debuggeeState) == Boolean.FALSE) {
+ // We did not reach the expected location so step again.
+ wrapper.enqueueCommandFirst(stepCommand);
+ return true;
+ }
+ return false;
+ }
+ };
+ return new JUnit3Wrapper.Command.StepCommand(stepKind, stepLevel, stepUntilFilter);
}
protected final JUnit3Wrapper.Command checkLocal(String localName) {
@@ -1512,23 +1538,14 @@
class StepCommand implements Command {
- private final byte stepDepth;
- private final byte stepSize;
+ private final StepKind stepDepth;
+ private final StepLevel stepSize;
private final StepFilter stepFilter;
- /**
- * A {@link Function} taking a {@link DebuggeeState} as input and returns {@code true} to
- * stop stepping, {@code false} to continue.
- */
- private final Function<JUnit3Wrapper.DebuggeeState, Boolean> stepUntil;
-
- public StepCommand(byte stepDepth,
- byte stepSize, StepFilter stepFilter,
- Function<DebuggeeState, Boolean> stepUntil) {
+ public StepCommand(StepKind stepDepth, StepLevel stepSize, StepFilter stepFilter) {
this.stepDepth = stepDepth;
this.stepSize = stepSize;
this.stepFilter = stepFilter;
- this.stepUntil = stepUntil;
}
@Override
@@ -1537,14 +1554,13 @@
int stepRequestID;
{
EventBuilder eventBuilder = Event.builder(EventKind.SINGLE_STEP, SuspendPolicy.ALL);
- eventBuilder.setStep(threadId, stepSize, stepDepth);
+ eventBuilder.setStep(threadId, stepSize.jdwpValue, stepDepth.jdwpValue);
stepFilter.getExcludedClasses().stream().forEach(s -> eventBuilder.setClassExclude(s));
ReplyPacket replyPacket = testBase.getMirror().setEvent(eventBuilder.build());
stepRequestID = replyPacket.getNextValueAsInt();
testBase.assertAllDataRead(replyPacket);
}
- testBase.events
- .put(stepRequestID, new StepEventHandler(this, stepRequestID, stepFilter, stepUntil));
+ testBase.events.put(stepRequestID, new StepEventHandler(this, stepRequestID, stepFilter));
// Resume all threads.
testBase.resume();
@@ -1552,8 +1568,8 @@
@Override
public String toString() {
- return String.format("step %s/%s", JDWPConstants.StepDepth.getName(stepDepth),
- JDWPConstants.StepSize.getName(stepSize));
+ return String.format("step %s/%s", JDWPConstants.StepDepth.getName(stepDepth.jdwpValue),
+ JDWPConstants.StepSize.getName(stepSize.jdwpValue));
}
}
@@ -1607,17 +1623,14 @@
private final JUnit3Wrapper.Command.StepCommand stepCommand;
private final int stepRequestID;
private final StepFilter stepFilter;
- private final Function<DebuggeeState, Boolean> stepUntil;
private StepEventHandler(
JUnit3Wrapper.Command.StepCommand stepCommand,
int stepRequestID,
- StepFilter stepFilter,
- Function<DebuggeeState, Boolean> stepUntil) {
+ StepFilter stepFilter) {
this.stepCommand = stepCommand;
this.stepRequestID = stepRequestID;
this.stepFilter = stepFilter;
- this.stepUntil = stepUntil;
}
@Override
@@ -1626,18 +1639,11 @@
testBase.getMirror().clearEvent(EventKind.SINGLE_STEP, stepRequestID);
testBase.events.remove(Integer.valueOf(stepRequestID));
- // Do we need to step again ?
- boolean repeatStep = false;
- if (stepFilter
- .skipLocation(testBase.getMirror(), testBase.getDebuggeeState().getLocation())) {
- repeatStep = true;
- } else if (stepUntil.apply(testBase.getDebuggeeState()) == Boolean.FALSE) {
- repeatStep = true;
- }
- if (repeatStep) {
- // In order to repeat the step now, we need to add it at the beginning of the queue.
- testBase.enqueueCommandFirst(stepCommand);
- }
+ // Let the filtering happen.
+ // Note: we don't need to know whether the location was skipped or not because we are
+ // going to process the next command(s) in the queue anyway.
+ stepFilter.skipLocation(testBase.getDebuggeeState(), testBase, stepCommand);
+
super.handle(testBase);
}
}
@@ -1659,7 +1665,8 @@
/**
* Indicates whether the given location must be skipped.
*/
- boolean skipLocation(VmMirror mirror, Location location);
+ boolean skipLocation(JUnit3Wrapper.DebuggeeState debuggeeState, JUnit3Wrapper wrapper,
+ JUnit3Wrapper.Command.StepCommand stepCommand);
/**
* A {@link StepFilter} that does not filter anything.
@@ -1672,7 +1679,8 @@
}
@Override
- public boolean skipLocation(VmMirror mirror, Location location) {
+ public boolean skipLocation(JUnit3Wrapper.DebuggeeState debuggeeState, JUnit3Wrapper wrapper,
+ JUnit3Wrapper.Command.StepCommand stepCommand) {
return false;
}
}
@@ -1687,7 +1695,7 @@
@Override
public List<String> getExcludedClasses() {
- return Arrays.asList(
+ return ImmutableList.of(
"com.sun.*",
"java.*",
"javax.*",
@@ -1707,8 +1715,10 @@
}
@Override
- public boolean skipLocation(VmMirror mirror, Location location) {
- // TODO(b/67225390) we also need to skip class loaders to act like IntelliJ.
+ public boolean skipLocation(JUnit3Wrapper.DebuggeeState debuggeeState, JUnit3Wrapper wrapper,
+ JUnit3Wrapper.Command.StepCommand stepCommand) {
+ VmMirror mirror = debuggeeState.getMirror();
+ Location location = debuggeeState.getLocation();
// Skip synthetic methods.
if (isLambdaMethod(mirror, location)) {
// Lambda methods are synthetic but we do want to stop there.
@@ -1722,14 +1732,37 @@
if (DEBUG_TESTS) {
System.out.println("Skipping lambda class wrapper method");
}
+ wrapper.enqueueCommandFirst(stepCommand);
return true;
}
if (isSyntheticMethod(mirror, location)) {
if (DEBUG_TESTS) {
System.out.println("Skipping synthetic method");
}
+ wrapper.enqueueCommandFirst(stepCommand);
return true;
}
+ if (isClassLoader(mirror, location)) {
+ if (DEBUG_TESTS) {
+ System.out.println("Skipping class loader");
+ }
+ wrapper.enqueueCommandFirst(
+ new JUnit3Wrapper.Command.StepCommand(StepKind.OUT, StepLevel.LINE, this));
+ return true;
+ }
+ return false;
+ }
+
+ private static boolean isClassLoader(VmMirror mirror, Location location) {
+ final long classLoaderClassID = mirror.getClassID("Ljava/lang/ClassLoader;");
+ assert classLoaderClassID != -1;
+ long classID = location.classID;
+ while (classID != 0) {
+ if (classID == classLoaderClassID) {
+ return true;
+ }
+ classID = mirror.getSuperclassId(classID);
+ }
return false;
}
@@ -1751,7 +1784,7 @@
return classSig.contains("$$Lambda$");
}
- private boolean isLambdaMethod(VmMirror mirror, Location location) {
+ private static boolean isLambdaMethod(VmMirror mirror, Location location) {
String methodName = mirror.getMethodName(location.classID, location.methodID);
return methodName.startsWith("lambda$");
}
diff --git a/src/test/java/com/android/tools/r8/debug/InterfaceMethodTest.java b/src/test/java/com/android/tools/r8/debug/InterfaceMethodTest.java
index 46ad815..713d042 100644
--- a/src/test/java/com/android/tools/r8/debug/InterfaceMethodTest.java
+++ b/src/test/java/com/android/tools/r8/debug/InterfaceMethodTest.java
@@ -22,10 +22,6 @@
@Test
public void testDefaultMethod() throws Throwable {
- // TODO(b/67225390) Dalvik steps into class loader first.
- Assume.assumeTrue("Dalvik suspends in class loader",
- ToolHelper.getDexVm().getVersion().isNewerThan(Version.V4_4_4));
-
String debuggeeClass = "DebugInterfaceMethod";
String parameterName = "msg";
String localVariableName = "name";
@@ -38,16 +34,16 @@
if (!supportsDefaultMethod()) {
// We desugared default method. This means we're going to step through an extra (forward)
// method first.
- commands.add(stepInto());
+ commands.add(stepInto(INTELLIJ_FILTER));
}
- commands.add(stepInto());
+ commands.add(stepInto(INTELLIJ_FILTER));
commands.add(checkLine(SOURCE_FILE, 9));
// TODO(shertz) we should see the local variable this even when desugaring.
if (supportsDefaultMethod()) {
commands.add(checkLocal("this"));
}
commands.add(checkLocal(parameterName));
- commands.add(stepOver());
+ commands.add(stepOver(INTELLIJ_FILTER));
commands.add(checkLocal(parameterName));
commands.add(checkLocal(localVariableName));
// TODO(shertz) check current method name ?
diff --git a/src/test/java/com/android/tools/r8/debug/MinificationTest.java b/src/test/java/com/android/tools/r8/debug/MinificationTest.java
index 6a297cd..db41173 100644
--- a/src/test/java/com/android/tools/r8/debug/MinificationTest.java
+++ b/src/test/java/com/android/tools/r8/debug/MinificationTest.java
@@ -3,14 +3,9 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.debug;
-import com.android.tools.r8.ToolHelper;
-import com.android.tools.r8.ToolHelper.DexVm;
-import com.android.tools.r8.debug.DebugTestBase.JUnit3Wrapper.Command;
import com.google.common.collect.ImmutableList;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
-import java.util.List;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -68,28 +63,18 @@
final String innerClassName = minifiedNames ? "a" : "Minified$Inner";
final String innerMethodName = minifiedNames ? "a" : "innerTest";
final String innerSignature = "()I";
- List<Command> commands =
- new ArrayList<>(
- Arrays.asList(
- breakpoint(className, methodName, signature),
- run(),
- checkMethod(className, methodName, signature),
- checkLine(SOURCE_FILE, 14),
- stepOver(),
- checkMethod(className, methodName, signature),
- checkLine(SOURCE_FILE, 15),
- stepInto()));
- // Dalvik first enters ClassLoader, step over it.
- // See also b/67225390.
- if (ToolHelper.getDexVm() == DexVm.ART_4_4_4_HOST) {
- commands.add(stepOver());
- }
- commands.addAll(
- Arrays.asList(
- checkMethod(innerClassName, innerMethodName, innerSignature),
- checkLine(SOURCE_FILE, 8),
- run()));
- runDebugTestR8(className, commands);
+ runDebugTestR8(className,
+ breakpoint(className, methodName, signature),
+ run(),
+ checkMethod(className, methodName, signature),
+ checkLine(SOURCE_FILE, 14),
+ stepOver(INTELLIJ_FILTER),
+ checkMethod(className, methodName, signature),
+ checkLine(SOURCE_FILE, 15),
+ stepInto(INTELLIJ_FILTER),
+ checkMethod(innerClassName, innerMethodName, innerSignature),
+ checkLine(SOURCE_FILE, 8),
+ run());
}
@Test
