RFR (S) 8042796: jvmtiRedefineClasses.cpp: guarantee(false) failed: OLD and/or OBSOLETE method(s) found

[serguei.spitsyn at oracle.com]

Coleen,


On 5/13/14 4:17 PM, Coleen Phillimore wrote:
>
> Serguei,
>
> I don't know if this is the right fix.  We shouldn't have deleted 
> methods in the cpCache because there's no way to keep these Method*s 
> from being deallocated.  I thought AdjustCpoolCacheAndVtable => 
> ConstantPoolCache::adjust_method_entries should fix these entries. For 
> deleted methods, shouldn't their cpCache entries be set to 
> unresolved?  Which class had the deleted methods?   The one being 
> redefined?

Thank you for reviewing this and for the questions.

Let's check if my understanding is right as it is relatively easy to 
miss something.

The original class has been redefined so that the new version does not 
have several private static methods anymore.
These methods are called "deleted" but they are still present in the 
original version of the class.
Even if we set their cpCache entries to unresolved these entries can be 
set to resolved again.
So that I've decided this approach is not going to work in general case.

The interesting question is:
  Q: What is going to happen with these entries when the original 
version of the class goes away?

I'll try to answer this question by looking at the code.
It won't be a surprise if there is nothing in place to cover this case.

>
> It looks like the code never did this.  I think this is missing 
> functionality.

Yes, that was my guess too.

> Do we not have any tests that delete a method?

Most likely, we don't.

> Your change should add a test anyway - there are class redefinition 
> tests in jdk/test/java/lang/instrument that can be adapted and we 
> should probably keep them all together there.

Right, it is better to have some test coverage here.
I'll check if any of the JLI or JDI tests can be extended to cover this.
I'm not sure the JLI is good for this task though as deleting the 
methods is not instrumentation. :)


Thanks!
Serguei

>
> Thanks,
> Coleen
>
> On 5/9/14, 2:20 PM, serguei.spitsyn at oracle.com wrote:
>> Please, review the fix for:
>>   https://bugs.openjdk.java.net/browse/JDK-8042796
>>
>>
>> Open webrev:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/8042796-JVMTI-OLD.1 
>>
>>
>> Summary:
>>
>>   This is a Nightly Stabilization issue that was caused by a 
>> combination of two problems:
>>     - A regression introduced by the fix of: 
>> https://bugs.openjdk.java.net/browse/JDK-7182152
>>     - An SQE testbase infra regression: 
>> https://bugs.openjdk.java.net/browse/INTJDK-7611018
>>
>>   A number of the vm.mlvm tests hits this guarantee taht was added by 
>> 7182152 (must be relaxed).
>>   The issue is with the deleted static private methods that are still 
>> present in the CP cache.
>>   The fix is to mark the deleted methods with the flag 
>> JVM_ACC_IS_DELETED and
>>   then use it to relax the guarantee condition.
>>
>>
>> Testing:
>>   Running the failing tests: vm.mlvm.indy.func.jvmti
>>   In progress: nsk.jvmti, nsk.jdi, java.lang.instrument test runs on 
>> sparcv9 and amd64.
>>
>>
>> Thanks,
>> Serguei
>