From 27ec3dbebd8539cd948649aed749b006a73d764e Mon Sep 17 00:00:00 2001 From: tstuefe Date: Thu, 10 Jul 2025 09:55:59 +0200 Subject: [PATCH 1/7] assert_correct --- .../share/gc/shenandoah/shenandoahAsserts.cpp | 63 ++++++++++++++----- src/hotspot/share/oops/compressedKlass.hpp | 3 + .../share/oops/compressedKlass.inline.hpp | 5 ++ src/hotspot/share/oops/oop.hpp | 1 + src/hotspot/share/oops/oop.inline.hpp | 11 ++++ 5 files changed, 66 insertions(+), 17 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp index 7c9fa7598355f..b83dae835e3e3 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp @@ -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: @@ -205,25 +208,13 @@ 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)) { - print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", - "Object klass pointer must go to metaspace", - file,line); - } - - if (!heap->is_in(obj)) { + if (!os::is_readable_pointer(obj)) { print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", - "Object should be in active region area", + "oop within heap bounds but at unreadable location", file, line); } + // Since we may need the forwardee (with +COH) to extract the Klass*, check it first oop fwd = ShenandoahForwarding::get_forwardee_raw_unchecked(obj); if (obj != fwd) { @@ -243,9 +234,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 +262,44 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char* } } + // Extract klass safely + const Klass* obj_klass = nullptr; + if (UseCompressedClassPointers) { + const narrowKlass nk = fwd->narrow_klass(); + if (!CompressedKlassPointers::is_valid_narrow_klass_id(nk)) { + print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", + "Object narrow klass pointer invalid outside range", + file,line); + } + obj_klass = CompressedKlassPointers::decode_without_asserts(nk); + } else { + obj_klass = fwd->klass_or_null(); + } + + 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)) { + print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", + "Object klass pointer must go to metaspace", + file,line); + } + + if (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); + } + + 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); + } + // 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 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"); From 18de09eb1f30a5f026898904895a0ae32df34d54 Mon Sep 17 00:00:00 2001 From: tstuefe Date: Thu, 10 Jul 2025 14:03:00 +0200 Subject: [PATCH 2/7] wip --- .../share/gc/shenandoah/shenandoahAsserts.cpp | 78 +++++++++++++------ .../share/gc/shenandoah/shenandoahAsserts.hpp | 11 +++ .../gc/shenandoah/shenandoahVerifier.cpp | 12 ++- .../gtest/oops/test_compressedKlass.cpp | 10 +++ 4 files changed, 86 insertions(+), 25 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp index b83dae835e3e3..16a4e022a0c8e 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp @@ -67,9 +67,17 @@ void ShenandoahAsserts::print_obj(ShenandoahMessageBuffer& msg, oop obj) { ShenandoahMarkingContext* const ctx = heap->marking_context(); - Klass* obj_klass = ShenandoahForwarding::klass(obj); + 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(); + } - msg.append(" " PTR_FORMAT " - klass " PTR_FORMAT " %s\n", p2i(obj), p2i(obj_klass), obj_klass->external_name()); + msg.append(" " PTR_FORMAT " - nk %u klass " PTR_FORMAT " %s\n", p2i(obj), nk, p2i(obj_klass), klass_text); 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"); @@ -124,6 +132,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); @@ -131,7 +143,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); @@ -153,7 +165,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); @@ -214,6 +226,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); + } + // Since we may need the forwardee (with +COH) to extract the Klass*, check it first oop fwd = ShenandoahForwarding::get_forwardee_raw_unchecked(obj); @@ -262,18 +280,12 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char* } } - // Extract klass safely const Klass* obj_klass = nullptr; - if (UseCompressedClassPointers) { - const narrowKlass nk = fwd->narrow_klass(); - if (!CompressedKlassPointers::is_valid_narrow_klass_id(nk)) { - print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", - "Object narrow klass pointer invalid outside range", - file,line); - } - obj_klass = CompressedKlassPointers::decode_without_asserts(nk); - } else { - obj_klass = fwd->klass_or_null(); + narrowKlass nk = 0; + if (!extract_klass_safely(obj, nk, obj_klass)) { + print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", + "Object klass pointer invalid", + file,line); } if (obj_klass == nullptr) { @@ -288,18 +300,12 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char* file,line); } - if (obj_klass != fwd->klass_or_null()) { + 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); } - 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); - } - // 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 @@ -548,3 +554,31 @@ 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()); } + +// Given a possibly invalid oop, extract narrowKlass (if UCCP) and Klass* from it safely, with checks. +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..2f6614251297c 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp @@ -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,16 @@ 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. + // Returns: + // - false, nk=0 && k=0 if oop is unreadable or (+COH) is forwarded and forwardee is unreadable + // or oop's narrowKlass was 0 + // - false, nk>0 && k=0 if narrowKlass is garbage + // - true, nk>0 && k!=0 if Klass* was successfully extracted. No further validity checks are done on the Klass*. + // 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..63a05732424d5 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp @@ -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/test/hotspot/gtest/oops/test_compressedKlass.cpp b/test/hotspot/gtest/oops/test_compressedKlass.cpp index 56bfd29782f1d..3ef576e073215 100644 --- a/test/hotspot/gtest/oops/test_compressedKlass.cpp +++ b/test/hotspot/gtest/oops/test_compressedKlass.cpp @@ -22,6 +22,7 @@ * questions. */ +#include "classfile/vmClasses.hpp" #include "oops/compressedKlass.inline.hpp" #include "utilities/globalDefinitions.hpp" @@ -107,3 +108,12 @@ 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)); +} From 2a5c9c0b915b369a7f530c1b561ff615803cd40f Mon Sep 17 00:00:00 2001 From: tstuefe Date: Thu, 10 Jul 2025 14:39:04 +0200 Subject: [PATCH 3/7] wip --- src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp | 1 - src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp | 5 ----- test/hotspot/gtest/oops/test_compressedKlass.cpp | 3 +++ 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp index 16a4e022a0c8e..be2b2ca83340f 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp @@ -555,7 +555,6 @@ void ShenandoahAsserts::assert_generations_reconciled(const char* file, int line report_vm_error(file, line, msg.buffer()); } -// Given a possibly invalid oop, extract narrowKlass (if UCCP) and Klass* from it safely, with checks. bool ShenandoahAsserts::extract_klass_safely(oop obj, narrowKlass& nk, const Klass*& k) { nk = 0; k = nullptr; diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp index 2f6614251297c..7f5ee40f6a76b 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp @@ -80,11 +80,6 @@ class ShenandoahAsserts { // Given a possibly invalid oop, extract narrowKlass (if UCCP) and Klass* // from it safely. - // Returns: - // - false, nk=0 && k=0 if oop is unreadable or (+COH) is forwarded and forwardee is unreadable - // or oop's narrowKlass was 0 - // - false, nk>0 && k=0 if narrowKlass is garbage - // - true, nk>0 && k!=0 if Klass* was successfully extracted. No further validity checks are done on the Klass*. // Note: For -UCCP, returned nk is always 0. static bool extract_klass_safely(oop obj, narrowKlass& nk, const Klass*& k); diff --git a/test/hotspot/gtest/oops/test_compressedKlass.cpp b/test/hotspot/gtest/oops/test_compressedKlass.cpp index 3ef576e073215..b9ee5c7eab629 100644 --- a/test/hotspot/gtest/oops/test_compressedKlass.cpp +++ b/test/hotspot/gtest/oops/test_compressedKlass.cpp @@ -116,4 +116,7 @@ TEST_VM(CompressedKlass, test_is_valid_narrow_klass) { 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)); + } } From c7aa454b0553411049ce7920725937fb292cafc9 Mon Sep 17 00:00:00 2001 From: tstuefe Date: Fri, 11 Jul 2025 15:55:40 +0200 Subject: [PATCH 4/7] wip --- .../share/gc/shenandoah/shenandoahAsserts.cpp | 48 +++++++++++-------- .../share/gc/shenandoah/shenandoahAsserts.hpp | 2 +- .../gc/shenandoah/shenandoahVerifier.cpp | 2 +- src/hotspot/share/utilities/ostream.hpp | 2 +- .../gtest/oops/test_compressedKlass.cpp | 2 +- 5 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp index be2b2ca83340f..0a9b87544085f 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 @@ -60,10 +60,7 @@ 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(); @@ -77,21 +74,34 @@ void ShenandoahAsserts::print_obj(ShenandoahMessageBuffer& msg, oop obj) { klass_text = obj_klass->external_name(); } - msg.append(" " PTR_FORMAT " - nk %u klass " PTR_FORMAT " %s\n", p2i(obj), nk, p2i(obj_klass), klass_text); - 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()))); + 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()) { + ss.print_cr("mirrored klass: " PTR_FORMAT, p2i(obj->metadata_field(java_lang_Class::klass_offset()))); + ss.print_cr("mirrored array klass: " PTR_FORMAT, p2i(obj->metadata_field(java_lang_Class::array_klass_offset()))); + } } + + static constexpr int num_bytes = 64; + const_address loc = cast_from_oop(obj); + os::print_hex_dump(&ss, loc, loc + num_bytes, 8, true, 32, loc); + + msg.append("%s", ss.base()); } void ShenandoahAsserts::print_non_obj(ShenandoahMessageBuffer& msg, void* loc) { diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp index 7f5ee40f6a76b..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. * diff --git a/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp b/src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp index 63a05732424d5..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. 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 b9ee5c7eab629..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. * From aab9d9c93af9ee9847cc368fadcf5534511a6dd9 Mon Sep 17 00:00:00 2001 From: tstuefe Date: Fri, 11 Jul 2025 16:42:00 +0200 Subject: [PATCH 5/7] wip --- .../share/gc/shenandoah/shenandoahAsserts.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp index 0a9b87544085f..d1875bfb7a8d3 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp @@ -92,14 +92,18 @@ void ShenandoahAsserts::print_obj(ShenandoahMessageBuffer& msg, oop obj) { r->print_on(&ss); ss.cr(); if (obj_klass == vmClasses::Class_klass()) { - ss.print_cr("mirrored klass: " PTR_FORMAT, p2i(obj->metadata_field(java_lang_Class::klass_offset()))); - ss.print_cr("mirrored array klass: " PTR_FORMAT, p2i(obj->metadata_field(java_lang_Class::array_klass_offset()))); + const Klass* mk = (const Klass*) obj->metadata_field(java_lang_Class::klass_offset()); + const bool mk_valid = Metaspace::contains(mk); + const Klass* amk = (const Klass*) obj->metadata_field(java_lang_Class::array_klass_offset()); + const bool amk_valid = Metaspace::contains(amk); + ss.print_cr("mirrored klass: " PTR_FORMAT " %s", p2i(mk), mk_valid ? "(in metaspace)" : "(invalid, not in metaspace)"); + ss.print_cr("mirrored array klass: " PTR_FORMAT " %s", p2i(amk), amk_valid ? "(in metaspace)" : "(invalid, not in metaspace)"); } } static constexpr int num_bytes = 64; const_address loc = cast_from_oop(obj); - os::print_hex_dump(&ss, loc, loc + num_bytes, 8, true, 32, loc); + os::print_hex_dump(&ss, loc, loc + num_bytes, 4, true, 32, loc); msg.append("%s", ss.base()); } @@ -246,6 +250,7 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char* oop fwd = ShenandoahForwarding::get_forwardee_raw_unchecked(obj); if (obj != fwd) { + // When Full GC moves the objects, we cannot trust fwdptrs. If we got here, it means something // tries fwdptr manipulation when Full GC is running. The only exception is using the fwdptr // that still points to the object itself. @@ -293,19 +298,19 @@ 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_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", + 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_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", + 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_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", + print_failure(_safe_oop, obj, interior_loc, nullptr, "Shenandoah assert_correct failed", "Object klass pointer must go to metaspace", file,line); } From e4e44046e07940e64071927566d2613f07bd0451 Mon Sep 17 00:00:00 2001 From: tstuefe Date: Fri, 11 Jul 2025 16:46:19 +0200 Subject: [PATCH 6/7] cosmetics --- .../share/gc/shenandoah/shenandoahAsserts.cpp | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp index d1875bfb7a8d3..d9249608d0d13 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp @@ -68,12 +68,9 @@ void ShenandoahAsserts::print_obj(ShenandoahMessageBuffer& msg, oop obj) { 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)) { + 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); @@ -100,11 +97,8 @@ void ShenandoahAsserts::print_obj(ShenandoahMessageBuffer& msg, oop obj) { ss.print_cr("mirrored array klass: " PTR_FORMAT " %s", p2i(amk), amk_valid ? "(in metaspace)" : "(invalid, not in metaspace)"); } } - - static constexpr int num_bytes = 64; const_address loc = cast_from_oop(obj); - os::print_hex_dump(&ss, loc, loc + num_bytes, 4, true, 32, loc); - + os::print_hex_dump(&ss, loc, loc + 64, 4, true, 32, loc); msg.append("%s", ss.base()); } @@ -246,11 +240,9 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char* file, line); } - // Since we may need the forwardee (with +COH) to extract the Klass*, check it first oop fwd = ShenandoahForwarding::get_forwardee_raw_unchecked(obj); if (obj != fwd) { - // When Full GC moves the objects, we cannot trust fwdptrs. If we got here, it means something // tries fwdptr manipulation when Full GC is running. The only exception is using the fwdptr // that still points to the object itself. From 94fe071e0b3ae779efba5697b75b5331319061b2 Mon Sep 17 00:00:00 2001 From: tstuefe Date: Sat, 12 Jul 2025 07:47:00 +0200 Subject: [PATCH 7/7] small revert --- src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp index d9249608d0d13..0ffdc40082668 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp @@ -89,12 +89,8 @@ void ShenandoahAsserts::print_obj(ShenandoahMessageBuffer& msg, oop obj) { r->print_on(&ss); ss.cr(); if (obj_klass == vmClasses::Class_klass()) { - const Klass* mk = (const Klass*) obj->metadata_field(java_lang_Class::klass_offset()); - const bool mk_valid = Metaspace::contains(mk); - const Klass* amk = (const Klass*) obj->metadata_field(java_lang_Class::array_klass_offset()); - const bool amk_valid = Metaspace::contains(amk); - ss.print_cr("mirrored klass: " PTR_FORMAT " %s", p2i(mk), mk_valid ? "(in metaspace)" : "(invalid, not in metaspace)"); - ss.print_cr("mirrored array klass: " PTR_FORMAT " %s", p2i(amk), amk_valid ? "(in metaspace)" : "(invalid, not in metaspace)"); + 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);