Re: JVMTI retransformation and addition of private methods

2018-04-16 Thread David Holmes 

On 17/04/2018 4:36 AM, Robert Field wrote:

Rather than reverse engineer the spec from the hotspot implementation...

Capabilities are the mechanism by which the level of functionality is 
defined.  Capabilities say what can be done, not what can't.


The wording "The redefinition must not add, remove or rename fields or 
methods, change the signatures of methods, change modifiers, or change 
inheritance. These restrictions may be lifted in future versions." was 
clearly left from an earlier version.  Probably should not have been 
there in the first place but certainly should have updated/removed as 
JVMTI evolved.


As written, it explicitly conflicts with the can_redefine_any_class 
capability:


   can_redefine_any_class    Can modify (retransform or redefine) any 
non-primitive non-array class. See IsModifiableClass.


I don't see any conflict there. can_redefine_any_class relates to the 
sets of classes amenable to redefinition, not to the set of changes that 
redefinition can make to a given class.


Arguably there could have been individual capabilities for each kind of 
change, but that seems somewhat extreme to me, unless there are good 
reasons why only subsets of changes would be reasonable to support. The 
existing "you can do anything except x, y, z" within the spec for 
redefineClass is coarse but not unreasonable. It is problematic because 
the exception list is incomplete but that's a different story.


as well as conflicting with the implementation of 
SetNativeMethodPrefixes et. al. and the RI in terms of 
private/final/static.


So two clear cases where the spec should have been updated but wasn't.

i would suggest removing the quoted text and adding to the text for the 
can_redefine_classes capability.   Maybe something like:


  can_redefine_classes    Can redefine classes with RedefineClasses, 
where the class is non-primitive and non-array and the redefinition does 
not add, remove or rename fields or methods, change the signatures of 
methods, change modifiers, or change inheritance.


Moving the text doesn't change the basic problem that the text and 
implementation are not in agreement.


David
-


-Robert

Re: JVMTI retransformation and addition of private methods

2018-04-16 Thread serguei.spit...@oracle.com 

Added to the JDK-8192936.

Thanks,
Serguei


On 4/16/18 11:36, Robert Field wrote:

Rather than reverse engineer the spec from the hotspot implementation...

Capabilities are the mechanism by which the level of functionality is 
defined.  Capabilities say what can be done, not what can't.


The wording "The redefinition must not add, remove or rename fields or 
methods, change the signatures of methods, change modifiers, or change 
inheritance. These restrictions may be lifted in future versions." was 
clearly left from an earlier version. Probably should not have been 
there in the first place but certainly should have updated/removed as 
JVMTI evolved.


As written, it explicitly conflicts with the can_redefine_any_class 
capability:


  can_redefine_any_class    Can modify (retransform or redefine) any 
non-primitive non-array class. See IsModifiableClass.


as well as conflicting with the implementation of 
SetNativeMethodPrefixes et. al. and the RI in terms of 
private/final/static.


i would suggest removing the quoted text and adding to the text for 
the can_redefine_classes capability.   Maybe something like:


 can_redefine_classes    Can redefine classes with 
RedefineClasses, where the class is non-primitive and non-array and 
the redefinition does not add, remove or rename fields or methods, 
change the signatures of methods, change modifiers, or change 
inheritance.


-Robert

Re: JVMTI retransformation and addition of private methods

2018-04-16 Thread serguei.spit...@oracle.com 

Added this and prev. suggestion to the JDK-8192936.

Thansk,
Serguei


On 4/16/18 11:36, Robert Field wrote:

Rather than reverse engineer the spec from the hotspot implementation...

Capabilities are the mechanism by which the level of functionality is 
defined.  Capabilities say what can be done, not what can't.


The wording "The redefinition must not add, remove or rename fields or 
methods, change the signatures of methods, change modifiers, or change 
inheritance. These restrictions may be lifted in future versions." was 
clearly left from an earlier version. Probably should not have been 
there in the first place but certainly should have updated/removed as 
JVMTI evolved.


As written, it explicitly conflicts with the can_redefine_any_class 
capability:


  can_redefine_any_class    Can modify (retransform or redefine) any 
non-primitive non-array class. See IsModifiableClass.


as well as conflicting with the implementation of 
SetNativeMethodPrefixes et. al. and the RI in terms of 
private/final/static.


i would suggest removing the quoted text and adding to the text for 
the can_redefine_classes capability.   Maybe something like:


 can_redefine_classes    Can redefine classes with 
RedefineClasses, where the class is non-primitive and non-array and 
the redefinition does not add, remove or rename fields or methods, 
change the signatures of methods, change modifiers, or change 
inheritance.


-Robert

Re: JVMTI retransformation and addition of private methods

2018-04-16 Thread Robert Field 

Rather than reverse engineer the spec from the hotspot implementation...

Capabilities are the mechanism by which the level of functionality is 
defined.  Capabilities say what can be done, not what can't.


The wording "The redefinition must not add, remove or rename fields or 
methods, change the signatures of methods, change modifiers, or change 
inheritance. These restrictions may be lifted in future versions." was 
clearly left from an earlier version.  Probably should not have been 
there in the first place but certainly should have updated/removed as 
JVMTI evolved.


As written, it explicitly conflicts with the can_redefine_any_class 
capability:


  can_redefine_any_class    Can modify (retransform or redefine) any 
non-primitive non-array class. See IsModifiableClass.


as well as conflicting with the implementation of 
SetNativeMethodPrefixes et. al. and the RI in terms of private/final/static.


i would suggest removing the quoted text and adding to the text for the 
can_redefine_classes capability.   Maybe something like:


 can_redefine_classes    Can redefine classes with RedefineClasses, 
where the class is non-primitive and non-array and the redefinition does 
not add, remove or rename fields or methods, change the signatures of 
methods, change modifiers, or change inheritance.


-Robert

