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

[serguei.spitsyn at oracle.com]

Dan,

Thank you a lot for reviewing!

On 5/13/14 10:42 AM, Daniel D. Daugherty wrote:
> > 
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/8042796-JVMTI-OLD.1
>
>
> src/share/vm/oops/method.hpp
>     No comments.
>
> src/share/vm/utilities/accessFlags.hpp
>     line 57: JVM_ACC_ON_STACK                = 0x00080000,     // 
> RedefinedClasses() is used on the stack
>         Typos: 'RedefinedClasses() is' -> 'RedefineClasses() was'
>         Not your typos, but would you mind fixing them?

Thanks, I'll fix it.

>
> src/share/vm/oops/cpCache.cpp
>     line 501: (!f1_as_method()->is_old() && 
> !f1_as_method()->is_obsolete() ||
>     line 502 f1_as_method()->is_deleted()));
>
>     Perhaps this would a little more clear (added parens and 
> reformatted):
>
>     line 501: ((!f1_as_method()->is_old() && 
> !f1_as_method()->is_obsolete()) ||
>     line 502   f1_as_method()->is_deleted()));
>
>     Or this one:
>
>     line 501: (f1_as_method()->is_deleted() ||
>     line 502:  (!f1_as_method()->is_old() && 
> !f1_as_method()->is_obsolete())));

I agree and will fix it as you suggested.


>
> src/share/vm/prims/jvmtiRedefineClasses.cpp
>     No comments
>
> Thumbs up even if you don't want to juggle the logic
> in cpCache.cpp...
>
> Minor clarification:
>
> > - A regression introduced by the fix of:
> > https://bugs.openjdk.java.net/browse/JDK-7182152
>
> The above fix modified the existing VM_RedefineClasses::check_class()
> code to use guarantee() instead of assert() and also modified the
> logic to catch "obsolete" in addition to "old" entries. I strongly
> suspect that the code before JDK-7182152 would still have fired an
> assert in the presence of deleted methods so the regression is
> probably older than JDK-7182152. Don't know if it is worth fixing
> that far back though...

Most likely, you are right.
In fact, it is not that easy to track it back in time so far. :)

Thanks,
Serguei

>
> Dan
>
>
> On 5/9/14 12: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
>