RFR: 8338530: CDS warning Skipping java/lang/invoke/BoundMethodHandle$Species_LLLL

[Matias Saavedra Silva]

On Wed, 4 Sep 2024 16:39:43 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>>> Yes, `Species_*` classes can be statically generated by jlink - using a build-time "training run" of the `HelloClasslist` program @ExE-Boss refers to to generate a list of classes to generate statically into the JDK image.
>>> 
>>> What happened in [JDK-8336856](https://bugs.openjdk.org/browse/JDK-8336856) is likely that the `String` concat expressions touched by `HelloClasslist` no longer needs `Species_LLLL`, but something touched when running `java -Xshare:dump` does, so we get a warning. A dynamically generated `Species_LLLL` wouldn't exist in the JDK image, so it makes sense to exclude it from a CDS dump.
>>> 
>>> Random thoughts:
>>> 
>>> 1. It would probably work to actually make these dumpable into the CDS archive - though we might need to also dump the full bytecode and patch that up when we load them.
>>> 2. Looking at what `-Xshare:dump` does there's some `linkMethodHandleConstant` upcall which ends up creating the `Species_LLLL` class seen here - via `MethodHandleImpl.bindCaller`. Anyone who can point me to what's making such upcalls during dump? The provided `Lookup` might need to have other privileges to avoid generating forms with caller sensitive bindings. @iklam ?
>> 
>> I added this inside `KlassFactory::create_from_stream()` 
>> 
>> 
>>   if (name->ends_with("BoundMethodHandle$Species_LLLL")) {
>>     fatal("here");
>>   }
>> 
>> 
>> The hs_err file shows
>> 
>> 
>> V  [libjvm.so+0x10e084b]  KlassFactory::create_from_stream(ClassFileStream*, Symbol*, ClassLoaderData*, ClassLoadInfo const&, JavaThread*)+0x15b  (klassFactory.cpp:185)
>> V  [libjvm.so+0x1547651]  SystemDictionary::resolve_class_from_stream(ClassFileStream*, Symbol*, Handle, ClassLoadInfo const&, JavaThread*)+0x13f  (systemDictionary.cpp:908)
>> V  [libjvm.so+0x15478f0]  SystemDictionary::resolve_from_stream(ClassFileStream*, Symbol*, Handle, ClassLoadInfo const&, JavaThread*)+0x6c  (systemDictionary.cpp:946)
>> V  [libjvm.so+0xf2510d]  jvm_lookup_define_class(_jclass*, char const*, signed char const*, int, _jobject*, unsigned char, int, _jobject*, JavaThread*)+0x5b0  (jvm.cpp:1011)
>> V  [libjvm.so+0xf256ab]  JVM_LookupDefineClass+0x133  (jvm.cpp:1086)
>> C  [libjava.so+0xec1b]  Java_java_lang_ClassLoader_defineClass0+0x14b
>> j  java.lang.ClassLoader.defineClass0(Ljava/lang/ClassLoader;Ljava/lang/Class;Ljava/lang/String;[BIILjava/security/ProtectionDomain;ZILjava/lang/Object;)Ljava/lang/Class;+0 java.base at 24-internal
>> j  java.lang.System$2.defineClass(Ljava/lang/ClassLoader;Ljava/lang/Class;Lja...
>
>> 1. It would probably work to actually make these dumpable into the CDS archive - though we might need to also dump the full bytecode and patch that up when we load them.
> 
> With  [JEP 483: Ahead-of-Time Class Loading & Linking](https://bugs.openjdk.org/browse/JDK-8315737), we can archive these classes when `-XX:+AOTClassLinking` is enabled:
> 
> 
>     if (k->name()->starts_with("java/lang/invoke/BoundMethodHandle$Species_")) {
>       // This class is dynamically generated by the JDK
> +     if (CDSConfig::is_dumping_aot_linked_classes() {
> +       // This class is archived without recording the original bytecodes. That's OK because the original
> +       // bytecodes are needed only for JVMTI ClassFileLoadHook. A CDS archived generated
> +       // with -XX:+AOTClassLinking will not be loadable if ClassFileLoadHooks are enabled.
> +       k->set_shared_classpath_index(0); 
> +    } else {
>         ResourceMark rm;
>         log_info(cds)("Skipping %s because it is dynamically generated", k->name()->as_C_string());
>         return true; // exclude without warning
> +    }
>     } else {
>       // These are classes loaded from unsupported locations (such as those loaded by JVMTI native
>       // agent during dump time).
>       return warn_excluded(k, "Unsupported location");
>     }
> 
> 
> We can't do this when `AOTClassLinking` is disabled, or else when CFLH asks for the original bytecodes, we will get an assert in [`FileMapInfo::open_stream_for_jvmti()`](https://github.com/openjdk/jdk/blob/12d060a255b9b783488714c6c2cb73a899d3f708/src/hotspot/share/cds/filemap.cpp#L2547):

Thanks for the reviews and discussion @iklam and @calvinccheung!

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

PR Comment: https://git.openjdk.org/jdk/pull/20799#issuecomment-2329988639