Proper error handling of JVMTI OPAQUE_FRAME errors in JDWP and JDI. Get at least one JDI test running in vthread mode to detect OPAQUE_FRAME for each of PopFrame, ForceEarlyReturn, and SetLocalXXX.

plummercj · plummercj · commit 6a9145b68e69 · 2022-03-11T17:45:35.000Z
diff --git a/src/jdk.jdi/share/classes/com/sun/tools/jdi/ThreadReferenceImpl.java b/src/jdk.jdi/share/classes/com/sun/tools/jdi/ThreadReferenceImpl.java
@@ -41,6 +41,7 @@
 import com.sun.jdi.MonitorInfo;
 import com.sun.jdi.NativeMethodException;
 import com.sun.jdi.ObjectReference;
+import com.sun.jdi.OpaqueFrameException;
 import com.sun.jdi.ReferenceType;
 import com.sun.jdi.StackFrame;
 import com.sun.jdi.ThreadGroupReference;
@@ -585,7 +586,12 @@ public void forceEarlyReturn(Value returnValue) throws InvalidTypeException,
         } catch (JDWPException exc) {
             switch (exc.errorCode()) {
             case JDWP.Error.OPAQUE_FRAME:
-                throw new NativeMethodException();
+                if (meth.isNative()) {
+                    throw new NativeMethodException();
+                } else {
+                    assert isVirtual(); // can only happen with virtual threads
+                    throw new OpaqueFrameException();
+                }
             case JDWP.Error.THREAD_NOT_SUSPENDED:
                 throw new IncompatibleThreadStateException(
                          "Thread not suspended");
diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/StackFrameImpl.c b/src/jdk.jdwp.agent/share/native/libjdwp/StackFrameImpl.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2022, 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
@@ -447,13 +447,6 @@ popFrames(PacketInputStream *in, PacketOutputStream *out)
         return JNI_TRUE;
     }
 
-    /* "popFrames" is not supported on a vthread. Although JVMTI will produce INVALID_THREAD,
-       we check here and force the error to make it obvious to the reader. */
-    if (isVThread(thread)) {
-        outStream_setError(out, JDWP_ERROR(INVALID_THREAD));
-        return JNI_TRUE;
-    }
-
     fnum = getFrameNumber(frame);
     error = threadControl_popFrames(thread, fnum);
     if (error != JVMTI_ERROR_NONE) {
diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/ThreadReferenceImpl.c b/src/jdk.jdwp.agent/share/native/libjdwp/ThreadReferenceImpl.c
@@ -595,13 +595,6 @@ forceEarlyReturn(PacketInputStream *in, PacketOutputStream *out)
         return JNI_TRUE;
     }
 
-    /* "forceEarlyReturn" is not supported on a vthread. Although JVMTI will produce INVALID_THREAD,
-       we check here and force the error to make it obvious to the reader. */
-    if (isVThread(thread)) {
-        outStream_setError(out, JDWP_ERROR(INVALID_THREAD));
-        return JNI_TRUE;
-    }
-
     typeKey = inStream_readByte(in);
     if (inStream_error(in)) {
         return JNI_TRUE;
diff --git a/test/hotspot/jtreg/ProblemList-vthread.txt b/test/hotspot/jtreg/ProblemList-vthread.txt
@@ -541,13 +541,12 @@ vmTestbase/nsk/jdb/repeat/repeat001/repeat001.java
 
 vmTestbase/nsk/jdi/ExceptionEvent/catchLocation/location002/TestDescription.java 8278470 generic-all
 
-####
-## JVMTI SetLocalXXX() is returning OPAQUE_FRAME because vthreads are not except with topmost farme.
-
-vmTestbase/nsk/jdi/StackFrame/_bounds_/bounds002/TestDescription.java
 
 ####
 ## JVMTI PopFrame() is returning OPAQUE_FRAME because vthreads are not supported.
+## Note: vmTestbase/nsk/jdi/ThreadReference/popFrames/popframes001 was converted
+## to support vthreads and expect the OPAQUE_FRAME error. The others were
+## not because they don't add any additional value.
 
 vmTestbase/nsk/jdi/Scenarios/invokeMethod/popframes001/TestDescription.java
 vmTestbase/nsk/jdi/BScenarios/hotswap/tc01x002/TestDescription.java
@@ -559,15 +558,15 @@ vmTestbase/nsk/jdi/BScenarios/hotswap/tc06x001/TestDescription.java
 vmTestbase/nsk/jdi/BScenarios/hotswap/tc08x001/TestDescription.java
 vmTestbase/nsk/jdi/BScenarios/hotswap/tc10x002/TestDescription.java
 
-vmTestbase/nsk/jdi/ThreadReference/popFrames/popframes001/TestDescription.java
 vmTestbase/nsk/jdi/ThreadReference/popFrames/popframes002/TestDescription.java
 vmTestbase/nsk/jdi/ThreadReference/popFrames/popframes003/TestDescription.java
 vmTestbase/nsk/jdi/ThreadReference/popFrames/popframes004/TestDescription.java
 
 ####
-## JVMTI ForceEarlyReturn not supported for vthreads (JVMTI_ERROR_INVALID_THREAD)
+## JVMTI ForceEarlyReturn not supported for vthreads (JVMTI_ERROR_OPAQUE_FRAME)
+## Note forceEarlyReturn002 was converted to support vthreads. The rest were not
+## since there is no added value (JVMTI_ERROR_OPAQUE_FRAME is expected).
 
-vmTestbase/nsk/jdi/ThreadReference/forceEarlyReturn/forceEarlyReturn002/forceEarlyReturn002.java
 vmTestbase/nsk/jdi/ThreadReference/forceEarlyReturn/forceEarlyReturn014/forceEarlyReturn014.java
 vmTestbase/nsk/jdi/stress/serial/forceEarlyReturn001/TestDescription.java
 vmTestbase/nsk/jdi/stress/serial/forceEarlyReturn002/TestDescription.java
diff --git a/test/hotspot/jtreg/vmTestbase/nsk/jdi/StackFrame/_bounds_/bounds002.java b/test/hotspot/jtreg/vmTestbase/nsk/jdi/StackFrame/_bounds_/bounds002.java
@@ -184,12 +184,24 @@ private void execTest() {
             complain("Unexpected " + e);
             exitStatus = Consts.TEST_FAILED;
         }
+        boolean vthreadMode = "Virtual".equals(System.getProperty("main.wrapper"));
         try {
             stackFrame.setValue(var, null);
-            display("OK");
+            if (vthreadMode) {
+                // should have failed because stackFrame is not the topmost frame
+                complain("Expected OpaqueFrameException");
+                exitStatus = Consts.TEST_FAILED;
+            } else {
+                display("OK");
+            }
         } catch(Exception e) {
-            complain("Unexpected " + e);
-            exitStatus = Consts.TEST_FAILED;
+            if (vthreadMode && e instanceof OpaqueFrameException) {
+                // pass
+                display("OK");
+            } else {
+                complain("Unexpected " + e);
+                exitStatus = Consts.TEST_FAILED;
+            }
         }
         display("");
 
diff --git a/test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/forceEarlyReturn/forceEarlyReturn002/forceEarlyReturn002.java b/test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/forceEarlyReturn/forceEarlyReturn002/forceEarlyReturn002.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2007, 2022, 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
@@ -147,17 +147,30 @@ public void doTest() {
 
         // get value for early return
         ObjectReference returnValue = (ObjectReference) referenceType.getValue(referenceType.fieldByName("expectedValue"));
-
+        boolean vthreadMode = "Virtual".equals(System.getProperty("main.wrapper"));
         try {
-            // don't expect any exception
+            // don't expect any exception, except for vthreads expect OpaqueFrameException
             threadReference.forceEarlyReturn(returnValue);
+            if (vthreadMode) {
+                setSuccess(false);
+                log.complain("Expected OpaqueFrameException");
+            }
         } catch (Exception e) {
-            setSuccess(false);
-            log.complain("Unexpected exception: " + e);
-            e.printStackTrace(log.getOutStream());
+            if (vthreadMode && (e instanceof OpaqueFrameException)) {
+                // pass
+            } else {
+                setSuccess(false);
+                log.complain("Unexpected exception: " + e);
+                e.printStackTrace(log.getOutStream());
+            }
         }
 
-        testMethodExitEvent(threadReference, ClassUsingTestClass.breakpointMethodName);
+        if (vthreadMode) {
+            // MethodExit event won't be as expected if using vthreads, so just resume
+            threadReference.resume();
+        } else {
+            testMethodExitEvent(threadReference, ClassUsingTestClass.breakpointMethodName);
+        }
 
         if (!isDebuggeeReady())
             return;
diff --git a/test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/forceEarlyReturn/forceEarlyReturn002/forceEarlyReturn002a.java b/test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/forceEarlyReturn/forceEarlyReturn002/forceEarlyReturn002a.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2007, 2022, 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
@@ -83,7 +83,9 @@ public boolean parseCommand(String command) {
             classUsingTestClass.createExpectedValue();
             Object value = classUsingTestClass.testClassMethod();
 
-            if (ClassUsingTestClass.expectedValue != value) {
+            boolean vthreadMode = "Virtual".equals(System.getProperty("main.wrapper"));
+            // expectedValue should be set as expected unless in vthread mode
+            if (vthreadMode == (ClassUsingTestClass.expectedValue == value)) {
                 setSuccess(false);
                 log.complain("Unexpected result of testClassMethod: " + value);
             }
diff --git a/test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/popFrames/popframes001.java b/test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/popFrames/popframes001.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2022, 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
@@ -93,6 +93,8 @@
 
 public class popframes001 extends JDIBase {
 
+    static boolean vthreadMode = "Virtual".equals(System.getProperty("main.wrapper"));
+
     public static void main (String argv[]) {
 
         int result = run(argv, System.out);
@@ -219,6 +221,10 @@ private int runTest() {
         try {
             testRun();
 
+            if (vthreadMode) {
+                return 0; // just exit. we already got the expected OpaqueFrameException
+            }
+
             log2("waiting for VMDeathEvent");
             getEventSet();
             if (eventIterator.nextEvent() instanceof VMDeathEvent)
@@ -350,10 +356,24 @@ private void testRun()
             log2("......thread2Ref.popFrames(stackFrame);");
             try {
                 thread2Ref.popFrames(stackFrame);
-            } catch ( IncompatibleThreadStateException e ) {
-                log3("ERROR: IncompatibleThreadStateException");
-                testExitCode = FAILED;
-                break;
+                if (vthreadMode) {
+                    log3("ERROR: Expected OpaqueFrameException");
+                    testExitCode = FAILED;
+                }
+            } catch ( Exception e ) {
+                if (vthreadMode && (e instanceof OpaqueFrameException)) {
+                    // pass. resume thread and exit
+                    log2("......got expected OpaqueFrameException");
+                    log2("......thread2Ref.resume();");
+                    thread2Ref.resume();
+                    breakpointForCommunication();
+                    vm.resume();
+                    break;
+                } else {
+                    log3("ERROR: " + e.getClass().getSimpleName());
+                    testExitCode = FAILED;
+                    throw e;
+                }
             }
 
             log2("......thread2Ref.resume();");
