8301988: VerifyLiveClosure::verify_liveness asserts on bad pointers outside heap

Reviewed-by: dholmes, ayang

Author: Thomas Schatzl

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

@@ -239,7 +239,7 @@ inline bool G1CollectedHeap::requires_barriers(stackChunkOop obj) const {
 }
 
 inline bool G1CollectedHeap::is_obj_filler(const oop obj) {
-  Klass* k = obj->klass();
+  Klass* k = obj->klass_raw();
   return k == Universe::fillerArrayKlassObj() || k == vmClasses::FillerObject_klass();
 }
 

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

@@ -494,61 +494,54 @@ class G1VerificationClosure : public BasicOopIterateClosure {
 };
 
 class VerifyLiveClosure : public G1VerificationClosure {
-public:
-  VerifyLiveClosure(G1CollectedHeap* g1h, VerifyOption vo) : G1VerificationClosure(g1h, vo) {}
-  virtual void do_oop(narrowOop* p) { do_oop_work(p); }
-  virtual void do_oop(oop* p) { do_oop_work(p); }
 
   template <class T>
   void do_oop_work(T* p) {
     assert(_containing_obj != NULL, "Precondition");
     assert(!_g1h->is_obj_dead_cond(_containing_obj, _vo),
       "Precondition");
-    verify_liveness(p);
-  }
 
-  template <class T>
-  void verify_liveness(T* p) {
     T heap_oop = RawAccess<>::oop_load(p);
-    Log(gc, verify) log;
-    if (!CompressedOops::is_null(heap_oop)) {
-      oop obj = CompressedOops::decode_not_null(heap_oop);
-      bool failed = false;
-      bool is_in_heap = _g1h->is_in(obj);
-      if (!is_in_heap || _g1h->is_obj_dead_cond(obj, _vo)) {
-        MutexLocker x(ParGCRareEvent_lock, Mutex::_no_safepoint_check_flag);
+    if (CompressedOops::is_null(heap_oop)) {
+      return;
+    }
 
-        if (!_failures) {
-          log.error("----------");
-        }
-        ResourceMark rm;
-        if (!is_in_heap) {
-          HeapRegion* from = _g1h->heap_region_containing(p);
-          log.error("Field " PTR_FORMAT " of live obj " PTR_FORMAT " in region " HR_FORMAT,
-                    p2i(p), p2i(_containing_obj), HR_FORMAT_PARAMS(from));
-          LogStream ls(log.error());
-          print_object(&ls, _containing_obj);
-          HeapRegion* const to = _g1h->heap_region_containing(obj);
-          log.error("points to obj " PTR_FORMAT " in region " HR_FORMAT " remset %s",
-                    p2i(obj), HR_FORMAT_PARAMS(to), to->rem_set()->get_state_str());
-        } else {
-          HeapRegion* from = _g1h->heap_region_containing(p);
-          HeapRegion* to = _g1h->heap_region_containing(obj);
-          log.error("Field " PTR_FORMAT " of live obj " PTR_FORMAT " in region " HR_FORMAT,
-                    p2i(p), p2i(_containing_obj), HR_FORMAT_PARAMS(from));
-          LogStream ls(log.error());
-          print_object(&ls, _containing_obj);
-          log.error("points to dead obj " PTR_FORMAT " in region " HR_FORMAT,
-                    p2i(obj), HR_FORMAT_PARAMS(to));
-          print_object(&ls, obj);
-        }
+    oop obj = CompressedOops::decode_raw_not_null(heap_oop);
+    bool is_in_heap = _g1h->is_in(obj);
+    if (!is_in_heap || _g1h->is_obj_dead_cond(obj, _vo)) {
+      MutexLocker x(ParGCRareEvent_lock, Mutex::_no_safepoint_check_flag);
+
+      Log(gc, verify) log;
+      if (!_failures) {
         log.error("----------");
-        _failures = true;
-        failed = true;
-        _n_failures++;
       }
+      ResourceMark rm;
+
+      HeapRegion* from = _g1h->heap_region_containing(p);
+      log.error("Field " PTR_FORMAT " of live obj " PTR_FORMAT " in region " HR_FORMAT,
+                p2i(p), p2i(_containing_obj), HR_FORMAT_PARAMS(from));
+      LogStream ls(log.error());
+      print_object(&ls, _containing_obj);
+
+      if (!is_in_heap) {
+        log.error("points to address " PTR_FORMAT " outside of heap", p2i(obj));
+      } else {
+        HeapRegion* to = _g1h->heap_region_containing(obj);
+        log.error("points to dead obj " PTR_FORMAT " in region " HR_FORMAT " remset %s",
+                  p2i(obj), HR_FORMAT_PARAMS(to), to->rem_set()->get_state_str());
+        print_object(&ls, obj);
+      }
+      log.error("----------");
+      _failures = true;
+      _n_failures++;
     }
   }
+
+public:
+  VerifyLiveClosure(G1CollectedHeap* g1h, VerifyOption vo) : G1VerificationClosure(g1h, vo) {}
+
+  virtual void do_oop(narrowOop* p) { do_oop_work(p); }
+  virtual void do_oop(oop* p) { do_oop_work(p); }
 };
 
 class VerifyRemSetClosure : public G1VerificationClosure {

src/hotspot/share/oops/compressedOops.hpp

@@ -127,6 +127,7 @@ class CompressedOops : public AllStatic {
   static inline narrowOop encode(oop v);
 
   // No conversions needed for these overloads
+  static inline oop decode_raw_not_null(oop v);
   static inline oop decode_not_null(oop v);
   static inline oop decode(oop v);
   static inline narrowOop encode_not_null(narrowOop v);

src/hotspot/share/oops/compressedOops.inline.hpp

@@ -78,6 +78,11 @@ inline narrowOop CompressedOops::encode(oop v) {
   return is_null(v) ? narrowOop::null : encode_not_null(v);
 }
 
+inline oop CompressedOops::decode_raw_not_null(oop v) {
+  assert(v != nullptr, "object is null");
+  return v;
+}
+
 inline oop CompressedOops::decode_not_null(oop v) {
   assert(Universe::is_in_heap(v), "object not in heap " PTR_FORMAT, p2i(v));
   return v;

src/hotspot/share/oops/oop.hpp

@@ -85,6 +85,8 @@ class oopDesc {
   inline Klass* klass() const;
   inline Klass* klass_or_null() const;
   inline Klass* klass_or_null_acquire() const;
+  // Get the raw value without any checks.
+  inline Klass* klass_raw() const;
 
   void set_narrow_klass(narrowKlass nk) NOT_CDS_JAVA_HEAP_RETURN;
   inline void set_klass(Klass* k);

src/hotspot/share/oops/oop.inline.hpp

@@ -107,6 +107,14 @@ Klass* oopDesc::klass_or_null_acquire() const {
   }
 }
 
+Klass* oopDesc::klass_raw() const {
+  if (UseCompressedClassPointers) {
+    return CompressedKlassPointers::decode_raw(_metadata._compressed_klass);
+  } else {
+    return _metadata._klass;
+  }
+}
+
 void oopDesc::set_klass(Klass* k) {
   assert(Universe::is_bootstrapping() || (k != NULL && k->is_klass()), "incorrect Klass");
   if (UseCompressedClassPointers) {