Re: JVMTI retransformation and addition of private methods

2018-02-21 Thread serguei.spit...@oracle.com 

On 2/21/18 16:59, David Holmes wrote:

On 22/02/2018 7:44 AM, serguei.spit...@oracle.com wrote:

Hi Karen,

Thank you for sorting this out!


On 2/21/18 09:55, Karen Kinnear wrote:

Dan,

Thank you for all the background digging. This is really helpful.

Serguei - do you know what tests exist for this behavior?


Dan already replied (thanks, Dan!)
There are two tests in the open/test/jdk/java/lang/instrument:
RedefineMethodAddInvoke*
RedefineMethodDelInvoke*



The way I read the source code - we currently allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?

The above does not look correct to me.
We have the same check for both ADD and DELETE method:
   if ((old_flags & JVM_ACC_PRIVATE) == 0
    // hack: private should be treated as final, but alas
   || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
  ) {
 // deleted methods must be private
 return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
   }

I read it as we allow ADD and DELETE for PRIVATE && (STATIC || FINAL) 
methods.

(Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC methods.)
As private should always be treated final then we can simplify the 
above to:

   We allow and and delete PRIVATE INSTANCE or PRIVATE STATIC methods
   which is equal to just "PRIVATE methods".


I read it as "PRIVATE" or "FINAL STATIC".


There are multiple negations above, so this needs to be interpreted in a 
right context:

  Return error if deleted method is !PRIVATE or (!FINAL and !STATIC)

After inversion:
  Allow to delete if method is PRIVATE and (FINAL or STATIC)

I feel myself stupid when reading this code. :)

Thanks,
Serguei


David
-

With the current implementation, I am not sure if deletion works for 
private methods - do we

have a test for that? Or could you add one as part of this exercise?


Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.


Today we create a vtable entry for private methods (my 
misunderstanding ~ 2006ish). After discussions

with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an assertion
  assert(!old_method->is_deleted(), “vtable methods may not be 
deleted”)


I may have read the code incorrectly - but I would expect to hit 
this assertion if you had a private

method you were deleting that was not final and not static.

option 1) we could explicitly tighten the restrictions to match what 
we have implemented
option 2) we could make this work by changing 
klassVtable.cpp::update_inherited_vtable
  handling of private fields to be done the way it handles final 
fields.

option 3) I read the code incorrectly?


If we create a vtable entry for private methods then we should hit 
the assert above.

If we no longer need this then we have no problem.

Thanks,
Serguei


thanks,
Karen

On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
> 
wrote:


On 2/21/18 2:45 AM, serguei.spit...@oracle.com wrote:

On 2/20/18 23:01, David Holmes wrote:

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI 
documentation to reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those 
who were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to 
fix it in 11.

However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of 
allowing this. I _think_ it is safe but ...


Valid concern.
Also, I'd love to collect more details on the initial motivation 
to relax the JVMTI spec.

Most likely we had no CCC/CSR filed on this change.



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too 
strict in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 
morphed into "Permit the adding or deleting of private 
final/static methods with redefine" - nor why those changes 
failed to make any change to the spec itself. It is also 
unclear whether the add/delete is restricted to final/static 
methods or any private method? I can see that the intent was to 
only allow something that would not perturb the vtable for 
existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than 
the one in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late attach in

Re: JVMTI retransformation and addition of private methods

2018-02-21 Thread David Holmes 

Correction ...

On 22/02/2018 10:59 AM, David Holmes wrote:

On 22/02/2018 7:44 AM, serguei.spit...@oracle.com wrote:

Hi Karen,

Thank you for sorting this out!


On 2/21/18 09:55, Karen Kinnear wrote:

Dan,

Thank you for all the background digging. This is really helpful.

Serguei - do you know what tests exist for this behavior?


Dan already replied (thanks, Dan!)
There are two tests in the open/test/jdk/java/lang/instrument:
RedefineMethodAddInvoke*
RedefineMethodDelInvoke*



The way I read the source code - we currently allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?

The above does not look correct to me.
We have the same check for both ADD and DELETE method:
   if ((old_flags & JVM_ACC_PRIVATE) == 0
    // hack: private should be treated as final, but alas
   || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
  ) {
 // deleted methods must be private
 return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
   }

I read it as we allow ADD and DELETE for PRIVATE && (STATIC || FINAL) 
methods.

(Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC methods.)
As private should always be treated final then we can simplify the 
above to:

   We allow and and delete PRIVATE INSTANCE or PRIVATE STATIC methods
   which is equal to just "PRIVATE methods".


I read it as "PRIVATE" or "FINAL STATIC".


Nope I read it wrong - Serguei is right.

Which is somewhat strange as the underlying problem being addressed 
seemed to require retransformation of Object.wait and Thread.sleep - the 
former is a public final instance method; the latter a public static 
method. Neither are private ... ???


David


David
-

With the current implementation, I am not sure if deletion works for 
private methods - do we

have a test for that? Or could you add one as part of this exercise?


Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.


Today we create a vtable entry for private methods (my 
misunderstanding ~ 2006ish). After discussions

with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an assertion
  assert(!old_method->is_deleted(), “vtable methods may not be deleted”)

I may have read the code incorrectly - but I would expect to hit this 
assertion if you had a private

method you were deleting that was not final and not static.

option 1) we could explicitly tighten the restrictions to match what 
we have implemented
option 2) we could make this work by changing 
klassVtable.cpp::update_inherited_vtable

  handling of private fields to be done the way it handles final fields.
option 3) I read the code incorrectly?


If we create a vtable entry for private methods then we should hit the 
assert above.

If we no longer need this then we have no problem.

Thanks,
Serguei


thanks,
Karen

On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
> 
wrote:


On 2/21/18 2:45 AM, serguei.spit...@oracle.com wrote:

