Skip to content

Commit 4fb70c7

Browse files
committed
8229012: When single stepping, the debug agent can cause the thread to remain in interpreter mode after single stepping completes
Reviewed-by: kevinw, sspitsyn
1 parent 4e60c2d commit 4fb70c7

File tree

2 files changed

+295
-3
lines changed

2 files changed

+295
-3
lines changed

src/jdk.jdwp.agent/share/native/libjdwp/stepControl.c

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1998, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1998, 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
@@ -858,6 +858,17 @@ stepControl_beginStep(JNIEnv *env, jthread thread, jint size, jint depth,
858858
return error;
859859
}
860860

861+
static jint
862+
getThreadState(jthread thread)
863+
{
864+
jint state = 0;
865+
jvmtiError error = JVMTI_FUNC_PTR(gdata->jvmti,GetThreadState)
866+
(gdata->jvmti, thread, &state);
867+
if (error != JVMTI_ERROR_NONE) {
868+
EXIT_ERROR(error, "getting thread state");
869+
}
870+
return state;
871+
}
861872

862873
static void
863874
clearStep(jthread thread, StepRequest *step)
@@ -877,15 +888,60 @@ clearStep(jthread thread, StepRequest *step)
877888
(void)eventHandler_free(step->methodEnterHandlerNode);
878889
step->methodEnterHandlerNode = NULL;
879890
}
880-
step->pending = JNI_FALSE;
881-
882891
/*
883892
* Warning: Do not clear step->method, step->lineEntryCount,
884893
* or step->lineEntries here, they will likely
885894
* be needed on the next step.
886895
*/
887896

897+
jvmtiError error;
898+
jboolean needsSuspending; // true if we needed to suspend this thread
899+
900+
// The thread needs suspending if it is not the current thread and is
901+
// not already suspended.
902+
if (isSameObject(getEnv(), threadControl_currentThread(), thread)) {
903+
needsSuspending = JNI_FALSE;
904+
} else {
905+
jint state = getThreadState(thread);
906+
needsSuspending = ((state & JVMTI_THREAD_STATE_SUSPENDED) == 0);
907+
}
908+
909+
if (needsSuspending) {
910+
// Don't use threadControl_suspendThread() here. It does a lot of
911+
// locking, increasing the risk of deadlock issues. None of that
912+
// locking is needed here.
913+
error = JVMTI_FUNC_PTR(gdata->jvmti,SuspendThread)
914+
(gdata->jvmti, thread);
915+
if (error != JVMTI_ERROR_NONE) {
916+
EXIT_ERROR(error, "suspending thread");
917+
}
918+
}
919+
920+
error = JVMTI_FUNC_PTR(gdata->jvmti,ClearAllFramePops)
921+
(gdata->jvmti, thread);
922+
if (error != JVMTI_ERROR_NONE) {
923+
#ifdef DEBUG
924+
jint currentDepth = getFrameCount(thread);
925+
jint state = getThreadState(thread);
926+
tty_message("JVMTI ERROR(%d): ClearAllFramePops (state=0x%x fromDepth=%d currentDepth=%d)",
927+
error, state, step->fromStackDepth, currentDepth);
928+
printThreadInfo(thread);
929+
printStackTrace(thread);
930+
threadControl_dumpThread(thread);
931+
#endif
932+
EXIT_ERROR(error, "clearing all frame pops");
933+
}
934+
935+
if (needsSuspending) {
936+
error = JVMTI_FUNC_PTR(gdata->jvmti,ResumeThread)
937+
(gdata->jvmti, thread);
938+
if (error != JVMTI_ERROR_NONE) {
939+
EXIT_ERROR(error, "resuming thread");
940+
}
941+
}
888942
}
943+
944+
step->pending = JNI_FALSE;
889945
}
890946

