From 5d991e894b2b5404a9a25c7e6c20a7e37b55c7ad Mon Sep 17 00:00:00 2001 From: Allan Gregersen Date: Wed, 18 Jun 2025 11:34:28 +0200 Subject: [PATCH] simplify step out logic and cleanup step handling --- .../jdwp/api/VMEventListenerImpl.java | 10 ++- .../truffle/espresso/jdwp/impl/Commands.java | 8 +-- .../espresso/jdwp/impl/DebuggerCommand.java | 15 +---- .../jdwp/impl/DebuggerConnection.java | 17 +---- .../jdwp/impl/DebuggerController.java | 64 +++++-------------- .../jdwp/impl/RequestedJDWPEvents.java | 6 +- .../espresso/jdwp/impl/SteppingInfo.java | 19 +----- 7 files changed, 38 insertions(+), 101 deletions(-) diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/VMEventListenerImpl.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/VMEventListenerImpl.java index 47b1a0326c2f..c1d584194649 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/VMEventListenerImpl.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/VMEventListenerImpl.java @@ -38,6 +38,7 @@ import com.oracle.truffle.api.interop.UnsupportedMessageException; import com.oracle.truffle.espresso.jdwp.impl.BreakpointInfo; import com.oracle.truffle.espresso.jdwp.impl.ClassPrepareRequest; +import com.oracle.truffle.espresso.jdwp.impl.DebuggerCommand; import com.oracle.truffle.espresso.jdwp.impl.DebuggerController; import com.oracle.truffle.espresso.jdwp.impl.FieldBreakpointEvent; import com.oracle.truffle.espresso.jdwp.impl.FieldBreakpointInfo; @@ -530,7 +531,14 @@ public void stepCompleted(SteppingInfo info, CallFrame currentFrame) { stream.writeByte(currentFrame.getTypeTag()); stream.writeLong(currentFrame.getClassId()); stream.writeLong(currentFrame.getMethodId()); - long codeIndex = info.getStepOutBCI() != -1 ? info.getStepOutBCI() : currentFrame.getCodeIndex(); + long codeIndex = currentFrame.getCodeIndex(); + if (info.getStepKind() == DebuggerCommand.Kind.STEP_OUT) { + // Step out in Truffle is implemented on the callee exit, where the event's top + // stack frame is set to the caller frame. Hence, to avoid sending the code index + // from the caller location, we must fetch the next bci from the frame to pass the + // correct location. + codeIndex = context.getNextBCI(currentFrame.getRootNode(), currentFrame.getFrame()); + } stream.writeLong(codeIndex); debuggerController.fine(() -> "Sending step completed event"); diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/Commands.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/Commands.java index 8de8baef8b6b..5a2461fc3a8f 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/Commands.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/Commands.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -25,11 +25,7 @@ import java.util.concurrent.Callable; public interface Commands { - void stepOver(Object thread, RequestFilter filter); - - void stepInto(Object thread, RequestFilter filter); - - void stepOut(Object thread, RequestFilter filter); + void step(Object thread, RequestFilter filter, DebuggerCommand.Kind stepKind); Callable createLineBreakpointCommand(BreakpointInfo info); diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerCommand.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerCommand.java index cae1ed3e1a5e..79ed5094e4be 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerCommand.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerCommand.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -22,9 +22,9 @@ */ package com.oracle.truffle.espresso.jdwp.impl; -final class DebuggerCommand { +public final class DebuggerCommand { - enum Kind { + public enum Kind { STEP_INTO, STEP_OVER, STEP_OUT, @@ -37,21 +37,12 @@ enum Kind { private final RequestFilter requestFilter; private SourceLocation location; private BreakpointInfo breakpointInfo; - private volatile boolean submitted; DebuggerCommand(Kind kind, RequestFilter filter) { this.kind = kind; this.requestFilter = filter; } - public boolean isSubmitted() { - return submitted; - } - - public void markSubmitted() { - submitted = true; - } - void setSourceLocation(SourceLocation location) { this.location = location; } diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerConnection.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerConnection.java index 37721004381e..327b88e93383 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerConnection.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerConnection.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -58,19 +58,8 @@ public void closeSocket() { } @Override - public void stepInto(Object thread, RequestFilter filter) { - controller.setCommandRequestId(thread, filter.getRequestId(), filter.getSuspendPolicy(), false, false, DebuggerCommand.Kind.STEP_INTO); - } - - @Override - public void stepOver(Object thread, RequestFilter filter) { - controller.setCommandRequestId(thread, filter.getRequestId(), filter.getSuspendPolicy(), false, false, DebuggerCommand.Kind.STEP_OVER); - } - - @Override - public void stepOut(Object thread, RequestFilter filter) { - controller.setCommandRequestId(thread, filter.getRequestId(), filter.getSuspendPolicy(), false, false, DebuggerCommand.Kind.STEP_OUT); - controller.stepOut(filter); + public void step(Object thread, RequestFilter filter, DebuggerCommand.Kind stepKind) { + controller.setCommandRequestId(thread, filter.getRequestId(), filter.getSuspendPolicy(), false, false, stepKind); } @Override diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java index f3980d573c29..ad5d90dac127 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -397,33 +397,6 @@ public void clearBreakpoints() { } } - void stepOut(RequestFilter filter) { - Object thread = filter.getStepInfo().getGuestThread(); - fine(() -> "STEP_OUT for thread: " + getThreadName(thread)); - - SuspendedInfo susp = suspendedInfos.get(thread); - if (susp != null && !(susp instanceof UnknownSuspendedInfo)) { - doStepOut(susp); - } else { - fine(() -> "not STEPPING OUT for thread: " + getThreadName(thread)); - } - } - - private void doStepOut(SuspendedInfo susp) { - assert susp != null && !(susp instanceof UnknownSuspendedInfo); - RootNode callerRoot = susp.getCallerRootNode(); - int stepOutBCI = context.getNextBCI(callerRoot, susp.getCallerFrame()); - SteppingInfo steppingInfo = commandRequestIds.get(susp.getThread()); - if (steppingInfo != null && stepOutBCI != -1) { - // record the location that we'll land on after the step out completes - MethodVersionRef method = context.getMethodFromRootNode(callerRoot); - if (method != null) { - KlassRef klass = method.getMethod().getDeclaringKlassRef(); - steppingInfo.setStepOutBCI(context.getIds().getIdAsLong(klass), context.getIds().getIdAsLong(method.getMethod()), stepOutBCI); - } - } - } - public void clearStepCommand(StepInfo stepInfo) { commandRequestIds.remove(stepInfo.getGuestThread()); } @@ -977,21 +950,21 @@ public void onSuspend(SuspendedEvent event) { context.steppingInProgress(hostThread, false); return; } - CallFrame[] callFrames = createCallFrames(ids.getIdAsLong(currentThread), event.getStackFrames(), 1, steppingInfo); + CallFrame[] callFrames = createCallFrames(ids.getIdAsLong(currentThread), event.getStackFrames(), 1, false); // get the top frame for checking instance filters CallFrame callFrame = null; if (callFrames.length > 0) { callFrame = callFrames[0]; } - if (checkExclusionFilters(steppingInfo, event, currentThread, callFrame)) { + if (checkExclusionFilters(steppingInfo, event, callFrame)) { fine(() -> "not suspending here: " + event.getSourceSection()); // continue stepping until completed commandRequestIds.put(currentThread, steppingInfo); return; } } - - CallFrame[] callFrames = createCallFrames(ids.getIdAsLong(currentThread), event.getStackFrames(), -1, steppingInfo); + boolean isStepOut = steppingInfo != null && event.isStep() && steppingInfo.getStepKind() == DebuggerCommand.Kind.STEP_OUT; + CallFrame[] callFrames = createCallFrames(ids.getIdAsLong(currentThread), event.getStackFrames(), -1, isStepOut); RootNode callerRootNode = callFrames.length > 1 ? callFrames[1].getRootNode() : null; SuspendedInfo suspendedInfo = new SuspendedInfo(context, event, callFrames, currentThread, callerRootNode); @@ -1014,8 +987,8 @@ public void onSuspend(SuspendedEvent event) { result = checkForBreakpoints(event, jobs, currentThread, callFrames); if (!result.breakpointHit) { // no breakpoint - continueStepping(event, steppingInfo, currentThread); commandRequestIds.put(currentThread, steppingInfo); + continueStepping(event, steppingInfo); } } } else { @@ -1159,10 +1132,10 @@ private boolean matchLocation(Pattern[] patterns, CallFrame callFrame) { return false; } - private boolean checkExclusionFilters(SteppingInfo info, SuspendedEvent event, Object thread, CallFrame frame) { + private boolean checkExclusionFilters(SteppingInfo info, SuspendedEvent event, CallFrame frame) { if (info != null) { if (isSingleSteppingSuspended()) { - continueStepping(event, info, thread); + continueStepping(event, info); return true; } if (frame == null) { @@ -1177,7 +1150,7 @@ private boolean checkExclusionFilters(SteppingInfo info, SuspendedEvent event, O Object filterObject = context.getIds().fromId((int) requestFilter.getThisFilterId()); Object thisObject = frame.getThisValue(); if (filterObject != thisObject) { - continueStepping(event, info, thread); + continueStepping(event, info); return true; } } @@ -1186,7 +1159,7 @@ private boolean checkExclusionFilters(SteppingInfo info, SuspendedEvent event, O if (klass != null && requestFilter.isKlassExcluded(klass)) { // should not suspend here then, tell the event to keep going - continueStepping(event, info, thread); + continueStepping(event, info); return true; } } @@ -1194,7 +1167,7 @@ private boolean checkExclusionFilters(SteppingInfo info, SuspendedEvent event, O return false; } - private void continueStepping(SuspendedEvent event, SteppingInfo steppingInfo, Object thread) { + private void continueStepping(SuspendedEvent event, SteppingInfo steppingInfo) { switch (steppingInfo.getStepKind()) { case STEP_INTO: // stepping into unwanted code which was filtered @@ -1205,17 +1178,14 @@ private void continueStepping(SuspendedEvent event, SteppingInfo steppingInfo, O event.prepareStepOver(STEP_CONFIG); break; case STEP_OUT: - SuspendedInfo info = getSuspendedInfo(thread); - if (info != null && !(info instanceof UnknownSuspendedInfo)) { - doStepOut(info); - } + event.prepareStepOut(STEP_CONFIG); break; default: break; } } - private CallFrame[] createCallFrames(long threadId, Iterable stackFrames, int frameLimit, SteppingInfo steppingInfo) { + private CallFrame[] createCallFrames(long threadId, Iterable stackFrames, int frameLimit, boolean isStepOut) { LinkedList list = new LinkedList<>(); int frameCount = 0; for (DebugStackFrame frame : stackFrames) { @@ -1247,10 +1217,10 @@ private CallFrame[] createCallFrames(long threadId, Iterable st klassId = ids.getIdAsLong(klass); methodId = ids.getIdAsLong(methodVersion.getMethod()); typeTag = TypeTag.getKind(klass); - - // check if we have a dedicated step out code index on the top frame - if (frameCount == 0 && steppingInfo != null && steppingInfo.isStepOutFrame(methodId, klassId)) { - codeIndex = steppingInfo.getStepOutBCI(); + if (isStepOut) { + // Truffle reports step out at the callers entry to the method, so we must fetch + // the BCI that follows to get the expected location within the frame. + codeIndex = context.getNextBCI(root, rawFrame); } else { codeIndex = context.getBCI(rawNode, rawFrame); } diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/RequestedJDWPEvents.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/RequestedJDWPEvents.java index 282c504a8c0b..6fa6f22aaafc 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/RequestedJDWPEvents.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/RequestedJDWPEvents.java @@ -95,13 +95,13 @@ public CommandResult registerEvent(Packet packet, Commands callback) { Object thread = stepInfo.getGuestThread(); switch (stepInfo.getDepth()) { case SteppingConstants.INTO: - callback.stepInto(thread, filter); + callback.step(thread, filter, DebuggerCommand.Kind.STEP_INTO); break; case SteppingConstants.OVER: - callback.stepOver(thread, filter); + callback.step(thread, filter, DebuggerCommand.Kind.STEP_OVER); break; case SteppingConstants.OUT: - callback.stepOut(thread, filter); + callback.step(thread, filter, DebuggerCommand.Kind.STEP_OUT); break; } break; diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SteppingInfo.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SteppingInfo.java index 0fdaa3291f64..e2f71b6eeede 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SteppingInfo.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SteppingInfo.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -26,9 +26,6 @@ public final class SteppingInfo { private final int requestId; private final byte suspendPolicy; - private long stepOutBCI = -1; - private long stepOutMethodId = -1; - private long stepOutKlassId = -1; private final boolean isPopFrames; private final boolean isForceEarlyReturn; private final DebuggerCommand.Kind stepKind; @@ -50,16 +47,6 @@ public byte getSuspendPolicy() { return suspendPolicy; } - public void setStepOutBCI(long klassId, long methodId, long stepOutBCI) { - this.stepOutKlassId = klassId; - this.stepOutMethodId = methodId; - this.stepOutBCI = stepOutBCI; - } - - public long getStepOutBCI() { - return stepOutBCI; - } - public boolean isPopFrames() { return isPopFrames; } @@ -68,10 +55,6 @@ public boolean isForceEarlyReturn() { return isForceEarlyReturn; } - public boolean isStepOutFrame(long methodId, long klassId) { - return stepOutMethodId == methodId && stepOutKlassId == klassId; - } - public DebuggerCommand.Kind getStepKind() { return stepKind; }