[jdk16] RFR: 8258236: Segfault in ClassListParser::resolve_indy dumping static AppCDS archive

[Calvin Cheung]

On Tue, 15 Dec 2020 23:39:04 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Please review this change for JDK 16.
>> 
>> In ClassListParser::resolve_indy, if a class has previously failed verification, don't proceed with resolve indy for that class to avoid dereferencing a null cpcache pointer.
>> 
>> Passed tiers 1,2,3,4 tests.
>
> src/hotspot/share/classfile/classListParser.cpp line 474:
> 
>> 472:       return;
>> 473:     }
>> 474:     MetaspaceShared::try_link_class(ik, THREAD);
> 
> Doesn't the check for failing verification belong after try_link_class(), which runs the verifier?

Linking attempt was done for the class and was marked as failed verification prior to getting into `ClassListParser::resolve_indy`.

Consider the following class list (from the test case):
BadInvokeDynamic
@lambda-proxy BadInvokeDynamic run ()Ljava/lang/Runnable; ()V REF_invokeStatic BadInvokeDynamic lambda$doTest$0 ()V ()V

The BadInvokeDynamic was linked via the following code path:
	MetaspaceShared::try_link_class() at metaspaceShared.cpp:1,161 0x7ffff63b1c10	
	MetaspaceShared::preload_classes() at metaspaceShared.cpp:1,125 0x7ffff63b1a42	
	MetaspaceShared::preload_and_dump() at metaspaceShared.cpp:1,047 0x7ffff63b1557	
	Threads::create_vm() at thread.cpp:3,809 0x7ffff666b3f9	
	JNI_CreateJavaVM_inner() at jni.cpp:3,769 0x7ffff604b1be	
	JNI_CreateJavaVM() at jni.cpp:3,852 0x7ffff604b4c3	
	InitializeJVM() at java.c:1,536 0x7ffff7fcd664	
	JavaMain() at java.c:416 0x7ffff7fca252	
	ThreadJavaMain() at java_md.c:651 0x7ffff7fd0f77	
	start_thread() at 0x7ffff79b0ea5

It was marked as failed verification in `MetaspaceShared::try_link_class` in the following:
    ik->link_class(THREAD);
    if (HAS_PENDING_EXCEPTION) {
      ResourceMark rm(THREAD);
      log_warning(cds)("Preload Warning: Verification failed for %s",
                    ik->external_name());
      CLEAR_PENDING_EXCEPTION;
      SystemDictionaryShared::set_class_has_failed_verification(ik);
      _has_error_classes = true;
    }
During the processing of the `@lambda-proxy` in the class list, the `ClassListParser::resolve_indy` would be called and the check is for checking if the caller class has failed verification before.

> src/hotspot/share/classfile/classListParser.cpp line 467:
> 
>> 465:   Handle class_loader(THREAD, SystemDictionary::java_system_loader());
>> 466:   Handle protection_domain;
>> 467:   Klass* klass = SystemDictionary::resolve_or_fail(class_name_symbol, class_loader, protection_domain, true, THREAD); // FIXME should really be just a lookup
> 
> If an exception is unexpected, this should be CHECK not THREAD.

I'll try CHECK and do more testing before posting an updated webrev.

Thanks for your review.

-------------

PR: https://git.openjdk.java.net/jdk16/pull/30