Skip to content

Commit 7a6fadc

Browse files
iklamiklam
committedSep 25, 2024
8340869: [premain] NullPointerException with LambdaWithUseImplMethodHandle.java
1 parent 6c83df4 commit 7a6fadc

File tree

5 files changed

+99
-132
lines changed

5 files changed

+99
-132
lines changed
 

‎src/hotspot/share/cds/aotClassInitializer.cpp

+55-95
Original file line numberDiff line numberDiff line change
@@ -34,76 +34,6 @@
3434
#include "oops/instanceKlass.inline.hpp"
3535
#include "runtime/mutexLocker.hpp"
3636

37-
// Warning -- this is fragile!!!
38-
//
39-
// This is a hard-coded list of classes that are safe to preinitialize at dump time. It needs
40-
// to be updated if the Java source code changes.
41-
bool AOTClassInitializer::is_forced_preinit_class(InstanceKlass* ik) {
42-
if (!CDSConfig::is_dumping_invokedynamic()) {
43-
return false;
44-
}
45-
46-
static const char* forced_preinit_classes[] = {
47-
"java/util/HexFormat",
48-
"jdk/internal/util/ClassFileDumper",
49-
"java/lang/reflect/ClassFileFormatVersion",
50-
"java/lang/Character$CharacterCache",
51-
"java/lang/invoke/Invokers",
52-
"java/lang/invoke/Invokers$Holder",
53-
"java/lang/invoke/MethodHandle",
54-
"java/lang/invoke/MethodHandleStatics",
55-
"java/lang/invoke/DelegatingMethodHandle",
56-
"java/lang/invoke/DelegatingMethodHandle$Holder",
57-
"java/lang/invoke/LambdaForm",
58-
"java/lang/invoke/LambdaForm$NamedFunction",
59-
"java/lang/invoke/ClassSpecializer",
60-
"java/lang/invoke/DirectMethodHandle",
61-
"java/lang/invoke/DirectMethodHandle$Holder",
62-
"java/lang/invoke/BoundMethodHandle$Specializer",
63-
"java/lang/invoke/MethodHandles$Lookup",
64-
65-
// TODO:
66-
// This is needed since JDK-8338532. Without this, when
67-
// archived heap objects are used, the class init order is not
68-
// expected by the jdk/internal/constant bootstrap code and we
69-
// will get a null pointer exception.
70-
//
71-
// When bootstraping has intricated/fragile order, it's probably
72-
// better to archive all related classes in an initialized state
73-
// (i.e., take a snapshot). The existing approach in
74-
// heapShared::resolve_or_init_classes_for_subgraph_of() won't work.
75-
"jdk/internal/constant/PrimitiveClassDescImpl",
76-
"jdk/internal/constant/ReferenceClassDescImpl",
77-
"java/lang/constant/ConstantDescs",
78-
"sun/invoke/util/Wrapper",
79-
80-
//TODO: these use java.lang.ClassValue$Entry which is a subtype of WeakReference
81-
//"java/lang/reflect/Proxy$ProxyBuilder",
82-
//"java/lang/reflect/Proxy",
83-
84-
// TODO -- need to clear internTable, etc
85-
//"java/lang/invoke/MethodType",
86-
87-
// TODO -- these need to link to native code
88-
//"java/lang/invoke/BoundMethodHandle",
89-
//"java/lang/invoke/BoundMethodHandle$Holder",
90-
//"java/lang/invoke/MemberName",
91-
//"java/lang/invoke/MethodHandleNatives",
92-
};
93-
94-
for (const char* class_name : forced_preinit_classes) {
95-
if (ik->name()->equals(class_name)) {
96-
if (log_is_enabled(Info, cds, init)) {
97-
ResourceMark rm;
98-
log_info(cds, init)("Force initialization %s", ik->external_name());
99-
}
100-
return true;
101-
}
102-
}
103-
104-
return false;
105-
}
106-
10737
// check_can_be_preinited() is quite costly, so we cache the results inside
10838
// DumpTimeClassInfo::_can_be_preinited. See also AOTClassInitializer::reset_preinit_check().
10939
bool AOTClassInitializer::check_can_be_preinited(InstanceKlass* ik) {
@@ -129,16 +59,17 @@ bool AOTClassInitializer::check_can_be_preinited(InstanceKlass* ik) {
12959
}
13060
}
13161

