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
--- a/hotspot/src/share/vm/gc_implementation/g1/collectionSetChooser.cpp Fri Aug 06 11:53:28 2010 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/collectionSetChooser.cpp Mon Aug 09 15:17:05 2010 -0700
@@ -158,13 +158,18 @@
// 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),
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegionSeq.cpp Fri Aug 06 11:53:28 2010 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegionSeq.cpp Mon Aug 09 15:17:05 2010 -0700
@@ -42,14 +42,19 @@
// 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),
--- a/hotspot/src/share/vm/memory/allocation.cpp Fri Aug 06 11:53:28 2010 -0700
+++ b/hotspot/src/share/vm/memory/allocation.cpp Mon Aug 09 15:17:05 2010 -0700
@@ -46,7 +46,7 @@
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::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() {
- 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
--- a/hotspot/src/share/vm/memory/allocation.hpp Fri Aug 06 11:53:28 2010 -0700
+++ b/hotspot/src/share/vm/memory/allocation.hpp Mon Aug 09 15:17:05 2010 -0700
@@ -317,6 +317,7 @@
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 @@
// 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 @@
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);
};
--- a/hotspot/src/share/vm/runtime/thread.cpp Fri Aug 06 11:53:28 2010 -0700
+++ b/hotspot/src/share/vm/runtime/thread.cpp Mon Aug 09 15:17:05 2010 -0700
@@ -807,7 +807,7 @@
// 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() {
--- a/hotspot/src/share/vm/runtime/thread.hpp Fri Aug 06 11:53:28 2010 -0700
+++ b/hotspot/src/share/vm/runtime/thread.hpp Mon Aug 09 15:17:05 2010 -0700
@@ -449,6 +449,11 @@
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; }