RFR(8u): 8038636, 8055008, 8156137: SIGSEGV in ReceiverTypeData::clean_weak_klass_links ...and 8057570.

[Kevin Walls]

Thanks Coleen!


On 16/11/2017 23:08, coleen.phillimore at oracle.com wrote:
>
> Hi Kevin, These changes look good.  It's nice to have the simpler 
> version of the InstanceKlass::_previous_versions in jdk8.  There was a 
> bug tail so there may be more changes after 8040237.  It's worth 
> following through all the bugs to get something close to what we have 
> in jdk9/10.  But this change looks good so far.
>
> Thanks,
> Coleen
>
> On 11/13/17 1:08 PM, Kevin Walls wrote:
>> Hi,
>>
>> I'd like to get a hotspot review of these backports from 9 to 8u. 
>> This is mainly runtime territory but some of the history is from the 
>> compiler side.
>>
>> webrev: http://cr.openjdk.java.net/~kevinw/8055008.8156137/webrev.00/
>>
>> The one we need is:
>>
>> 8156137: SIGSEGV in ReceiverTypeData::clean_weak_klass_links
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8156137
>> 9 changeset: 
>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/882e8cda60b3
>>
>> The 9 changeset is short, just changing klass.cpp, but not possible 
>> in 8 without additional work, so...
>>
>> 1.
>>
>> 8038636: speculative traps break when classes are redefined
>> https://bugs.openjdk.java.net/browse/JDK-8038636
>>
>> 9 changeset: 
>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/a7784ddacbef
>>
>> 9 review thread: 
>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2014-April/013948.html 
>> (in 9 since April 2014)
>>
>> That one imports cleanly into 8u.
>>
>>
>> 2.
>>
>> With that imported, we need:
>>
>> 8055008:Clean up code that saves the previous versions of redefined 
>> classes
>> https://bugs.openjdk.java.net/browse/JDK-8055008
>>
>> 9 changeset: 
>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/e3fb51ae8d7d
>>
>> This is the change to stop using PreviousVersionNode and get an 
>> InstanceKlass* from previous_versions().
>>
>>
>> My webrev: http://cr.openjdk.java.net/~kevinw/8055008.8156137/webrev.00/
>>
>> ...is 8055008 and 8156137 in 8u. The klass.cpp change here is 
>> 8156137.  The rest is 8055008, so I can commit them separately.
>>
>> 8156137 doesn't have its own test, but I have found if you get this 
>> area wrong, the test runtime/RedefineObject crashes.
>>
>>
>> Notes:
>> For 8055008 there was a change in 
>> src/share/vm/classfile/classLoaderData.cpp which is already in 8u.
>>
>> src/share/vm/classfile/metadataOnStackMark.cpp:
>> +MetadataOnStackMark::MetadataOnStackMark(bool has_redefined_a_class) {
>>
>> The change is to stop using JvmtiExport::has_redefined_a_class() here 
>> which is already in 8u at this point, but the param is already there 
>> with a different name, so renamed as per 8055008.
>>
>> universe.cpp: had a change in 8055008 but following that there was:
>>
>> 8057570: RedefineClasses() tests fail 
>> assert(((Metadata*)obj)->is_valid()) failed: obj is valid
>> https://bugs.openjdk.java.net/browse/JDK-8057570
>> 9 changeset: 
>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/479ed4234a9d
>>
>> ..this puts back a few lines in nmethod.cpp which 8055008 removed:
>>
>> + // Call function Method*, not embedded in these other places.
>> + if (_method != NULL) f(_method);
>>
>> ..and in universe.cpp:
>>
>> + // Make the dependent methods not entrant (in VM_Deoptimize they 
>> are made zombies)
>> + CodeCache::make_marked_nmethods_not_entrant();
>>
>> ..and removes: CodeCache::make_marked_nmethods_zombies();
>>
>> My webrev takes these into account.
>>
>>
>> Then there are 8038636's two follow-on bugs which I'd like to 
>> follow-up in a separate thread.
>> 8039960 is a test change
>> 8040237: nsk/jvmti/RetransformClasses/retransform001 crashed the VM 
>> on all platforms when run with with -server -Xcomp
>> https://bugs.openjdk.java.net/browse/JDK-8040237
>>
>>
>> Thanks!
>> Kevin
>>
>>
>>
>