Skip to content

Commit f58e08e

Browse files
author
Thomas Schatzl
committed
8290715: Fix incorrect uses of G1CollectedHeap::heap_region_containing()
Reviewed-by: kbarrett, sangheki
1 parent 18cd16d commit f58e08e

File tree

5 files changed

+15
-22
lines changed

5 files changed

+15
-22
lines changed

src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,10 @@ inline HeapWord* G1CollectedHeap::bottom_addr_for_region(uint index) const {
9797
return _hrm.reserved().start() + index * HeapRegion::GrainWords;
9898
}
9999

100+
100101
inline HeapRegion* G1CollectedHeap::heap_region_containing(const void* addr) const {
101-
assert(addr != nullptr, "invariant");
102-
assert(is_in_reserved(addr),
103-
"Address " PTR_FORMAT " is outside of the heap ranging from [" PTR_FORMAT " to " PTR_FORMAT ")",
104-
p2i((void*)addr), p2i(reserved().start()), p2i(reserved().end()));
105-
return _hrm.addr_to_region((HeapWord*)addr);
102+
uint const region_idx = addr_to_region(addr);
103+
return region_at(region_idx);
106104
}
107105

108106
inline HeapRegion* G1CollectedHeap::heap_region_containing_or_null(const void* addr) const {

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1889,16 +1889,16 @@ HeapRegion* G1ConcurrentMark::claim_region(uint worker_id) {
18891889
while (finger < _heap.end()) {
18901890
assert(_g1h->is_in_reserved(finger), "invariant");
18911891

1892-
HeapRegion* curr_region = _g1h->heap_region_containing(finger);
1892+
HeapRegion* curr_region = _g1h->heap_region_containing_or_null(finger);
18931893
// Make sure that the reads below do not float before loading curr_region.
18941894
OrderAccess::loadload();
18951895
// Above heap_region_containing may return NULL as we always scan claim
18961896
// until the end of the heap. In this case, just jump to the next region.
1897-
HeapWord* end = curr_region != NULL ? curr_region->end() : finger + HeapRegion::GrainWords;
1897+
HeapWord* end = curr_region != nullptr ? curr_region->end() : finger + HeapRegion::GrainWords;
18981898

18991899
// Is the gap between reading the finger and doing the CAS too long?
19001900
HeapWord* res = Atomic::cmpxchg(&_finger, finger, end);
1901-
if (res == finger && curr_region != NULL) {
1901+
if (res == finger && curr_region != nullptr) {
19021902
// we succeeded
19031903
HeapWord* bottom = curr_region->bottom();
19041904
HeapWord* limit = curr_region->top_at_mark_start();
@@ -1915,7 +1915,7 @@ HeapRegion* G1ConcurrentMark::claim_region(uint worker_id) {
19151915
"the region limit should be at bottom");
19161916
// we return NULL and the caller should try calling
19171917
// claim_region() again.
1918-
return NULL;
1918+
return nullptr;
19191919
}
19201920
} else {
19211921
assert(_finger > finger, "the finger should have moved forward");
@@ -1924,7 +1924,7 @@ HeapRegion* G1ConcurrentMark::claim_region(uint worker_id) {
19241924
}
19251925
}
19261926

1927-
return NULL;
1927+
return nullptr;
19281928
}
19291929

19301930
#ifndef PRODUCT
@@ -1972,11 +1972,11 @@ void G1ConcurrentMark::verify_no_collection_set_oops() {
19721972

19731973
// Verify the global finger
19741974
HeapWord* global_finger = finger();
1975-
if (global_finger != NULL && global_finger < _heap.end()) {
1976-
// Since we always iterate over all regions, we might get a NULL HeapRegion
1975+
if (global_finger != nullptr && global_finger < _heap.end()) {
1976+
// Since we always iterate over all regions, we might get a nullptr HeapRegion
19771977
// here.
1978-
HeapRegion* global_hr = _g1h->heap_region_containing(global_finger);
1979-
guarantee(global_hr == NULL || global_finger == global_hr->bottom(),
1978+
HeapRegion* global_hr = _g1h->heap_region_containing_or_null(global_finger);
1979+
guarantee(global_hr == nullptr || global_finger == global_hr->bottom(),
19801980
"global finger: " PTR_FORMAT " region: " HR_FORMAT,
19811981
p2i(global_finger), HR_FORMAT_PARAMS(global_hr));
19821982
}
@@ -1986,10 +1986,10 @@ void G1ConcurrentMark::verify_no_collection_set_oops() {
19861986
for (uint i = 0; i < _num_concurrent_workers; ++i) {
19871987
G1CMTask* task = _tasks[i];
19881988
HeapWord* task_finger = task->finger();
1989-
if (task_finger != NULL && task_finger < _heap.end()) {
1989+
if (task_finger != nullptr && task_finger < _heap.end()) {
19901990
// See above note on the global finger verification.
1991-
HeapRegion* r = _g1h->heap_region_containing(task_finger);
1992-
guarantee(r == NULL || task_finger == r->bottom() ||
1991+
HeapRegion* r = _g1h->heap_region_containing_or_null(task_finger);
1992+
guarantee(r == nullptr || task_finger == r->bottom() ||
19931993
!r->in_collection_set() || !r->has_index_in_opt_cset(),
19941994
"task finger: " PTR_FORMAT " region: " HR_FORMAT,
19951995
p2i(task_finger), HR_FORMAT_PARAMS(r));

src/hotspot/share/gc/g1/g1OopClosures.inline.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ inline static void check_obj_during_refinement(T* p, oop const obj) {
122122

123123
HeapRegion* from = g1h->heap_region_containing(p);
124124

125-
assert(from != NULL, "from region must be non-NULL");
126125
assert(from->is_in_reserved(p) ||
127126
(from->is_humongous() &&
128127
g1h->heap_region_containing(p)->is_humongous() &&

src/hotspot/share/gc/g1/g1SATBMarkQueueSet.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ static inline bool requires_marking(const void* entry, G1CollectedHeap* g1h) {
8686
"Non-heap pointer in SATB buffer: " PTR_FORMAT, p2i(entry));
8787

8888
HeapRegion* region = g1h->heap_region_containing(entry);
89-
assert(region != NULL, "No region for " PTR_FORMAT, p2i(entry));
9089
if (entry >= region->top_at_mark_start()) {
9190
return false;
9291
}

src/hotspot/share/prims/whitebox.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,9 +383,6 @@ WB_ENTRY(jboolean, WB_isObjectInOldGen(JNIEnv* env, jobject o, jobject obj))
383383
if (UseG1GC) {
384384
G1CollectedHeap* g1h = G1CollectedHeap::heap();
385385
const HeapRegion* hr = g1h->heap_region_containing(p);
386-
if (hr == NULL) {
387-
return false;
388-
}
389386
return !(hr->is_young());
390387
}
391388
#endif

0 commit comments

Comments
 (0)