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

[Coleen Phillimore]

On Wed, 16 Dec 2020 01:15:07 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> 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.

For this change, don't change it to CHECK in case something new will fail, and any new exceptions will caught by checking k != NULL.  It just looks strange but can be cleaned up later.

>> 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.

Okay so you're trying to catch a previous link time failure.  Why do you expect the next try_link_class call to never fail?

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

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