request for review (S): rev JVMTI version number

[Daniel D. Daugherty]

Thumbs up from me. Don't forget to update the copyright years in
jvmti.xml and jvmtiH.xsl.

Dan


On 12/6/2010 1:53 PM, Keith McGuigan wrote:
>
> 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 {
>>
>