Skip to content

Commit 80aaa40

Browse files
entlicherentlicher
committed
[GR-67909] Do not step over roots and use the next BCI depending on the suspend anchor.
PullRequest: graal/21592
2 parents e9181df + 889a5b6 commit 80aaa40

File tree

9 files changed

+34
-35
lines changed

9 files changed

+34
-35
lines changed

‎espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/JDWPContext.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -399,11 +399,12 @@ public interface JDWPContext {
399399
/**
400400
* Returns the bci of the next bytecode instruction within the current frame
401401
*
402-
* @param callerRoot the root node of the caller frame
402+
* @param method the current method
403+
* @param rawNode the current node
403404
* @param frame the frame to read the current bci from
404405
* @return the bci of the next instruction
405406
*/
406-
int getNextBCI(RootNode callerRoot, Frame frame);
407+
int getNextBCI(MethodRef method, Node rawNode, Frame frame);
407408

408409
/**
409410
* Returns the current BCI or -1 if the BCI cannot be read.
@@ -412,7 +413,7 @@ public interface JDWPContext {
412413
* @param frame the frame to read the bci from
413414
* @return the BCI or -1
414415
*/
415-
long readBCIFromFrame(RootNode root, Frame frame);
416+
int readBCIFromFrame(RootNode root, Frame frame);
416417

417418
/**
418419
* Returns a {@link CallFrame} representation of the location of
@@ -478,7 +479,7 @@ public interface JDWPContext {
478479
* @param frame the current frame
479480
* @return the current bci
480481
*/
481-
long getBCI(Node rawNode, Frame frame);
482+
int getBCI(Node rawNode, Frame frame);
482483

483484
/**
484485
* Returns the instrumentable delegate node for the language root node or <code>rootNode</code>

‎espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/MethodRef.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ public interface MethodRef {
231231
*
232232
* @return last bci
233233
*/
234-
long getLastBCI();
234+
int getLastBCI();
235235

236236
/**
237237
* Determines if the method is a constructor.

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
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;
4241
import com.oracle.truffle.espresso.jdwp.impl.DebuggerController;
4342
import com.oracle.truffle.espresso.jdwp.impl.FieldBreakpointEvent;
4443
import com.oracle.truffle.espresso.jdwp.impl.FieldBreakpointInfo;
@@ -532,13 +531,6 @@ public void stepCompleted(SteppingInfo info, CallFrame currentFrame) {
532531
stream.writeLong(currentFrame.getClassId());
533532
stream.writeLong(currentFrame.getMethodId());
534533
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-
}
542534
stream.writeLong(codeIndex);
543535
debuggerController.fine(() -> "Sending step completed event");
544536

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public void initialize(Debugger debug, JDWPOptions jdwpOptions, JDWPContext jdwp
128128
ids.injectLogger(jdwpLogger);
129129

130130
// set up the debug session object early to make sure instrumentable nodes are materialized
131-
debuggerSession = debug.startSession(new SuspendedCallbackImpl(), SourceElement.ROOT, SourceElement.STATEMENT);
131+
debuggerSession = debug.startSession(new SuspendedCallbackImpl(), SourceElement.STATEMENT);
132132
debuggerSession.setSteppingFilter(SuspensionFilter.newBuilder().ignoreLanguageContextInitialization(true).build());
133133

134134
init(jdwpContext);
@@ -963,8 +963,8 @@ public void onSuspend(SuspendedEvent event) {
963963
return;
964964
}
965965
}
966-
boolean isStepOut = steppingInfo != null && event.isStep() && steppingInfo.getStepKind() == DebuggerCommand.Kind.STEP_OUT;
967-
CallFrame[] callFrames = createCallFrames(ids.getIdAsLong(currentThread), event.getStackFrames(), -1, isStepOut);
966+
boolean isAfter = event.getSuspendAnchor() == SuspendAnchor.AFTER;
967+
CallFrame[] callFrames = createCallFrames(ids.getIdAsLong(currentThread), event.getStackFrames(), -1, isAfter);
968968
RootNode callerRootNode = callFrames.length > 1 ? callFrames[1].getRootNode() : null;
969969

970970
SuspendedInfo suspendedInfo = new SuspendedInfo(context, event, callFrames, currentThread, callerRootNode);
@@ -1185,7 +1185,7 @@ private void continueStepping(SuspendedEvent event, SteppingInfo steppingInfo) {
11851185
}
11861186
}
11871187

1188-
private CallFrame[] createCallFrames(long threadId, Iterable<DebugStackFrame> stackFrames, int frameLimit, boolean isStepOut) {
1188+
private CallFrame[] createCallFrames(long threadId, Iterable<DebugStackFrame> stackFrames, int frameLimit, boolean isAfter) {
11891189
LinkedList<CallFrame> list = new LinkedList<>();
11901190
int frameCount = 0;
11911191
for (DebugStackFrame frame : stackFrames) {
@@ -1212,15 +1212,16 @@ private CallFrame[] createCallFrames(long threadId, Iterable<DebugStackFrame> st
12121212

12131213
Frame rawFrame = frame.getRawFrame(context.getLanguageClass(), FrameInstance.FrameAccess.READ_WRITE);
12141214
MethodVersionRef methodVersion = context.getMethodFromRootNode(root);
1215-
KlassRef klass = methodVersion.getMethod().getDeclaringKlassRef();
1215+
MethodRef method = methodVersion.getMethod();
1216+
KlassRef klass = method.getDeclaringKlassRef();
12161217

12171218
klassId = ids.getIdAsLong(klass);
1218-
methodId = ids.getIdAsLong(methodVersion.getMethod());
1219+
methodId = ids.getIdAsLong(method);
12191220
typeTag = TypeTag.getKind(klass);
1220-
if (isStepOut) {
1221-
// Truffle reports step out at the callers entry to the method, so we must fetch
1221+
if (isAfter && frameCount == 0) {
1222+
// Truffle reports anchor after this instruction, so we must fetch
12221223
// the BCI that follows to get the expected location within the frame.
1223-
codeIndex = context.getNextBCI(root, rawFrame);
1224+
codeIndex = context.getNextBCI(method, rawNode, rawFrame);
12241225
} else {
12251226
codeIndex = context.getBCI(rawNode, rawFrame);
12261227
}

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/impl/Method.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1390,7 +1390,7 @@ public int getLastLine() {
13901390
}
13911391

13921392
@Override
1393-
public long getLastBCI() {
1393+
public int getLastBCI() {
13941394
int bci = 0;
13951395
BytecodeStream bs = new BytecodeStream(getOriginalCode());
13961396
int end = bs.endBCI();

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/JDWPContextImpl.java‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -665,19 +665,21 @@ public int getCatchLocation(MethodRef method, Object guestException, int bci) {
665665
}
666666

667667
@Override
668-
public int getNextBCI(RootNode callerRoot, Frame frame) {
669-
if (callerRoot instanceof EspressoRootNode espressoRootNode) {
670-
int bci = (int) readBCIFromFrame(callerRoot, frame);
671-
if (bci >= 0) {
672-
BytecodeStream bs = new BytecodeStream(espressoRootNode.getMethodVersion().getOriginalCode());
673-
return bs.nextBCI(bci);
668+
public int getNextBCI(MethodRef method, Node rawNode, Frame frame) {
669+
int bci = getBCI(rawNode, frame);
670+
if (bci >= 0) {
671+
BytecodeStream bs = new BytecodeStream(method.getOriginalCode());
672+
int nextBci = bs.nextBCI(bci);
673+
if (nextBci < bs.endBCI()) {
674+
// Use the next only if it's in bounds.
675+
bci = nextBci;
674676
}
675677
}
676-
return -1;
678+
return bci;
677679
}
678680

679681
@Override
680-
public long readBCIFromFrame(RootNode root, Frame frame) {
682+
public int readBCIFromFrame(RootNode root, Frame frame) {
681683
if (root instanceof EspressoRootNode rootNode && frame != null) {
682684
return rootNode.readBCI(frame);
683685
}
@@ -803,7 +805,7 @@ private BciProvider getBciProviderNode(Node node) {
803805
return null;
804806
}
805807

806-
public long getBCI(Node rawNode, Frame frame) {
808+
public int getBCI(Node rawNode, Frame frame) {
807809
BciProvider bciProvider = getBciProviderNode(rawNode);
808810
if (bciProvider == null) {
809811
return -1;

‎tools/src/com.oracle.truffle.tools.chromeinspector.test/src/com/oracle/truffle/tools/chromeinspector/test/SLInspectDebugTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1322,7 +1322,7 @@ public void testRestartFrame() throws Exception {
13221322
"{\"name\":\"global\",\"type\":\"global\",\"object\":{\"description\":\"global\",\"type\":\"object\",\"objectId\":\"10\"}}]," +
13231323
"\"this\":null," +
13241324
"\"functionLocation\":{\"scriptId\":\"1\",\"columnNumber\":9,\"lineNumber\":4}," +
1325-
"\"location\":{\"scriptId\":\"1\",\"columnNumber\":28,\"lineNumber\":8}," +
1325+
"\"location\":{\"scriptId\":\"1\",\"columnNumber\":12,\"lineNumber\":8}," +
13261326
"\"url\":\"" + srcURL + "\"}," +
13271327
"{\"callFrameId\":\"1\",\"functionName\":\"factorial\"," +
13281328
"\"scopeChain\":[{\"name\":\"factorial\",\"type\":\"local\",\"object\":{\"description\":\"factorial\",\"type\":\"object\",\"objectId\":\"11\"}}," +
@@ -1347,7 +1347,7 @@ public void testRestartFrame() throws Exception {
13471347
"{\"name\":\"global\",\"type\":\"global\",\"object\":{\"description\":\"global\",\"type\":\"object\",\"objectId\":\"16\"}}]," +
13481348
"\"this\":null," +
13491349
"\"functionLocation\":{\"scriptId\":\"1\",\"columnNumber\":9,\"lineNumber\":0}," +
1350-
"\"location\":{\"scriptId\":\"1\",\"columnNumber\":14,\"lineNumber\":2}," +
1350+
"\"location\":{\"scriptId\":\"1\",\"columnNumber\":2,\"lineNumber\":2}," +
13511351
"\"url\":\"" + srcURL + "\"}]}," +
13521352
"\"id\":5}\n"));
13531353
tester.sendMessage("{\"id\":6,\"method\":\"Debugger.resume\"}");

‎truffle/src/com.oracle.truffle.api.debug.test/src/com/oracle/truffle/api/debug/test/ReenterStackFrameTest.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242

4343
import com.oracle.truffle.api.debug.DebugStackFrame;
4444
import com.oracle.truffle.api.debug.DebuggerSession;
45+
import com.oracle.truffle.api.debug.SuspendAnchor;
4546
import com.oracle.truffle.api.debug.SuspendedEvent;
4647
import java.util.Iterator;
4748
import org.graalvm.polyglot.Source;
@@ -88,6 +89,8 @@ public void testReenterCurrent() throws Throwable {
8889
expectSuspended((SuspendedEvent event) -> {
8990
DebugStackFrame currentFrame = event.getTopStackFrame();
9091
try {
92+
// Assert that we're before the CALL again after unwind
93+
Assert.assertEquals(SuspendAnchor.BEFORE, event.getSuspendAnchor());
9194
Assert.assertEquals("CALL(a)", currentFrame.getSourceSection().getCharacters());
9295
Iterator<DebugStackFrame> frames = event.getStackFrames().iterator();
9396
Assert.assertEquals("", frames.next().getName());

‎truffle/src/com.oracle.truffle.api.debug/src/com/oracle/truffle/api/debug/DebuggerSession.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1140,7 +1140,7 @@ private void notifyUnwindCallback(MaterializedFrame frame, InsertableNode insert
11401140
// Fake the caller context
11411141
Caller caller = findCurrentCaller(this, includeInternal);
11421142
SuspendedContext context = SuspendedContext.create(caller.node, ((SteppingStrategy.Unwind) s).unwind);
1143-
doSuspend(context, SuspendAnchor.AFTER, caller.frame, insertableNode, false, true);
1143+
doSuspend(context, SuspendAnchor.BEFORE, caller.frame, insertableNode, false, true);
11441144
}
11451145

11461146
static Caller findCurrentCaller(DebuggerSession session, boolean includeInternal) {

0 commit comments

Comments
 (0)