132-
if (HeapShared::is_lambda_form_klass(ik) || is_forced_preinit_class(ik)) {
62+
if (HeapShared::is_lambda_form_klass(ik)) {
13363
// We allow only these to have <clinit> or non-default static fields
134-
} else {
135-
if (ik->class_initializer() != nullptr) {
136-
log_info(cds, init)("cannot initialize %s (has <clinit>)", ik->external_name());
137-
return false;
138-
}
139-
if (ik->is_initialized() && !has_default_static_fields(ik)) {
140-
return false;
141-
}
64+
return true;
65+
}
66+
67+
if (ik->class_initializer() != nullptr) {
68+
log_info(cds, init)("cannot initialize %s (has <clinit>)", ik->external_name());
69+
return false;
70+
}
71+
if (ik->is_initialized() && !has_default_static_fields(ik)) {
72+
return false;
14273
}
14374

14475
return true;
@@ -254,24 +185,53 @@ bool AOTClassInitializer::can_archive_preinitialized_mirror(InstanceKlass* ik) {
254185

255186
if (ik->is_hidden()) {
256187
return HeapShared::is_archivable_hidden_klass(ik);
257-
} else if (ik->is_initialized()) {
258-
if (ik->java_super() == vmClasses::Enum_klass()) {
259-
return true;
260-
}
261-
Symbol* name = ik->name();
262-
if (name->equals("jdk/internal/constant/PrimitiveClassDescImpl") ||
263-
name->equals("jdk/internal/constant/ReferenceClassDescImpl") ||
264-
name->equals("java/lang/constant/ConstantDescs") ||
265-
name->equals("sun/invoke/util/Wrapper")) {
188+
}
189+
190+
if (ik->is_initialized() && ik->java_super() == vmClasses::Enum_klass()) {
191+
return true;
192+
}
193+
194+
Symbol* name = ik->name();
195+
if (name->equals("jdk/internal/constant/PrimitiveClassDescImpl") ||
196+
name->equals("jdk/internal/constant/ReferenceClassDescImpl") ||
197+
name->equals("java/lang/constant/ConstantDescs")) {
198+
assert(ik->is_initialized(), "must be");
199+
// The above 3 classes are special cases needed to support the aot-caching of
200+
// java.lang.invoke.MethodType instances:
201+
// - MethodType points to sun.invoke.util.Wrapper enums
202+
// - The Wrapper enums point to static final fields in the above 3 classes.
203+
// E.g., ConstantDescs.CD_Boolean.
204+
// - If we re-run the <clinit> of these 3 classes again during the production
205+
// run, ConstantDescs.CD_Boolean will get a new value that has a different
206+
// object identity than the value referenced by the the Wrapper enums.
207+
// - However, Wrapper requires object identity (it allows the use of == to
208+
// test the equality of ClassDesc, etc).
209+
// Therefore, we must preserve the static fields of these 3 classes from
210+
// the assembly phase.
211+
return true;
212+
}
213+
if (CDSConfig::is_dumping_invokedynamic()) {
214+
if (name->equals("java/lang/Boolean$AOTHolder") ||
215+
name->equals("java/lang/Character$CharacterCache") ||
216+
name->equals("java/lang/invoke/BoundMethodHandle$Specializer") ||
217+
name->equals("java/lang/invoke/ClassSpecializer") ||
218+
name->equals("java/lang/invoke/DelegatingMethodHandle") ||
219+
name->equals("java/lang/invoke/DelegatingMethodHandle$Holder") ||
220+
name->equals("java/lang/invoke/DirectMethodHandle") ||
221+
name->equals("java/lang/invoke/DirectMethodHandle$AOTHolder") ||
222+
name->equals("java/lang/invoke/DirectMethodHandle$Holder") ||
223+
name->equals("java/lang/invoke/Invokers") ||
224+
name->equals("java/lang/invoke/Invokers$Holder") ||
225+
name->equals("java/lang/invoke/LambdaForm") ||
226+
name->equals("java/lang/invoke/LambdaForm$NamedFunction") ||
227+
name->equals("java/lang/invoke/LambdaForm$NamedFunction$AOTHolder") ||
228+
name->equals("java/lang/invoke/MethodHandle") ||
229+
name->equals("java/lang/invoke/MethodHandles$Lookup") ||
230+
name->equals("java/lang/invoke/MethodType$AOTHolder") ||
231+
name->starts_with("java/lang/invoke/ClassSpecializer$")) {
232+
assert(ik->is_initialized(), "must be");
266233
return true;
267234
}
268-
if (CDSConfig::is_dumping_invokedynamic()) {
269-
if (name->equals("java/lang/invoke/DirectMethodHandle$AOTHolder") ||
270-
name->equals("java/lang/invoke/LambdaForm$NamedFunction$AOTHolder") ||
271-
name->equals("java/lang/invoke/MethodType$AOTHolder")) {
272-
return true;
273-
}
274-
}
275235
}
276236

277237
return AOTClassInitializer::can_be_preinited_locked(ik);

‎src/hotspot/share/cds/aotClassInitializer.hpp

-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ class InstanceKlass;
3232

3333
class AOTClassInitializer : AllStatic {
3434
static bool has_default_static_fields(InstanceKlass* ik);
35-
static bool is_forced_preinit_class(InstanceKlass* ik);
3635

3736
static bool check_can_be_preinited(InstanceKlass* ik);
3837
static bool can_be_preinited(InstanceKlass* ik);

‎src/hotspot/share/cds/heapShared.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,8 @@ void HeapShared::copy_preinitialized_mirror(Klass* orig_k, oop orig_mirror, oop
634634
}
635635
}
636636

637+
java_lang_Class::set_class_data(m, java_lang_Class::class_data(orig_mirror));
638+
637639
// Class::reflectData use SoftReference, which cannot be archived. Set it
638640
// to null and it will be recreated at runtime.
639641
java_lang_Class::set_reflection_data(m, nullptr);

‎src/hotspot/share/cds/metaspaceShared.cpp

+33-35
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,27 @@ void VM_PopulateDumpSharedSpace::doit() {
572572

573573
DEBUG_ONLY(SystemDictionaryShared::NoClassLoadingMark nclm);
574574

575+
_method_handle_intrinsics = new (mtClassShared) GrowableArray<Method*>(256, mtClassShared);
576+
SystemDictionary::get_all_method_handle_intrinsics(_method_handle_intrinsics);
577+
_method_handle_intrinsics->sort([] (Method** a, Method** b) -> int {
578+
Symbol* a_holder = (*a)->method_holder()->name();
579+
Symbol* b_holder = (*b)->method_holder()->name();
580+
if (a_holder != b_holder) {
581+
return a_holder->cmp(b_holder);
582+
}
583+
Symbol* a_name = (*a)->name();
584+
Symbol* b_name = (*b)->name();
585+
if (a_name != b_name) {
586+
return a_name->cmp(b_name);
587+
}
588+
Symbol* a_signature = (*a)->signature();
589+
Symbol* b_signature = (*b)->signature();
590+
if (a_signature != b_signature) {
591+
return a_signature->cmp(b_signature);
592+
}
593+
return 0;
594+
});
595+
575596
FileMapInfo::check_nonempty_dir_in_shared_path_table();
576597

577598
NOT_PRODUCT(SystemDictionary::verify();)
@@ -894,20 +915,6 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS
894915
}
895916
}
896917

897-
#if INCLUDE_CDS_JAVA_HEAP
898-
if (CDSConfig::is_dumping_invokedynamic()) {
899-
// This makes sure that the MethodType and MethodTypeForm tables won't be updated
900-
// concurrently when we are saving their contents into a side table.
901-
assert(CDSConfig::allow_only_single_java_thread(), "Required");
902-
903-
JavaValue result(T_VOID);
904-
JavaCalls::call_static(&result, vmClasses::MethodType_klass(),
905-
vmSymbols::createArchivedObjects(),
906-
vmSymbols::void_method_signature(),
907-
CHECK);
908-
}
909-
#endif
910-
911918
// Rewrite and link classes
912919
log_info(cds)("Rewriting and linking classes ...");
913920
// Link any classes which got missed. This would happen if we have loaded classes that
@@ -924,27 +931,6 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS
924931

925932
TrainingData::init_dumptime_table(CHECK); // captures TrainingDataSetLocker
926933

927-
_method_handle_intrinsics = new (mtClassShared) GrowableArray<Method*>(256, mtClassShared);
928-
SystemDictionary::get_all_method_handle_intrinsics(_method_handle_intrinsics);
929-
_method_handle_intrinsics->sort([] (Method** a, Method** b) -> int {
930-
Symbol* a_holder = (*a)->method_holder()->name();
931-
Symbol* b_holder = (*b)->method_holder()->name();
932-
if (a_holder != b_holder) {
933-
return a_holder->cmp(b_holder);
934-
}
935-
Symbol* a_name = (*a)->name();
936-
Symbol* b_name = (*b)->name();
937-
if (a_name != b_name) {
938-
return a_name->cmp(b_name);
939-
}
940-
Symbol* a_signature = (*a)->signature();
941-
Symbol* b_signature = (*b)->signature();
942-
if (a_signature != b_signature) {
943-
return a_signature->cmp(b_signature);
944-
}
945-
return 0;
946-
});
947-
948934
#if INCLUDE_CDS_JAVA_HEAP
949935
if (CDSConfig::is_dumping_heap()) {
950936
if (!HeapShared::is_archived_boot_layer_available(THREAD)) {
@@ -961,6 +947,18 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS
961947
SystemDictionaryShared::create_loader_positive_lookup_cache(CHECK);
962948
}
963949

950+
if (CDSConfig::is_dumping_invokedynamic()) {
951+
// This makes sure that the MethodType and MethodTypeForm tables won't be updated
952+
// concurrently when we are saving their contents into a side table.
953+
assert(CDSConfig::allow_only_single_java_thread(), "Required");
954+
955+
JavaValue result(T_VOID);
956+
JavaCalls::call_static(&result, vmClasses::MethodType_klass(),
957+
vmSymbols::createArchivedObjects(),
958+
vmSymbols::void_method_signature(),
959+
CHECK);
960+
}
961+
964962
// Do this at the very end, when no Java code will be executed. Otherwise
965963
// some new strings may be added to the intern table.
966964
StringTable::allocate_shared_strings_array(CHECK);

‎test/hotspot/jtreg/runtime/cds/appcds/LambdaWithUseImplMethodHandle.java

+9-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -43,6 +43,11 @@ public class LambdaWithUseImplMethodHandle {
4343

4444
// See pkg2/Child.jcod for details about the condition that triggers JDK-8290417
4545
public static void main(String[] args) throws Exception {
46+
test(false);
47+
test(true);
48+
}
49+
50+
static void test(boolean aotClassLinking) throws Exception {
4651
String appJar = ClassFileInstaller.getJarPath("test.jar");
4752
String mainClass = "LambdaWithUseImplMethodHandleApp";
4853
String expectedMsg = "Called BaseWithProtectedMethod::protectedMethod";
@@ -57,6 +62,9 @@ public static void main(String[] args) throws Exception {
5762
.addPrefix("-XX:ExtraSharedClassListFile=" + classList,
5863
"-cp", appJar)
5964
.setArchiveName(archiveName);
65+
if (aotClassLinking) {
66+
opts.addPrefix("-XX:+AOTClassLinking");
67+
}
6068
CDSTestUtils.createArchiveAndCheck(opts);
6169

6270
// run with archive

0 commit comments

Comments
 (0)
Please sign in to comment.