891947
jvmtiError
Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
// THIS TEST IS LINE NUMBER SENSITIVE
25+
26+
/**
27+
* @test
28+
* @bug 8229012
29+
* @summary Verify that during single stepping a method is not compiled and
30+
* after single stepping it is compiled.
31+
* @requires vm.compMode == "Xmixed"
32+
* @library /test/lib
33+
* @build jdk.test.whitebox.WhiteBox
34+
* @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
35+
* @run build TestScaffold VMConnection TargetListener TargetAdapter
36+
* @run compile -g SingleStepCompilationTest.java
37+
* @run driver SingleStepCompilationTest
38+
*/
39+
40+
import com.sun.jdi.*;
41+
import com.sun.jdi.event.*;
42+
import com.sun.jdi.request.*;
43+
44+
import java.util.*;
45+
import java.lang.reflect.Method;
46+
47+
import jdk.test.whitebox.WhiteBox;
48+
49+
class TestTarg {
50+
public final static int BKPT_LINE = 66;
51+
52+
public static void main(String[] args) {
53+
// Call buswork() and verify that is get compiled.
54+
busyWork("Warmup");
55+
if (!isCompiled()) {
56+
throw new RuntimeException("busywork() did not get compiled after warmup");
57+
}
58+
59+
// We need to force deopt the method now. Although we are about to force interp_only
60+
// mode by enabling single stepping, this does not result in the method being
61+
// deopt right away. It just causes the compiled method not to be used.
62+
deoptimizeMethod();
63+
64+
// Call busywork() again. This time the debugger will have single stepping
65+
// enabled. After calling, verify that busywork() is not compiled.
66+
busyWork("StepOver"); // BKPT_LINE: We do a STEP_OVER here
67+
if (isCompiled()) {
68+
throw new RuntimeException("busywork() compiled during single stepping");
69+
}
70+
71+
// Call busywork a 3rd time. This time the debugger will not have single stepping
72+
// enabled. After calling, verify that busywork is compiled.
73+
busyWork("AfterStep");
74+
if (!isCompiled()) {
75+
throw new RuntimeException("busywork() not compiled after single stepping completes");
76+
}
77+
}
78+
79+
private static final WhiteBox WB = WhiteBox.getWhiteBox();
80+
private static Method busyWorkMethod;
81+
static {
82+
try {
83+
busyWorkMethod = TestTarg.class.getDeclaredMethod("busyWork", String.class);
84+
} catch (NoSuchMethodException e) {
85+
throw new RuntimeException(e);
86+
}
87+
}
88+
89+
private static boolean isCompiled() {
90+
return WB.isMethodCompiled(busyWorkMethod, true) || WB.isMethodCompiled(busyWorkMethod, false);
91+
}
92+
93+
private static void deoptimizeMethod() {
94+
System.out.println("Decompile count before: " + WB.getMethodDecompileCount(busyWorkMethod));
95+
WB.deoptimizeMethod(busyWorkMethod, true);
96+
WB.deoptimizeMethod(busyWorkMethod, false);
97+
System.out.println("Decompile count after: " + WB.getMethodDecompileCount(busyWorkMethod));
98+
}
99+
100+
// We put the array and the result variables in static volatiles just to make sure
101+
// the compiler doesn't optimize them away.
102+
public static final int ARRAY_SIZE = 1000*1024;
103+
public static volatile int[] a = new int[ARRAY_SIZE];
104+
public static volatile long result = 0;
105+
106+
// Do something compute intensive to trigger compilation.
107+
public static void busyWork(String phase) {
108+
// Although timing is not necessary for this test, it is useful to have in the
109+
// log output for troubleshooting. The timing when step over is enabled should
110+
// be sigficantly slower than when not.
111+
long start = System.currentTimeMillis();
112+
113+
// Burn some CPU time and trigger OSR compilation.
114+
for (int j = 0; j < 500; j++) {
115+
for (int i = 0; i < ARRAY_SIZE; i++) {
116+
a[i] = j*i + i << 5 + j;
117+
}
118+
}
119+
for (int i = 0; i < ARRAY_SIZE; i++) {
120+
result += a[i];
121+
}
122+
123+
long end = System.currentTimeMillis();
124+
long totalTime = end - start;
125+
System.out.println(phase + " time: " + totalTime);
126+
}
127+
}
128+
129+
/********** test program **********/
130+
131+
public class SingleStepCompilationTest extends TestScaffold {
132+
ClassType targetClass;
133+
ThreadReference mainThread;
134+
135+
SingleStepCompilationTest (String args[]) {
136+
super(args);
137+
}
138+
139+
public static void main(String[] args) throws Exception {
140+
/*
141+
* We need to pass some extra args to the debuggee for WhiteBox support and
142+
* for more consistent compilation behavior.
143+
*/
144+
String[] newArgs = new String[5];
145+
if (args.length != 0) {
146+
throw new RuntimeException("Unexpected arguments passed to test");
147+
}
148+
149+
// These are all needed for WhiteBoxAPI
150+
newArgs[0] = "-Xbootclasspath/a:.";
151+
newArgs[1] = "-XX:+UnlockDiagnosticVMOptions";
152+
newArgs[2] = "-XX:+WhiteBoxAPI";
153+
154+
// In order to make sure the compilation is complete before we exit busyWork(),
155+
// we don't allow background compilations.
156+
newArgs[3] = "-XX:-BackgroundCompilation";
157+
158+
// Disabled tiered compilations so a new compilation doesn't start in the middle
159+
// of executing busyWork(). (Might not really be needed)
160+
newArgs[4] = "-XX:-TieredCompilation";
161+
162+
new SingleStepCompilationTest(newArgs).startTests();
163+
}
164+
165+
/********** event handlers **********/
166+
167+
EventRequestManager erm;
168+
BreakpointRequest bkptRequest;
169+
StepRequest stepRequest;
170+
171+
// When we get a bkpt we want to disable the request and enable single stepping.
172+
public void breakpointReached(BreakpointEvent event) {
173+
System.out.println("Got BreakpointEvent: " + event);
174+
EventRequest req = event.request();
175+
req.disable();
176+
stepRequest = erm.createStepRequest(mainThread,
177+
StepRequest.STEP_LINE,
178+
StepRequest.STEP_OVER);
179+
stepRequest.enable();
180+
}
181+
182+
public void stepCompleted(StepEvent event) {
183+
System.out.println("Got StepEvent: " + event);
184+
EventRequest req = event.request();
185+
req.disable();
186+
}
187+
188+
public void eventSetComplete(EventSet set) {
189+
set.resume();
190+
}
191+
192+
public void vmDisconnected(VMDisconnectEvent event) {
193+
System.out.println("Got VMDisconnectEvent");
194+
}
195+
196+
/********** test core **********/
197+
198+
protected void runTests() throws Exception {
199+
/*
200+
* Get to the top of main() to determine targetClass and mainThread.
201+
*/
202+
BreakpointEvent bpe = startToMain("TestTarg");
203+
targetClass = (ClassType)bpe.location().declaringType();
204+
mainThread = bpe.thread();
205+
erm = vm().eventRequestManager();
206+
207+
// The BKPT_LINE is the line we will STEP_OVER.
208+
Location loc1 = findLocation(targetClass, TestTarg.BKPT_LINE);
209+
bkptRequest = erm.createBreakpointRequest(loc1);
210+
bkptRequest.enable();
211+
212+
try {
213+
addListener(this);
214+
} catch (Exception ex) {
215+
ex.printStackTrace();
216+
failure("failure: Could not add listener");
217+
throw new RuntimeException("SingleStepCompilationTest: failed", ex);
218+
}
219+
220+
vm().resume();
221+
waitForVMDisconnect();
222+
223+
System.out.println("done with loop");
224+
removeListener(this);
225+
226+
/*
227+
* Deal with results of test. If anything has called failure("foo")
228+
* then testFailed will be true.
229+
*/
230+
if (!testFailed) {
231+
System.out.println("SingleStepCompilationTest: passed");
232+
} else {
233+
throw new Exception("SingleStepCompilationTest: failed");
234+
}
235+
}
236+
}

0 commit comments

Comments
 (0)