Skip to content

Commit

Permalink
8248003: [lworld] [lw3] VM crashes when classes with inline type fiel…
Browse files Browse the repository at this point in the history
…ds are loaded from CDS archive
  • Loading branch information
fparain committed Jun 24, 2020
1 parent 45400b1 commit 3f8947b
Show file tree
Hide file tree
Showing 26 changed files with 322 additions and 59 deletions.
2 changes: 1 addition & 1 deletion src/hotspot/cpu/x86/macroAssembler_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3616,7 +3616,7 @@ void MacroAssembler::zero_memory(Register address, Register length_in_bytes, int
}

void MacroAssembler::get_value_field_klass(Register klass, Register index, Register value_klass) {
movptr(value_klass, Address(klass, InstanceKlass::value_field_klasses_offset()));
movptr(value_klass, Address(klass, InstanceKlass::inline_type_field_klasses_offset()));
#ifdef ASSERT
{
Label done;
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/ci/ciInstanceKlass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ GrowableArray<ciField*>* ciInstanceKlass::compute_nonstatic_fields_impl(Growable
// Value type fields are embedded
int field_offset = fd.offset();
// Get ValueKlass and adjust number of fields
Klass* k = get_instanceKlass()->get_value_field_klass(fd.index());
Klass* k = get_instanceKlass()->get_inline_type_field_klass(fd.index());
ciValueKlass* vk = CURRENT_ENV->get_klass(k)->as_value_klass();
flen += vk->nof_nonstatic_fields() - 1;
// Iterate over fields of the flattened value type and copy them to 'this'
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/ci/ciReplay.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ class CompileReplay : public StackObj {
break;
}
case T_VALUETYPE: {
ValueKlass* vk = ValueKlass::cast(fd->field_holder()->get_value_field_klass(fd->index()));
ValueKlass* vk = ValueKlass::cast(fd->field_holder()->get_inline_type_field_klass(fd->index()));
if (fd->is_inlined()) {
int field_offset = fd->offset() - vk->first_field_offset();
oop obj = (oop)(cast_from_oop<address>(_vt) + field_offset);
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/classfile/classFileParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6432,7 +6432,7 @@ void ClassFileParser::fill_instance_klass(InstanceKlass* ik,
int nfields = ik->java_fields_count();
if (ik->is_inline_klass()) nfields++;
for (int i = 0; i < nfields; i++) {
if (ik->field_is_inline_type(i)) {
if (ik->field_is_inline_type(i) && ((ik->field_access_flags(i) & JVM_ACC_STATIC) == 0)) {
Symbol* klass_name = ik->field_signature(i)->fundamental_name(CHECK);
// Inline classes for instance fields must have been pre-loaded
// Inline classes for static fields might not have been loaded yet
Expand All @@ -6441,7 +6441,7 @@ void ClassFileParser::fill_instance_klass(InstanceKlass* ik,
Handle(THREAD, ik->protection_domain()), CHECK);
if (klass != NULL) {
assert(klass->access_flags().is_inline_type(), "Inline type expected");
ik->set_value_field_klass(i, klass);
ik->set_inline_type_field_klass(i, klass);
}
klass_name->decrement_refcount();
} else
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/classfile/fieldLayoutBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ bool FieldLayout::reconstruct_layout(const InstanceKlass* ik) {
has_instance_fields = true;
LayoutRawBlock* block;
if (type == T_VALUETYPE) {
ValueKlass* vk = ValueKlass::cast(ik->get_value_field_klass(fs.index()));
ValueKlass* vk = ValueKlass::cast(ik->get_inline_type_field_klass(fs.index()));
block = new LayoutRawBlock(fs.index(), LayoutRawBlock::INHERITED, vk->get_exact_size_in_bytes(),
vk->get_alignment(), false);

Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/classfile/javaClasses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1071,6 +1071,8 @@ void java_lang_Class::create_mirror(Klass* k, Handle class_loader,
// set the reference projection type
set_ref_type_mirror(mirror(), ref_type_oop);

assert(oopDesc::is_oop(ref_type_oop), "Sanity check");

// set the value and reference projection types
set_val_type_mirror(ref_type_oop, mirror());
set_ref_type_mirror(ref_type_oop, ref_type_oop);
Expand Down Expand Up @@ -1128,6 +1130,7 @@ class ResetMirrorField: public FieldClosure {
case T_BOOLEAN:
_m()->bool_field_put(fd->offset(), false);
break;
case T_VALUETYPE:
case T_ARRAY:
case T_OBJECT: {
// It might be useful to cache the String field, but
Expand Down
42 changes: 41 additions & 1 deletion src/hotspot/share/classfile/systemDictionary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
#include "oops/oopHandle.inline.hpp"
#include "oops/symbol.hpp"
#include "oops/typeArrayKlass.hpp"
#include "oops/valueKlass.hpp"
#include "oops/valueKlass.inline.hpp"
#include "prims/jvmtiExport.hpp"
#include "prims/methodHandles.hpp"
#include "runtime/arguments.hpp"
Expand Down Expand Up @@ -1480,6 +1480,24 @@ InstanceKlass* SystemDictionary::load_shared_class(InstanceKlass* ik,
return NULL;
}


if (ik->has_inline_type_fields()) {
for (AllFieldStream fs(ik->fields(), ik->constants()); !fs.done(); fs.next()) {
if (Signature::basic_type(fs.signature()) == T_VALUETYPE) {
if (!fs.access_flags().is_static()) {
// Pre-load inline class
Klass* real_k = SystemDictionary::resolve_inline_type_field_or_fail(&fs,
class_loader, protection_domain, true, CHECK_NULL);
Klass* k = ik->get_inline_type_field_klass_or_null(fs.index());
if (real_k != k) {
// oops, the app has substituted a different version of k!
return NULL;
}
}
}
}
}

InstanceKlass* new_ik = NULL;
// CFLH check is skipped for VM hidden or anonymous classes (see KlassFactory::create_from_stream).
// It will be skipped for shared VM hidden lambda proxy classes.
Expand Down Expand Up @@ -1516,6 +1534,13 @@ InstanceKlass* SystemDictionary::load_shared_class(InstanceKlass* ik,
}

load_shared_class_misc(ik, loader_data, CHECK_NULL);

if (ik->is_inline_klass()) {
ValueKlass* vk = ValueKlass::cast(ik);
oop val = ik->allocate_instance(CHECK_NULL);
vk->set_default_value(val);
}

return ik;
}

Expand Down Expand Up @@ -1579,6 +1604,21 @@ void SystemDictionary::quick_resolve(InstanceKlass* klass, ClassLoaderData* load
}
}

if (klass->has_inline_type_fields()) {
for (AllFieldStream fs(klass->fields(), klass->constants()); !fs.done(); fs.next()) {
if (Signature::basic_type(fs.signature()) == T_VALUETYPE) {
if (!fs.access_flags().is_static()) {
Klass* real_k = SystemDictionary::resolve_inline_type_field_or_fail(&fs,
Handle(THREAD, loader_data->class_loader()), domain, true, CHECK);
Klass* k = klass->get_inline_type_field_klass_or_null(fs.index());
assert(real_k == k, "oops, the app has substituted a different version of k!");
} else {
klass->reset_inline_type_field_klass(fs.index());
}
}
}
}

klass->restore_unshareable_info(loader_data, domain, NULL, THREAD);
load_shared_class_misc(klass, loader_data, CHECK);
Dictionary* dictionary = loader_data->dictionary();
Expand Down
8 changes: 4 additions & 4 deletions src/hotspot/share/interpreter/interpreterRuntime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ JRT_ENTRY(int, InterpreterRuntime::withfield(JavaThread* thread, ConstantPoolCac
if (cp_entry->is_inlined()) {
oop vt_oop = *(oop*)f.interpreter_frame_expression_stack_at(tos_idx);
assert(vt_oop != NULL && oopDesc::is_oop(vt_oop) && vt_oop->is_inline_type(),"argument must be an inline type");
ValueKlass* field_vk = ValueKlass::cast(vklass->get_value_field_klass(field_index));
ValueKlass* field_vk = ValueKlass::cast(vklass->get_inline_type_field_klass(field_index));
assert(vt_oop != NULL && field_vk == vt_oop->klass(), "Must match");
field_vk->write_inlined_field(new_value_h(), offset, vt_oop, CHECK_(return_offset));
} else { // not inlined
Expand Down Expand Up @@ -392,14 +392,14 @@ JRT_ENTRY(void, InterpreterRuntime::uninitialized_static_value_field(JavaThread*
InstanceKlass* klass = InstanceKlass::cast(java_lang_Class::as_Klass(mirror));
if (klass->is_being_initialized() && klass->is_reentrant_initialization(THREAD)) {
int offset = klass->field_offset(index);
Klass* field_k = klass->get_value_field_klass_or_null(index);
Klass* field_k = klass->get_inline_type_field_klass_or_null(index);
if (field_k == NULL) {
field_k = SystemDictionary::resolve_or_fail(klass->field_signature(index)->fundamental_name(THREAD),
Handle(THREAD, klass->class_loader()),
Handle(THREAD, klass->protection_domain()),
true, CHECK);
assert(field_k != NULL, "Should have been loaded or an exception thrown above");
klass->set_value_field_klass(index, field_k);
klass->set_inline_type_field_klass(index, field_k);
}
field_k->initialize(CHECK);
oop defaultvalue = ValueKlass::cast(field_k)->default_value();
Expand Down Expand Up @@ -435,7 +435,7 @@ JRT_ENTRY(void, InterpreterRuntime::read_inlined_field(JavaThread* thread, oopDe

assert(klass->field_is_inlined(index), "Sanity check");

ValueKlass* field_vklass = ValueKlass::cast(klass->get_value_field_klass(index));
ValueKlass* field_vklass = ValueKlass::cast(klass->get_inline_type_field_klass(index));
assert(field_vklass->is_initialized(), "Must be initialized at this point");

oop res = field_vklass->read_inlined_field(obj_h(), klass->field_offset(index), CHECK);
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/memory/heapInspection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ static void print_inlined_field(outputStream* st, int level, int offset, Instanc
fd.is_inline_type(), fd.holder()->field_is_inlined(fd.index()));
if (fd.holder()->field_is_inlined(fd.index())) {
print_inlined_field(st, level + 1, offset2 ,
InstanceKlass::cast(fd.holder()->get_value_field_klass(fd.index())));
InstanceKlass::cast(fd.holder()->get_inline_type_field_klass(fd.index())));
}
}
}
Expand Down Expand Up @@ -606,7 +606,7 @@ void PrintClassLayout::print_class_layout(outputStream* st, char* class_name) {
print_field(st, 0, fd.offset(), fd, fd.is_inline_type(), fd.holder()->field_is_inlined(fd.index()));
if (fd.holder()->field_is_inlined(fd.index())) {
print_inlined_field(st, 1, fd.offset(),
InstanceKlass::cast(fd.holder()->get_value_field_klass(fd.index())));
InstanceKlass::cast(fd.holder()->get_inline_type_field_klass(fd.index())));
}
}
}
Expand Down
11 changes: 8 additions & 3 deletions src/hotspot/share/oops/constantPool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -402,14 +402,15 @@ void ConstantPool::remove_unshareable_info() {
_flags |= (_on_stack | _is_shared);
int num_klasses = 0;
for (int index = 1; index < length(); index++) { // Index 0 is unused
jbyte qdesc_bit = tag_at(index).is_Qdescriptor_klass() ? (jbyte) JVM_CONSTANT_QDescBit : 0;
if (!DynamicDumpSharedSpaces) {
assert(!tag_at(index).is_unresolved_klass_in_error(), "This must not happen during static dump time");
} else {
if (tag_at(index).is_unresolved_klass_in_error() ||
tag_at(index).is_method_handle_in_error() ||
tag_at(index).is_method_type_in_error() ||
tag_at(index).is_dynamic_constant_in_error()) {
tag_at_put(index, JVM_CONSTANT_UnresolvedClass);
tag_at_put(index, JVM_CONSTANT_UnresolvedClass | qdesc_bit);
}
}
if (tag_at(index).is_klass()) {
Expand All @@ -429,7 +430,7 @@ void ConstantPool::remove_unshareable_info() {
int name_index = kslot.name_index();
assert(tag_at(name_index).is_symbol(), "sanity");
resolved_klasses()->at_put(resolved_klass_index, NULL);
tag_at_put(index, JVM_CONSTANT_UnresolvedClass);
tag_at_put(index, JVM_CONSTANT_UnresolvedClass | qdesc_bit);
assert(klass_name_at(index) == symbol_at(name_index), "sanity");
}
}
Expand Down Expand Up @@ -564,7 +565,11 @@ Klass* ConstantPool::klass_at_impl(const constantPoolHandle& this_cp, int which,
// to resolve this constant pool entry fail with the same error (JVMS 5.4.3).
if (HAS_PENDING_EXCEPTION) {
if (save_resolution_error) {
save_and_throw_exception(this_cp, which, constantTag(JVM_CONSTANT_UnresolvedClass), CHECK_NULL);
jbyte tag = JVM_CONSTANT_UnresolvedClass;
if (this_cp->tag_at(which).is_Qdescriptor_klass()) {
tag |= JVM_CONSTANT_QDescBit;
}
save_and_throw_exception(this_cp, which, constantTag(tag), CHECK_NULL);
// If CHECK_NULL above doesn't return the exception, that means that
// some other thread has beaten us and has resolved the class.
// To preserve old behavior, we return the resolved class.
Expand Down
29 changes: 21 additions & 8 deletions src/hotspot/share/oops/instanceKlass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ InstanceKlass::InstanceKlass(const ClassFileParser& parser, unsigned kind, Klass
_init_state(allocated),
_reference_type(parser.reference_type()),
_init_thread(NULL),
_value_field_klasses(NULL),
_inline_type_field_klasses(NULL),
_adr_valueklass_fixed_block(NULL)
{
set_vtable_length(parser.vtable_size());
Expand All @@ -592,7 +592,7 @@ InstanceKlass::InstanceKlass(const ClassFileParser& parser, unsigned kind, Klass
set_layout_helper(Klass::instance_layout_helper(parser.layout_size(),
false));
if (parser.has_inline_fields()) {
set_has_inline_fields();
set_has_inline_type_fields();
}
_java_fields_count = parser.java_fields_count();

Expand All @@ -605,8 +605,8 @@ InstanceKlass::InstanceKlass(const ClassFileParser& parser, unsigned kind, Klass
if (UseBiasedLocking && BiasedLocking::enabled()) {
set_prototype_header(markWord::biased_locking_prototype());
}
if (has_inline_fields()) {
_value_field_klasses = (const Klass**) adr_value_fields_klasses();
if (has_inline_type_fields()) {
_inline_type_field_klasses = (const Klass**) adr_inline_type_field_klasses();
}
}

Expand Down Expand Up @@ -1260,9 +1260,8 @@ void InstanceKlass::initialize_impl(TRAPS) {
{
for (AllFieldStream fs(this); !fs.done(); fs.next()) {
if (Signature::basic_type(fs.signature()) == T_VALUETYPE) {
Klass* klass = this->get_value_field_klass_or_null(fs.index());
if (klass == NULL) {
assert(fs.access_flags().is_static(), "Otherwise should have been pre-loaded");
Klass* klass = get_inline_type_field_klass_or_null(fs.index());
if (fs.access_flags().is_static() && klass == NULL) {
klass = SystemDictionary::resolve_or_fail(field_signature(fs.index())->fundamental_name(THREAD),
Handle(THREAD, class_loader()),
Handle(THREAD, protection_domain()),
Expand All @@ -1273,7 +1272,7 @@ void InstanceKlass::initialize_impl(TRAPS) {
if (!klass->is_inline_klass()) {
THROW(vmSymbols::java_lang_IncompatibleClassChangeError());
}
this->set_value_field_klass(fs.index(), klass);
set_inline_type_field_klass(fs.index(), klass);
}
InstanceKlass::cast(klass)->initialize(CHECK);
if (fs.access_flags().is_static()) {
Expand Down Expand Up @@ -2623,6 +2622,12 @@ void InstanceKlass::metaspace_pointers_do(MetaspaceClosure* it) {
it->push(&_nest_members);
it->push(&_permitted_subclasses);
it->push(&_record_components);

if (has_inline_type_fields()) {
for (int i = 0; i < java_fields_count(); i++) {
it->push(&((Klass**)adr_inline_type_field_klasses())[i]);
}
}
}

void InstanceKlass::remove_unshareable_info() {
Expand Down Expand Up @@ -2658,6 +2663,14 @@ void InstanceKlass::remove_unshareable_info() {
array_klasses()->remove_unshareable_info();
}

if (has_inline_type_fields()) {
for (AllFieldStream fs(fields(), constants()); !fs.done(); fs.next()) {
if (Signature::basic_type(fs.signature()) == T_VALUETYPE) {
reset_inline_type_field_klass(fs.index());
}
}
}

// These are not allocated from metaspace. They are safe to set to NULL.
_source_debug_extension = NULL;
_dep_context = NULL;
Expand Down
Loading

0 comments on commit 3f8947b

Please sign in to comment.