request for review (S): rev JVMTI version number

[Keith McGuigan]

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

Comme ça?

--
- Keith

On Dec 6, 2010, at 11:29 AM, Daniel D. Daugherty wrote:

> 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 {
>