8272107: Removal of Unsafe::defineAnonymousClass left a dangling C++ class

Reviewed-by: coleenp, iklam, dholmes

src/hotspot/share/memory/iterator.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2021, 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
@@ -341,17 +341,6 @@ public:
 class SymbolClosure : public StackObj {
  public:
   virtual void do_symbol(Symbol**) = 0;
-
-  // Clear LSB in symbol address; it can be set by CPSlot.
-  static Symbol* load_symbol(Symbol** p) {
-    return (Symbol*)(intptr_t(*p) & ~1);
-  }
-
-  // Store symbol, adjusting new pointer if the original pointer was adjusted
-  // (symbol references in constant pool slots have their LSB set to 1).
-  static void store_symbol(Symbol** p, Symbol* sym) {
-    *p = (Symbol*)(intptr_t(sym) | (intptr_t(*p) & 1));
-  }
 };
 
 template <typename E>

src/hotspot/share/memory/metaspaceClosure.hpp

@@ -128,8 +128,8 @@ public:
     virtual ~Ref() {}
 
     address obj() const {
-      // In some rare cases (see CPSlot in constantPool.hpp) we store some flags in the lowest
-      // 2 bits of a MetaspaceObj pointer. Unmask these when manipulating the pointer.
+      // In some rare cases we store some flags in the lowest 2 bits of a
+      // MetaspaceObj pointer. Unmask these when manipulating the pointer.
       uintx p = (uintx)*mpp();
       return (address)(p & (~FLAG_MASK));
     }

src/hotspot/share/oops/constantPool.cpp

@@ -2391,11 +2391,11 @@ void ConstantPool::verify_on(outputStream* st) {
     if (tag.is_klass() || tag.is_unresolved_klass()) {
       guarantee(klass_name_at(i)->refcount() != 0, "should have nonzero reference count");
     } else if (tag.is_symbol()) {
-      CPSlot entry = slot_at(i);
-      guarantee(entry.get_symbol()->refcount() != 0, "should have nonzero reference count");
+      Symbol* entry = symbol_at(i);
+      guarantee(entry->refcount() != 0, "should have nonzero reference count");
     } else if (tag.is_string()) {
-      CPSlot entry = slot_at(i);
-      guarantee(entry.get_symbol()->refcount() != 0, "should have nonzero reference count");
+      Symbol* entry = unresolved_string_at(i);
+      guarantee(entry->refcount() != 0, "should have nonzero reference count");
     }
   }
   if (pool_holder() != NULL) {

src/hotspot/share/oops/constantPool.hpp

@@ -49,22 +49,6 @@
 
 class SymbolHashMap;
 
-class CPSlot {
- friend class ConstantPool;
-  intptr_t _ptr;
-  enum TagBits  {_pseudo_bit = 1};
- public:
-
-  CPSlot(intptr_t ptr): _ptr(ptr) {}
-  CPSlot(Symbol* ptr, int tag_bits = 0): _ptr((intptr_t)ptr | tag_bits) {}
-
-  intptr_t value()   { return _ptr; }
-
-  Symbol* get_symbol() {
-    return (Symbol*)(_ptr & ~_pseudo_bit);
-  }
-};
-
 // This represents a JVM_CONSTANT_Class, JVM_CONSTANT_UnresolvedClass, or
 // JVM_CONSTANT_UnresolvedClassInError slot in the constant pool.
 class CPKlassSlot {
@@ -152,13 +136,6 @@ class ConstantPool : public Metadata {
  private:
   intptr_t* base() const { return (intptr_t*) (((char*) this) + sizeof(ConstantPool)); }
 
-  CPSlot slot_at(int which) const;
-
-  void slot_at_put(int which, CPSlot s) const {
-    assert(is_within_bounds(which), "index out of bounds");
-    assert(s.value() != 0, "Caught something");
-    *(intptr_t*)&base()[which] = s.value();
-  }
   intptr_t* obj_at_addr(int which) const {
     assert(is_within_bounds(which), "index out of bounds");
     return (intptr_t*) &base()[which];
@@ -343,8 +320,12 @@ class ConstantPool : public Metadata {
   }
 
   void unresolved_string_at_put(int which, Symbol* s) {
-    release_tag_at_put(which, JVM_CONSTANT_String);
-    slot_at_put(which, CPSlot(s));
+    assert(s->refcount() != 0, "should have nonzero refcount");
+    // Note that release_tag_at_put is not needed here because this is called only
+    // when constructing a ConstantPool in a single thread, with no possibility
+    // of concurrent access.
+    tag_at_put(which, JVM_CONSTANT_String);
+    *symbol_at_addr(which) = s;
   }
 
   void int_at_put(int which, jint i) {
@@ -497,8 +478,7 @@ class ConstantPool : public Metadata {
 
   Symbol* unresolved_string_at(int which) {
     assert(tag_at(which).is_string(), "Corrupted constant pool");
-    Symbol* sym = slot_at(which).get_symbol();
-    return sym;
+    return *symbol_at_addr(which);
   }
 
   // Returns an UTF8 for a CONSTANT_String entry at a given index.

src/hotspot/share/oops/constantPool.inline.hpp

@@ -30,15 +30,6 @@
 #include "oops/cpCache.inline.hpp"
 #include "runtime/atomic.hpp"
 
-inline CPSlot ConstantPool::slot_at(int which) const {
-  assert(is_within_bounds(which), "index out of bounds");
-  assert(!tag_at(which).is_unresolved_klass() && !tag_at(which).is_unresolved_klass_in_error(), "Corrupted constant pool");
-  // Uses volatile because the klass slot changes without a lock.
-  intptr_t adr = Atomic::load_acquire(obj_at_addr(which));
-  assert(adr != 0 || which == 0, "cp entry for klass should not be zero");
-  return CPSlot(adr);
-}
-
 inline Klass* ConstantPool::resolved_klass_at(int which) const {  // Used by Compiler
   guarantee(tag_at(which).is_klass(), "Corrupted constant pool");
   // Must do an acquire here in case another thread resolved the klass

src/hotspot/share/oops/symbol.hpp

@@ -45,8 +45,8 @@
 // saved in persistent storage.  This does not include the pointer
 // in the SymbolTable bucket (the _literal field in HashtableEntry)
 // that points to the Symbol.  All other stores of a Symbol*
-// to a field of a persistent variable (e.g., the _name filed in
-// fieldDescriptor or _ptr in a CPSlot) is reference counted.
+// to a field of a persistent variable (e.g., the _name field in
+// fieldDescriptor or symbol in a constant pool) is reference counted.
 //
 // 1) The lookup of a "name" in the SymbolTable either creates a Symbol F for
 // "name" and returns a pointer to F or finds a pre-existing Symbol F for

src/hotspot/share/services/heapDumper.cpp

@@ -1305,7 +1305,7 @@ class SymbolTableDumper : public SymbolClosure {
 
 void SymbolTableDumper::do_symbol(Symbol** p) {
   ResourceMark rm;
-  Symbol* sym = load_symbol(p);
+  Symbol* sym = *p;
   int len = sym->utf8_length();
   if (len > 0) {
     char* s = sym->as_utf8();