8290715: Fix incorrect uses of G1CollectedHeap::heap_region_containing()

Reviewed-by: kbarrett, sangheki
This commit is contained in:
Thomas Schatzl 2022-07-29 15:43:43 +00:00
parent 18cd16d2ea
commit f58e08e258
5 changed files with 15 additions and 22 deletions

View File

@ -97,12 +97,10 @@ inline HeapWord* G1CollectedHeap::bottom_addr_for_region(uint index) const {
return _hrm.reserved().start() + index * HeapRegion::GrainWords;
}
inline HeapRegion* G1CollectedHeap::heap_region_containing(const void* addr) const {
assert(addr != nullptr, "invariant");
assert(is_in_reserved(addr),
"Address " PTR_FORMAT " is outside of the heap ranging from [" PTR_FORMAT " to " PTR_FORMAT ")",
p2i((void*)addr), p2i(reserved().start()), p2i(reserved().end()));
return _hrm.addr_to_region((HeapWord*)addr);
uint const region_idx = addr_to_region(addr);
return region_at(region_idx);
}
inline HeapRegion* G1CollectedHeap::heap_region_containing_or_null(const void* addr) const {

View File

@ -1889,16 +1889,16 @@ HeapRegion* G1ConcurrentMark::claim_region(uint worker_id) {
while (finger < _heap.end()) {
assert(_g1h->is_in_reserved(finger), "invariant");
HeapRegion* curr_region = _g1h->heap_region_containing(finger);
HeapRegion* curr_region = _g1h->heap_region_containing_or_null(finger);
// Make sure that the reads below do not float before loading curr_region.
OrderAccess::loadload();
// Above heap_region_containing may return NULL as we always scan claim
// until the end of the heap. In this case, just jump to the next region.
HeapWord* end = curr_region != NULL ? curr_region->end() : finger + HeapRegion::GrainWords;
HeapWord* end = curr_region != nullptr ? curr_region->end() : finger + HeapRegion::GrainWords;
// Is the gap between reading the finger and doing the CAS too long?
HeapWord* res = Atomic::cmpxchg(&_finger, finger, end);
if (res == finger && curr_region != NULL) {
if (res == finger && curr_region != nullptr) {
// we succeeded
HeapWord* bottom = curr_region->bottom();
HeapWord* limit = curr_region->top_at_mark_start();
@ -1915,7 +1915,7 @@ HeapRegion* G1ConcurrentMark::claim_region(uint worker_id) {
"the region limit should be at bottom");
// we return NULL and the caller should try calling
// claim_region() again.
return NULL;
return nullptr;
}
} else {
assert(_finger > finger, "the finger should have moved forward");
@ -1924,7 +1924,7 @@ HeapRegion* G1ConcurrentMark::claim_region(uint worker_id) {
}
}
return NULL;
return nullptr;
}
#ifndef PRODUCT
@ -1972,11 +1972,11 @@ void G1ConcurrentMark::verify_no_collection_set_oops() {
// Verify the global finger
HeapWord* global_finger = finger();
if (global_finger != NULL && global_finger < _heap.end()) {
// Since we always iterate over all regions, we might get a NULL HeapRegion
if (global_finger != nullptr && global_finger < _heap.end()) {
// Since we always iterate over all regions, we might get a nullptr HeapRegion
// here.
HeapRegion* global_hr = _g1h->heap_region_containing(global_finger);
guarantee(global_hr == NULL || global_finger == global_hr->bottom(),
HeapRegion* global_hr = _g1h->heap_region_containing_or_null(global_finger);
guarantee(global_hr == nullptr || global_finger == global_hr->bottom(),
"global finger: " PTR_FORMAT " region: " HR_FORMAT,
p2i(global_finger), HR_FORMAT_PARAMS(global_hr));
}
@ -1986,10 +1986,10 @@ void G1ConcurrentMark::verify_no_collection_set_oops() {
for (uint i = 0; i < _num_concurrent_workers; ++i) {
G1CMTask* task = _tasks[i];
HeapWord* task_finger = task->finger();
if (task_finger != NULL && task_finger < _heap.end()) {
if (task_finger != nullptr && task_finger < _heap.end()) {
// See above note on the global finger verification.
HeapRegion* r = _g1h->heap_region_containing(task_finger);
guarantee(r == NULL || task_finger == r->bottom() ||
HeapRegion* r = _g1h->heap_region_containing_or_null(task_finger);
guarantee(r == nullptr || task_finger == r->bottom() ||
!r->in_collection_set() || !r->has_index_in_opt_cset(),
"task finger: " PTR_FORMAT " region: " HR_FORMAT,
p2i(task_finger), HR_FORMAT_PARAMS(r));

View File

@ -122,7 +122,6 @@ inline static void check_obj_during_refinement(T* p, oop const obj) {
HeapRegion* from = g1h->heap_region_containing(p);
assert(from != NULL, "from region must be non-NULL");
assert(from->is_in_reserved(p) ||
(from->is_humongous() &&
g1h->heap_region_containing(p)->is_humongous() &&

View File

@ -86,7 +86,6 @@ static inline bool requires_marking(const void* entry, G1CollectedHeap* g1h) {
"Non-heap pointer in SATB buffer: " PTR_FORMAT, p2i(entry));
HeapRegion* region = g1h->heap_region_containing(entry);
assert(region != NULL, "No region for " PTR_FORMAT, p2i(entry));
if (entry >= region->top_at_mark_start()) {
return false;
}

View File

@ -383,9 +383,6 @@ WB_ENTRY(jboolean, WB_isObjectInOldGen(JNIEnv* env, jobject o, jobject obj))
if (UseG1GC) {
G1CollectedHeap* g1h = G1CollectedHeap::heap();
const HeapRegion* hr = g1h->heap_region_containing(p);
if (hr == NULL) {
return false;
}
return !(hr->is_young());
}
#endif