6975078: assert(allocated_on_res_area() || allocated_on_C_heap() || allocated_on_arena()

Summary: Pass the check in ResourceObj() if _allocation value is already set and object is allocated on stack.
Reviewed-by: dholmes, johnc

src/share/vm/gc_implementation/g1/collectionSetChooser.cpp

@@ -158,13 +158,18 @@ CollectionSetChooser::CollectionSetChooser() :
   // The line below is the worst bit of C++ hackery I've ever written
   // (Detlefs, 11/23).  You should think of it as equivalent to
   // "_regions(100, true)": initialize the growable array and inform it
-  // that it should allocate its elem array(s) on the C heap.  The first
-  // argument, however, is actually a comma expression (new-expr, 100).
-  // The purpose of the new_expr is to inform the growable array that it
-  // is *already* allocated on the C heap: it uses the placement syntax to
-  // keep it from actually doing any allocation.
-  _markedRegions((ResourceObj::operator new (sizeof(GrowableArray<HeapRegion*>),
-                                             (void*)&_markedRegions,
+  // that it should allocate its elem array(s) on the C heap.
+  //
+  // The first argument, however, is actually a comma expression
+  // (set_allocation_type(this, C_HEAP), 100). The purpose of the
+  // set_allocation_type() call is to replace the default allocation
+  // type for embedded objects STACK_OR_EMBEDDED with C_HEAP. It will
+  // allow to pass the assert in GenericGrowableArray() which checks
+  // that a growable array object must be on C heap if elements are.
+  //
+  // Note: containing object is allocated on C heap since it is CHeapObj.
+  //
+  _markedRegions((ResourceObj::set_allocation_type((address)&_markedRegions,
                                              ResourceObj::C_HEAP),
                   100),
                  true),

src/share/vm/gc_implementation/g1/heapRegionSeq.cpp

@@ -42,14 +42,19 @@ HeapRegionSeq::HeapRegionSeq(const size_t max_size) :
   // The line below is the worst bit of C++ hackery I've ever written
   // (Detlefs, 11/23).  You should think of it as equivalent to
   // "_regions(100, true)": initialize the growable array and inform it
-  // that it should allocate its elem array(s) on the C heap.  The first
-  // argument, however, is actually a comma expression (new-expr, 100).
-  // The purpose of the new_expr is to inform the growable array that it
-  // is *already* allocated on the C heap: it uses the placement syntax to
-  // keep it from actually doing any allocation.
-  _regions((ResourceObj::operator new (sizeof(GrowableArray<HeapRegion*>),
-                                       (void*)&_regions,
-                                       ResourceObj::C_HEAP),
+  // that it should allocate its elem array(s) on the C heap.
+  //
+  // The first argument, however, is actually a comma expression
+  // (set_allocation_type(this, C_HEAP), 100). The purpose of the
+  // set_allocation_type() call is to replace the default allocation
+  // type for embedded objects STACK_OR_EMBEDDED with C_HEAP. It will
+  // allow to pass the assert in GenericGrowableArray() which checks
+  // that a growable array object must be on C heap if elements are.
+  //
+  // Note: containing object is allocated on C heap since it is CHeapObj.
+  //
+  _regions((ResourceObj::set_allocation_type((address)&_regions,
+                                             ResourceObj::C_HEAP),
             (int)max_size),
            true),
   _next_rr_candidate(0),

src/share/vm/memory/allocation.cpp

@@ -46,7 +46,7 @@ void* ResourceObj::operator new(size_t size, allocation_type type) {
     DEBUG_ONLY(set_allocation_type(res, C_HEAP);)
     break;
    case RESOURCE_AREA:
-    // Will set allocation type in the resource object.
+    // new(size) sets allocation type RESOURCE_AREA.
     res = (address)operator new(size);
     break;
    default:
@@ -66,26 +66,30 @@ void ResourceObj::operator delete(void* p) {
 void ResourceObj::set_allocation_type(address res, allocation_type type) {
     // Set allocation type in the resource object
     uintptr_t allocation = (uintptr_t)res;
-    assert((allocation & allocation_mask) == 0, "address should be aligned ot 4 bytes at least");
+    assert((allocation & allocation_mask) == 0, "address should be aligned to 4 bytes at least");
     assert(type <= allocation_mask, "incorrect allocation type");
     ((ResourceObj *)res)->_allocation = ~(allocation + type);
 }
 
-ResourceObj::allocation_type ResourceObj::get_allocation_type() {
+ResourceObj::allocation_type ResourceObj::get_allocation_type() const {
     assert(~(_allocation | allocation_mask) == (uintptr_t)this, "lost resource object");
     return (allocation_type)((~_allocation) & allocation_mask);
 }
 
-ResourceObj::ResourceObj() { // default construtor
+ResourceObj::ResourceObj() { // default constructor
     if (~(_allocation | allocation_mask) != (uintptr_t)this) {
       set_allocation_type((address)this, STACK_OR_EMBEDDED);
+    } else if (allocated_on_stack()) {
+      // For some reason we got a value which looks like an allocation on stack.
+      // Pass if it is really allocated on stack.
+      assert(Thread::current()->on_local_stack((address)this),"should be on stack");
     } else {
       assert(allocated_on_res_area() || allocated_on_C_heap() || allocated_on_arena(),
              "allocation_type should be set by operator new()");
     }
 }
 
-ResourceObj::ResourceObj(const ResourceObj& r) { // default copy construtor
+ResourceObj::ResourceObj(const ResourceObj& r) { // default copy constructor
     // Used in ClassFileParser::parse_constant_pool_entries() for ClassFileStream.
     set_allocation_type((address)this, STACK_OR_EMBEDDED);
 }
@@ -98,8 +102,9 @@ ResourceObj& ResourceObj::operator=(const ResourceObj& r) { // default copy assi
 }
 
 ResourceObj::~ResourceObj() {
-    if (!allocated_on_C_heap()) { // operator delete() checks C_heap allocation_type.
-      _allocation = badHeapOopVal;
+    // allocated_on_C_heap() also checks that encoded (in _allocation) address == this.
+    if (!allocated_on_C_heap()) {  // ResourceObj::delete() zaps _allocation for C_heap.
+      _allocation = badHeapOopVal; // zap type
     }
 }
 #endif // ASSERT

src/share/vm/memory/allocation.hpp

@@ -317,6 +317,7 @@ extern void resource_free_bytes( char *old, size_t size );
 class ResourceObj ALLOCATION_SUPER_CLASS_SPEC {
  public:
   enum allocation_type { STACK_OR_EMBEDDED = 0, RESOURCE_AREA, C_HEAP, ARENA, allocation_mask = 0x3 };
+  static void set_allocation_type(address res, allocation_type type) NOT_DEBUG_RETURN;
 #ifdef ASSERT
  private:
   // When this object is allocated on stack the new() operator is not
@@ -324,12 +325,11 @@ class ResourceObj ALLOCATION_SUPER_CLASS_SPEC {
   // Store negated 'this' pointer when new() is called to distinguish cases.
   uintptr_t _allocation;
  public:
-  static void set_allocation_type(address res, allocation_type type);
-  allocation_type get_allocation_type();
-  bool allocated_on_stack()     { return get_allocation_type() == STACK_OR_EMBEDDED; }
-  bool allocated_on_res_area()  { return get_allocation_type() == RESOURCE_AREA; }
-  bool allocated_on_C_heap()    { return get_allocation_type() == C_HEAP; }
-  bool allocated_on_arena()     { return get_allocation_type() == ARENA; }
+  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; }
   ResourceObj(); // default construtor
   ResourceObj(const ResourceObj& r); // default copy construtor
   ResourceObj& operator=(const ResourceObj& r); // default copy assignment
@@ -348,11 +348,6 @@ class ResourceObj ALLOCATION_SUPER_CLASS_SPEC {
       DEBUG_ONLY(set_allocation_type(res, RESOURCE_AREA);)
       return res;
   }
-  void* operator new(size_t size, void* where, allocation_type type) {
-      address res = (address)where;
-      DEBUG_ONLY(set_allocation_type(res, type);)
-      return res;
-  }
   void  operator delete(void* p);
 };
 

src/share/vm/runtime/thread.cpp

@@ -807,7 +807,7 @@ bool Thread::is_in_stack(address adr) const {
 // should be revisited, and they should be removed if possible.
 
 bool Thread::is_lock_owned(address adr) const {
-  return (_stack_base >= adr && adr >= (_stack_base - _stack_size));
+  return on_local_stack(adr);
 }
 
 bool Thread::set_as_starting_thread() {

src/share/vm/runtime/thread.hpp

@@ -449,6 +449,11 @@ public:
   void    set_stack_size(size_t size)  { _stack_size = size; }
   void    record_stack_base_and_size();
 
+  bool    on_local_stack(address adr) const {
+    /* QQQ this has knowledge of direction, ought to be a stack method */
+    return (_stack_base >= adr && adr >= (_stack_base - _stack_size));
+  }
+
   int     lgrp_id() const                 { return _lgrp_id; }
   void    set_lgrp_id(int value)          { _lgrp_id = value; }