On 2/20/18 23:01, David Holmes wrote:

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI documentation 
to reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those 
who were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to 
fix it in 11.

However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of 
allowing this. I _think_ it is safe but ...


Valid concern.
Also, I'd love to collect more details on the initial motivation to 
relax the JVMTI spec.

Most likely we had no CCC/CSR filed on this change.



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too 
strict in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 
morphed into "Permit the adding or deleting of private 
final/static methods with redefine" - nor why those changes 
failed to make any change to the spec itself. It is also unclear 
whether the add/delete is restricted to final/static methods or 
any private method? I can see that the intent was to only allow 
something that would not perturb the vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than 
the one in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late

Re: JVMTI retransformation and addition of private methods

2018-02-21 Thread serguei.spit...@oracle.com 

  
  
Karen,
  
  
  On 2/21/18 16:46, Karen Kinnear wrote:


  
  Sergeui,
  
  
  You were right - I read the sources incorrectly.


This code is easy to misunderstand - I read it incorrectly multiple
times. :)
This parts causes most of confusion:
  (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0


   Would still help to understand both
  the motivation and the reason to not add to the
spec.
  
  
  Robert - do you remember why we didn’t add this to
the specification? (6404550)
  

  
On Feb 21, 2018, at 4:44 PM, serguei.spit...@oracle.com
  wrote:


  
Hi Karen,
  
  Thank you for sorting this out!
  
  
  On 2/21/18 09:55, Karen Kinnear wrote:

 Dan,
  
  
  Thank you for all the background
digging. This is really helpful.
  
  
  Serguei - do you know what tests exist
for this behavior?


Dan already replied (thanks, Dan!)
There are two tests in the
open/test/jdk/java/lang/instrument:
  RedefineMethodAddInvoke*
  RedefineMethodDelInvoke*
  
  


  The way I read the source code - we
currently allow ADD and DELETE for
  PRIVATE OR STATIC OR FINAL methods. Did
I read that correctly?

The above does not look correct to me.
We have the same check for both ADD and DELETE method:
  if ((old_flags & JVM_ACC_PRIVATE) == 0
   // hack: private should be treated as final,
but alas
  || (old_flags &
(JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
 ) {
    // deleted methods must be private
    return
JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
  }

I read it as we allow ADD and DELETE for PRIVATE
&& (STATIC || FINAL) methods.
(Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC
methods.)
As private should always be treated final then we can
simplify the above to:
  We allow and and delete PRIVATE INSTANCE or PRIVATE
STATIC methods
  which is equal to just "PRIVATE methods”.
  

  
  
  
  Sergeui - you are right - I misread this today.
And I played with the tests a bit - thanks Dan - and it
  matches the way you read this.


So today, private methods that are not marked as final in
  the source code can not be added -
I tried that variation by modifying the
  RedefineMethodAddInvokeTarget_1.java and changing
private final void myMethod1() to private void myMethod1()
  and got an UnsupportedOperationException.


So - private methods are not marked as ACC_FINAL today so I
  think the simplification doesn’t
apply, so we are left with the ability to ADD or DELETE
  PRIVATE && (STATIC || FINAL) methods  - at least
  that is what we support today.
  


Thank you for confirmation!
I suspected this because of the comment:
   // hack: private should be treated as final, but alas


  

  

  

  With the current implementation, I am
not sure if deletion works for private methods - do
we
  have a test for that? Or could you add
one as part of this exercise?


Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.


  
  I tried the test. And it works because of the requirement that
  FINAL or STATIC are set,
which therefore means no vtable entry.

  


Good to know.


  

  

  

  Today we create a vtable entry for
private methods (my misunderstanding ~ 2006ish).
After discussions
  with David I no longer believe we need
those.
  Today,

Re: JVMTI retransformation and addition of private methods

2018-02-21 Thread David Holmes 

On 22/02/2018 7:44 AM, serguei.spit...@oracle.com wrote:

Hi Karen,

Thank you for sorting this out!


On 2/21/18 09:55, Karen Kinnear wrote:

Dan,

Thank you for all the background digging. This is really helpful.

Serguei - do you know what tests exist for this behavior?


Dan already replied (thanks, Dan!)
There are two tests in the open/test/jdk/java/lang/instrument:
RedefineMethodAddInvoke*
RedefineMethodDelInvoke*



The way I read the source code - we currently allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?

The above does not look correct to me.
We have the same check for both ADD and DELETE method:
   if ((old_flags & JVM_ACC_PRIVATE) == 0
    // hack: private should be treated as final, but alas
   || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
  ) {
     // deleted methods must be private
     return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
   }

I read it as we allow ADD and DELETE for PRIVATE && (STATIC || FINAL) 
methods.

(Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC methods.)
As private should always be treated final then we can simplify the above to:
   We allow and and delete PRIVATE INSTANCE or PRIVATE STATIC methods
   which is equal to just "PRIVATE methods".


I read it as "PRIVATE" or "FINAL STATIC".

David
-

With the current implementation, I am not sure if deletion works for 
private methods - do we

have a test for that? Or could you add one as part of this exercise?


Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.


Today we create a vtable entry for private methods (my 
misunderstanding ~ 2006ish). After discussions

with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an assertion
  assert(!old_method->is_deleted(), “vtable methods may not be deleted”)

I may have read the code incorrectly - but I would expect to hit this 
assertion if you had a private

method you were deleting that was not final and not static.

option 1) we could explicitly tighten the restrictions to match what 
we have implemented
option 2) we could make this work by changing 
klassVtable.cpp::update_inherited_vtable

  handling of private fields to be done the way it handles final fields.
option 3) I read the code incorrectly?


If we create a vtable entry for private methods then we should hit the 
assert above.

If we no longer need this then we have no problem.

Thanks,
Serguei


thanks,
Karen

On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
> wrote:


On 2/21/18 2:45 AM, serguei.spit...@oracle.com wrote:

