8355490: Make VM_RedefineClasses::merge_constant_pools only take reference arguments

Reviewed-by: amenkov, sspitsyn, coleenp

Author: jdksjolen

src/hotspot/share/prims/jvmtiRedefineClasses.cpp

@@ -1572,28 +1572,20 @@ void VM_RedefineClasses::map_operand_index(int old_index, int new_index) {
 
 
 // Merge old_cp and scratch_cp and return the results of the merge via
-// merge_cp_p. The number of entries in *merge_cp_p is returned via
+// merge_cp_p. The number of entries in merge_cp_p is returned via
 // merge_cp_length_p. The entries in old_cp occupy the same locations
-// in *merge_cp_p. Also creates a map of indices from entries in
-// scratch_cp to the corresponding entry in *merge_cp_p. Index map
+// in merge_cp_p. Also creates a map of indices from entries in
+// scratch_cp to the corresponding entry in merge_cp_p. Index map
 // entries are only created for entries in scratch_cp that occupy a
-// different location in *merged_cp_p.
+// different location in merged_cp_p.
 bool VM_RedefineClasses::merge_constant_pools(const constantPoolHandle& old_cp,
-       const constantPoolHandle& scratch_cp, constantPoolHandle *merge_cp_p,
-       int *merge_cp_length_p, TRAPS) {
+       const constantPoolHandle& scratch_cp, constantPoolHandle& merge_cp_p,
+       int& merge_cp_length_p, TRAPS) {
 
-  if (merge_cp_p == nullptr) {
-    assert(false, "caller must provide scratch constantPool");
-    return false; // robustness
-  }
-  if (merge_cp_length_p == nullptr) {
-    assert(false, "caller must provide scratch CP length");
-    return false; // robustness
-  }
   // Worst case we need old_cp->length() + scratch_cp()->length(),
   // but the caller might be smart so make sure we have at least
   // the minimum.
-  if ((*merge_cp_p)->length() < old_cp->length()) {
+  if (merge_cp_p->length() < old_cp->length()) {
     assert(false, "merge area too small");
     return false; // robustness
   }
@@ -1621,36 +1613,36 @@ bool VM_RedefineClasses::merge_constant_pools(const constantPoolHandle& old_cp,
         // revert the copy to JVM_CONSTANT_UnresolvedClass
         // May be resolving while calling this so do the same for
         // JVM_CONSTANT_UnresolvedClass (klass_name_at() deals with transition)
-        (*merge_cp_p)->temp_unresolved_klass_at_put(old_i,
+        merge_cp_p->temp_unresolved_klass_at_put(old_i,
           old_cp->klass_name_index_at(old_i));
         break;
 
       case JVM_CONSTANT_Double:
       case JVM_CONSTANT_Long:
-        // just copy the entry to *merge_cp_p, but double and long take
+        // just copy the entry to merge_cp_p, but double and long take
         // two constant pool entries
-        ConstantPool::copy_entry_to(old_cp, old_i, *merge_cp_p, old_i);
+        ConstantPool::copy_entry_to(old_cp, old_i, merge_cp_p, old_i);
         old_i++;
         break;
 
       default:
-        // just copy the entry to *merge_cp_p
-        ConstantPool::copy_entry_to(old_cp, old_i, *merge_cp_p, old_i);
+        // just copy the entry to merge_cp_p
+        ConstantPool::copy_entry_to(old_cp, old_i, merge_cp_p, old_i);
         break;
       }
     } // end for each old_cp entry
 
-    ConstantPool::copy_operands(old_cp, *merge_cp_p, CHECK_false);
-    (*merge_cp_p)->extend_operands(scratch_cp, CHECK_false);
+    ConstantPool::copy_operands(old_cp, merge_cp_p, CHECK_false);
+    merge_cp_p->extend_operands(scratch_cp, CHECK_false);
 
     // We don't need to sanity check that *merge_cp_length_p is within
     // *merge_cp_p bounds since we have the minimum on-entry check above.
-    (*merge_cp_length_p) = old_i;
+    merge_cp_length_p = old_i;
   }
 
   // merge_cp_len should be the same as old_cp->length() at this point
   // so this trace message is really a "warm-and-breathing" message.
-  log_debug(redefine, class, constantpool)("after pass 0: merge_cp_len=%d", *merge_cp_length_p);
+  log_debug(redefine, class, constantpool)("after pass 0: merge_cp_len=%d", merge_cp_length_p);
 
   int scratch_i;  // index into scratch_cp
   {
@@ -1674,13 +1666,13 @@ bool VM_RedefineClasses::merge_constant_pools(const constantPoolHandle& old_cp,
         break;
       }
 
-      bool match = scratch_cp->compare_entry_to(scratch_i, *merge_cp_p, scratch_i);
+      bool match = scratch_cp->compare_entry_to(scratch_i, merge_cp_p, scratch_i);
       if (match) {
         // found a match at the same index so nothing more to do
         continue;
       }
 
-      int found_i = scratch_cp->find_matching_entry(scratch_i, *merge_cp_p);
+      int found_i = scratch_cp->find_matching_entry(scratch_i, merge_cp_p);
       if (found_i != 0) {
         guarantee(found_i != scratch_i,
           "compare_entry_to() and find_matching_entry() do not agree");
@@ -1692,14 +1684,14 @@ bool VM_RedefineClasses::merge_constant_pools(const constantPoolHandle& old_cp,
       }
 
       // No match found so we have to append this entry and any unique
-      // referenced entries to *merge_cp_p.
-      append_entry(scratch_cp, scratch_i, merge_cp_p, merge_cp_length_p);
+      // referenced entries to merge_cp_p.
+      append_entry(scratch_cp, scratch_i, &merge_cp_p, &merge_cp_length_p);
     }
   }
 
   log_debug(redefine, class, constantpool)
     ("after pass 1a: merge_cp_len=%d, scratch_i=%d, index_map_len=%d",
-     *merge_cp_length_p, scratch_i, _index_map_count);
+     merge_cp_length_p, scratch_i, _index_map_count);
 
   if (scratch_i < scratch_cp->length()) {
     // Pass 1b:
@@ -1721,24 +1713,24 @@ bool VM_RedefineClasses::merge_constant_pools(const constantPoolHandle& old_cp,
       }
 
       int found_i =
-        scratch_cp->find_matching_entry(scratch_i, *merge_cp_p);
+        scratch_cp->find_matching_entry(scratch_i, merge_cp_p);
       if (found_i != 0) {
-        // Found a matching entry somewhere else in *merge_cp_p so
+        // Found a matching entry somewhere else in merge_cp_p so
         // just need a mapping entry.
         map_index(scratch_cp, scratch_i, found_i);
         continue;
       }
 
       // No match found so we have to append this entry and any unique
-      // referenced entries to *merge_cp_p.
-      append_entry(scratch_cp, scratch_i, merge_cp_p, merge_cp_length_p);
+      // referenced entries to merge_cp_p.
+      append_entry(scratch_cp, scratch_i, &merge_cp_p, &merge_cp_length_p);
     }
 
     log_debug(redefine, class, constantpool)
       ("after pass 1b: merge_cp_len=%d, scratch_i=%d, index_map_len=%d",
-       *merge_cp_length_p, scratch_i, _index_map_count);
+       merge_cp_length_p, scratch_i, _index_map_count);
   }
