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

[serguei.spit...@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.spit...@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