4-th round RFR (XS) 8068162: jvmtiRedefineClasses.cpp: guarantee(false) failed: OLD and/or OBSOLETE method(s) found

[serguei.spitsyn at oracle.com]

On 1/19/15 5:40 PM, David Holmes wrote:
> On 20/01/2015 1:51 AM, Coleen Phillimore wrote:
>>
>> On 1/19/15, 12:47 AM, David Holmes wrote:
>>> On 17/01/2015 12:24 PM, Daniel D. Daugherty wrote:
>>>>  >
>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8068162-JVMTI-old.4/ 
>>>>
>>>>
>>>>
>>>>
>>>> src/share/vm/memory/universe.hpp
>>>>      No comments.
>>>>
>>>> src/share/vm/memory/universe.cpp
>>>>      No comments.
>>>>
>>>> src/share/vm/prims/jvmtiRedefineClasses.cpp
>>>>      So redefining the Unsafe class is now very expensive because
>>>>      we have to visit the i-table and v-table of every class (and
>>>>      maybe interface?)...
>>>
>>> Sorry I didn't follow this through all the rounds, but the bug report
>>> states (and the code I looked at seems to be consistent with it) that
>>> the throw_IllegalAccessError call is only added to itables - so why
>>> are we also walking vtables?
>>
>> We are thinking about using a similar mechanism to fill in exceptions in
>> the vtable instead of overpass methods.   Basically, a function in
>> Unsafe can appear in both tables.
>
> Okay. But as we are in the process of trying to reduce Unsafe to its 
> minimal form and potentially convert to supported API's where needed, 
> perhaps this is not the best mechanism to be expanding. :)
>
> On the other hand ... if in a modular world Unsafe is truly 
> inaccessible then perhaps we can prohibit it from being redefined in 
> the first place. :)

Even if in a modular world the Unsafe is truly inaccessible, it should 
not be applied to the agents as they need redefineabilty, not 
accessibility. :)
I do not see how we can prohibit any class from being redefined 
(instrumented).
At least, this approach will impact the JVMTI spec.

Thanks,
Serguei


>
> Cheers,
> David
>
>> Coleen
>>>
>>> David H.
>>> -----
>>>
>>>>      Based on the bug report 'Unsafe::throw_illegal_access' is some
>>>>      magical method that can appear in any i-table or v-table entry.
>>>>      Maybe only as part of some default methods thing? That's not
>>>>      clear to me so I'm just guessing...
>>>>
>>>>      Is there some way to limit this visit to classes where the
>>>>      magical method can appear? Or can it really appear anywhere?
>>>>
>>>> Dan
>>>>
>>>>
>>>> On 1/16/15 12:14 PM, serguei.spitsyn at oracle.com wrote:
>>>>> Dan, David H. or David C.,
>>>>>
>>>>> May I ask one of you to look at the webrev below?
>>>>> The issue itself is a little bit tricky, so it is not easy to review
>>>>> despite the small size.
>>>>>
>>>>> Coleen,
>>>>>
>>>>> Does the webrev matches what we discussed with you?
>>>>> Do you give me a thumbs up?
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>> May I ask
>>>>>
>>>>> On 1/13/15 9:47 PM, serguei.spitsyn at oracle.com wrote:
>>>>>> Please, review the fix for:
>>>>>>   https://bugs.openjdk.java.net/browse/JDK-8068162
>>>>>>
>>>>>>
>>>>>> Open webrevs:
>>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8068162-JVMTI-old.4/ 
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/jdk/8068162-Test-IsModifiableAgent/ 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Summary:
>>>>>>
>>>>>>    The sun.misc.Unsafe:throwIllegalAccessError() method is used in
>>>>>> place of a default
>>>>>>    interface method in the itable if a default method was not 
>>>>>> defined
>>>>>> in the interface.
>>>>>>    In fact, it happens for two interfaces that purhaps are
>>>>>> auto-generated:
>>>>>>      java/nio/CharBuffer
>>>>>>      java/nio/HeapCharBuffer
>>>>>>
>>>>>>    This approach creates a problem when the class sun.misc.Unsafe is
>>>>>> retransformed.
>>>>>>    The Method* pointer to the old (redefined) method in the itable
>>>>>> triggers an assert
>>>>>>    (see the hs_err log in the bug report).
>>>>>>    Coleen told me that a similar approach is going to be implemented
>>>>>> for some vtable entries.
>>>>>>    Coleen, thanks for suggesting a better fix for this issue!
>>>>>>
>>>>>>    The fix is to replace the old Unsafe method in the itable/vtable
>>>>>> with the latest method version.
>>>>>>
>>>>>>
>>>>>> Testing:
>>>>>>   In progress: nsk.jdi.testlist, JTREG java/lang/instrument tests
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>
>>>>
>>