8361712: Improve ShenandoahAsserts printing

Reviewed-by: rkennke, asmehra

Author: tstuefe

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<HeapWord*>(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<HeapWord*>(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<const_address>(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,14 +136,18 @@ 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);
 
   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;
+}

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__)

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");
 

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) {

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));

src/hotspot/share/oops/oop.hpp

@@ -91,6 +91,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);
 

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");

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)) {

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));
+  }
+}