8337981: ShenandoahHeap::is_in should check for alive regions

Reviewed-by: rkennke, wkemper
This commit is contained in:
Aleksey Shipilev 2024-08-20 08:40:45 +00:00
parent 9775d57168
commit b9d49dcef2
12 changed files with 111 additions and 60 deletions

View File

@ -37,7 +37,7 @@ void print_raw_memory(ShenandoahMessageBuffer &msg, void* loc) {
// should be in heap, in known committed region, within that region.
ShenandoahHeap* heap = ShenandoahHeap::heap();
if (!heap->is_in(loc)) return;
if (!heap->is_in_reserved(loc)) return;
ShenandoahHeapRegion* r = heap->heap_region_containing(loc);
if (r != nullptr && r->is_committed()) {
@ -77,7 +77,7 @@ void ShenandoahAsserts::print_obj(ShenandoahMessageBuffer& msg, oop obj) {
void ShenandoahAsserts::print_non_obj(ShenandoahMessageBuffer& msg, void* loc) {
ShenandoahHeap* heap = ShenandoahHeap::heap();
if (heap->is_in(loc)) {
if (heap->is_in_reserved(loc)) {
msg.append(" inside Java heap\n");
ShenandoahHeapRegion *r = heap->heap_region_containing(loc);
stringStream ss;
@ -96,7 +96,7 @@ void ShenandoahAsserts::print_non_obj(ShenandoahMessageBuffer& msg, void* loc) {
void ShenandoahAsserts::print_obj_safe(ShenandoahMessageBuffer& msg, void* loc) {
ShenandoahHeap* heap = ShenandoahHeap::heap();
msg.append(" " PTR_FORMAT " - safe print, no details\n", p2i(loc));
if (heap->is_in(loc)) {
if (heap->is_in_reserved(loc)) {
ShenandoahHeapRegion* r = heap->heap_region_containing(loc);
if (r != nullptr) {
stringStream ss;
@ -113,7 +113,7 @@ void ShenandoahAsserts::print_failure(SafeLevel level, oop obj, void* interior_l
ShenandoahHeap* heap = ShenandoahHeap::heap();
ResourceMark rm;
bool loc_in_heap = (loc != nullptr && heap->is_in(loc));
bool loc_in_heap = (loc != nullptr && heap->is_in_reserved(loc));
ShenandoahMessageBuffer msg("%s; %s\n\n", phase, label);
@ -166,22 +166,22 @@ void ShenandoahAsserts::print_failure(SafeLevel level, oop obj, void* interior_l
report_vm_error(file, line, msg.buffer());
}
void ShenandoahAsserts::assert_in_heap(void* interior_loc, oop obj, const char *file, int line) {
void ShenandoahAsserts::assert_in_heap_bounds(void* interior_loc, oop obj, const char *file, int line) {
ShenandoahHeap* heap = ShenandoahHeap::heap();
if (!heap->is_in(obj)) {
print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_in_heap failed",
"oop must point to a heap address",
if (!heap->is_in_reserved(obj)) {
print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_in_heap_bounds failed",
"oop must be in heap bounds",
file, line);
}
}
void ShenandoahAsserts::assert_in_heap_or_null(void* interior_loc, oop obj, const char *file, int line) {
void ShenandoahAsserts::assert_in_heap_bounds_or_null(void* interior_loc, oop obj, const char *file, int line) {
ShenandoahHeap* heap = ShenandoahHeap::heap();
if (obj != nullptr && !heap->is_in(obj)) {
print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_in_heap_or_null failed",
"oop must point to a heap address",
if (obj != nullptr && !heap->is_in_reserved(obj)) {
print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_in_heap_bounds_or_null failed",
"oop must be in heap bounds",
file, line);
}
}
@ -191,9 +191,9 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char*
// Step 1. Check that obj is correct.
// After this step, it is safe to call heap_region_containing().
if (!heap->is_in(obj)) {
if (!heap->is_in_reserved(obj)) {
print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
"oop must point to a heap address",
"oop must be in heap bounds",
file, line);
}
@ -210,6 +210,12 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char*
file,line);
}
if (!heap->is_in(obj)) {
print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
"Object should be in active region area",
file, line);
}
oop fwd = ShenandoahForwarding::get_forwardee_raw_unchecked(obj);
if (obj != fwd) {
@ -223,9 +229,9 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char*
}
// Step 2. Check that forwardee is correct
if (!heap->is_in(fwd)) {
if (!heap->is_in_reserved(fwd)) {
print_failure(_safe_oop, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
"Forwardee must point to a heap address",
"Forwardee must be in heap bounds",
file, line);
}
@ -236,9 +242,15 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char*
}
// Step 3. Check that forwardee points to correct region
if (!heap->is_in(fwd)) {
print_failure(_safe_oop, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
"Forwardee should be in active region area",
file, line);
}
if (heap->heap_region_index_containing(fwd) == heap->heap_region_index_containing(obj)) {
print_failure(_safe_all, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
"Non-trivial forwardee should in another region",
"Non-trivial forwardee should be in another region",
file, line);
}

View File

@ -53,8 +53,8 @@ public:
static void print_rp_failure(const char *label, BoolObjectClosure* actual,
const char *file, int line);
static void assert_in_heap(void* interior_loc, oop obj, const char* file, int line);
static void assert_in_heap_or_null(void* interior_loc, oop obj, const char* file, int line);
static void assert_in_heap_bounds(void* interior_loc, oop obj, const char* file, int line);
static void assert_in_heap_bounds_or_null(void* interior_loc, oop obj, const char* file, int line);
static void assert_in_correct_region(void* interior_loc, oop obj, const char* file, int line);
static void assert_correct(void* interior_loc, oop obj, const char* file, int line);
@ -74,10 +74,10 @@ public:
static void assert_heaplocked_or_safepoint(const char* file, int line);
#ifdef ASSERT
#define shenandoah_assert_in_heap(interior_loc, obj) \
ShenandoahAsserts::assert_in_heap(interior_loc, obj, __FILE__, __LINE__)
#define shenandoah_assert_in_heap_or_null(interior_loc, obj) \
ShenandoahAsserts::assert_in_heap_or_null(interior_loc, obj, __FILE__, __LINE__)
#define shenandoah_assert_in_heap_bounds(interior_loc, obj) \
ShenandoahAsserts::assert_in_heap_bounds(interior_loc, obj, __FILE__, __LINE__)
#define shenandoah_assert_in_heap_bounds_or_null(interior_loc, obj) \
ShenandoahAsserts::assert_in_heap_bounds_or_null(interior_loc, obj, __FILE__, __LINE__)
#define shenandoah_assert_in_correct_region(interior_loc, obj) \
ShenandoahAsserts::assert_in_correct_region(interior_loc, obj, __FILE__, __LINE__)
@ -164,8 +164,8 @@ public:
#define shenandoah_assert_heaplocked_or_safepoint() \
ShenandoahAsserts::assert_heaplocked_or_safepoint(__FILE__, __LINE__)
#else
#define shenandoah_assert_in_heap(interior_loc, obj)
#define shenandoah_assert_in_heap_or_null(interior_loc, obj)
#define shenandoah_assert_in_heap_bounds(interior_loc, obj)
#define shenandoah_assert_in_heap_bounds_or_null(interior_loc, obj)
#define shenandoah_assert_in_correct_region(interior_loc, obj)
#define shenandoah_assert_correct_if(interior_loc, obj, condition)

View File

@ -41,12 +41,12 @@ bool ShenandoahCollectionSet::is_in(ShenandoahHeapRegion* r) const {
}
bool ShenandoahCollectionSet::is_in(oop p) const {
shenandoah_assert_in_heap_or_null(nullptr, p);
shenandoah_assert_in_heap_bounds_or_null(nullptr, p);
return is_in_loc(cast_from_oop<void*>(p));
}
bool ShenandoahCollectionSet::is_in_loc(void* p) const {
assert(p == nullptr || _heap->is_in(p), "Must be in the heap");
assert(p == nullptr || _heap->is_in_reserved(p), "Must be in the heap");
uintx index = ((uintx) p) >> _region_size_bytes_shift;
// no need to subtract the bottom of the heap from p,
// _biased_cset_map is biased

View File

@ -718,7 +718,7 @@ void ShenandoahEvacUpdateCleanupOopStorageRootsClosure::do_oop(oop* p) {
const oop obj = RawAccess<>::oop_load(p);
if (!CompressedOops::is_null(obj)) {
if (!_mark_context->is_marked(obj)) {
shenandoah_assert_correct(p, obj);
// Note: The obj is dead here. Do not touch it, just clear.
ShenandoahHeap::atomic_clear_oop(p, obj);
} else if (_evac_in_progress && _heap->in_collection_set(obj)) {
oop resolved = ShenandoahBarrierSet::resolve_forwarded_not_null(obj);

View File

@ -32,7 +32,7 @@
#include "runtime/javaThread.hpp"
inline oop ShenandoahForwarding::get_forwardee_raw(oop obj) {
shenandoah_assert_in_heap(nullptr, obj);
shenandoah_assert_in_heap_bounds(nullptr, obj);
return get_forwardee_raw_unchecked(obj);
}

View File

@ -730,9 +730,18 @@ size_t ShenandoahHeap::initial_capacity() const {
}
bool ShenandoahHeap::is_in(const void* p) const {
HeapWord* heap_base = (HeapWord*) base();
HeapWord* last_region_end = heap_base + ShenandoahHeapRegion::region_size_words() * num_regions();
return p >= heap_base && p < last_region_end;
if (is_in_reserved(p)) {
if (is_full_gc_move_in_progress()) {
// Full GC move is running, we do not have a consistent region
// information yet. But we know the pointer is in heap.
return true;
}
// Now check if we point to a live section in active region.
ShenandoahHeapRegion* r = heap_region_containing(p);
return (r->is_active() && p < r->top());
} else {
return false;
}
}
void ShenandoahHeap::maybe_uncommit(double shrink_before, size_t shrink_until) {

View File

@ -493,6 +493,8 @@ private:
public:
bool is_maximal_no_gc() const override shenandoah_not_implemented_return(false);
// Check the pointer is in active part of Java heap.
// Use is_in_reserved to check if object is within heap bounds.
bool is_in(const void* p) const override;
bool requires_barriers(stackChunkOop obj) const override;

View File

@ -122,7 +122,7 @@ void ShenandoahMarkBitMap::clear_range_large(MemRegion mr) {
#ifdef ASSERT
void ShenandoahMarkBitMap::check_mark(HeapWord* addr) const {
assert(ShenandoahHeap::heap()->is_in(addr),
assert(ShenandoahHeap::heap()->is_in_reserved(addr),
"Trying to access bitmap " PTR_FORMAT " for address " PTR_FORMAT " not in the heap.",
p2i(this), p2i(addr));
}

View File

@ -63,8 +63,10 @@ public:
inline bool mark_weak(oop obj);
// Simple versions of marking accessors, to be used outside of marking (e.g. no possible concurrent updates)
inline bool is_marked(oop) const;
inline bool is_marked(oop obj) const;
inline bool is_marked(HeapWord* raw_obj) const;
inline bool is_marked_strong(oop obj) const;
inline bool is_marked_strong(HeapWord* raw_obj) const;
inline bool is_marked_weak(oop obj) const;
inline HeapWord* get_next_marked_addr(HeapWord* addr, HeapWord* limit) const;

View File

@ -38,11 +38,19 @@ inline bool ShenandoahMarkingContext::mark_weak(oop obj) {
}
inline bool ShenandoahMarkingContext::is_marked(oop obj) const {
return allocated_after_mark_start(obj) || _mark_bit_map.is_marked(cast_from_oop<HeapWord *>(obj));
return is_marked(cast_from_oop<HeapWord*>(obj));
}
inline bool ShenandoahMarkingContext::is_marked(HeapWord* raw_obj) const {
return allocated_after_mark_start(raw_obj) || _mark_bit_map.is_marked(raw_obj);
}
inline bool ShenandoahMarkingContext::is_marked_strong(oop obj) const {
return allocated_after_mark_start(obj) || _mark_bit_map.is_marked_strong(cast_from_oop<HeapWord*>(obj));
return is_marked_strong(cast_from_oop<HeapWord*>(obj));
}
inline bool ShenandoahMarkingContext::is_marked_strong(HeapWord* raw_obj) const {
return allocated_after_mark_start(raw_obj) || _mark_bit_map.is_marked_strong(raw_obj);
}
inline bool ShenandoahMarkingContext::is_marked_weak(oop obj) const {

View File

@ -83,10 +83,21 @@ static volatile T* reference_referent_addr(oop reference) {
return (volatile T*)java_lang_ref_Reference::referent_addr_raw(reference);
}
inline oop reference_coop_decode_raw(narrowOop v) {
return CompressedOops::is_null(v) ? nullptr : CompressedOops::decode_raw(v);
}
inline oop reference_coop_decode_raw(oop v) {
return v;
}
// Raw referent, it can be dead. You cannot treat it as oop without additional safety
// checks, this is why it is HeapWord*. The decoding uses a special-case inlined
// CompressedOops::decode method that bypasses normal oop-ness checks.
template <typename T>
static oop reference_referent(oop reference) {
T heap_oop = Atomic::load(reference_referent_addr<T>(reference));
return CompressedOops::decode(heap_oop);
static HeapWord* reference_referent_raw(oop reference) {
T raw_oop = Atomic::load(reference_referent_addr<T>(reference));
return cast_from_oop<HeapWord*>(reference_coop_decode_raw(raw_oop));
}
static void reference_clear_referent(oop reference) {
@ -278,8 +289,8 @@ bool ShenandoahReferenceProcessor::should_discover(oop reference, ReferenceType
template <typename T>
bool ShenandoahReferenceProcessor::should_drop(oop reference, ReferenceType type) const {
const oop referent = reference_referent<T>(reference);
if (referent == nullptr) {
HeapWord* raw_referent = reference_referent_raw<T>(reference);
if (raw_referent == nullptr) {
// Reference has been cleared, by a call to Reference.enqueue()
// or Reference.clear() from the application, which means we
// should drop the reference.
@ -289,9 +300,9 @@ bool ShenandoahReferenceProcessor::should_drop(oop reference, ReferenceType type
// Check if the referent is still alive, in which case we should
// drop the reference.
if (type == REF_PHANTOM) {
return ShenandoahHeap::heap()->complete_marking_context()->is_marked(referent);
return ShenandoahHeap::heap()->complete_marking_context()->is_marked(raw_referent);
} else {
return ShenandoahHeap::heap()->complete_marking_context()->is_marked_strong(referent);
return ShenandoahHeap::heap()->complete_marking_context()->is_marked_strong(raw_referent);
}
}
@ -303,7 +314,7 @@ void ShenandoahReferenceProcessor::make_inactive(oop reference, ReferenceType ty
// next field. An application can't call FinalReference.enqueue(), so there is
// no race to worry about when setting the next field.
assert(reference_next<T>(reference) == nullptr, "Already inactive");
assert(ShenandoahHeap::heap()->marking_context()->is_marked(reference_referent<T>(reference)), "only make inactive final refs with alive referents");
assert(ShenandoahHeap::heap()->marking_context()->is_marked(reference_referent_raw<T>(reference)), "only make inactive final refs with alive referents");
reference_set_next(reference, reference);
} else {
// Clear referent
@ -376,8 +387,8 @@ oop ShenandoahReferenceProcessor::drop(oop reference, ReferenceType type) {
log_trace(gc, ref)("Dropped Reference: " PTR_FORMAT " (%s)", p2i(reference), reference_type_name(type));
#ifdef ASSERT
oop referent = reference_referent<T>(reference);
assert(referent == nullptr || ShenandoahHeap::heap()->marking_context()->is_marked(referent),
HeapWord* raw_referent = reference_referent_raw<T>(reference);
assert(raw_referent == nullptr || ShenandoahHeap::heap()->marking_context()->is_marked(raw_referent),
"only drop references with alive referents");
#endif

View File

@ -51,13 +51,6 @@ static bool is_instance_ref_klass(Klass* k) {
return k->is_instance_klass() && InstanceKlass::cast(k)->reference_type() != REF_NONE;
}
class ShenandoahIgnoreReferenceDiscoverer : public ReferenceDiscoverer {
public:
virtual bool discover_reference(oop obj, ReferenceType type) {
return true;
}
};
class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
private:
const char* _phase;
@ -68,6 +61,7 @@ private:
ShenandoahLivenessData* _ld;
void* _interior_loc;
oop _loc;
ReferenceIterationMode _ref_mode;
public:
ShenandoahVerifyOopClosure(ShenandoahVerifierStack* stack, MarkBitMap* map, ShenandoahLivenessData* ld,
@ -82,10 +76,20 @@ public:
_loc(nullptr) {
if (options._verify_marked == ShenandoahVerifier::_verify_marked_complete_except_references ||
options._verify_marked == ShenandoahVerifier::_verify_marked_disable) {
set_ref_discoverer_internal(new ShenandoahIgnoreReferenceDiscoverer());
// Unknown status for Reference.referent field. Do not touch it, it might be dead.
// Normally, barriers would prevent us from seeing the dead referents, but verifier
// runs with barriers disabled.
_ref_mode = DO_FIELDS_EXCEPT_REFERENT;
} else {
// Otherwise do all fields.
_ref_mode = DO_FIELDS;
}
}
ReferenceIterationMode reference_iteration_mode() override {
return _ref_mode;
}
private:
void check(ShenandoahAsserts::SafeLevel level, oop obj, bool test, const char* label) {
if (!test) {
@ -119,8 +123,8 @@ private:
// that failure report would not try to touch something that was not yet verified to be
// safe to process.
check(ShenandoahAsserts::_safe_unknown, obj, _heap->is_in(obj),
"oop must be in heap");
check(ShenandoahAsserts::_safe_unknown, obj, _heap->is_in_reserved(obj),
"oop must be in heap bounds");
check(ShenandoahAsserts::_safe_unknown, obj, is_object_aligned(obj),
"oop must be aligned");
@ -177,8 +181,8 @@ private:
ShenandoahHeapRegion* fwd_reg = nullptr;
if (obj != fwd) {
check(ShenandoahAsserts::_safe_oop, obj, _heap->is_in(fwd),
"Forwardee must be in heap");
check(ShenandoahAsserts::_safe_oop, obj, _heap->is_in_reserved(fwd),
"Forwardee must be in heap bounds");
check(ShenandoahAsserts::_safe_oop, obj, !CompressedOops::is_null(fwd),
"Forwardee is set");
check(ShenandoahAsserts::_safe_oop, obj, is_object_aligned(fwd),
@ -195,6 +199,9 @@ private:
fwd_reg = _heap->heap_region_containing(fwd);
check(ShenandoahAsserts::_safe_oop, obj, fwd_reg->is_active(),
"Forwardee should be in active region");
// Verify that forwardee is not in the dead space:
check(ShenandoahAsserts::_safe_oop, obj, !fwd_reg->is_humongous(),
"Should have no humongous forwardees");
@ -324,8 +331,8 @@ public:
_loc = nullptr;
}
virtual void do_oop(oop* p) { do_oop_work(p); }
virtual void do_oop(narrowOop* p) { do_oop_work(p); }
virtual void do_oop(oop* p) override { do_oop_work(p); }
virtual void do_oop(narrowOop* p) override { do_oop_work(p); }
};
class ShenandoahCalculateRegionStatsClosure : public ShenandoahHeapRegionClosure {