-  finalize_operands_merge(*merge_cp_p, CHECK_false);
+  finalize_operands_merge(merge_cp_p, CHECK_false);
 
   return true;
 } // end merge_constant_pools()
@@ -1815,8 +1807,8 @@ jvmtiError VM_RedefineClasses::merge_cp_and_rewrite(
 
   // reference to the cp holder is needed for copy_operands()
   merge_cp->set_pool_holder(scratch_class);
-  bool result = merge_constant_pools(old_cp, scratch_cp, &merge_cp,
-                  &merge_cp_length, THREAD);
+  bool result = merge_constant_pools(old_cp, scratch_cp, merge_cp,
+                  merge_cp_length, THREAD);
   merge_cp->set_pool_holder(nullptr);
 
   if (!result) {

src/hotspot/share/prims/jvmtiRedefineClasses.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 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
@@ -441,8 +441,8 @@ class VM_RedefineClasses: public VM_Operation {
   void map_index(const constantPoolHandle& scratch_cp, int old_index, int new_index);
   void map_operand_index(int old_bootstrap_spec_index, int new_bootstrap_spec_index);
   bool merge_constant_pools(const constantPoolHandle& old_cp,
-    const constantPoolHandle& scratch_cp, constantPoolHandle *merge_cp_p,
-    int *merge_cp_length_p, TRAPS);
+    const constantPoolHandle& scratch_cp, constantPoolHandle& merge_cp_p,
+    int& merge_cp_length_p, TRAPS);
   jvmtiError merge_cp_and_rewrite(InstanceKlass* the_class,
     InstanceKlass* scratch_class, TRAPS);
   u2 rewrite_cp_ref_in_annotation_data(