Skip to content

Commit 5d991e8

Browse files
committed
simplify step out logic and cleanup step handling
1 parent e4bd1ea commit 5d991e8

File tree

7 files changed

+38
-101
lines changed

7 files changed

+38
-101
lines changed

espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/VMEventListenerImpl.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import com.oracle.truffle.api.interop.UnsupportedMessageException;
3939
import com.oracle.truffle.espresso.jdwp.impl.BreakpointInfo;
4040
import com.oracle.truffle.espresso.jdwp.impl.ClassPrepareRequest;
41+
import com.oracle.truffle.espresso.jdwp.impl.DebuggerCommand;
4142
import com.oracle.truffle.espresso.jdwp.impl.DebuggerController;
4243
import com.oracle.truffle.espresso.jdwp.impl.FieldBreakpointEvent;
4344
import com.oracle.truffle.espresso.jdwp.impl.FieldBreakpointInfo;
@@ -530,7 +531,14 @@ public void stepCompleted(SteppingInfo info, CallFrame currentFrame) {
530531
stream.writeByte(currentFrame.getTypeTag());
531532
stream.writeLong(currentFrame.getClassId());
532533
stream.writeLong(currentFrame.getMethodId());
533-
long codeIndex = info.getStepOutBCI() != -1 ? info.getStepOutBCI() : currentFrame.getCodeIndex();
534+
long codeIndex = currentFrame.getCodeIndex();
535+
if (info.getStepKind() == DebuggerCommand.Kind.STEP_OUT) {
536+
// Step out in Truffle is implemented on the callee exit, where the event's top
537+
// stack frame is set to the caller frame. Hence, to avoid sending the code index
538+
// from the caller location, we must fetch the next bci from the frame to pass the
539+
// correct location.
540+
codeIndex = context.getNextBCI(currentFrame.getRootNode(), currentFrame.getFrame());
541+
}
534542
stream.writeLong(codeIndex);
535543
debuggerController.fine(() -> "Sending step completed event");
536544

espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/Commands.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -25,11 +25,7 @@
2525
import java.util.concurrent.Callable;
2626

2727
public interface Commands {
28-
void stepOver(Object thread, RequestFilter filter);
29-
30-
void stepInto(Object thread, RequestFilter filter);
31-
32-
void stepOut(Object thread, RequestFilter filter);
28+
void step(Object thread, RequestFilter filter, DebuggerCommand.Kind stepKind);
3329

3430
Callable<Void> createLineBreakpointCommand(BreakpointInfo info);
3531

espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerCommand.java

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -22,9 +22,9 @@
2222
*/
2323
package com.oracle.truffle.espresso.jdwp.impl;
2424

25-
final class DebuggerCommand {
25+
public final class DebuggerCommand {
2626

27-
enum Kind {
27+
public enum Kind {
2828
STEP_INTO,
2929
STEP_OVER,
3030
STEP_OUT,
@@ -37,21 +37,12 @@ enum Kind {
3737
private final RequestFilter requestFilter;
3838
private SourceLocation location;
3939
private BreakpointInfo breakpointInfo;
40-
private volatile boolean submitted;
4140

4241
DebuggerCommand(Kind kind, RequestFilter filter) {
4342
this.kind = kind;
4443
this.requestFilter = filter;
4544
}
4645

47-
public boolean isSubmitted() {
48-
return submitted;
49-
}
50-
51-
public void markSubmitted() {
52-
submitted = true;
53-
}
54-
5546
void setSourceLocation(SourceLocation location) {
5647
this.location = location;
5748
}

espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerConnection.java

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -58,19 +58,8 @@ public void closeSocket() {
5858
}
5959

6060
@Override
61-
public void stepInto(Object thread, RequestFilter filter) {
62-
controller.setCommandRequestId(thread, filter.getRequestId(), filter.getSuspendPolicy(), false, false, DebuggerCommand.Kind.STEP_INTO);
63-
}
64-
65-
@Override
66-
public void stepOver(Object thread, RequestFilter filter) {
67-
controller.setCommandRequestId(thread, filter.getRequestId(), filter.getSuspendPolicy(), false, false, DebuggerCommand.Kind.STEP_OVER);
68-
}
69-
70-
@Override
71-
public void stepOut(Object thread, RequestFilter filter) {
72-
controller.setCommandRequestId(thread, filter.getRequestId(), filter.getSuspendPolicy(), false, false, DebuggerCommand.Kind.STEP_OUT);
73-
controller.stepOut(filter);
61+
public void step(Object thread, RequestFilter filter, DebuggerCommand.Kind stepKind) {
62+
controller.setCommandRequestId(thread, filter.getRequestId(), filter.getSuspendPolicy(), false, false, stepKind);
7463
}
7564

7665
@Override

espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java

Lines changed: 17 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -397,33 +397,6 @@ public void clearBreakpoints() {
397397
}
398398
}
399399

400-
void stepOut(RequestFilter filter) {
401-
Object thread = filter.getStepInfo().getGuestThread();
402-
fine(() -> "STEP_OUT for thread: " + getThreadName(thread));
403-
404-
SuspendedInfo susp = suspendedInfos.get(thread);
405-
if (susp != null && !(susp instanceof UnknownSuspendedInfo)) {
406-
doStepOut(susp);
407-
} else {
408-
fine(() -> "not STEPPING OUT for thread: " + getThreadName(thread));
409-
}
410-
}
411-
412-
private void doStepOut(SuspendedInfo susp) {
413-
assert susp != null && !(susp instanceof UnknownSuspendedInfo);
414-
RootNode callerRoot = susp.getCallerRootNode();
415-
int stepOutBCI = context.getNextBCI(callerRoot, susp.getCallerFrame());
416-
SteppingInfo steppingInfo = commandRequestIds.get(susp.getThread());
417-
if (steppingInfo != null && stepOutBCI != -1) {
418-
// record the location that we'll land on after the step out completes
419-
MethodVersionRef method = context.getMethodFromRootNode(callerRoot);
420-
if (method != null) {
421-
KlassRef klass = method.getMethod().getDeclaringKlassRef();
422-
steppingInfo.setStepOutBCI(context.getIds().getIdAsLong(klass), context.getIds().getIdAsLong(method.getMethod()), stepOutBCI);
423-
}
424-
}
425-
}
426-
427400
public void clearStepCommand(StepInfo stepInfo) {
428401
commandRequestIds.remove(stepInfo.getGuestThread());
429402
}
@@ -977,21 +950,21 @@ public void onSuspend(SuspendedEvent event) {
977950
context.steppingInProgress(hostThread, false);
978951
return;
979952
}
980-
CallFrame[] callFrames = createCallFrames(ids.getIdAsLong(currentThread), event.getStackFrames(), 1, steppingInfo);
953+
CallFrame[] callFrames = createCallFrames(ids.getIdAsLong(currentThread), event.getStackFrames(), 1, false);
981954
// get the top frame for checking instance filters
982955
CallFrame callFrame = null;
983956
if (callFrames.length > 0) {
984957
callFrame = callFrames[0];
985958
}
986-
if (checkExclusionFilters(steppingInfo, event, currentThread, callFrame)) {
959+
if (checkExclusionFilters(steppingInfo, event, callFrame)) {
987960
fine(() -> "not suspending here: " + event.getSourceSection());
988961
// continue stepping until completed
989962
commandRequestIds.put(currentThread, steppingInfo);
990963
return;
991964
}
992965
}
993-
994-
CallFrame[] callFrames = createCallFrames(ids.getIdAsLong(currentThread), event.getStackFrames(), -1, steppingInfo);
966+
boolean isStepOut = steppingInfo != null && event.isStep() && steppingInfo.getStepKind() == DebuggerCommand.Kind.STEP_OUT;
967+
CallFrame[] callFrames = createCallFrames(ids.getIdAsLong(currentThread), event.getStackFrames(), -1, isStepOut);
995968
RootNode callerRootNode = callFrames.length > 1 ? callFrames[1].getRootNode() : null;
996969

997970
SuspendedInfo suspendedInfo = new SuspendedInfo(context, event, callFrames, currentThread, callerRootNode);
@@ -1014,8 +987,8 @@ public void onSuspend(SuspendedEvent event) {
1014987
result = checkForBreakpoints(event, jobs, currentThread, callFrames);
1015988
if (!result.breakpointHit) {
1016989
// no breakpoint
1017-
continueStepping(event, steppingInfo, currentThread);
1018990
commandRequestIds.put(currentThread, steppingInfo);
991+
continueStepping(event, steppingInfo);
1019992
}
1020993
}
1021994
} else {
@@ -1159,10 +1132,10 @@ private boolean matchLocation(Pattern[] patterns, CallFrame callFrame) {
11591132
return false;
11601133
}
11611134

1162-
private boolean checkExclusionFilters(SteppingInfo info, SuspendedEvent event, Object thread, CallFrame frame) {
1135+
private boolean checkExclusionFilters(SteppingInfo info, SuspendedEvent event, CallFrame frame) {
11631136
if (info != null) {
11641137
if (isSingleSteppingSuspended()) {
1165-
continueStepping(event, info, thread);
1138+
continueStepping(event, info);
11661139
return true;
11671140
}
11681141
if (frame == null) {
@@ -1177,7 +1150,7 @@ private boolean checkExclusionFilters(SteppingInfo info, SuspendedEvent event, O
11771150
Object filterObject = context.getIds().fromId((int) requestFilter.getThisFilterId());
11781151
Object thisObject = frame.getThisValue();
11791152
if (filterObject != thisObject) {
1180-
continueStepping(event, info, thread);
1153+
continueStepping(event, info);
11811154
return true;
11821155
}
11831156
}
@@ -1186,15 +1159,15 @@ private boolean checkExclusionFilters(SteppingInfo info, SuspendedEvent event, O
11861159

11871160
if (klass != null && requestFilter.isKlassExcluded(klass)) {
11881161
// should not suspend here then, tell the event to keep going
1189-
continueStepping(event, info, thread);
1162+
continueStepping(event, info);
11901163
return true;
11911164
}
11921165
}
11931166
}
11941167
return false;
11951168
}
11961169

1197-
private void continueStepping(SuspendedEvent event, SteppingInfo steppingInfo, Object thread) {
1170+
private void continueStepping(SuspendedEvent event, SteppingInfo steppingInfo) {
11981171
switch (steppingInfo.getStepKind()) {
11991172
case STEP_INTO:
12001173
// stepping into unwanted code which was filtered
@@ -1205,17 +1178,14 @@ private void continueStepping(SuspendedEvent event, SteppingInfo steppingInfo, O
12051178
event.prepareStepOver(STEP_CONFIG);
12061179
break;
12071180
case STEP_OUT:
1208-
SuspendedInfo info = getSuspendedInfo(thread);
1209-
if (info != null && !(info instanceof UnknownSuspendedInfo)) {
1210-
doStepOut(info);
1211-
}
1181+
event.prepareStepOut(STEP_CONFIG);
12121182
break;
12131183
default:
12141184
break;
12151185
}
12161186
}
12171187

1218-
private CallFrame[] createCallFrames(long threadId, Iterable<DebugStackFrame> stackFrames, int frameLimit, SteppingInfo steppingInfo) {
1188+
private CallFrame[] createCallFrames(long threadId, Iterable<DebugStackFrame> stackFrames, int frameLimit, boolean isStepOut) {
12191189
LinkedList<CallFrame> list = new LinkedList<>();
12201190
int frameCount = 0;
12211191
for (DebugStackFrame frame : stackFrames) {
@@ -1247,10 +1217,10 @@ private CallFrame[] createCallFrames(long threadId, Iterable<DebugStackFrame> st
12471217
klassId = ids.getIdAsLong(klass);
12481218
methodId = ids.getIdAsLong(methodVersion.getMethod());
12491219
typeTag = TypeTag.getKind(klass);
1250-
1251-
// check if we have a dedicated step out code index on the top frame
1252-
if (frameCount == 0 && steppingInfo != null && steppingInfo.isStepOutFrame(methodId, klassId)) {
1253-
codeIndex = steppingInfo.getStepOutBCI();
1220+
if (isStepOut) {
1221+
// Truffle reports step out at the callers entry to the method, so we must fetch
1222+
// the BCI that follows to get the expected location within the frame.
1223+
codeIndex = context.getNextBCI(root, rawFrame);
12541224
} else {
12551225
codeIndex = context.getBCI(rawNode, rawFrame);
12561226
}

espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/RequestedJDWPEvents.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,13 @@ public CommandResult registerEvent(Packet packet, Commands callback) {
9595
Object thread = stepInfo.getGuestThread();
9696
switch (stepInfo.getDepth()) {
9797
case SteppingConstants.INTO:
98-
callback.stepInto(thread, filter);
98+
callback.step(thread, filter, DebuggerCommand.Kind.STEP_INTO);
9999
break;
100100
case SteppingConstants.OVER:
101-
callback.stepOver(thread, filter);
101+
callback.step(thread, filter, DebuggerCommand.Kind.STEP_OVER);
102102
break;
103103
case SteppingConstants.OUT:
104-
callback.stepOut(thread, filter);
104+
callback.step(thread, filter, DebuggerCommand.Kind.STEP_OUT);
105105
break;
106106
}
107107
break;

espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SteppingInfo.java

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -26,9 +26,6 @@ public final class SteppingInfo {
2626

2727
private final int requestId;
2828
private final byte suspendPolicy;
29-
private long stepOutBCI = -1;
30-
private long stepOutMethodId = -1;
31-
private long stepOutKlassId = -1;
3229
private final boolean isPopFrames;
3330
private final boolean isForceEarlyReturn;
3431
private final DebuggerCommand.Kind stepKind;
@@ -50,16 +47,6 @@ public byte getSuspendPolicy() {
5047
return suspendPolicy;
5148
}
5249

53-
public void setStepOutBCI(long klassId, long methodId, long stepOutBCI) {
54-
this.stepOutKlassId = klassId;
55-
this.stepOutMethodId = methodId;
56-
this.stepOutBCI = stepOutBCI;
57-
}
58-
59-
public long getStepOutBCI() {
60-
return stepOutBCI;
61-
}
62-
6350
public boolean isPopFrames() {
6451
return isPopFrames;
6552
}
@@ -68,10 +55,6 @@ public boolean isForceEarlyReturn() {
6855
return isForceEarlyReturn;
6956
}
7057

71-
public boolean isStepOutFrame(long methodId, long klassId) {
72-
return stepOutMethodId == methodId && stepOutKlassId == klassId;
73-
}
74-
7558
public DebuggerCommand.Kind getStepKind() {
7659
return stepKind;
7760
}

0 commit comments

Comments
 (0)