Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/hotspot/share/cds/aotConstantPoolResolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ bool AOTConstantPoolResolver::check_lambda_metafactory_signature(ConstantPool* c
}

bool AOTConstantPoolResolver::check_lambda_metafactory_methodtype_arg(ConstantPool* cp, int bsms_attribute_index, int arg_i) {
int mt_index = cp->operand_argument_index_at(bsms_attribute_index, arg_i);
int mt_index = cp->bsm_attribute_entry(bsms_attribute_index)->argument_index(arg_i);
if (!cp->tag_at(mt_index).is_method_type()) {
// malformed class?
return false;
Expand All @@ -408,7 +408,7 @@ bool AOTConstantPoolResolver::check_lambda_metafactory_methodtype_arg(ConstantPo
}

bool AOTConstantPoolResolver::check_lambda_metafactory_methodhandle_arg(ConstantPool* cp, int bsms_attribute_index, int arg_i) {
int mh_index = cp->operand_argument_index_at(bsms_attribute_index, arg_i);
int mh_index = cp->bsm_attribute_entry(bsms_attribute_index)->argument_index(arg_i);
if (!cp->tag_at(mh_index).is_method_handle()) {
// malformed class?
return false;
Expand Down Expand Up @@ -514,7 +514,7 @@ bool AOTConstantPoolResolver::is_indy_resolution_deterministic(ConstantPool* cp,
}

int bsms_attribute_index = cp->bootstrap_methods_attribute_index(cp_index);
int arg_count = cp->operand_argument_count_at(bsms_attribute_index);
int arg_count = cp->bsm_attribute_entry(bsms_attribute_index)->argument_count();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your fix made it possible to do a bit more simplifications.
For instance, each bsms_attribute_index parameter can be replaced with a bsme parameter.
But this does not look that important at the moment.

if (arg_count != 3) {
// Malformed class?
return false;
Expand Down
14 changes: 8 additions & 6 deletions src/hotspot/share/oops/constantPool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1938,18 +1938,20 @@ int ConstantPool::find_matching_entry(int pattern_i,
// Compare this constant pool's bootstrap specifier at idx1 to the constant pool
// cp2's bootstrap specifier at idx2.
bool ConstantPool::compare_operand_to(int idx1, const constantPoolHandle& cp2, int idx2) {
int k1 = operand_bootstrap_method_ref_index_at(idx1);
int k2 = cp2->operand_bootstrap_method_ref_index_at(idx2);
BSMAttributeEntry* e1 = bsm_attribute_entry(idx1);
BSMAttributeEntry* e2 = cp2->bsm_attribute_entry(idx2);
int k1 = e1->bootstrap_method_index();
int k2 = e2->bootstrap_method_index();
bool match = compare_entry_to(k1, cp2, k2);

if (!match) {
return false;
}
int argc = operand_argument_count_at(idx1);
if (argc == cp2->operand_argument_count_at(idx2)) {
int argc = e1->argument_count();
if (argc == e2->argument_count()) {
for (int j = 0; j < argc; j++) {
k1 = operand_argument_index_at(idx1, j);
k2 = cp2->operand_argument_index_at(idx2, j);
k1 = e1->argument_index(j);
k2 = e2->argument_index(j);
match = compare_entry_to(k1, cp2, k2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'd suggest to define two locals to simplify the code as below:

  BSMAttributeEntry* e1 = bsm_attribute_entry(idx1);
  BSMAttributeEntry& e2 = cp2->bsm_attribute_entry(idx12);
 
  int k1 = e1->bootstrap_method_index();
  int k2 = e2->bootstrap_method_index();
  bool match = compare_entry_to(k1, cp2, k2);

  if (!match) {
    return false;
  }
  int argc = e1->argument_count();
  if (argc == e2->argument_count()) {
    for (int j = 0; j < argc; j++) {
      k1 = e1->argument_index(j);
      k2 = e2->argument_index(j);
      match = compare_entry_to(k1, cp2, k2);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a good simplification.

if (!match) {
return false;
Expand Down
98 changes: 53 additions & 45 deletions src/hotspot/share/oops/constantPool.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,43 @@ class CPKlassSlot {
}
};

class BSMAttributeEntry {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider moving the description comment from lines #569-#572 ahead of this structure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This?

  // The first part of the operands array consists of an index into the second part.
  // Extract a 32-bit index value from the first part.

Wouldn't it be better to leave it where it is, but add a comment ahead of the structure?

friend class ConstantPool;
u2 _bootstrap_method_index;
u2 _argument_count;

// The argument indexes are stored right after the object, in a contiguous array.
// [ bsmi_0 argc_0 arg_00 arg_01 ... arg_0N bsmi_1 argc_1 arg_10 ... arg_1N ... ]
// So in order to find the argument array, jump over ourselves.
const u2* argument_indexes() const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Lois that some comments here would be useful. I think the original comment can stay where it is and you can add some extra details here.

return reinterpret_cast<const u2*>(this + 1);
}
u2* argument_indexes() {
return reinterpret_cast<u2*>(this + 1);
}
// These are overlays on top of the operands array. Do not construct.
BSMAttributeEntry() = delete;

public:
// Offsets for SA
enum {
_bsmi_offset = 0,
_argc_offset = 1,
_argv_offset = 2
};

int bootstrap_method_index() const {
return _bootstrap_method_index;
}
int argument_count() const {
return _argument_count;
}
int argument_index(int n) const {
assert(checked_cast<u2>(n) < _argument_count, "oob");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the assert contain a check that _argument_count is >= 0 as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, it's a u2 so that's just part of its type.

return argument_indexes()[n];
}
};

class ConstantPool : public Metadata {
friend class VMStructs;
friend class JVMCIVMStructs;
Expand Down Expand Up @@ -519,10 +556,6 @@ class ConstantPool : public Metadata {
assert(tag_at(cp_index).has_bootstrap(), "Corrupted constant pool");
return extract_low_short_from_int(*int_at_addr(cp_index));
}
int bootstrap_operand_base(int cp_index) {
int bsms_attribute_index = bootstrap_methods_attribute_index(cp_index);
return operand_offset_at(operands(), bsms_attribute_index);
}
// The first part of the operands array consists of an index into the second part.
// Extract a 32-bit index value from the first part.
static int operand_offset_at(Array<u2>* operands, int bsms_attribute_index) {
Expand Down Expand Up @@ -560,47 +593,26 @@ class ConstantPool : public Metadata {
else
return operand_offset_at(operands, nextidx);
}
int bootstrap_operand_limit(int cp_index) {
int bsms_attribute_index = bootstrap_methods_attribute_index(cp_index);
return operand_limit_at(operands(), bsms_attribute_index);
}
#endif //ASSERT

// Layout of InvokeDynamic and Dynamic bootstrap method specifier
// data in second part of operands array. This encodes one record in
// the BootstrapMethods attribute. The whole specifier also includes
// the name and type information from the main constant pool entry.
enum {
_indy_bsm_offset = 0, // CONSTANT_MethodHandle bsm
_indy_argc_offset = 1, // u2 argc
_indy_argv_offset = 2 // u2 argv[argc]
};

Comment on lines -573 to -578
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plummercj , this is where the enum constants come from.

// These functions are used in RedefineClasses for CP merge

int operand_offset_at(int bsms_attribute_index) {
assert(0 <= bsms_attribute_index &&
bsms_attribute_index < operand_array_length(operands()),
"Corrupted CP operands");
return operand_offset_at(operands(), bsms_attribute_index);
}
u2 operand_bootstrap_method_ref_index_at(int bsms_attribute_index) {
int offset = operand_offset_at(bsms_attribute_index);
return operands()->at(offset + _indy_bsm_offset);
}
u2 operand_argument_count_at(int bsms_attribute_index) {
int offset = operand_offset_at(bsms_attribute_index);
u2 argc = operands()->at(offset + _indy_argc_offset);
return argc;
}
u2 operand_argument_index_at(int bsms_attribute_index, int j) {

BSMAttributeEntry* bsm_attribute_entry(int bsms_attribute_index) {
int offset = operand_offset_at(bsms_attribute_index);
return operands()->at(offset + _indy_argv_offset + j);
return reinterpret_cast<BSMAttributeEntry*>(operands()->adr_at(offset));
}

int operand_next_offset_at(int bsms_attribute_index) {
int offset = operand_offset_at(bsms_attribute_index) + _indy_argv_offset
+ operand_argument_count_at(bsms_attribute_index);
return offset;
BSMAttributeEntry* bsme = bsm_attribute_entry(bsms_attribute_index);
u2* argv_start = bsme->argument_indexes();
int offset = argv_start - operands()->data();
return offset + bsme->argument_count();
}
// Compare a bootstrap specifier data in the operands arrays
bool compare_operand_to(int bsms_attribute_index1, const constantPoolHandle& cp2,
Expand All @@ -617,23 +629,19 @@ class ConstantPool : public Metadata {

u2 bootstrap_method_ref_index_at(int cp_index) {
assert(tag_at(cp_index).has_bootstrap(), "Corrupted constant pool");
int op_base = bootstrap_operand_base(cp_index);
return operands()->at(op_base + _indy_bsm_offset);
int bsmai = bootstrap_methods_attribute_index(cp_index);
return bsm_attribute_entry(bsmai)->bootstrap_method_index();
}
u2 bootstrap_argument_count_at(int cp_index) {
assert(tag_at(cp_index).has_bootstrap(), "Corrupted constant pool");
int op_base = bootstrap_operand_base(cp_index);
u2 argc = operands()->at(op_base + _indy_argc_offset);
DEBUG_ONLY(int end_offset = op_base + _indy_argv_offset + argc;
int next_offset = bootstrap_operand_limit(cp_index));
assert(end_offset == next_offset, "matched ending");
return argc;
int bsmai = bootstrap_methods_attribute_index(cp_index);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "matched ending" assert was the only code that used bootstrap_operand_limit(), so that method could be removed as well. This comment applies to bootstrap_operand_base() as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

return bsm_attribute_entry(bsmai)->argument_count();
}
u2 bootstrap_argument_index_at(int cp_index, int j) {
int op_base = bootstrap_operand_base(cp_index);
DEBUG_ONLY(int argc = operands()->at(op_base + _indy_argc_offset));
assert((uint)j < (uint)argc, "oob");
return operands()->at(op_base + _indy_argv_offset + j);
int bsmai = bootstrap_methods_attribute_index(cp_index);
BSMAttributeEntry* bsme = bsm_attribute_entry(bsmai);
assert((uint)j < (uint)bsme->argument_count(), "oob");
return bsm_attribute_entry(bsmai)->argument_index(j);
}

// The following methods (name/signature/klass_ref_at, klass_ref_at_noresolve,
Expand Down
10 changes: 5 additions & 5 deletions src/hotspot/share/prims/jvmtiClassFileReconstituter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ void JvmtiClassFileReconstituter::write_bootstrapmethod_attribute() {
// calculate length of attribute
u4 length = sizeof(u2); // num_bootstrap_methods
for (int n = 0; n < num_bootstrap_methods; n++) {
u2 num_bootstrap_arguments = cpool()->operand_argument_count_at(n);
u2 num_bootstrap_arguments = cpool()->bsm_attribute_entry(n)->argument_count();
length += sizeof(u2); // bootstrap_method_ref
length += sizeof(u2); // num_bootstrap_arguments
length += (u4)sizeof(u2) * num_bootstrap_arguments; // bootstrap_arguments[num_bootstrap_arguments]
Expand All @@ -406,12 +406,12 @@ void JvmtiClassFileReconstituter::write_bootstrapmethod_attribute() {
// write attribute
write_u2(checked_cast<u2>(num_bootstrap_methods));
for (int n = 0; n < num_bootstrap_methods; n++) {
u2 bootstrap_method_ref = cpool()->operand_bootstrap_method_ref_index_at(n);
u2 num_bootstrap_arguments = cpool()->operand_argument_count_at(n);
write_u2(bootstrap_method_ref);
BSMAttributeEntry* bsme = cpool()->bsm_attribute_entry(n);
u2 num_bootstrap_arguments = bsme->argument_count();
write_u2(bsme->bootstrap_method_index());
write_u2(num_bootstrap_arguments);
for (int arg = 0; arg < num_bootstrap_arguments; arg++) {
u2 bootstrap_argument = cpool()->operand_argument_index_at(n, arg);
u2 bootstrap_argument = bsme->argument_index(arg);
write_u2(bootstrap_argument);
}
}
Expand Down
9 changes: 5 additions & 4 deletions src/hotspot/share/prims/jvmtiRedefineClasses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -659,10 +659,11 @@ u2 VM_RedefineClasses::find_or_append_indirect_entry(const constantPoolHandle& s
// Append a bootstrap specifier into the merge_cp operands that is semantically equal
// to the scratch_cp operands bootstrap specifier passed by the old_bs_i index.
// Recursively append new merge_cp entries referenced by the new bootstrap specifier.
void VM_RedefineClasses::append_operand(const constantPoolHandle& scratch_cp, int old_bs_i,
void VM_RedefineClasses::append_operand(const constantPoolHandle& scratch_cp, const int old_bs_i,
constantPoolHandle *merge_cp_p, int *merge_cp_length_p) {

u2 old_ref_i = scratch_cp->operand_bootstrap_method_ref_index_at(old_bs_i);
BSMAttributeEntry* old_bsme = scratch_cp->bsm_attribute_entry(old_bs_i);
u2 old_ref_i = old_bsme->bootstrap_method_index();
u2 new_ref_i = find_or_append_indirect_entry(scratch_cp, old_ref_i, merge_cp_p,
merge_cp_length_p);
if (new_ref_i != old_ref_i) {
Expand All @@ -676,14 +677,14 @@ void VM_RedefineClasses::append_operand(const constantPoolHandle& scratch_cp, in
// However, the operand_offset_at(0) was set in the extend_operands() call.
int new_base = (new_bs_i == 0) ? (*merge_cp_p)->operand_offset_at(0)
: (*merge_cp_p)->operand_next_offset_at(new_bs_i - 1);
u2 argc = scratch_cp->operand_argument_count_at(old_bs_i);
u2 argc = old_bsme->argument_count();

ConstantPool::operand_offset_at_put(merge_ops, _operands_cur_length, new_base);
merge_ops->at_put(new_base++, new_ref_i);
merge_ops->at_put(new_base++, argc);

for (int i = 0; i < argc; i++) {
u2 old_arg_ref_i = scratch_cp->operand_argument_index_at(old_bs_i, i);
u2 old_arg_ref_i = old_bsme->argument_index(i);
u2 new_arg_ref_i = find_or_append_indirect_entry(scratch_cp, old_arg_ref_i, merge_cp_p,
merge_cp_length_p);
merge_ops->at_put(new_base++, new_arg_ref_i);
Expand Down
12 changes: 6 additions & 6 deletions src/hotspot/share/runtime/vmStructs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1482,13 +1482,13 @@
\
declare_constant(Symbol::max_symbol_length) \
\
/***********************************************/ \
/* ConstantPool* layout enum for InvokeDynamic */ \
/***********************************************/ \
/******************************************************/ \
/* BSMAttributeEntry* - layout enum for InvokeDynamic */ \
/******************************************************/ \
\
declare_constant(ConstantPool::_indy_bsm_offset) \
declare_constant(ConstantPool::_indy_argc_offset) \
declare_constant(ConstantPool::_indy_argv_offset) \
declare_constant(BSMAttributeEntry::_bsmi_offset) \
declare_constant(BSMAttributeEntry::_argc_offset) \
declare_constant(BSMAttributeEntry::_argv_offset) \
\
/***************************************/ \
/* JavaThreadStatus enum */ \
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2025, 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
Expand Down Expand Up @@ -100,9 +100,9 @@ private static synchronized void initialize(TypeDataBase db) throws WrongTypeExc
headerSize = type.getSize();
elementSize = 0;
// fetch constants:
INDY_BSM_OFFSET = db.lookupIntConstant("ConstantPool::_indy_bsm_offset").intValue();
INDY_ARGC_OFFSET = db.lookupIntConstant("ConstantPool::_indy_argc_offset").intValue();
INDY_ARGV_OFFSET = db.lookupIntConstant("ConstantPool::_indy_argv_offset").intValue();
INDY_BSM_OFFSET = db.lookupIntConstant("BSMAttributeEntry::_bsmi_offset").intValue();
INDY_ARGC_OFFSET = db.lookupIntConstant("BSMAttributeEntry::_argc_offset").intValue();
INDY_ARGV_OFFSET = db.lookupIntConstant("BSMAttributeEntry::_argv_offset").intValue();
}

public ConstantPool(Address addr) {
Expand Down