Re: request for review (S): rev JVMTI version number

[Daniel D. Daugherty]

On 12/6/2010 8:22 AM, Keith McGuigan wrote:

This change updates the JVMTI revision number from 1.1 to 1.2, when 
Hotspot is placed in a jdk >= 7.

http://cr.openjdk.java.net/~kamg/7003782/webrev.00/

Thanks for any review.

src/share/vm/prims/jvmti.xml
   No comments.

src/share/vm/prims/jvmtiEnv.cpp
   Since JVM/TI version 1.0, 1.1 and (presumably) 1.2 are compatible
   with each other, you don't need to do this check. The relevant
   part of the JVM/TI spec is:

   
http://download.oracle.com/javase/6/docs/platform/jvmti/jvmti.html#jvmtiEnvAccess

   The key sentence is:

   > The returned environment may have a different version than the
   > requested version but the returned environment must be compatible.

   If a client asks for version 1.0 or version 1.1, then it is okay
   to return version 1.2 as long as the common parts between those
   versions are compatible. If a client running on JDK5 or JDK6 is
   smart enough to ask for JVM/TI version 1.2, then it can be allowed
   to have it.

src/share/vm/prims/jvmtiExport.cpp
   I don't think you need these changes either.

src/share/vm/prims/jvmtiH.xsl
   This change just adds JVMTI_VERSION_1_2. It does not change the
   JVMTI_VERSION value to the 1.2 version, but it should.

src/share/vm/prims/jvmtiEnvBase.hpp
   You should add JDK17_JVMTI_VERSION to the enum set here. You should
   also add a declaration for JvmtiEnvBase::use_version_1_2_semantics()
   here.

src/share/vm/prims/jvmtiEnvBase.cpp
   You should add a definition for 
JvmtiEnvBase::use_version_1_2_semantics()
   here.

If a JVM/TI version simply adds new functions, then nothing needs to
be done. An older agent that asks for JVM/TI version 1.0 or JVM/TI
version 1.1 won't know about or try to use the new function.

However, if a new JVM/TI version changes semantics of an existing function,
then a little more hoop jumping is needed.

The way to get different version semantics is like this (from 6849968,
just the use_version_...() stuff):

hg diff -r 1081 -r 1082 src/share/vm/prims/jvmtiEnv.cpp
diff -r 9127aa69352e -r 98cd9901c161 src/share/vm/prims/jvmtiEnv.cpp
--- a/src/share/vm/prims/jvmtiEnv.cpp   Mon Dec 14 09:51:09 2009 -0700
+++ b/src/share/vm/prims/jvmtiEnv.cpp   Mon Dec 14 10:05:36 2009 -0700
@@ -32,15 +32,15 @@
 // FIXLATER: hook into JvmtiTrace
#define TraceJVMTICalls false

-JvmtiEnv::JvmtiEnv() : JvmtiEnvBase() {
+JvmtiEnv::JvmtiEnv(jint version) : JvmtiEnvBase(version) {
}

JvmtiEnv::~JvmtiEnv() {
}

JvmtiEnv*
-JvmtiEnv::create_a_jvmti() {
-  return new JvmtiEnv();
+JvmtiEnv::create_a_jvmti(jint version) {
+  return new JvmtiEnv(version);
}

// VM operation class to copy jni function table at safepoint.
@@ -408,6 +408,11 @@
  if (phase == JVMTI_PHASE_ONLOAD) {
    Arguments::append_sysclasspath(segment);
    return JVMTI_ERROR_NONE;
+  } else if (use_version_1_0_semantics()) {
+    // This JvmtiEnv requested version 1.0 semantics and this function
+    // is only allowed in the ONLOAD phase in version 1.0 so we need to
+    // return an error here.
+    return JVMTI_ERROR_WRONG_PHASE;
  } else if (phase == JVMTI_PHASE_LIVE) {
    // The phase is checked by the wrapper that called this function,
    // but this thread could be racing with the thread that is
@@ -2857,6 +2862,14 @@
// is_obsolete_ptr - pre-checked for NULL
jvmtiError
JvmtiEnv::IsMethodObsolete(methodOop method_oop, jboolean* 
is_obsolete_ptr) {
+  if (use_version_1_0_semantics() &&
+      get_capabilities()->can_redefine_classes == 0) {
+    // This JvmtiEnv requested version 1.0 semantics and this function
+    // requires the can_redefine_classes capability in version 1.0 so
+    // we need to return an error here.
+    return JVMTI_ERROR_MUST_POSSESS_CAPABILITY;
+  }
+
  if (method_oop == NULL || method_oop->is_obsolete()) {
    *is_obsolete_ptr = true;
  } else {