On 2/20/18 23:01, David Holmes wrote:

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI documentation 
to reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those 
who were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to fix 
it in 11.

However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of 
allowing this. I _think_ it is safe but ...


Valid concern.
Also, I'd love to collect more details on the initial motivation to 
relax the JVMTI spec.

Most likely we had no CCC/CSR filed on this change.



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too 
strict in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 
morphed into "Permit the adding or deleting of private 
final/static methods with redefine" - nor why those changes 
failed to make any change to the spec itself. It is also unclear 
whether the add/delete is restricted to final/static methods or 
any private method? I can see that the intent was to only allow 
something that would not perturb the vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than 
the one in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late attach in NetBeans Profiler due to 
missing functionality in JVMTI


Digging deeper ... to fix the problem described in that bug they 
augmented JVM TI to allow private method redefinition as an 
alternate to the "native rebinding" technique that had been used 
previously. See the final comment in:


https://bugs.openjdk.java.net/browse/JDK-6341303

"JVMTI Spec: Need

Re: JVMTI retransformation and addition of private methods

2018-02-21 Thread Karen Kinnear 
Sergeui,

You were right - I read the sources incorrectly. Would still help to understand 
both
the motivation and the reason to not add to the spec.

Robert - do you remember why we didn’t add this to the specification? (6404550)

> On Feb 21, 2018, at 4:44 PM, serguei.spit...@oracle.com wrote:
> 
> Hi Karen,
> 
> Thank you for sorting this out!
> 
> 
> On 2/21/18 09:55, Karen Kinnear wrote:
>> Dan,
>> 
>> Thank you for all the background digging. This is really helpful.
>> 
>> Serguei - do you know what tests exist for this behavior?
> 
> Dan already replied (thanks, Dan!)
> There are two tests in the open/test/jdk/java/lang/instrument:
>   RedefineMethodAddInvoke*
>   RedefineMethodDelInvoke*
> 
> 
>> The way I read the source code - we currently allow ADD and DELETE for
>> PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?
> The above does not look correct to me.
> We have the same check for both ADD and DELETE method:
>   if ((old_flags & JVM_ACC_PRIVATE) == 0
>// hack: private should be treated as final, but alas
>   || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
>  ) {
> // deleted methods must be private
> return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
>   }
> 
> I read it as we allow ADD and DELETE for PRIVATE && (STATIC || FINAL) methods.
> (Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC methods.)
> As private should always be treated final then we can simplify the above to:
>   We allow and and delete PRIVATE INSTANCE or PRIVATE STATIC methods
>   which is equal to just "PRIVATE methods”.

Sergeui - you are right - I misread this today.
And I played with the tests a bit - thanks Dan - and it matches the way you 
read this.

So today, private methods that are not marked as final in the source code can 
not be added -
I tried that variation by modifying the RedefineMethodAddInvokeTarget_1.java 
and changing
private final void myMethod1() to private void myMethod1() and got an 
UnsupportedOperationException.

So - private methods are not marked as ACC_FINAL today so I think the 
simplification doesn’t
apply, so we are left with the ability to ADD or DELETE
  PRIVATE && (STATIC || FINAL) methods  - at least that is what we support 
today.
> 
>> With the current implementation, I am not sure if deletion works for private 
>> methods - do we
>> have a test for that? Or could you add one as part of this exercise?
> 
> Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.
I tried the test. And it works because of the requirement that FINAL or STATIC 
are set,
which therefore means no vtable entry.
> 
> 
>> Today we create a vtable entry for private methods (my misunderstanding ~ 
>> 2006ish). After discussions
>> with David I no longer believe we need those.
>> Today, klassVtable::adjust_method_entries has an assertion
>>   assert(!old_method->is_deleted(), “vtable methods may not be deleted”)
>> 
>> I may have read the code incorrectly - but I would expect to hit this 
>> assertion if you had a private
>> method you were deleting that was not final and not static.
>> 
>> option 1) we could explicitly tighten the restrictions to match what we have 
>> implemented
>> option 2) we could make this work by changing 
>> klassVtable.cpp::update_inherited_vtable
>>   handling of private fields to be done the way it handles final fields.
>> option 3) I read the code incorrectly?
> 
> If we create a vtable entry for private methods then we should hit the assert 
> above.
> If we no longer need this then we have no problem.
We do create a vtable entry for private methods; however if FINAL or STATIC is 
set, then
we do not create a vtable entry.

That is why we don’t ever get here.

thanks,
Karen
> 
> Thanks,
> Serguei
> 
>> thanks,
>> Karen
>> 
>>> On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
>>> > wrote:
>>> 
>>> On 2/21/18 2:45 AM, serguei.spit...@oracle.com 
>>>  wrote:
 On 2/20/18 23:01, David Holmes wrote: 
> On 21/02/2018 4:50 PM, serguei.spit...@oracle.com 
>  wrote: 
>> Hi Karen and David, 
>> 
>> 
>> On 2/20/18 19:52, David Holmes wrote: 
>>> Hi Karen, 
>>> 
>>> On 21/02/2018 1:54 AM, Karen Kinnear wrote: 
 Folks, 
 
 As part of the Valhalla EG discussions for JVMTI changes for nestmates 
 (thank you Serguei and David), 
 IBM brought up a request that we update the JVMTI documentation to 
 reflect that we allow addition 
 of private methods. 
 
 Is there a reason we do not document this? I’m inviting those who were 
 involved at the time - please include 
 others if needed. 
>> 
>> I support documenting this in the JVMTI spec and had a plan to fix it in 
>> 11. 
>> However, it is not clear to me yet if we have a consensus on

Re: JVMTI retransformation and addition of private methods

2018-02-21 Thread serguei.spit...@oracle.com 

  
  
