diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp index 7c9fa7598355f..0ffdc40082668 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 2020, Red Hat, Inc. All rights reserved. + * Copyright (c) 2018, 2025, Red Hat, Inc. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -29,7 +29,10 @@ #include "gc/shenandoah/shenandoahHeapRegionSet.inline.hpp" #include "gc/shenandoah/shenandoahMarkingContext.inline.hpp" #include "gc/shenandoah/shenandoahUtils.hpp" +#include "oops/oop.inline.hpp" #include "memory/resourceArea.hpp" +#include "runtime/os.hpp" +#include "utilities/vmError.hpp" void print_raw_memory(ShenandoahMessageBuffer &msg, void* loc) { // Be extra safe. Only access data that is guaranteed to be safe: @@ -57,30 +60,42 @@ void ShenandoahAsserts::print_obj(ShenandoahMessageBuffer& msg, oop obj) { ResourceMark rm; stringStream ss; - r->print_on(&ss); - - stringStream mw_ss; - obj->mark().print_on(&mw_ss); + StreamIndentor si(&ss); ShenandoahMarkingContext* const ctx = heap->marking_context(); - Klass* obj_klass = ShenandoahForwarding::klass(obj); - - msg.append(" " PTR_FORMAT " - klass " PTR_FORMAT " %s\n", p2i(obj), p2i(obj_klass), obj_klass->external_name()); - msg.append(" %3s allocated after mark start\n", ctx->allocated_after_mark_start(obj) ? "" : "not"); - msg.append(" %3s after update watermark\n", cast_from_oop(obj) >= r->get_update_watermark() ? "" : "not"); - msg.append(" %3s marked strong\n", ctx->is_marked_strong(obj) ? "" : "not"); - msg.append(" %3s marked weak\n", ctx->is_marked_weak(obj) ? "" : "not"); - msg.append(" %3s in collection set\n", heap->in_collection_set(obj) ? "" : "not"); - if (heap->mode()->is_generational() && !obj->is_forwarded()) { - msg.append(" age: %d\n", obj->age()); - } - msg.append(" mark:%s\n", mw_ss.freeze()); - msg.append(" region: %s", ss.freeze()); - if (obj_klass == vmClasses::Class_klass()) { - msg.append(" mirrored klass: " PTR_FORMAT "\n", p2i(obj->metadata_field(java_lang_Class::klass_offset()))); - msg.append(" mirrored array klass: " PTR_FORMAT "\n", p2i(obj->metadata_field(java_lang_Class::array_klass_offset()))); + narrowKlass nk = 0; + const Klass* obj_klass = nullptr; + const bool klass_valid = extract_klass_safely(obj, nk, obj_klass); + const char* klass_text = "(invalid)"; + if (klass_valid && os::is_readable_pointer(obj_klass) && Metaspace::contains(obj_klass)) { + klass_text = obj_klass->external_name(); + } + ss.print_cr(PTR_FORMAT " - nk %u klass " PTR_FORMAT " %s\n", p2i(obj), nk, p2i(obj_klass), klass_text); + { + StreamIndentor si(&ss); + ss.print_cr("%3s allocated after mark start", ctx->allocated_after_mark_start(obj) ? "" : "not"); + ss.print_cr("%3s after update watermark", cast_from_oop(obj) >= r->get_update_watermark() ? "" : "not"); + ss.print_cr("%3s marked strong", ctx->is_marked_strong(obj) ? "" : "not"); + ss.print_cr("%3s marked weak", ctx->is_marked_weak(obj) ? "" : "not"); + ss.print_cr("%3s in collection set", heap->in_collection_set(obj) ? "" : "not"); + if (heap->mode()->is_generational() && !obj->is_forwarded()) { + ss.print_cr("age: %d", obj->age()); + } + ss.print_raw("mark: "); + obj->mark().print_on(&ss); + ss.cr(); + ss.print_raw("region: "); + r->print_on(&ss); + ss.cr(); + if (obj_klass == vmClasses::Class_klass()) { + msg.append(" mirrored klass: " PTR_FORMAT "\n", p2i(obj->metadata_field(java_lang_Class::klass_offset()))); + msg.append(" mirrored array klass: " PTR_FORMAT "\n", p2i(obj->metadata_field(java_lang_Class::array_klass_offset()))); + } } + const_address loc = cast_from_oop(obj); + os::print_hex_dump(&ss, loc, loc + 64, 4, true, 32, loc); + msg.append("%s", ss.base()); } void ShenandoahAsserts::print_non_obj(ShenandoahMessageBuffer& msg, void* loc) { @@ -121,6 +136,10 @@ void ShenandoahAsserts::print_failure(SafeLevel level, oop obj, void* interior_l ShenandoahHeap* heap = ShenandoahHeap::heap(); ResourceMark rm; + if (!os::is_readable_pointer(obj)) { + level = _safe_unknown; + } + bool loc_in_heap = (loc != nullptr && heap->is_in_reserved(loc)); ShenandoahMessageBuffer msg("%s; %s\n\n", phase, label); @@ -128,7 +147,7 @@ void ShenandoahAsserts::print_failure(SafeLevel level, oop obj, void* interior_l msg.append("Referenced from:\n"); if (interior_loc != nullptr) { msg.append(" interior location: " PTR_FORMAT "\n", p2i(interior_loc)); - if (loc_in_heap) { + if (loc_in_heap && os::is_readable_pointer(loc)) { print_obj(msg, loc); } else { print_non_obj(msg, interior_loc); @@ -150,7 +169,7 @@ void ShenandoahAsserts::print_failure(SafeLevel level, oop obj, void* interior_l oop fwd = ShenandoahForwarding::get_forwardee_raw_unchecked(obj); msg.append("Forwardee:\n"); if (obj != fwd) { - if (level >= _safe_oop_fwd) { + if (level >= _safe_oop_fwd && os::is_readable_pointer(fwd)) { print_obj(msg, fwd); } else { print_obj_safe(msg, fwd); @@ -205,17 +224,10 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char* file, line); } - Klass* obj_klass = ShenandoahForwarding::klass(obj); - if (obj_klass == nullptr) { - print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", - "Object klass pointer should not be null", - file,line); - } - - if (!Metaspace::contains(obj_klass)) { + if (!os::is_readable_pointer(obj)) { print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", - "Object klass pointer must go to metaspace", - file,line); + "oop within heap bounds but at unreadable location", + file, line); } if (!heap->is_in(obj)) { @@ -243,9 +255,9 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char* file, line); } - if (obj_klass != ShenandoahForwarding::klass(fwd)) { + if (!os::is_readable_pointer(fwd)) { print_failure(_safe_oop, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", - "Forwardee klass disagrees with object class", + "Forwardee within heap bounds but at unreadable location", file, line); } @@ -271,6 +283,32 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char* } } + const Klass* obj_klass = nullptr; + narrowKlass nk = 0; + if (!extract_klass_safely(obj, nk, obj_klass)) { + print_failure(_safe_oop, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", + "Object klass pointer invalid", + file,line); + } + + if (obj_klass == nullptr) { + print_failure(_safe_oop, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", + "Object klass pointer should not be null", + file,line); + } + + if (!Metaspace::contains(obj_klass)) { + print_failure(_safe_oop, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", + "Object klass pointer must go to metaspace", + file,line); + } + + if (!UseCompactObjectHeaders && obj_klass != fwd->klass_or_null()) { + print_failure(_safe_oop, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", + "Forwardee klass disagrees with object class", + file, line); + } + // Do additional checks for special objects: their fields can hold metadata as well. // We want to check class loading/unloading did not corrupt them. We can only reasonably // trust the forwarded objects, as the from-space object can have the klasses effectively @@ -519,3 +557,30 @@ void ShenandoahAsserts::assert_generations_reconciled(const char* file, int line ShenandoahMessageBuffer msg("Active(%d) & GC(%d) Generations aren't reconciled", agen->type(), ggen->type()); report_vm_error(file, line, msg.buffer()); } + +bool ShenandoahAsserts::extract_klass_safely(oop obj, narrowKlass& nk, const Klass*& k) { + nk = 0; + k = nullptr; + + if (!os::is_readable_pointer(obj)) { + return false; + } + if (UseCompressedClassPointers) { + if (UseCompactObjectHeaders) { // look in forwardee + oop fwd = ShenandoahForwarding::get_forwardee_raw_unchecked(obj); + if (!os::is_readable_pointer(fwd)) { + return false; + } + nk = fwd->mark().narrow_klass(); + } else { + nk = obj->narrow_klass(); + } + if (!CompressedKlassPointers::is_valid_narrow_klass_id(nk)) { + return false; + } + k = CompressedKlassPointers::decode_not_null_without_asserts(nk); + } else { + k = obj->klass(); + } + return k != nullptr; +} diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp index 31a99bf438cf9..e31ef7c99aae3 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 2019, Red Hat, Inc. All rights reserved. + * Copyright (c) 2018, 2025, Red Hat, Inc. All rights reserved. * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -27,6 +27,7 @@ #define SHARE_GC_SHENANDOAH_SHENANDOAHASSERTS_HPP #include "memory/iterator.hpp" +#include "oops/compressedKlass.hpp" #include "runtime/mutex.hpp" #include "utilities/formatBuffer.hpp" @@ -77,6 +78,11 @@ class ShenandoahAsserts { static void assert_generational(const char* file, int line); static void assert_generations_reconciled(const char* file, int line); + // Given a possibly invalid oop, extract narrowKlass (if UCCP) and Klass* + // from it safely. + // Note: For -UCCP, returned nk is always 0. + static bool extract_klass_safely(oop obj, narrowKlass& nk, const Klass*& k); + #ifdef ASSERT #define shenandoah_assert_in_heap_bounds(interior_loc, obj) \ ShenandoahAsserts::assert_in_heap_bounds(interior_loc, obj, __FILE__, __LINE__) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp b/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp index 727b90e82a2a5..33b8744be3d88 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2021, Red Hat, Inc. All rights reserved. + * Copyright (c) 2017, 2025, Red Hat, Inc. All rights reserved. * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. @@ -149,15 +149,21 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure { "oop must be in heap bounds"); check(ShenandoahAsserts::_safe_unknown, obj, is_object_aligned(obj), "oop must be aligned"); + check(ShenandoahAsserts::_safe_unknown, obj, os::is_readable_pointer(obj), + "oop must be accessible"); ShenandoahHeapRegion *obj_reg = _heap->heap_region_containing(obj); - Klass* obj_klass = ShenandoahForwarding::klass(obj); + + narrowKlass nk = 0; + const Klass* obj_klass = nullptr; + const bool klass_valid = ShenandoahAsserts::extract_klass_safely(obj, nk, obj_klass); + + check(ShenandoahAsserts::_safe_unknown, obj, klass_valid, + "Object klass pointer unreadable or invalid"); // Verify that obj is not in dead space: { // Do this before touching obj->size() - check(ShenandoahAsserts::_safe_unknown, obj, obj_klass != nullptr, - "Object klass pointer should not be null"); check(ShenandoahAsserts::_safe_unknown, obj, Metaspace::contains(obj_klass), "Object klass pointer must go to metaspace"); diff --git a/src/hotspot/share/oops/compressedKlass.hpp b/src/hotspot/share/oops/compressedKlass.hpp index a8d79ab838e9f..a2c6d177f430e 100644 --- a/src/hotspot/share/oops/compressedKlass.hpp +++ b/src/hotspot/share/oops/compressedKlass.hpp @@ -251,6 +251,9 @@ class CompressedKlassPointers : public AllStatic { inline static void check_valid_narrow_klass_id(narrowKlass nk); #endif + // Given a narrow Klass ID, returns true if it appears to be valid + inline static bool is_valid_narrow_klass_id(narrowKlass nk); + // Returns whether the pointer is in the memory region used for encoding compressed // class pointers. This includes CDS. static inline bool is_encodable(const void* addr) { diff --git a/src/hotspot/share/oops/compressedKlass.inline.hpp b/src/hotspot/share/oops/compressedKlass.inline.hpp index c80900b5ea081..b3634313c0760 100644 --- a/src/hotspot/share/oops/compressedKlass.inline.hpp +++ b/src/hotspot/share/oops/compressedKlass.inline.hpp @@ -93,6 +93,11 @@ inline void CompressedKlassPointers::check_valid_narrow_klass_id(narrowKlass nk) } #endif // ASSERT +// Given a narrow Klass ID, returns true if it appears to be valid +inline bool CompressedKlassPointers::is_valid_narrow_klass_id(narrowKlass nk) { + return nk >= _lowest_valid_narrow_klass_id && nk < _highest_valid_narrow_klass_id; +} + inline address CompressedKlassPointers::encoding_range_end() { const int max_bits = narrow_klass_pointer_bits() + _shift; return (address)((uintptr_t)_base + nth_bit(max_bits)); diff --git a/src/hotspot/share/oops/oop.hpp b/src/hotspot/share/oops/oop.hpp index 8048c8770c2ba..68ded0c9c0928 100644 --- a/src/hotspot/share/oops/oop.hpp +++ b/src/hotspot/share/oops/oop.hpp @@ -90,6 +90,7 @@ class oopDesc { inline Klass* klass_without_asserts() const; void set_narrow_klass(narrowKlass nk) NOT_CDS_JAVA_HEAP_RETURN; + inline narrowKlass narrow_klass() const; inline void set_klass(Klass* k); static inline void release_set_klass(HeapWord* mem, Klass* k); diff --git a/src/hotspot/share/oops/oop.inline.hpp b/src/hotspot/share/oops/oop.inline.hpp index 3dad778a73a47..9d3f3e6b43a24 100644 --- a/src/hotspot/share/oops/oop.inline.hpp +++ b/src/hotspot/share/oops/oop.inline.hpp @@ -141,6 +141,17 @@ Klass* oopDesc::klass_without_asserts() const { } } +narrowKlass oopDesc::narrow_klass() const { + switch (ObjLayout::klass_mode()) { + case ObjLayout::Compact: + return mark().narrow_klass(); + case ObjLayout::Compressed: + return _metadata._compressed_klass; + default: + ShouldNotReachHere(); + } +} + void oopDesc::set_klass(Klass* k) { assert(Universe::is_bootstrapping() || (k != nullptr && k->is_klass()), "incorrect Klass"); assert(!UseCompactObjectHeaders, "don't set Klass* with compact headers"); diff --git a/src/hotspot/share/utilities/ostream.hpp b/src/hotspot/share/utilities/ostream.hpp index 79e95734a5345..a148557fd324a 100644 --- a/src/hotspot/share/utilities/ostream.hpp +++ b/src/hotspot/share/utilities/ostream.hpp @@ -182,7 +182,7 @@ class StreamIndentor { NONCOPYABLE(StreamIndentor); public: - StreamIndentor(outputStream* os, int indentation) : + StreamIndentor(outputStream* os, int indentation = 2) : _stream(os), _indentation(indentation), _old_autoindent(_stream->set_autoindent(true)) { diff --git a/test/hotspot/gtest/oops/test_compressedKlass.cpp b/test/hotspot/gtest/oops/test_compressedKlass.cpp index 56bfd29782f1d..c8120e3519dbf 100644 --- a/test/hotspot/gtest/oops/test_compressedKlass.cpp +++ b/test/hotspot/gtest/oops/test_compressedKlass.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, Red Hat, Inc. All rights reserved. + * Copyright (c) 2024, 2025, Red Hat, Inc. All rights reserved. * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -22,6 +22,7 @@ * questions. */ +#include "classfile/vmClasses.hpp" #include "oops/compressedKlass.inline.hpp" #include "utilities/globalDefinitions.hpp" @@ -107,3 +108,15 @@ TEST_VM(CompressedKlass, test_good_address) { addr = CompressedKlassPointers::klass_range_end() - alignment; ASSERT_TRUE(CompressedKlassPointers::is_encodable(addr)); } + +TEST_VM(CompressedKlass, test_is_valid_narrow_klass) { + if (!UseCompressedClassPointers) { + return; + } + ASSERT_FALSE(CompressedKlassPointers::is_valid_narrow_klass_id(0)); + narrowKlass nk_jlC = CompressedKlassPointers::encode((Klass*)vmClasses::Class_klass()); + ASSERT_TRUE(CompressedKlassPointers::is_valid_narrow_klass_id(nk_jlC)); + if (CompressedClassSpaceSize < 4 * G && CompressedKlassPointers::base() != nullptr) { + ASSERT_FALSE(CompressedKlassPointers::is_valid_narrow_klass_id(0xFFFFFFFF)); + } +}