RFR: 8042235: redefining method used by multiple MethodHandles crashes VM

[KEVIN WALLS]

Hi,

I need to get comments on this review, but I'm also seeing a crash 
related to the constant pool cache in debug builds, which I haven't yet 
figured out...  So if anybody has thoughts on the following that would 
be great:

The testcase uses two MethodHandles, and redefines the target method.  
The problem is an assert during the redefinition, in
jdk7u-dev/hotspot/src/share/vm/oops/cpCacheOop.cpp:618.  Flags say that 
"is_vfinal" is true, but the constantPoolCacheEntry has a null _f2 which 
is illegal.

The assert here is  different to the original source just to isolate 
that it was NULL:

...
   [7] report_vm_error(file = 0xfe7a4fb8 
"/export/kwalls/work/bugs/8042235/jdk7u-dev/hotspot/src/share/vm/oops/cpCacheOop.cpp", 
line = 618, error_msg = 0xfe7a500c "assert(m != NULL) failed", 
detail_msg = 0xfe7a5025 "kjw null methodOop check"), line 227 in "debug.cpp"
   [8] ConstantPoolCacheEntry::is_interesting_method_entry(this = 
0xb5dfe4f8, k = 0xb5e0d560), line 618 in "cpCacheOop.cpp"
   [9] constantPoolCacheOopDesc::adjust_method_entries(this = 
0xb5dfe1e8, old_methods = 0x8117028, new_methods = 0x8117030, 
methods_length = 2, trace_name_printed = 0xb2b3ea23), line 682 in 
"cpCacheOop.cpp"
   [10] VM_RedefineClasses::adjust_cpool_cache_and_vtable(k_oop = 
0xb5dfd578, initiating_loader = 0xe4640a98, __the_thread__ = 0x8201000), 
line 2773 in "jvmtiRedefineClasses.cpp"
   [11] Dictionary::classes_do(this = 0x81297c8, f = 0xfe12ce90 = 
&VM_RedefineClasses::adjust_cpool_cache_and_vtable(klassOopDesc*,oopDesc*,Thread*), 
__the_thread__ = 0x8201000), line 354 in "dictionary.cpp"
   [12] SystemDictionary::classes_do(f = 0xfe12ce90 = 
&VM_RedefineClasses::adjust_cpool_cache_and_vtable(klassOopDesc*,oopDesc*,Thread*), 
__the_thread__ = 0x8201000), line 1754 in "systemDictionary.cpp"
   [13] VM_RedefineClasses::redefine_single_class(this = 0xb254d81c, 
the_jclass = 0x87147cc, scratch_class = CLASS, __the_thread__ = 
0x8201000), line 3448 in "jvmtiRedefineClasses.cpp"
   [14] VM_RedefineClasses::doit(this = 0xb254d81c), line 124 in 
"jvmtiRedefineClasses.cpp"
   [15] VM_Operation::evaluate(this = 0xb254d81c), line 71 in 
"vm_operations.cpp"
   [16] VMThread::evaluate_operation(this = 0x8201000, op = 0xb254d81c), 
line 386 in "vmThread.cpp"
...

dbx> frame 8
Current function is ConstantPoolCacheEntry::is_interesting_method_entry
   618     assert(m != NULL, "kjw null methodOop check");
dbx> print *this
*this = {
     _indices = 720896
     _f1      = 0xe47125d8
     _f2      = 0
     _flags   = 1901068290
}

_f1 is an instance of Oop for java/lang/invoke/MethodType

Adding an assert in set_f2_as_vfinal_method to check the passed 
methodOop is non-null, that does not trigger, i.e. we don't appear to 
place the null there.

When we crash during the class redefinition operation, 
ConstantPoolCacheEntry::adjust_method_entry(old, new) has been called, 
and that method already asserts that what it stores in _f2 is not null.

It's taking me a while, so I wanted to ping the alias here for other 
ideas!...

Thanks
Kevin


On 17/12/2014 15:48, KEVIN WALLS wrote:
> Hi,
>
> This is a request for review of a backport to 7u of 8042235. There are 
> a few changes from the original, hence the request here.
>
> On JDK7 this is not a crash, but we run the wrong method, i.e. 
> invocation through a MethodHandle invokes the old version of the 
> method, if it has been redefined.
>
> The test is different also: in jdk8 we have the ASM library, and can 
> visit methods and bytecodes.  Here in 7, I wrote a non-bytecode aware 
> byte replacer method, and replaced some literal bytecode sequence with 
> another.   As we're crafting a method that we will rewrite, we can do 
> something that avoids use of the constant pool (which we haven't 
> actually understood in this trivial rewriter), so we rewrite some 
> simple arithmetic, and from the result of the method it's obvious 
> whether we are running the correct code.
>
> Coleen: thanks for your earlier hints on oop / obj_field vs. 
> address_field.
>
> bug
> https://bugs.openjdk.java.net/browse/JDK-8042235
>
> webrev
> http://cr.openjdk.java.net/~kevinw/8042235/webrev.00/
>
> Thanks
> Kevin