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 <https://bugs.openjdk.java.net/secure/ViewProfile.jspa?name=rfield> 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 methods :-)

Thanks for finding this Dan,
Karen

On Dec 11, 2012, at 8:24 PM, Mikael Vidstedt wrote:

That does indeed explain why the transform agent code I was looking at works at 
all. The joy indeed. Let's pretend we never added that for now? :)

/M

On 2012-12-11 16:52, Daniel D. Daugherty wrote:
Mikael, Karen and Serguei,

Ahhh the joys of hidden features...

In src/share/vm/prims/jvmtiRedefineClasses.cpp, there is a
wonderful little block of code:

   760      case added:
   761        // method added, see if it is OK
   762        new_flags = (jushort) k_new_method->access_flags().get_flags();
   763        if ((new_flags & JVM_ACC_PRIVATE) == 0
   764             // hack: private should be treated as final, but alas
   765            || (new_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
   766           ) {
   767          // new methods must be private
   768          return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_ADDED;
   769        }
   770        {
   771          u2 num = the_class->next_method_idnum();
   772          if (num == ConstMethod::UNSET_IDNUM) {
   773            // cannot add any more methods
   774            return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_ADDED;
   775          }
   776          u2 new_num = k_new_method->method_idnum();
   777          Method* idnum_owner = scratch_class->method_with_idnum(num);
   778          if (idnum_owner != NULL) {
   779            // There is already a method assigned this idnum -- switch the
m
   780            idnum_owner->set_method_idnum(new_num);
   781          }
   782          k_new_method->set_method_idnum(num);
   783          swap_all_method_annotations(new_num, num, scratch_class, thread)
;
   784          if (thread->has_pending_exception()) {
   785            return JVMTI_ERROR_OUT_OF_MEMORY;
   786          }
   787        }
   788        RC_TRACE(0x00008000, ("Method added: new: %s [%d]",
   789 k_new_method->name_and_sig_as_C_string(), ni
));
   790        ++ni; // advance to next new method
   791        break;
   792      case deleted:
   793        // method deleted, see if it is OK
   794        old_flags = (jushort) k_old_method->access_flags().get_flags();
   795        if ((old_flags & JVM_ACC_PRIVATE) == 0
   796             // hack: private should be treated as final, but alas
   797            || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
   798           ) {
   799          // deleted methods must be private
   800          return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
   801        }
   802        RC_TRACE(0x00008000, ("Method deleted: old: %s [%d]",
   803 k_old_method->name_and_sig_as_C_string(), oi
));
   804        ++oi; // advance to next old method
   805        break;

so you can add private methods or final, static methods...
and you can delete them too...

Looks like these little gems were added by these deltas:

D 1.65.2.7 06/04/26 15:26:16 rfield 144 143     00010/00008/03286
MRs:
COMMENTS:
6404550: missing late attach (JVM TI redefine) functionality
            Add/delete private methods, continued: changes per review


D 1.65.2.6 06/04/26 13:59:12 rfield 143 142     00011/00003/03283
MRs:
COMMENTS:
6404550: missing late attach (JVM TI redefine) functionality
            Add/delete private methods, continued:
               JLS says private is final, but VM doesn't think so --
                      hack: require added/deleted methods to be final or static
               The vtable of classes implementing redefined interfaces need upda
te too



D 1.65.2.5 06/04/26 00:31:36 rfield 142 141     00002/00002/03284
MRs:
COMMENTS:
6404550: missing late attach (JVM TI redefine) functionality
            Add/delete private methods, continued: changes per review, yada yada


D 1.65.2.4 06/04/26 00:24:24 rfield 141 140     00002/00002/03284
MRs:
COMMENTS:
6404550: missing late attach (JVM TI redefine) functionality
            Add/delete private methods, continued: changes per review, yada yada


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

D 1.57.1.2 06/04/21 12:23:09 rfield 129 114     00375/00132/02143
MRs:
COMMENTS:
6404550: missing late attach (JVM TI redefine) functionality
           Permit the adding or deleting of private methods with redefine
           A method now has an assigned idnum, rather than an index into the met
hods() array
           jmethodIDs are now JNI handles.


I don't see anything in the JVM/TI spec about this...

Dan








--- End Message ---

Reply via email to