Hi Karen,
  
  Thank you for sorting this out!
  
  
  On 2/21/18 09:55, Karen Kinnear wrote:


  
  Dan,
  
  
  Thank you for all the background digging. This is
really helpful.
  
  
  Serguei - do you know what tests exist for this
behavior?


Dan already replied (thanks, Dan!)
There are two tests in the open/test/jdk/java/lang/instrument:
  RedefineMethodAddInvoke*
  RedefineMethodDelInvoke*
  
  


  The way I read the source code - we currently allow
ADD and DELETE for
  PRIVATE OR STATIC OR FINAL methods. Did I read that
correctly?

The above does not look correct to me.
We have the same check for both ADD and DELETE method:
  if ((old_flags & JVM_ACC_PRIVATE) == 0
   // hack: private should be treated as final, but alas
  || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
 ) {
    // deleted methods must be private
    return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
  }

I read it as we allow ADD and DELETE for PRIVATE && (STATIC
|| FINAL) methods.
(Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC methods.)
As private should always be treated final then we can simplify the
above to:
  We allow and and delete PRIVATE INSTANCE or PRIVATE STATIC methods
  which is equal to just "PRIVATE methods".


  With the current implementation, I am not sure if
deletion works for private methods - do we
  have a test for that? Or could you add one as part
of this exercise?


Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.



  Today we create a vtable entry for private methods
(my misunderstanding ~ 2006ish). After discussions
  with David I no longer believe we need those.
  Today, klassVtable::adjust_method_entries has an
assertion
    assert(!old_method->is_deleted(), “vtable
methods may not be deleted”)
  
  
  I may have read the code incorrectly - but I would
expect to hit this assertion if you had a private
  method you were deleting that was not final and not
static.
  
  
  option 1) we could explicitly tighten the
restrictions to match what we have implemented
  option 2) we could make this work by changing
klassVtable.cpp::update_inherited_vtable
    handling of private fields to be done the way it
handles final fields.
  option 3) I read the code incorrectly?


If we create a vtable entry for private methods then we should hit
the assert above.
If we no longer need this then we have no problem.

Thanks,
Serguei


  thanks,
  Karen
  

  
On Feb 21, 2018, at 10:40 AM, Daniel D.
  Daugherty 
  wrote:


   On 2/21/18 2:45 AM, serguei.spit...@oracle.com
wrote:
On 2/20/18 23:01, David Holmes wrote: 
  On 21/02/2018 4:50
PM, serguei.spit...@oracle.com
wrote: 
Hi Karen and David,
  
  
  
  On 2/20/18 19:52, David Holmes wrote: 
  Hi Karen, 

On 21/02/2018 1:54 AM, Karen Kinnear wrote: 
Folks, 
  
  As part of the Valhalla EG discussions for
  JVMTI changes for nestmates (thank you Serguei
  and David), 
  IBM brought up a request that we update the
  JVMTI documentation to reflect that we allow
  addition 
  of private methods. 
  
  Is there a reason we do not document this? I’m
  inviting those who were involved at the time -
  please include 
  others if needed. 

  
  
  I support documenting this in the JVMTI spec and
  had a plan to fix it in 11. 
  However, it is not clear to me yet if we have a
  consensus on it. 


I would like to see a detailed analysis of the
implications of allowing this. I _think_ it is safe
but ... 
  
  
  Valid concern. 
  Also, I'd love to collect more details

Re: JVMTI retransformation and addition of private methods

2018-02-21 Thread serguei.spit...@oracle.com 

  
  
Hi Dan,
  
  On 2/21/18 10:02, Daniel D. Daugherty wrote:


  
  On 2/21/18 12:55 PM, Karen Kinnear wrote:
   Dan,


Thank you for all the background digging. This is
  really helpful.


Serguei - do you know what tests exist for this
  behavior?
  
   
I mentioned a test that I found in the email that I attached to
my
previous reply:


  Subject: 
  Re: adding/deleting methods in JVM/TI
  RedefineClasses()/RetransformClasses()
  

  


  
  

  From: 
  "Daniel D. Daugherty" 
  
  

  Date: 
  12/17/12, 3:03 PM
  

  
  

  

  To: 
  Karen Kinnear 
  
  

  CC: 
  Mikael Vidstedt ,
  Serguei Spitsyn 
  

  
  
  Looks like I added a JLI/JPLIS test that exercises this
  "feature" 
  by calling the newly added method via reflection: 
  
      $ ls
/work/shared/bug_hunt/jdk8/exp/test/java/lang/instrument/RedefineMethodAdd*
      RedefineMethodAddInvoke.sh 
      RedefineMethodAddInvokeAgent.java 
      RedefineMethodAddInvokeApp.java 
      RedefineMethodAddInvokeTarget.java 
      RedefineMethodAddInvokeTarget_1.java 
      RedefineMethodAddInvokeTarget_2.java 
  
  This test was added for the following bug: 
  
      6667089 3/3 multiple redefinitions of a class break
  reflection 
  
  All these little pieces of information keep coming back at
  me... 
  

I've added a comment to the JDK-8192936 with the most important
fragments
from this email exchange in a hope, it will stop this bad loop. :)


 Dan

I haven't checked to see if that test is still there or if any
additional tests have been added.
  

Thank you for pointing to this!

Also, there is a test for DELETE method in the
open/test/jdk/java/lang/instrument:
  RedefineMethodDelInvokeAgent.java
  RedefineMethodDelInvokeApp.java
  RedefineMethodDelInvoke.sh
  RedefineMethodDelInvokeTarget_1.java
  RedefineMethodDelInvokeTarget_2.java
  RedefineMethodDelInvokeTarget.java

Thanks,
Serguei

 
Dan


  
  


The way I read the source code - we currently
  allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read
  that correctly?


With the current implementation, I am not sure if
  deletion works for private methods - do we
have a test for that? Or could you add one as part
  of this exercise?


