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

[David Holmes]

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. :)

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
>>>>>
>>>>
>>>
>