8258048: Placeholder hash code is the same as Dictionary hash code

Reviewed-by: dholmes, hseigel, lfoltan

src/hotspot/share/classfile/dictionary.cpp

@@ -296,9 +296,11 @@ InstanceKlass* Dictionary::find(unsigned int hash, Symbol* name,
   }
 }
 
-InstanceKlass* Dictionary::find_class(int index, unsigned int hash,
+InstanceKlass* Dictionary::find_class(unsigned int hash,
                                       Symbol* name) {
   assert_locked_or_safepoint(SystemDictionary_lock);
+
+  int index = hash_to_index(hash);
   assert (index == index_for(name), "incorrect index?");
 
   DictionaryEntry* entry = get_entry(index, hash, name);

src/hotspot/share/classfile/dictionary.hpp

@@ -60,7 +60,7 @@ class Dictionary : public Hashtable<InstanceKlass*, mtClass> {
 
   void add_klass(unsigned int hash, Symbol* class_name, InstanceKlass* obj);
 
-  InstanceKlass* find_class(int index, unsigned int hash, Symbol* name);
+  InstanceKlass* find_class(unsigned int hash, Symbol* name);
 
   void classes_do(void f(InstanceKlass*));
   void classes_do(void f(InstanceKlass*, TRAPS), TRAPS);

src/hotspot/share/classfile/loaderConstraints.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2020, 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
@@ -440,20 +440,16 @@ void LoaderConstraintTable::verify(PlaceholderTable* placeholders) {
         Symbol* name = ik->name();
         ClassLoaderData* loader_data = ik->class_loader_data();
         Dictionary* dictionary = loader_data->dictionary();
-        unsigned int d_hash = dictionary->compute_hash(name);
-        int d_index = dictionary->hash_to_index(d_hash);
-        InstanceKlass* k = dictionary->find_class(d_index, d_hash, name);
+        unsigned int name_hash = dictionary->compute_hash(name);
+        InstanceKlass* k = dictionary->find_class(name_hash, name);
         if (k != NULL) {
           // We found the class in the dictionary, so we should
           // make sure that the Klass* matches what we already have.
           guarantee(k == probe->klass(), "klass should be in dictionary");
         } else {
           // If we don't find the class in the dictionary, it
           // has to be in the placeholders table.
-          unsigned int p_hash = placeholders->compute_hash(name);
-          int p_index = placeholders->hash_to_index(p_hash);
-          PlaceholderEntry* entry = placeholders->get_entry(p_index, p_hash,
-                                                            name, loader_data);
+          PlaceholderEntry* entry = placeholders->get_entry(name_hash, name, loader_data);
 
           // The InstanceKlass might not be on the entry, so the only
           // thing we can check here is whether we were successful in

src/hotspot/share/classfile/placeholders.cpp

@@ -25,8 +25,7 @@
 #include "precompiled.hpp"
 #include "classfile/classLoaderData.inline.hpp"
 #include "classfile/placeholders.hpp"
-#include "classfile/systemDictionary.hpp"
-#include "oops/oop.inline.hpp"
+#include "runtime/mutexLocker.hpp"
 #include "utilities/hashtable.inline.hpp"
 
 // Placeholder methods
@@ -61,26 +60,29 @@ void PlaceholderTable::free_entry(PlaceholderEntry* entry) {
 // All threads examining the placeholder table must hold the
 // SystemDictionary_lock, so we don't need special precautions
 // on store ordering here.
-void PlaceholderTable::add_entry(int index, unsigned int hash,
-                                 Symbol* class_name, ClassLoaderData* loader_data,
-                                 bool havesupername, Symbol* supername){
+PlaceholderEntry* PlaceholderTable::add_entry(unsigned int hash,
+                                              Symbol* class_name, ClassLoaderData* loader_data,
+                                              bool havesupername, Symbol* supername){
   assert_locked_or_safepoint(SystemDictionary_lock);
   assert(class_name != NULL, "adding NULL obj");
 
   // Both readers and writers are locked so it's safe to just
   // create the placeholder and insert it in the list without a membar.
   PlaceholderEntry* entry = new_entry(hash, class_name, loader_data, havesupername, supername);
-  add_entry(index, entry);
+  int index = hash_to_index(hash);
+  Hashtable<Symbol*, mtClass>::add_entry(index, entry);
+  return entry;
 }
 
 
 // Remove a placeholder object.
-void PlaceholderTable::remove_entry(int index, unsigned int hash,
+void PlaceholderTable::remove_entry(unsigned int hash,
                                     Symbol* class_name,
                                     ClassLoaderData* loader_data) {
   assert_locked_or_safepoint(SystemDictionary_lock);
+  int index = hash_to_index(hash);
   PlaceholderEntry** p = bucket_addr(index);
-  while (*p) {
+  while (*p != NULL) {
     PlaceholderEntry *probe = *p;
     if (probe->hash() == hash && probe->equals(class_name, loader_data)) {
       // Delete entry
@@ -92,11 +94,12 @@ void PlaceholderTable::remove_entry(int index, unsigned int hash,
   }
 }
 
-PlaceholderEntry* PlaceholderTable::get_entry(int index, unsigned int hash,
-                                       Symbol* class_name,
-                                       ClassLoaderData* loader_data) {
+PlaceholderEntry* PlaceholderTable::get_entry(unsigned int hash,
+                                              Symbol* class_name,
+                                              ClassLoaderData* loader_data) {
   assert_locked_or_safepoint(SystemDictionary_lock);
 
+  int index = hash_to_index(hash);
   for (PlaceholderEntry *place_probe = bucket(index);
                          place_probe != NULL;
                          place_probe = place_probe->next()) {
@@ -108,36 +111,35 @@ PlaceholderEntry* PlaceholderTable::get_entry(int index, unsigned int hash,
   return NULL;
 }
 
-Symbol* PlaceholderTable::find_entry(int index, unsigned int hash,
-                                       Symbol* class_name,
-                                       ClassLoaderData* loader_data) {
-  PlaceholderEntry* probe = get_entry(index, hash, class_name, loader_data);
-  return (probe? probe->klassname(): (Symbol*)NULL);
+Symbol* PlaceholderTable::find_entry(unsigned int hash,
+                                     Symbol* class_name,
+                                     ClassLoaderData* loader_data) {
+  PlaceholderEntry* probe = get_entry(hash, class_name, loader_data);
+  return (probe != NULL ? probe->klassname() : NULL);
 }
 
   // find_and_add returns probe pointer - old or new
   // If no entry exists, add a placeholder entry
   // If entry exists, reuse entry
   // For both, push SeenThread for classloadAction
   // if havesupername: this is used for circularity for instanceklass loading
-PlaceholderEntry* PlaceholderTable::find_and_add(int index, unsigned int hash,
+PlaceholderEntry* PlaceholderTable::find_and_add(unsigned int hash,
                                                  Symbol* name,
                                                  ClassLoaderData* loader_data,
                                                  classloadAction action,
                                                  Symbol* supername,
                                                  Thread* thread) {
-  PlaceholderEntry* probe = get_entry(index, hash, name, loader_data);
+  PlaceholderEntry* probe = get_entry(hash, name, loader_data);
   if (probe == NULL) {
     // Nothing found, add place holder
-    add_entry(index, hash, name, loader_data, (action == LOAD_SUPER), supername);
-    probe = get_entry(index, hash, name, loader_data);
+    probe = add_entry(hash, name, loader_data, (action == LOAD_SUPER), supername);
   } else {
     if (action == LOAD_SUPER) {
       probe->set_havesupername(true);
       probe->set_supername(supername);
     }
   }
-  if (probe) probe->add_seen_thread(thread, action);
+  probe->add_seen_thread(thread, action);
   return probe;
 }
 
@@ -155,18 +157,18 @@ PlaceholderEntry* PlaceholderTable::find_and_add(int index, unsigned int hash,
 // Note: you can be in both placeholders and systemDictionary
 // Therefore - must always check SD first
 // Ignores the case where entry is not found
-void PlaceholderTable::find_and_remove(int index, unsigned int hash,
+void PlaceholderTable::find_and_remove(unsigned int hash,
                                        Symbol* name, ClassLoaderData* loader_data,
                                        classloadAction action,
                                        Thread* thread) {
     assert_locked_or_safepoint(SystemDictionary_lock);
-    PlaceholderEntry *probe = get_entry(index, hash, name, loader_data);
+    PlaceholderEntry *probe = get_entry(hash, name, loader_data);
     if (probe != NULL) {
        probe->remove_seen_thread(thread, action);
        // If no other threads using this entry, and this thread is not using this entry for other states
        if ((probe->superThreadQ() == NULL) && (probe->loadInstanceThreadQ() == NULL)
           && (probe->defineThreadQ() == NULL) && (probe->definer() == NULL)) {
-         remove_entry(index, hash, name, loader_data);
+         remove_entry(hash, name, loader_data);
        }
     }
   }

src/hotspot/share/classfile/placeholders.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2020, 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
@@ -50,19 +50,16 @@ class PlaceholderTable : public Hashtable<Symbol*, mtClass> {
     return (PlaceholderEntry**)Hashtable<Symbol*, mtClass>::bucket_addr(i);
   }
 
-  void add_entry(int index, PlaceholderEntry* new_entry) {
-    Hashtable<Symbol*, mtClass>::add_entry(index, (HashtableEntry<Symbol*, mtClass>*)new_entry);
-  }
-
-  void add_entry(int index, unsigned int hash, Symbol* name,
-                ClassLoaderData* loader_data, bool havesupername, Symbol* supername);
+  PlaceholderEntry* add_entry(unsigned int hash, Symbol* name,
+                              ClassLoaderData* loader_data,
+                              bool havesupername, Symbol* supername);
 
   // This returns a Symbol* to match type for SystemDictionary
-  Symbol* find_entry(int index, unsigned int hash,
-                       Symbol* name, ClassLoaderData* loader_data);
+  Symbol* find_entry(unsigned int hash,
+                     Symbol* name, ClassLoaderData* loader_data);
 
-  PlaceholderEntry* get_entry(int index, unsigned int hash,
-                       Symbol* name, ClassLoaderData* loader_data);
+  PlaceholderEntry* get_entry(unsigned int hash,
+                              Symbol* name, ClassLoaderData* loader_data);
 
 // caller to create a placeholder entry must enumerate an action
 // caller claims ownership of that action
@@ -83,17 +80,17 @@ class PlaceholderTable : public Hashtable<Symbol*, mtClass> {
   // find_and_add returns probe pointer - old or new
   // If no entry exists, add a placeholder entry and push SeenThread for classloadAction
   // If entry exists, reuse entry and push SeenThread for classloadAction
-  PlaceholderEntry* find_and_add(int index, unsigned int hash,
+  PlaceholderEntry* find_and_add(unsigned int hash,
                                  Symbol* name, ClassLoaderData* loader_data,
                                  classloadAction action, Symbol* supername,
                                  Thread* thread);
 
-  void remove_entry(int index, unsigned int hash,
+  void remove_entry(unsigned int hash,
                     Symbol* name, ClassLoaderData* loader_data);
 
   // find_and_remove first removes SeenThread for classloadAction
   // If all queues are empty and definer is null, remove the PlacheholderEntry completely
-  void find_and_remove(int index, unsigned int hash,
+  void find_and_remove(unsigned int hash,
                        Symbol* name, ClassLoaderData* loader_data,
                        classloadAction action, Thread* thread);