RFR (S) 8181171: Deleting method for RedefineClasses breaks ResolvedMethodName

[David Holmes]

On 25/02/2019 11:04 pm, coleen.phillimore at oracle.com wrote:
> 
> David, Thank you for the code review.
> 
> On 2/24/19 8:04 PM, David Holmes wrote:
>> Hi Coleen,
>>
>> On 23/02/2019 9:36 am, coleen.phillimore at oracle.com wrote:
>>> 8210457: JVM crash in ResolvedMethodTable::add_method(Handle)
>>> Summary: Add a function to call NSME in ResolvedMethodTable to 
>>> replace deleted methods.
>>
>> Given the existing code just seems to say "I don't know what to do 
>> here", this change seems quite reasonable.
>>
>> A few minor style nits in test/jdk/java/lang/instrument/NamedBuffer.java:
>>
>> +         final byte FirstByte = className.getBytes()[0];
>>
>> No need for final
> Fixed and the name changed to firstByte.
>>
>> s/First/first/
>>
>> +             if(checkMatch
>>
>> Space after if
> 
> Fixed.
>>
>> Also could you add a comment on bytesForHostClass explaining exactly 
>> what it does please. I have to keep re-reading it to figure out what 
>> is the name of the actual classfile on disk and how it then gets 
>> transformed. IIUC given className it replaces the first letter with 
>> "replace" and reads that classfile from disk, and then iterates 
>> through the bytes looking for the modified name and updating it to be 
>> "className" (which just requires changing the first byte).
> 
> How about:
> 
>      // This function reads a class file from disk and replaces the 
> first character of
>      // the name with the one passed as "replace".  Then goes through 
> the bytecodes to
>      // replace all instances of the name of the class with the new 
> name.  The
>      // redefinition tests use this to redefine Host$ classes with 
> precompiled class files
>      // Xost.java, Yost.java and Zost.java.

Yep that's good - thanks.

David

> Thanks,
> Coleen
> 
>>
>> Thanks,
>> David
>> -----
>>
>>> This Unsafe.throwX trick is also used for vtable initialization for 
>>> throwing IllegalAccessError.  Tested with redefinition tests in the 
>>> repository and tier1-3, and added tests.
>>>
>>> open webrev at 
>>> http://cr.openjdk.java.net/~coleenp/2019/8181171.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8181171
>>>
>>> Thanks,
>>> Coleen
>