Fix derived pointers assertions and debugging

Author: pron

src/hotspot/share/compiler/oopMap.cpp

@@ -498,6 +498,7 @@ void ImmutableOopMap::oops_do(const frame *fr, const RegisterMap *reg_map,
     derived_cl = &process_cl;
     break;
   case DerivedPointerIterationMode::_with_table:
+    // if (has_derived_oops()) { tty->print_cr(">>> ImmutableOopMap::oops_do derived table"); fr->print_on(tty); }
     derived_cl = &add_cl;
     break;
   case DerivedPointerIterationMode::_ignore:
@@ -1011,6 +1012,8 @@ void DerivedPointerTable::update_pointers() {
     *derived_loc = derived_base + offset;
     assert(*derived_loc - derived_base == offset, "sanity check");
 
+    // assert (offset >= 0 && offset <= (intptr_t)(base->size() << LogHeapWordSize), "offset: %ld base->size: %zu relative: %d", offset, base->size() << LogHeapWordSize, *(intptr_t*)derived_loc <= 0);
+
     if (TraceDerivedPointers) {
       tty->print_cr("Updating derived pointer@" INTPTR_FORMAT
                     " - Derived: " INTPTR_FORMAT "  Base: " INTPTR_FORMAT " (Offset: " INTX_FORMAT ")",

src/hotspot/share/compiler/oopMap.inline.hpp

@@ -71,7 +71,7 @@ void OopMapDo<OopFnT, DerivedOopFnT, ValueFilterT>::iterate_oops_do(const frame
 
       address loc = fr->oopmapreg_to_location(omv.reg(), reg_map);
 
-      DEBUG_ONLY(if (reg_map->should_skip_missing()) continue;)
+      DEBUG_ONLY(if (loc == NULL && reg_map->should_skip_missing()) continue;)
       guarantee(loc != NULL, "missing saved register");
       derived_pointer* derived_loc = (derived_pointer*)loc;
       oop* base_loc = fr->oopmapreg_to_oop_location(omv.content_reg(), reg_map);

src/hotspot/share/oops/instanceStackChunkKlass.cpp

@@ -273,14 +273,14 @@ class RelativizeDerivedPointers : public DerivedOopClosure {
 
       OrderAccess::loadload();
       intptr_t derived_int_val = Atomic::load((intptr_t*)derived_loc); // *derived_loc;
-      if (derived_int_val <= 0) { // an offset of 0 was observed on AArch64
+      if (derived_int_val <= 0) {
         return;
       }
 
       // at this point, we've seen a non-offset value *after* we've read the base, but we write the offset *before* fixing the base,
       // so we are guaranteed that the value in derived_loc is consistent with base (i.e. points into the object).
       intptr_t offset = derived_int_val - cast_from_oop<intptr_t>(base);
-      assert (offset >= 0, "Derived pointer offset is %ld", offset); // an offset of 0 was observed on AArch64
+      assert (offset >= 0, "Derived pointer offset is %ld", offset);
       // assert (offset >= 0 && offset <= (base->size() << LogHeapWordSize), "offset: %ld size: %d", offset, (base->size() << LogHeapWordSize)); -- base might be invalid at this point
       Atomic::store((intptr_t*)derived_loc, -offset); // there could be a benign race here; we write a negative offset to let the sign bit signify it's an offset rather than an address
     } else {
@@ -304,17 +304,11 @@ class DerelativizeDerivedPointers : public DerivedOopClosure {
 
       // at this point, we've seen a non-offset value *after* we've read the base, but we write the offset *before* fixing the base,
       // so we are guaranteed that the value in derived_loc is consistent with base (i.e. points into the object).
-      if (offset <= 0) { // an offset of 0 was observed on AArch64
+      if (offset <= 0) {
         offset = -offset;
         assert (offset >= 0 && (size_t)offset <= (base->size() << LogHeapWordSize), "");
         Atomic::store((intptr_t*)derived_loc, cast_from_oop<intptr_t>(base) + offset);
       }
-  #ifdef ASSERT
-      else {
-        offset = offset - cast_from_oop<intptr_t>(base);
-        assert (offset >= 0 && (size_t)offset <= (base->size() << LogHeapWordSize), "offset: " PTR_FORMAT " size: %zu", offset, (base->size() << LogHeapWordSize));
-      }
-  #endif
     }
   }
 };
@@ -744,10 +738,10 @@ class StackChunkVerifyBitmapClosure : public BitMapClosure {
 
 class StackChunkVerifyOopsClosure : public OopClosure {
   stackChunkOop _chunk;
-  intptr_t* _sp;
+  intptr_t* _unextended_sp;
   int _count;
 public:
-  StackChunkVerifyOopsClosure(stackChunkOop chunk, intptr_t* sp) : _chunk(chunk), _sp(sp), _count(0) {}
+  StackChunkVerifyOopsClosure(stackChunkOop chunk, intptr_t* unextended_sp) : _chunk(chunk), _unextended_sp(unextended_sp), _count(0) {}
   int count() { return _count; }
   void do_oop(oop* p) override { (_chunk->has_bitmap() && UseCompressedOops) ? do_oop_work((narrowOop*)p) : do_oop_work(p); }
   void do_oop(narrowOop* p) override { do_oop_work(p); }
@@ -768,8 +762,10 @@ class StackChunkVerifyOopsClosure : public OopClosure {
 
 class StackChunkVerifyDerivedPointersClosure : public DerivedOopClosure {
   stackChunkOop _chunk;
+  intptr_t* _unextended_sp;
 public:
-  StackChunkVerifyDerivedPointersClosure(stackChunkOop chunk) : _chunk(chunk) {}
+
+  StackChunkVerifyDerivedPointersClosure(stackChunkOop chunk, intptr_t* unextended_sp) : _chunk(chunk), _unextended_sp(unextended_sp) {}
 
   virtual void do_derived_oop(oop* base_loc, derived_pointer* derived_loc) override {
     log_develop_trace(jvmcont)("debug_verify_stack_chunk base: " INTPTR_FORMAT " derived: " INTPTR_FORMAT, p2i(base_loc), p2i(derived_loc));
@@ -784,11 +780,12 @@ class StackChunkVerifyDerivedPointersClosure : public DerivedOopClosure {
       ZGC_ONLY(assert (!UseZGC || ZAddress::is_good(cast_from_oop<uintptr_t>(base)), "");)
       OrderAccess::loadload();
       intptr_t offset = Atomic::load((intptr_t*)derived_loc);
-      offset = offset <= 0 // an offset of 0 was observed on AArch64
+      offset = offset <= 0
                   ? -offset
                   : offset - cast_from_oop<intptr_t>(base);
 
       // Has been seen to fail on AArch64 for some reason
+      // It looks as if a derived pointer appears live in the oopMap but isn't used.
       // assert (offset >= 0 && offset <= (intptr_t)(base->size() << LogHeapWordSize), "offset: %ld base->size: %zu relative: %d", offset, base->size() << LogHeapWordSize, *(intptr_t*)derived_loc <= 0);
     } else {
       assert (*derived_loc == derived_pointer(0), "");
@@ -850,11 +847,11 @@ class VerifyStackClosure {
     //   }
     // }
 
-    StackChunkVerifyOopsClosure oops_closure(_chunk, f.sp());
+    StackChunkVerifyOopsClosure oops_closure(_chunk, f.unextended_sp());
     f.iterate_oops(&oops_closure, map);
     assert (oops_closure.count() == num_oops, "oops: %d oopmap->num_oops(): %d", oops_closure.count(), num_oops);
 
-    StackChunkVerifyDerivedPointersClosure derived_oops_closure(_chunk);
+    StackChunkVerifyDerivedPointersClosure derived_oops_closure(_chunk, f.unextended_sp());
     f.iterate_derived_pointers(&derived_oops_closure, map);
 
     _callee_interpreted = f.is_interpreted();

src/hotspot/share/runtime/frame.cpp

@@ -1317,11 +1317,15 @@ class FrameValuesOopClosure: public OopClosure, public DerivedOopClosure {
       values.describe(frame_no, (intptr_t*)derived, err_msg("derived pointer (base: " INTPTR_FORMAT ") for #%d", p2i(base), frame_no));
     }
   }
-  virtual void do_oop(oop* p)       { _oops->push(p); }
-  virtual void do_oop(narrowOop* p) { _narrow_oops->push(p); }
-  virtual void do_derived_oop(oop* base, derived_pointer* derived) {
-    _base->push(base);
-    _derived->push(derived);
+  virtual void do_oop(oop* p) override { _oops->push(p); }
+  virtual void do_oop(narrowOop* p) override { _narrow_oops->push(p); }
+  virtual void do_derived_oop(oop* base_loc, derived_pointer* derived_loc) override {
+    // oop base = *base_loc;
+    // intptr_t offset = *(intptr_t*)derived_loc - cast_from_oop<intptr_t>(base);
+    // assert (offset >= 0 && offset <= (intptr_t)(base->size() << LogHeapWordSize), "offset: %ld base->size: %zu relative: %d", offset, base->size() << LogHeapWordSize, *(intptr_t*)derived_loc <= 0);
+
+    _base->push(base_loc);
+    _derived->push(derived_loc);
   }
 };
 
@@ -1335,7 +1339,7 @@ class FrameValuesOopMapClosure: public OopMapClosure {
   FrameValuesOopMapClosure(const frame* fr, const RegisterMap* reg_map, FrameValues& values, int frame_no)
    : _fr(fr), _reg_map(reg_map), _values(values), _frame_no(frame_no) {}
 
-  virtual void do_value(VMReg reg, OopMapValue::oop_types type) {
+  virtual void do_value(VMReg reg, OopMapValue::oop_types type) override {
     intptr_t* p = (intptr_t*)_fr->oopmapreg_to_location(reg, _reg_map);
     if (p != NULL && (((intptr_t)p & WordAlignmentMask) == 0)) {
       const char* type_name = NULL;