Today we create a vtable entry for private methods
  (my misunderstanding ~ 2006ish). After discussions
with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an
  assertion
  assert(!old_method->is_deleted(), “vtable
  methods may not be deleted”)


I may have read the code incorrectly - but I would
  expect to hit this assertion if you had a private
method you were deleting that was not final and
  not static.


option 1) we could explicitly tighten the
  restrictions to match what we have implemented
option 2) we could make this work by changing
  klassVtable.cpp::update_inherited_vtable
  handling of private fields to be done the way it
  handles final fields.
option 3) I read the code incorrectly?


thanks,
Karen

  

  On Feb 21, 2018, at 10:40 AM, Daniel D.
Daugherty 
wrote:
  
  
 On 2/21/18 2:45 AM, serguei.spit...@oracle.com
  wrote:
  On 2/20/18 23:01, David Holmes wrote: 
On 21/02/2018 4:50
  PM, serguei.spit...@oracle.com
  wrote: 
  Hi Karen and
David, 


On 2/20/18 19:52, David Holmes wrote:

Re: JVMTI retransformation and addition of private methods

2018-02-21 Thread Daniel D. Daugherty 

On 2/21/18 12:55 PM, Karen Kinnear wrote:

Dan,

Thank you for all the background digging. This is really helpful.

Serguei - do you know what tests exist for this behavior?


I mentioned a test that I found in the email that I attached to my
previous reply:


Subject:
Re: adding/deleting methods in JVM/TI 
RedefineClasses()/RetransformClasses()


From:
"Daniel D. Daugherty" 
Date:
12/17/12, 3:03 PM

To:
Karen Kinnear 
CC:
Mikael Vidstedt , Serguei Spitsyn 




Looks like I added a JLI/JPLIS test that exercises this "feature"
by calling the newly added method via reflection:

    $ ls 
/work/shared/bug_hunt/jdk8/exp/test/java/lang/instrument/RedefineMethodAdd*

    RedefineMethodAddInvoke.sh
    RedefineMethodAddInvokeAgent.java
    RedefineMethodAddInvokeApp.java
    RedefineMethodAddInvokeTarget.java
    RedefineMethodAddInvokeTarget_1.java
    RedefineMethodAddInvokeTarget_2.java

This test was added for the following bug:

    6667089 3/3 multiple redefinitions of a class break reflection

All these little pieces of information keep coming back at me...

Dan


I haven't checked to see if that test is still there or if any
additional tests have been added.

Dan




The way I read the source code - we currently allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?

With the current implementation, I am not sure if deletion works for 
private methods - do we

have a test for that? Or could you add one as part of this exercise?

Today we create a vtable entry for private methods (my 
misunderstanding ~ 2006ish). After discussions

with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an assertion
  assert(!old_method->is_deleted(), “vtable methods may not be deleted”)

I may have read the code incorrectly - but I would expect to hit this 
assertion if you had a private

method you were deleting that was not final and not static.

option 1) we could explicitly tighten the restrictions to match what 
we have implemented
option 2) we could make this work by changing 
klassVtable.cpp::update_inherited_vtable

  handling of private fields to be done the way it handles final fields.
option 3) I read the code incorrectly?

thanks,
Karen

On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
> wrote:


On 2/21/18 2:45 AM, serguei.spit...@oracle.com wrote:

On 2/20/18 23:01, David Holmes wrote:

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI documentation 
to reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those 
who were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to fix 
it in 11.

However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of 
allowing this. I _think_ it is safe but ...


Valid concern.
Also, I'd love to collect more details on the initial motivation to 
relax the JVMTI spec.

Most likely we had no CCC/CSR filed on this change.



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too 
strict in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 
morphed into "Permit the adding or deleting of private 
final/static methods with redefine" - nor why those changes 
failed to make any change to the spec itself. It is also unclear 
whether the add/delete is restricted to final/static methods or 
any private method? I can see that the intent was to only allow 
something that would not perturb the vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than 
the one in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late attach in NetBeans Profiler due to 
missing functionality in JVMTI


Digging deeper ... to fix the problem described in that bug they 
augmented JVM TI to allow private method redefinition as an 
alternate to the "native rebinding" technique that had been used 
previously. See the final comment in:


https://bugs.openjdk.java.net/browse/JDK-6341303

"JVMTI Spec: Need a way how to rebind Object.wait and Thread.sleep 
with late attach"


which was closed as a duplicate.


Thank you for the point.
This explains it.
It

Re: JVMTI retransformation and addition of private methods

2018-02-21 Thread Karen Kinnear 
Dan,

Thank you for all the background digging. This is really helpful.

Serguei - do you know what tests exist for this behavior?

The way I read the source code - we currently allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?

With the current implementation, I am not sure if deletion works for private 
methods - do we
have a test for that? Or could you add one as part of this exercise?

Today we create a vtable entry for private methods (my misunderstanding ~ 
2006ish). After discussions
with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an assertion
  assert(!old_method->is_deleted(), “vtable methods may not be deleted”)

I may have read the code incorrectly - but I would expect to hit this assertion 
if you had a private
method you were deleting that was not final and not static.

option 1) we could explicitly tighten the restrictions to match what we have 
implemented
option 2) we could make this work by changing 
klassVtable.cpp::update_inherited_vtable
  handling of private fields to be done the way it handles final fields.
option 3) I read the code incorrectly?

thanks,
Karen

> On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
>  wrote:
> 
> On 2/21/18 2:45 AM, serguei.spit...@oracle.com 
>  wrote:
>> On 2/20/18 23:01, David Holmes wrote: 
>>> On 21/02/2018 4:50 PM, serguei.spit...@oracle.com 
>>>  wrote: 
 Hi Karen and David, 
 
 
 On 2/20/18 19:52, David Holmes wrote: 
