8213481: [REDO] Fix incorrect copy constructors in hotspot

Fix and use ResourceObj copy constructor.

Reviewed-by: coleenp, dholmes, kvn

src/hotspot/share/classfile/stackMapFrame.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2018, 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
@@ -63,6 +63,7 @@ class StackMapFrame : public ResourceObj {
   ClassVerifier* _verifier;  // the verifier verifying this method
 
   StackMapFrame(const StackMapFrame& cp) :
+      ResourceObj(cp),
       _offset(cp._offset), _locals_size(cp._locals_size),
       _stack_size(cp._stack_size), _stack_mark(cp._stack_mark),
       _max_locals(cp._max_locals), _max_stack(cp._max_stack),

src/hotspot/share/libadt/dict.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, 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
@@ -161,7 +161,7 @@ void Dict::doubhash(void) {
 
 //------------------------------Dict-----------------------------------------
 // Deep copy a dictionary.
-Dict::Dict( const Dict &d ) : _arena(d._arena), _size(d._size), _cnt(d._cnt), _hash(d._hash), _cmp(d._cmp) {
+Dict::Dict( const Dict &d ) : ResourceObj(d), _arena(d._arena), _size(d._size), _cnt(d._cnt), _hash(d._hash), _cmp(d._cmp) {
   _bin = (bucket*)_arena->Amalloc_4(sizeof(bucket)*_size);
   memcpy( (void*)_bin, (void*)d._bin, sizeof(bucket)*_size );
   for( uint i=0; i<_size; i++ ) {

src/hotspot/share/libadt/set.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, 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
@@ -112,16 +112,15 @@ class SetI_;
 
 //------------------------------Set--------------------------------------------
 class Set : public ResourceObj {
- public:
+ protected:
 
   // Creates a new, empty set.
-  // DO NOT CONSTRUCT A Set.  THIS IS AN ABSTRACT CLASS, FOR INHERITENCE ONLY
   Set(Arena *arena) : _set_arena(arena) {};
 
   // Creates a new set from an existing set
-  // DO NOT CONSTRUCT A Set.  THIS IS AN ABSTRACT CLASS, FOR INHERITENCE ONLY
+  Set(const Set & s) : ResourceObj(s) {};
-  Set(const Set &) {};
 
+ public:
   // Set assignment; deep-copy guts
   virtual Set &operator =(const Set &s)=0;
   virtual Set &clone(void) const=0;

src/hotspot/share/memory/allocation.cpp

@@ -162,78 +162,81 @@ void ResourceObj::operator delete [](void* p) {
 
 #ifdef ASSERT
 void ResourceObj::set_allocation_type(address res, allocation_type type) {
-    // Set allocation type in the resource object
+  // Set allocation type in the resource object
-    uintptr_t allocation = (uintptr_t)res;
+  uintptr_t allocation = (uintptr_t)res;
-    assert((allocation & allocation_mask) == 0, "address should be aligned to 4 bytes at least: " INTPTR_FORMAT, p2i(res));
+  assert((allocation & allocation_mask) == 0, "address should be aligned to 4 bytes at least: " INTPTR_FORMAT, p2i(res));
-    assert(type <= allocation_mask, "incorrect allocation type");
+  assert(type <= allocation_mask, "incorrect allocation type");
-    ResourceObj* resobj = (ResourceObj *)res;
+  ResourceObj* resobj = (ResourceObj *)res;
-    resobj->_allocation_t[0] = ~(allocation + type);
+  resobj->_allocation_t[0] = ~(allocation + type);
-    if (type != STACK_OR_EMBEDDED) {
+  if (type != STACK_OR_EMBEDDED) {
-      // Called from operator new() and CollectionSetChooser(),
+    // Called from operator new() and CollectionSetChooser(),
-      // set verification value.
+    // set verification value.
-      resobj->_allocation_t[1] = (uintptr_t)&(resobj->_allocation_t[1]) + type;
+    resobj->_allocation_t[1] = (uintptr_t)&(resobj->_allocation_t[1]) + type;
-    }
+  }
 }
 
 ResourceObj::allocation_type ResourceObj::get_allocation_type() const {
-    assert(~(_allocation_t[0] | allocation_mask) == (uintptr_t)this, "lost resource object");
+  assert(~(_allocation_t[0] | allocation_mask) == (uintptr_t)this, "lost resource object");
-    return (allocation_type)((~_allocation_t[0]) & allocation_mask);
+  return (allocation_type)((~_allocation_t[0]) & allocation_mask);
 }
 
 bool ResourceObj::is_type_set() const {
-    allocation_type type = (allocation_type)(_allocation_t[1] & allocation_mask);
+  allocation_type type = (allocation_type)(_allocation_t[1] & allocation_mask);
-    return get_allocation_type()  == type &&
+  return get_allocation_type()  == type &&
-           (_allocation_t[1] - type) == (uintptr_t)(&_allocation_t[1]);
+         (_allocation_t[1] - type) == (uintptr_t)(&_allocation_t[1]);
 }
 
-ResourceObj::ResourceObj() { // default constructor
+// This whole business of passing information from ResourceObj::operator new
-    if (~(_allocation_t[0] | allocation_mask) != (uintptr_t)this) {
+// to the ResourceObj constructor via fields in the "object" is technically UB.
-      // Operator new() is not called for allocations
+// But it seems to work within the limitations of HotSpot usage (such as no
-      // on stack and for embedded objects.
+// multiple inheritance) with the compilers and compiler options we're using.
-      set_allocation_type((address)this, STACK_OR_EMBEDDED);
+// And it gives some possibly useful checking for misuse of ResourceObj.
-    } else if (allocated_on_stack()) { // STACK_OR_EMBEDDED
+void ResourceObj::initialize_allocation_info() {
-      // For some reason we got a value which resembles
+  if (~(_allocation_t[0] | allocation_mask) != (uintptr_t)this) {
-      // an embedded or stack object (operator new() does not
+    // Operator new() is not called for allocations
-      // set such type). Keep it since it is valid value
+    // on stack and for embedded objects.
-      // (even if it was garbage).
-      // Ignore garbage in other fields.
-    } else if (is_type_set()) {
-      // Operator new() was called and type was set.
-      assert(!allocated_on_stack(),
-             "not embedded or stack, this(" PTR_FORMAT ") type %d a[0]=(" PTR_FORMAT ") a[1]=(" PTR_FORMAT ")",
-             p2i(this), get_allocation_type(), _allocation_t[0], _allocation_t[1]);
-    } else {
-      // Operator new() was not called.
-      // Assume that it is embedded or stack object.
-      set_allocation_type((address)this, STACK_OR_EMBEDDED);
-    }
-    _allocation_t[1] = 0; // Zap verification value
-}
-
-ResourceObj::ResourceObj(const ResourceObj& r) { // default copy constructor
-    // Used in ClassFileParser::parse_constant_pool_entries() for ClassFileStream.
-    // Note: garbage may resembles valid value.
-    assert(~(_allocation_t[0] | allocation_mask) != (uintptr_t)this || !is_type_set(),
-           "embedded or stack only, this(" PTR_FORMAT ") type %d a[0]=(" PTR_FORMAT ") a[1]=(" PTR_FORMAT ")",
-           p2i(this), get_allocation_type(), _allocation_t[0], _allocation_t[1]);
     set_allocation_type((address)this, STACK_OR_EMBEDDED);
-    _allocation_t[1] = 0; // Zap verification value
+  } else if (allocated_on_stack()) { // STACK_OR_EMBEDDED
+    // For some reason we got a value which resembles
+    // an embedded or stack object (operator new() does not
+    // set such type). Keep it since it is valid value
+    // (even if it was garbage).
+    // Ignore garbage in other fields.
+  } else if (is_type_set()) {
+    // Operator new() was called and type was set.
+    assert(!allocated_on_stack(),
+           "not embedded or stack, this(" PTR_FORMAT ") type %d a[0]=(" PTR_FORMAT ") a[1]=(" PTR_FORMAT ")",
+           p2i(this), get_allocation_type(), _allocation_t[0], _allocation_t[1]);
+  } else {
+    // Operator new() was not called.
+    // Assume that it is embedded or stack object.
+    set_allocation_type((address)this, STACK_OR_EMBEDDED);
+  }
+  _allocation_t[1] = 0; // Zap verification value
 }
 
-ResourceObj& ResourceObj::operator=(const ResourceObj& r) { // default copy assignment
+ResourceObj::ResourceObj() {
-    // Used in InlineTree::ok_to_inline() for WarmCallInfo.
+  initialize_allocation_info();
-    assert(allocated_on_stack(),
+}
-           "copy only into local, this(" PTR_FORMAT ") type %d a[0]=(" PTR_FORMAT ") a[1]=(" PTR_FORMAT ")",
+
-           p2i(this), get_allocation_type(), _allocation_t[0], _allocation_t[1]);
+ResourceObj::ResourceObj(const ResourceObj&) {
-    // Keep current _allocation_t value;
+  // Initialize _allocation_t as a new object, ignoring object being copied.
-    return *this;
+  initialize_allocation_info();
+}
+
+ResourceObj& ResourceObj::operator=(const ResourceObj& r) {
+  assert(allocated_on_stack(),
+         "copy only into local, this(" PTR_FORMAT ") type %d a[0]=(" PTR_FORMAT ") a[1]=(" PTR_FORMAT ")",
+         p2i(this), get_allocation_type(), _allocation_t[0], _allocation_t[1]);
+  // Keep current _allocation_t value;
+  return *this;
 }
 
 ResourceObj::~ResourceObj() {
-    // allocated_on_C_heap() also checks that encoded (in _allocation) address == this.
+  // allocated_on_C_heap() also checks that encoded (in _allocation) address == this.
-    if (!allocated_on_C_heap()) { // ResourceObj::delete() will zap _allocation for C_heap.
+  if (!allocated_on_C_heap()) { // ResourceObj::delete() will zap _allocation for C_heap.
-      _allocation_t[0] = (uintptr_t)badHeapOopVal; // zap type
+    _allocation_t[0] = (uintptr_t)badHeapOopVal; // zap type
-    }
+  }
 }
 #endif // ASSERT
 

src/hotspot/share/memory/allocation.hpp

@@ -365,12 +365,14 @@ class ResourceObj ALLOCATION_SUPER_CLASS_SPEC {
   // Use second array's element for verification value to distinguish garbage.
   uintptr_t _allocation_t[2];
   bool is_type_set() const;
+  void initialize_allocation_info();
  public:
   allocation_type get_allocation_type() const;
   bool allocated_on_stack()    const { return get_allocation_type() == STACK_OR_EMBEDDED; }
   bool allocated_on_res_area() const { return get_allocation_type() == RESOURCE_AREA; }
   bool allocated_on_C_heap()   const { return get_allocation_type() == C_HEAP; }
   bool allocated_on_arena()    const { return get_allocation_type() == ARENA; }
+protected:
   ResourceObj(); // default constructor
   ResourceObj(const ResourceObj& r); // default copy constructor
   ResourceObj& operator=(const ResourceObj& r); // default copy assignment