8297284: ResolutionErrorTable's key is wrong

Reviewed-by: matsaave, iklam

src/hotspot/share/classfile/resolutionErrors.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2022, 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,7 +33,30 @@
 #include "runtime/mutexLocker.hpp"
 #include "utilities/resourceHash.hpp"
 
-ResourceHashtable<uintptr_t, ResolutionErrorEntry*, 107, AnyObj::C_HEAP, mtClass> _resolution_error_table;
+class ResolutionErrorKey {
+  ConstantPool* _cpool;
+  int           _index;
+
+ public:
+  ResolutionErrorKey(ConstantPool* cpool, int index) : _cpool(cpool), _index(index) {
+    assert(_index > 0, "should be already encoded or otherwise greater than zero");
+  }
+
+  ConstantPool* cpool() const { return _cpool; }
+
+  static unsigned hash(const ResolutionErrorKey& key) {
+    Symbol* name = key._cpool->pool_holder()->name();
+    return (unsigned int)(name->identity_hash() ^ key._index);
+  }
+
+  static bool equals(const ResolutionErrorKey& l, const ResolutionErrorKey& r) {
+    return (l._cpool == r._cpool) && (l._index == r._index);
+  }
+};
+
+ResourceHashtable<ResolutionErrorKey, ResolutionErrorEntry*, 107, AnyObj::C_HEAP, mtClass,
+                  ResolutionErrorKey::hash,
+                  ResolutionErrorKey::equals> _resolution_error_table;
 
 // create new error entry
 void ResolutionErrorTable::add_entry(const constantPoolHandle& pool, int cp_index,
@@ -43,8 +66,9 @@ void ResolutionErrorTable::add_entry(const constantPoolHandle& pool, int cp_inde
   assert_locked_or_safepoint(SystemDictionary_lock);
   assert(!pool.is_null() && error != NULL, "adding NULL obj");
 
-  ResolutionErrorEntry* entry = new ResolutionErrorEntry(pool(), cp_index, error, message, cause, cause_msg);
-  _resolution_error_table.put(convert_key(pool, cp_index), entry);
+  ResolutionErrorKey key(pool(), cp_index);
+  ResolutionErrorEntry *entry = new ResolutionErrorEntry(error, message, cause, cause_msg);
+  _resolution_error_table.put(key, entry);
 }
 
 // create new nest host error entry
@@ -54,78 +78,54 @@ void ResolutionErrorTable::add_entry(const constantPoolHandle& pool, int cp_inde
   assert_locked_or_safepoint(SystemDictionary_lock);
   assert(!pool.is_null() && message != NULL, "adding NULL obj");
 
-  ResolutionErrorEntry* entry = new ResolutionErrorEntry(pool(), cp_index, message);
-  _resolution_error_table.put(convert_key(pool, cp_index), entry);
+  ResolutionErrorKey key(pool(), cp_index);
+  ResolutionErrorEntry *entry = new ResolutionErrorEntry(message);
+  _resolution_error_table.put(key, entry);
 }
 
 // find entry in the table
 ResolutionErrorEntry* ResolutionErrorTable::find_entry(const constantPoolHandle& pool, int cp_index) {
   assert_locked_or_safepoint(SystemDictionary_lock);
-  ResolutionErrorEntry** entry = _resolution_error_table.get(convert_key(pool, cp_index));
-  if (entry != nullptr) {
-    return *entry;
-  } else {
-    return nullptr;
-  }
-
+  ResolutionErrorKey key(pool(), cp_index);
+  ResolutionErrorEntry** entry = _resolution_error_table.get(key);
+  return entry == nullptr ? nullptr : *entry;
 }
 
-ResolutionErrorEntry::ResolutionErrorEntry(ConstantPool* pool, int cp_index, Symbol* error, Symbol* message,
+ResolutionErrorEntry::ResolutionErrorEntry(Symbol* error, Symbol* message,
       Symbol* cause, Symbol* cause_msg):
-        _cp_index(cp_index),
         _error(error),
         _message(message),
         _cause(cause),
         _cause_msg(cause_msg),
-        _pool(pool),
         _nest_host_error(nullptr) {
 
-  if (_error != nullptr) {
-    _error->increment_refcount();
-  }
-
-  if (_message != nullptr) {
-    _message->increment_refcount();
-  }
-
-  if (_cause != nullptr) {
-    _cause->increment_refcount();
-  }
-
-  if (_cause_msg != nullptr) {
-    _cause_msg->increment_refcount();
-  }
+  Symbol::maybe_increment_refcount(_error);
+  Symbol::maybe_increment_refcount(_message);
+  Symbol::maybe_increment_refcount(_cause);
+  Symbol::maybe_increment_refcount(_cause_msg);
 }
 
 ResolutionErrorEntry::~ResolutionErrorEntry() {
   // decrement error refcount
-  if (error() != NULL) {
-    error()->decrement_refcount();
-  }
-  if (message() != NULL) {
-    message()->decrement_refcount();
-  }
-  if (cause() != NULL) {
-    cause()->decrement_refcount();
-  }
-  if (cause_msg() != NULL) {
-    cause_msg()->decrement_refcount();
-  }
+  Symbol::maybe_decrement_refcount(_error);
+  Symbol::maybe_decrement_refcount(_message);
+  Symbol::maybe_decrement_refcount(_cause);
+  Symbol::maybe_decrement_refcount(_cause_msg);
+
   if (nest_host_error() != NULL) {
     FREE_C_HEAP_ARRAY(char, nest_host_error());
   }
 }
 
-class ResolutionErrorDeleteIterate : StackObj{
-private:
+class ResolutionErrorDeleteIterate : StackObj {
   ConstantPool* p;
 
 public:
   ResolutionErrorDeleteIterate(ConstantPool* pool):
     p(pool) {};
 
-  bool do_entry(uintptr_t key, ResolutionErrorEntry* value){
-    if (value -> pool() == p) {
+  bool do_entry(const ResolutionErrorKey& key, ResolutionErrorEntry* value){
+    if (key.cpool() == p) {
       delete value;
       return true;
     } else {
@@ -142,10 +142,10 @@ void ResolutionErrorTable::delete_entry(ConstantPool* c) {
   _resolution_error_table.unlink(&deleteIterator);
 }
 
-class ResolutionIteratePurgeErrors : StackObj{
+class ResolutionIteratePurgeErrors : StackObj {
 public:
-  bool do_entry(uintptr_t key, ResolutionErrorEntry* value) {
-    ConstantPool* pool = value -> pool();
+  bool do_entry(const ResolutionErrorKey& key, ResolutionErrorEntry* value){
+    ConstantPool* pool = key.cpool();
     if (!(pool->pool_holder()->is_loader_alive())) {
       delete value;
       return true;

src/hotspot/share/classfile/resolutionErrors.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2022, 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
@@ -32,18 +32,11 @@ class ResolutionErrorEntry;
 // ResolutionError objects are used to record errors encountered during
 // constant pool resolution (JVMS 5.4.3).
 
-// This value is added to the cpCache index of an invokedynamic instruction when
-// storing the resolution error resulting from that invokedynamic instruction.
-// This prevents issues where the cpCache index is the same as the constant pool
-// index of another entry in the table.
-const int CPCACHE_INDEX_MANGLE_VALUE = 1000000;
-
 class ResolutionErrorTable : AllStatic {
 
 public:
-
   static void add_entry(const constantPoolHandle& pool, int which, Symbol* error, Symbol* message,
-                 Symbol* cause, Symbol* cause_msg);
+                        Symbol* cause, Symbol* cause_msg);
 
   static void add_entry(const constantPoolHandle& pool, int which, const char* message);
 
@@ -56,6 +49,12 @@ public:
   // RedefineClasses support - remove obsolete constant pool entry
   static void delete_entry(ConstantPool* c);
 
+  // This value is added to the cpCache index of an invokedynamic instruction when
+  // storing the resolution error resulting from that invokedynamic instruction.
+  // This prevents issues where the cpCache index is the same as the constant pool
+  // index of another entry in the table.
+  static const int CPCACHE_INDEX_MANGLE_VALUE = 1000000;
+
   // This function is used to encode an index to differentiate it from a
   // constant pool index.  It assumes it is being called with a cpCache index
   // (that is less than 0).
@@ -63,35 +62,27 @@ public:
     assert(index < 0, "Unexpected non-negative cpCache index");
     return index + CPCACHE_INDEX_MANGLE_VALUE;
   }
-
-  static uintptr_t convert_key(const constantPoolHandle& pool, int cp_index) {
-    return (uintptr_t) (pool() + cp_index);
-  }
 };
 
 
 class ResolutionErrorEntry : public CHeapObj<mtClass> {
  private:
-  int               _cp_index;
   Symbol*           _error;
   Symbol*           _message;
   Symbol*           _cause;
   Symbol*           _cause_msg;
-  ConstantPool*     _pool;
   const char*       _nest_host_error;
 
+  NONCOPYABLE(ResolutionErrorEntry);
+
  public:
+    ResolutionErrorEntry(Symbol* error, Symbol* message, Symbol* cause, Symbol* cause_msg);
 
-    ResolutionErrorEntry(ConstantPool* pool, int cp_index, Symbol* error, Symbol* message,
-      Symbol* cause, Symbol* cause_msg);
-
-    ResolutionErrorEntry(ConstantPool* pool, int cp_index, const char* message):
-        _cp_index(cp_index),
+    ResolutionErrorEntry(const char* message):
         _error(nullptr),
         _message(nullptr),
         _cause(nullptr),
         _cause_msg(nullptr),
-        _pool(pool),
         _nest_host_error(message) {}
 
     ~ResolutionErrorEntry();
@@ -101,14 +92,11 @@ class ResolutionErrorEntry : public CHeapObj<mtClass> {
     }
 
 
-  ConstantPool*      pool() const               { return _pool; }
-  int                cp_index() const           { return _cp_index; }
   Symbol*            error() const              { return _error; }
   Symbol*            message() const            { return _message; }
   Symbol*            cause() const              { return _cause; }
   Symbol*            cause_msg() const          { return _cause_msg; }
   const char*        nest_host_error() const    { return _nest_host_error; }
-
 };
 
 #endif // SHARE_CLASSFILE_RESOLUTIONERRORS_HPP