> Hi Karen, 
> 
> On 21/02/2018 1:54 AM, Karen Kinnear wrote: 
>> Folks, 
>> 
>> As part of the Valhalla EG discussions for JVMTI changes for nestmates 
>> (thank you Serguei and David), 
>> IBM brought up a request that we update the JVMTI documentation to 
>> reflect that we allow addition 
>> of private methods. 
>> 
>> Is there a reason we do not document this? I’m inviting those who were 
>> involved at the time - please include 
>> others if needed. 
 
 I support documenting this in the JVMTI spec and had a plan to fix it in 
 11. 
 However, it is not clear to me yet if we have a consensus on it. 
>>> 
>>> I would like to see a detailed analysis of the implications of allowing 
>>> this. I _think_ it is safe but ... 
>> 
>> Valid concern. 
>> Also, I'd love to collect more details on the initial motivation to relax 
>> the JVMTI spec. 
>> Most likely we had no CCC/CSR filed on this change. 
>> 
>> 
> This issue is tracked by: 
> 
> https://bugs.openjdk.java.net/browse/JDK-8192936 
>  
> 
> "RI does not follow the JVMTI RedefineClasses spec that is too strict in 
> the definition" 
 
 Yes, this is the one. 
 Thank you, David, for posting the link. 
 
 
> As I wrote there ... It is not at all clear how JDK-6404550 morphed into 
> "Permit the adding or deleting of private final/static methods with 
> redefine" - nor why those changes failed to make any change to the spec 
> itself. It is also unclear whether the add/delete is restricted to 
> final/static methods or any private method? I can see that the intent was 
> to only allow something that would not perturb the vtable for existing 
> instances. 
 
 I agree, there is a confusion somewhere. 
 Is it possible, the JDK-6404550 in JIRA is a different bug than the one in 
 the Bugtraq system? 
 
 The JDK-6404550 in JIRA has a different synopsis: 
 https://bugs.openjdk.java.net/browse/JDK-6404550 
  
   Cannot implement late attach in NetBeans Profiler due to missing 
 functionality in JVMTI 
>>> 
>>> Digging deeper ... to fix the problem described in that bug they augmented 
>>> JVM TI to allow private method redefinition as an alternate to the "native 
>>> rebinding" technique that had been used previously. See the final comment 
>>> in: 
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-6341303 
>>>  
>>> 
>>> "JVMTI Spec: Need a way how to rebind Object.wait and Thread.sleep with 
>>> late attach" 
>>> 
>>> which was closed as a duplicate. 
>> 
>> Thank you for the point. 
>> This explains it. 
>> It seems, the bug synopsis was changed at some moment. 
> 
> The synopsis for 6404550 has never changed. Here's the subject line when
> it was created on 2006.03.27:
> 
> > CR 6404550 *HOT* Created P1 hotspot/jvmti Cannot implement late attach in 
> > NetBeans Profiler due to missing functionality in JVMTI
> 
> I think the confusion arises over comments like this in 6341303:
> 
>>  Robert Field 
>>  added a 
>> comment - 2006-05-04 11:54
>> BT2:EVALUATION
>> 
>> This can now be

Re: JVMTI retransformation and addition of private methods

2018-02-21 Thread Daniel D. Daugherty 

On 2/21/18 2:45 AM, serguei.spit...@oracle.com wrote:

On 2/20/18 23:01, David Holmes wrote:

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI documentation to 
reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those who 
were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to fix 
it in 11.

However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of 
allowing this. I _think_ it is safe but ...


Valid concern.
Also, I'd love to collect more details on the initial motivation to 
relax the JVMTI spec.

Most likely we had no CCC/CSR filed on this change.



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too 
strict in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 morphed 
into "Permit the adding or deleting of private final/static methods 
with redefine" - nor why those changes failed to make any change to 
the spec itself. It is also unclear whether the add/delete is 
restricted to final/static methods or any private method? I can see 
that the intent was to only allow something that would not perturb 
the vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than the 
one in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late attach in NetBeans Profiler due to 
missing functionality in JVMTI


Digging deeper ... to fix the problem described in that bug they 
augmented JVM TI to allow private method redefinition as an alternate 
to the "native rebinding" technique that had been used previously. 
See the final comment in:


https://bugs.openjdk.java.net/browse/JDK-6341303

"JVMTI Spec: Need a way how to rebind Object.wait and Thread.sleep 
with late attach"


which was closed as a duplicate.


Thank you for the point.
This explains it.
It seems, the bug synopsis was changed at some moment.


The synopsis for 6404550 has never changed. Here's the subject line when
it was created on 2006.03.27:

> CR 6404550 *HOT* Created P1 hotspot/jvmti Cannot implement late 
attach in NetBeans Profiler due to missing functionality in JVMTI


I think the confusion arises over comments like this in 6341303:

rfield Robert Field 
 
added a comment - 2006-05-04 11:54

BT2:EVALUATION

This can now be accomplished with Java programming language 
instrumentation, via:


 6404550: missing late attach (JVM TI redefine) functionality
   Permit the adding or deleting of private final/static 
methods with redefine


Closing this bug as a duplicate.


That's just Robert's style for an sccs delta comment:

D 1.65.2.3 06/04/25 23:36:35 rfield 140 139 00023/00013/03263
MRs:
COMMENTS:
6404550: missing late attach (JVM TI redefine) functionality
    Add/delete private methods, continued: changes per review

Back in the ancient past we tried to include some brief
info about the change in the delta comment. This was one of many
deltas associated with 6404550.

Please see the attached email that I sent on 2012.12.17 about the
history behind this issue... (sent to Karen, Mikael V, and Serguei)

It seems I forwarded that same email to Coleen, Markus G and Serguei
back on 2014.05.20. Since Markus is on that thread, it must have had
something to do with research about JFR...

I need to do a detailed read thru my e-mail archive for 6404550 to
see if I can spot some clues about why we didn't do a spec update.

