JVMTI retransformation and addition of private methods

Wed Feb 21 18:02:07 UTC 2018

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" <daniel.daugherty at oracle.com>
> Date:
> 12/17/12, 3:03 PM
>
> To:
> Karen Kinnear <Karen.Kinnear at Oracle.com>
> CC:
> Mikael Vidstedt <mikael.vidstedt at oracle.com>, Serguei Spitsyn 
> <serguei.spitsyn at oracle.com>
>
>
> 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 
>> <daniel.daugherty at oracle.com <mailto:daniel.daugherty at oracle.com>> wrote:
>>
>> On 2/21/18 2:45 AM, serguei.spitsyn at oracle.com wrote:
>>> On 2/20/18 23:01, David Holmes wrote:
>>>> On 21/02/2018 4:50 PM, serguei.spitsyn at 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
>>>>>>>
>>>>>
>>>
>>>
>>
>> <Attached Message.eml>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180221/7ec5bd0e/attachment-0001.html>