From c5150c7b81e2b7b8c9e13c228d3b7bcb9dfe5024 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Wed, 10 Apr 2024 12:38:07 +0000 Subject: [PATCH] 8309751: Duplicate constant pool entries added during default method processing Co-authored-by: Ashutosh Mehra Reviewed-by: matsaave, dholmes --- .../share/classfile/bytecodeAssembler.cpp | 60 +++++++++++++++---- .../share/classfile/bytecodeAssembler.hpp | 28 +++++---- .../share/classfile/defaultMethods.cpp | 15 +++-- 3 files changed, 75 insertions(+), 28 deletions(-) diff --git a/src/hotspot/share/classfile/bytecodeAssembler.cpp b/src/hotspot/share/classfile/bytecodeAssembler.cpp index 0ddc6282806..e1e54b56e41 100644 --- a/src/hotspot/share/classfile/bytecodeAssembler.cpp +++ b/src/hotspot/share/classfile/bytecodeAssembler.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -33,31 +33,63 @@ #include "utilities/bytes.hpp" #include "utilities/checkedCast.hpp" +void BytecodeConstantPool::init() { + for (int i = 1; i < _orig->length(); i++) { + BytecodeCPEntry entry; + switch(_orig->tag_at(i).value()) { + case JVM_CONSTANT_Class: + case JVM_CONSTANT_UnresolvedClass: + entry = BytecodeCPEntry::klass(_orig->klass_slot_at(i).name_index()); + break; + case JVM_CONSTANT_Utf8: + entry = BytecodeCPEntry::utf8(_orig->symbol_at(i)); + break; + case JVM_CONSTANT_NameAndType: + entry = BytecodeCPEntry::name_and_type(_orig->name_ref_index_at(i), _orig->signature_ref_index_at(i)); + break; + case JVM_CONSTANT_Methodref: + entry = BytecodeCPEntry::methodref(_orig->uncached_klass_ref_index_at(i), _orig->uncached_name_and_type_ref_index_at(i)); + break; + case JVM_CONSTANT_String: + entry = BytecodeCPEntry::string(_orig->unresolved_string_at(i)); + break; + } + if (entry._tag != BytecodeCPEntry::tag::ERROR_TAG) { + bool created = false; + _index_map.put_if_absent(entry, i, &created); + if (created) { + _orig_cp_added += 1; + _added_entries.append(entry); + } + } + } +} + u2 BytecodeConstantPool::find_or_add(BytecodeCPEntry const& bcpe, TRAPS) { // Check for overflow - int new_size = _orig->length() + _entries.length(); + int new_size = _orig->length() + _added_entries.length() - _orig_cp_added; if (new_size > USHRT_MAX) { THROW_MSG_0(vmSymbols::java_lang_InternalError(), "default methods constant pool overflowed"); } - u2 index = checked_cast(_entries.length()); + u2 index = checked_cast(new_size); bool created = false; - u2* probe = _indices.put_if_absent(bcpe, index, &created); + u2* probe = _index_map.put_if_absent(bcpe, index, &created); if (created) { - _entries.append(bcpe); + _added_entries.append(bcpe); } else { index = *probe; } - return checked_cast(index + _orig->length()); + return index; } ConstantPool* BytecodeConstantPool::create_constant_pool(TRAPS) const { - if (_entries.length() == 0) { + if (_added_entries.length() == 0) { return _orig; } - int new_size = _orig->length() + _entries.length(); + int new_size = _orig->length() + _added_entries.length() - _orig_cp_added; ConstantPool* cp = ConstantPool::allocate( _orig->pool_holder()->class_loader_data(), new_size, CHECK_NULL); @@ -69,9 +101,13 @@ ConstantPool* BytecodeConstantPool::create_constant_pool(TRAPS) const { // Preserve dynamic constant information from the original pool cp->copy_fields(_orig); - for (int i = 0; i < _entries.length(); ++i) { - BytecodeCPEntry entry = _entries.at(i); - int idx = i + _orig->length(); + for (int i = _orig_cp_added; i < _added_entries.length(); ++i) { + // Add new entries that we added to the temporary constant pool, to the + // newly creatd constant pool. + BytecodeCPEntry entry = _added_entries.at(i); + // Get the constant pool index saved in the hashtable for this new entry. + u2* value = _index_map.get(entry); + int idx = *value; switch (entry._tag) { case BytecodeCPEntry::UTF8: entry._u.utf8->increment_refcount(); @@ -83,7 +119,7 @@ ConstantPool* BytecodeConstantPool::create_constant_pool(TRAPS) const { break; case BytecodeCPEntry::STRING: cp->unresolved_string_at_put( - idx, cp->symbol_at(entry._u.string)); + idx, entry._u.utf8); break; case BytecodeCPEntry::NAME_AND_TYPE: cp->name_and_type_at_put(idx, diff --git a/src/hotspot/share/classfile/bytecodeAssembler.hpp b/src/hotspot/share/classfile/bytecodeAssembler.hpp index 4505db28aef..fb51924a9c6 100644 --- a/src/hotspot/share/classfile/bytecodeAssembler.hpp +++ b/src/hotspot/share/classfile/bytecodeAssembler.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -66,7 +66,6 @@ class BytecodeCPEntry { union { Symbol* utf8; u2 klass; - u2 string; struct { u2 name_index; u2 type_index; @@ -93,9 +92,9 @@ class BytecodeCPEntry { return bcpe; } - static BytecodeCPEntry string(u2 index) { + static BytecodeCPEntry string(Symbol* symbol) { BytecodeCPEntry bcpe(STRING); - bcpe._u.string = index; + bcpe._u.utf8 = symbol; return bcpe; } @@ -114,6 +113,8 @@ class BytecodeCPEntry { } static bool equals(BytecodeCPEntry const& e0, BytecodeCPEntry const& e1) { + // The hash is the "union trick" value of the information saved for the tag, + // so can be compared for equality. return e0._tag == e1._tag && e0._u.hash == e1._u.hash; } @@ -122,23 +123,27 @@ class BytecodeCPEntry { } }; -class BytecodeConstantPool : ResourceObj { +class BytecodeConstantPool : public ResourceObj { private: typedef ResourceHashtable IndexHash; ConstantPool* _orig; - GrowableArray _entries; - IndexHash _indices; + GrowableArray _added_entries; + IndexHash _index_map; + int _orig_cp_added; u2 find_or_add(BytecodeCPEntry const& bcpe, TRAPS); + void init(); public: - BytecodeConstantPool(ConstantPool* orig) : _orig(orig) {} + BytecodeConstantPool(ConstantPool* orig) : _orig(orig), _orig_cp_added(0) { + init(); + } - BytecodeCPEntry const& at(u2 index) const { return _entries.at(index); } + BytecodeCPEntry const& at(u2 index) const { return _added_entries.at(index); } InstanceKlass* pool_holder() const { return _orig->pool_holder(); @@ -154,8 +159,9 @@ class BytecodeConstantPool : ResourceObj { } u2 string(Symbol* str, TRAPS) { - u2 utf8_entry = utf8(str, CHECK_0); - return find_or_add(BytecodeCPEntry::string(utf8_entry), THREAD); + // Create the utf8_entry in the hashtable but use Symbol for matching. + (void)utf8(str, CHECK_0); + return find_or_add(BytecodeCPEntry::string(str), THREAD); } u2 name_and_type(Symbol* name, Symbol* sig, TRAPS) { diff --git a/src/hotspot/share/classfile/defaultMethods.cpp b/src/hotspot/share/classfile/defaultMethods.cpp index 60c75a44007..dd221efb409 100644 --- a/src/hotspot/share/classfile/defaultMethods.cpp +++ b/src/hotspot/share/classfile/defaultMethods.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -939,9 +939,10 @@ static void create_defaults_and_exceptions(GrowableArray* slot GrowableArray overpasses; GrowableArray defaults; - BytecodeConstantPool bpool(klass->constants()); BytecodeBuffer* buffer = nullptr; // Lazily create a reusable buffer + BytecodeConstantPool* bpool = nullptr; + for (int i = 0; i < slots->length(); ++i) { EmptyVtableSlot* slot = slots->at(i); @@ -974,11 +975,15 @@ static void create_defaults_and_exceptions(GrowableArray* slot } else { buffer->clear(); } - int max_stack = BytecodeAssembler::assemble_method_error(&bpool, buffer, + // Lazily allocate bytecode constant pool also. + if (bpool == nullptr) { + bpool = new BytecodeConstantPool(klass->constants()); + } + int max_stack = BytecodeAssembler::assemble_method_error(bpool, buffer, method->get_exception_name(), method->get_exception_message(), CHECK); AccessFlags flags = accessFlags_from( JVM_ACC_PUBLIC | JVM_ACC_SYNTHETIC | JVM_ACC_BRIDGE); - Method* m = new_method(&bpool, buffer, slot->name(), slot->signature(), + Method* m = new_method(bpool, buffer, slot->name(), slot->signature(), flags, max_stack, slot->size_of_parameters(), ConstMethod::OVERPASS, CHECK); // We push to the methods list: @@ -995,7 +1000,7 @@ static void create_defaults_and_exceptions(GrowableArray* slot log_debug(defaultmethods)("Created %d default methods", defaults.length()); if (overpasses.length() > 0) { - switchover_constant_pool(&bpool, klass, &overpasses, CHECK); + switchover_constant_pool(bpool, klass, &overpasses, CHECK); merge_in_new_methods(klass, &overpasses, CHECK); } if (defaults.length() > 0) {