Dan




Thanks,
Serguei



David
-



Thanks,
Serguei



--
David



thanks,
Karen








--- Begin Message ---

Looks like I added a JLI/JPLIS test that exercises this "feature"
by calling the newly added method via reflection:

$ ls 
/work/shared/bug_hunt/jdk8/exp/test/java/lang/instrument/RedefineMethodAdd*

RedefineMethodAddInvoke.sh
RedefineMethodAddInvokeAgent.java
RedefineMethodAddInvokeApp.java
RedefineMethodAddInvokeTarget.java
RedefineMethodAddInvokeTarget_1.java
RedefineMethodAddInvokeTarget_2.java

This test was added for the following bug:

6667089 3/3 multiple redefinitions of a class break reflection

All these little pieces of information keep coming back at me... :-)

Dan



On 12/11/12 8:10 PM, Karen Kinnear wrote:

But what a wonderful way to bound the add/delete

Re: JVMTI retransformation and addition of private methods

2018-02-21 Thread serguei.spit...@oracle.com 

On 2/20/18 23:01, David Holmes wrote:

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI documentation to 
reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those who 
were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to fix it 
in 11.

However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of 
allowing this. I _think_ it is safe but ...


Valid concern.
Also, I'd love to collect more details on the initial motivation to 
relax the JVMTI spec.

Most likely we had no CCC/CSR filed on this change.



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too 
strict in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 morphed 
into "Permit the adding or deleting of private final/static methods 
with redefine" - nor why those changes failed to make any change to 
the spec itself. It is also unclear whether the add/delete is 
restricted to final/static methods or any private method? I can see 
that the intent was to only allow something that would not perturb 
the vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than the 
one in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late attach in NetBeans Profiler due to 
missing functionality in JVMTI


Digging deeper ... to fix the problem described in that bug they 
augmented JVM TI to allow private method redefinition as an alternate 
to the "native rebinding" technique that had been used previously. See 
the final comment in:


https://bugs.openjdk.java.net/browse/JDK-6341303

"JVMTI Spec: Need a way how to rebind Object.wait and Thread.sleep 
with late attach"


which was closed as a duplicate.


Thank you for the point.
This explains it.
It seems, the bug synopsis was changed at some moment.

Thanks,
Serguei



David
-



Thanks,
Serguei



--
David



thanks,
Karen

Re: JVMTI retransformation and addition of private methods

2018-02-20 Thread David Holmes 

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI documentation to 
reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those who 
were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to fix it in 11.
However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of allowing 
this. I _think_ it is safe but ...



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too strict 
in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 morphed 
into "Permit the adding or deleting of private final/static methods 
with redefine" - nor why those changes failed to make any change to 
the spec itself. It is also unclear whether the add/delete is 
restricted to final/static methods or any private method? I can see 
that the intent was to only allow something that would not perturb the 
vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than the one 
in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late attach in NetBeans Profiler due to missing 
functionality in JVMTI


Digging deeper ... to fix the problem described in that bug they 
augmented JVM TI to allow private method redefinition as an alternate to 
the "native rebinding" technique that had been used previously. See the 
final comment in:


https://bugs.openjdk.java.net/browse/JDK-6341303

"JVMTI Spec: Need a way how to rebind Object.wait and Thread.sleep with 
late attach"


which was closed as a duplicate.

David
-



Thanks,
Serguei



--
David



thanks,
Karen

Re: JVMTI retransformation and addition of private methods

2018-02-20 Thread serguei.spit...@oracle.com 

  
  
Hi Karen and David,
  
  
  On 2/20/18 19:52, David Holmes wrote:

Hi
  Karen,
  
  
  On 21/02/2018 1:54 AM, Karen Kinnear wrote:
  
  Folks,


As part of the Valhalla EG discussions for JVMTI changes for
nestmates (thank you Serguei and David),

IBM brought up a request that we update the JVMTI documentation
to reflect that we allow addition

of private methods.


Is there a reason we do not document this? I’m inviting those
who were involved at the time - please include

others if needed.
  


I support documenting this in the JVMTI spec and had a plan to fix
it in 11.
However, it is not clear to me yet if we have a consensus on it.

This
  issue is tracked by:
  
  
  https://bugs.openjdk.java.net/browse/JDK-8192936
  
  
  "RI does not follow the JVMTI RedefineClasses spec that is too
  strict in the definition"
  


Yes, this is the one.
Thank you, David, for posting the link.


As I
  wrote there ... It is not at all clear how JDK-6404550 morphed
  into "Permit the adding or deleting of private final/static
  methods with redefine" - nor why those changes failed to make any
  change to the spec itself. It is also unclear whether the
  add/delete is restricted to final/static methods or any private
  method? I can see that the intent was to only allow something that
  would not perturb the vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than the
one in the Bugtraq system?

The JDK-6404550 in JIRA has a different synopsis:
   https://bugs.openjdk.java.net/browse/JDK-6404550
 Cannot implement late attach in NetBeans Profiler due to
missing functionality in JVMTI


Thanks,
Serguei


--
  
  David
  
  
  
  thanks,

Karen

Re: JVMTI retransformation and addition of private methods

2018-02-20 Thread David Holmes 

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for nestmates (thank 
you Serguei and David),
IBM brought up a request that we update the JVMTI documentation to reflect that 
we allow addition
of private methods.

Is there a reason we do not document this? I’m inviting those who were involved 
at the time - please include
others if needed.


This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too strict in 
the definition"


As I wrote there ... It is not at all clear how JDK-6404550 morphed into 
"Permit the adding or deleting of private final/static methods with 
redefine" - nor why those changes failed to make any change to the spec 
itself. It is also unclear whether the add/delete is restricted to 
final/static methods or any private method? I can see that the intent 
was to only allow something that would not perturb the vtable for 
existing instances.


--
David



thanks,
Karen