8358735: GenShen: block_start() may be incorrect after class unloading

Co-authored-by: Y. Srinivas Ramakrishna <[email protected]>
Reviewed-by: wkemper

Author: kdnilsen

src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp

@@ -270,6 +270,7 @@ class ShenandoahHeap : public CollectedHeap {
 public:
 
   inline HeapWord* base() const { return _heap_region.start(); }
+  inline HeapWord* end()  const { return _heap_region.end(); }
 
   inline size_t num_regions() const { return _num_regions; }
   inline bool is_heap_region_special() { return _heap_region_special; }

src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.cpp

@@ -57,6 +57,26 @@ bool ShenandoahMarkBitMap::is_bitmap_clear_range(const HeapWord* start, const He
   return (result == end);
 }
 
+HeapWord* ShenandoahMarkBitMap::get_prev_marked_addr(const HeapWord* limit,
+                                                     const HeapWord* addr) const {
+#ifdef ASSERT
+  ShenandoahHeap* heap = ShenandoahHeap::heap();
+  ShenandoahHeapRegion* r = heap->heap_region_containing(addr);
+  ShenandoahMarkingContext* ctx = heap->marking_context();
+  HeapWord* tams = ctx->top_at_mark_start(r);
+  assert(limit != nullptr, "limit must not be null");
+  assert(limit >= r->bottom(), "limit must be more than bottom");
+  assert(addr <= tams, "addr must be less than TAMS");
+#endif
+
+  // Round addr down to a possible object boundary to be safe.
+  size_t const addr_offset = address_to_index(align_down(addr, HeapWordSize << LogMinObjAlignment));
+  size_t const limit_offset = address_to_index(limit);
+  size_t const last_offset = get_prev_one_offset(limit_offset, addr_offset);
+
+  // cast required to remove const-ness of the value pointed to.  We won't modify that object, but my caller might.
+  return (last_offset > addr_offset)? (HeapWord*) addr + 1: index_to_address(last_offset);
+}
 
 HeapWord* ShenandoahMarkBitMap::get_next_marked_addr(const HeapWord* addr,
                                                      const HeapWord* limit) const {

src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.hpp

@@ -119,9 +119,21 @@ class ShenandoahMarkBitMap {
   template<bm_word_t flip, bool aligned_right>
   inline idx_t get_next_bit_impl(idx_t l_index, idx_t r_index) const;
 
-  inline idx_t get_next_one_offset (idx_t l_index, idx_t r_index) const;
+  // Helper for get_prev_{zero,one}_bit variants.
+  // - flip designates whether searching for 1s or 0s.  Must be one of
+  //   find_{zeros,ones}_flip.
+  // - aligned_left is true if l_index is a priori on a bm_word_t boundary.
+  template<bm_word_t flip, bool aligned_left>
+  inline idx_t get_prev_bit_impl(idx_t l_index, idx_t r_index) const;
+
+  // Search for the first marked address in the range [l_index, r_index), or r_index if none found.
+  inline idx_t get_next_one_offset(idx_t l_index, idx_t r_index) const;
 
-  void clear_large_range (idx_t beg, idx_t end);
+  // Search for last one in the range [l_index, r_index).  Return r_index if not found.
+  inline idx_t get_prev_one_offset(idx_t l_index, idx_t r_index) const;
+
+  // Clear the strong and weak mark bits for all index positions >= l_index and < r_index.
+  void clear_large_range(idx_t beg, idx_t end);
 
   // Verify bit is less than size().
   void verify_index(idx_t bit) const NOT_DEBUG_RETURN;
@@ -162,12 +174,14 @@ class ShenandoahMarkBitMap {
 
   bool is_bitmap_clear_range(const HeapWord* start, const HeapWord* end) const;
 
-  // Return the address corresponding to the next marked bit at or after
-  // "addr", and before "limit", if "limit" is non-null.  If there is no
-  // such bit, returns "limit" if that is non-null, or else "endWord()".
+  // Return the first marked address in the range [addr, limit), or limit if none found.
   HeapWord* get_next_marked_addr(const HeapWord* addr,
                                  const HeapWord* limit) const;
 
+  // Return the last marked address in the range [limit, addr], or addr+1 if none found.
+  HeapWord* get_prev_marked_addr(const HeapWord* limit,
+                                 const HeapWord* addr) const;
+
   bm_word_t inverted_bit_mask_for_range(idx_t beg, idx_t end) const;
   void  clear_range_within_word    (idx_t beg, idx_t end);
   void clear_range (idx_t beg, idx_t end);

src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.inline.hpp

@@ -29,6 +29,7 @@
 #include "gc/shenandoah/shenandoahMarkBitMap.hpp"
 
 #include "runtime/atomicAccess.hpp"
+#include "utilities/count_leading_zeros.hpp"
 #include "utilities/count_trailing_zeros.hpp"
 
 inline size_t ShenandoahMarkBitMap::address_to_index(const HeapWord* addr) const {
@@ -169,10 +170,99 @@ inline ShenandoahMarkBitMap::idx_t ShenandoahMarkBitMap::get_next_bit_impl(idx_t
   return r_index;
 }
 
+template<ShenandoahMarkBitMap::bm_word_t flip, bool aligned_left>
+inline ShenandoahMarkBitMap::idx_t ShenandoahMarkBitMap::get_prev_bit_impl(idx_t l_index, idx_t r_index) const {
+  STATIC_ASSERT(flip == find_ones_flip || flip == find_zeros_flip);
+  verify_range(l_index, r_index);
+  assert(!aligned_left || is_aligned(l_index, BitsPerWord), "l_index not aligned");
+
+  // The first word often contains an interesting bit, either due to
+  // density or because of features of the calling algorithm.  So it's
+  // important to examine that first word with a minimum of fuss,
+  // minimizing setup time for later words that will be wasted if the
+  // first word is indeed interesting.
+
+  // The benefit from aligned_left being true is relatively small.
+  // It saves an operation in the setup for the word search loop.
+  // It also eliminates the range check on the final result.
+  // However, callers often have a comparison with l_index, and
+  // inlining often allows the two comparisons to be combined; it is
+  // important when !aligned_left that return paths either return
+  // l_index or a value dominating a comparison with l_index.
+  // aligned_left is still helpful when the caller doesn't have a
+  // range check because features of the calling algorithm guarantee
+  // an interesting bit will be present.
+
+  if (l_index < r_index) {
+    // Get the word containing r_index, and shift out the high-order bits (representing objects that come after r_index)
+    idx_t index = to_words_align_down(r_index);
+    assert(BitsPerWord - 2 >= bit_in_word(r_index), "sanity");
+    size_t shift = BitsPerWord - 2 - bit_in_word(r_index);
+    bm_word_t cword = (map(index) ^ flip) << shift;
+    // After this shift, the highest order bits correspond to r_index.
+
+    // We give special handling if either of the two most significant bits (Weak or Strong) is set.  With 64-bit
+    // words, the mask of interest is 0xc000_0000_0000_0000.  Symbolically, this constant is represented by:
+    const bm_word_t first_object_mask = ((bm_word_t) 0x3) << (BitsPerWord - 2);
+    if ((cword & first_object_mask) != 0) {
+      // The first object is similarly often interesting. When it matters
+      // (density or features of the calling algorithm make it likely
+      // the first bit is set), going straight to the next clause compares
+      // poorly with doing this check first; count_leading_zeros can be
+      // relatively expensive, plus there is the additional range check.
+      // But when the first bit isn't set, the cost of having tested for
+      // it is relatively small compared to the rest of the search.
+      return r_index;
+    } else if (cword != 0) {
+      // Note that there are 2 bits corresponding to every index value (Weak and Strong), and every odd index value
+      //  corresponds to the same object as index-1
+      // Flipped and shifted first word is non-zero.  If leading_zeros is 0 or 1, we return r_index (above).
+      // if leading zeros is 2 or 3, we return (r_index - 1) or (r_index - 2), and so forth
+      idx_t result = r_index + 1 - count_leading_zeros(cword);
+      if (aligned_left || (result >= l_index)) return result;
+      else {
+        // Sentinel value means no object found within specified range.
+        return r_index + 2;
+      }
+    } else {
+      // Flipped and shifted first word is zero.  Word search through
+      // aligned up r_index for a non-zero flipped word.
+      idx_t limit = aligned_left
+                    ? to_words_align_down(l_index) // Minuscule savings when aligned.
+                    : to_words_align_up(l_index);
+      // Unsigned index is always >= unsigned limit if limit equals zero, so test for strictly greater than before decrement.
+      while (index-- > limit) {
+        cword = map(index) ^ flip;
+        if (cword != 0) {
+          // cword hods bits:
+          //    0x03 for the object corresponding to index (and index+1)       (count_leading_zeros is 62 or 63)
+          //    0x0c for the object corresponding to index + 2 (and index+3)   (count_leading_zeros is 60 or 61)
+          //    and so on.
+          idx_t result = bit_index(index + 1) - (count_leading_zeros(cword) + 1);
+          if (aligned_left || (result >= l_index)) return result;
+          else {
+            // Sentinel value means no object found within specified range.
+            return r_index + 2;
+          }
+        }
+      }
+      // No bits in range; return r_index+2.
+      return r_index + 2;
+    }
+  }
+  else {
+    return r_index + 2;
+  }
+}
+
 inline ShenandoahMarkBitMap::idx_t ShenandoahMarkBitMap::get_next_one_offset(idx_t l_offset, idx_t r_offset) const {
   return get_next_bit_impl<find_ones_flip, false>(l_offset, r_offset);
 }
 
+inline ShenandoahMarkBitMap::idx_t ShenandoahMarkBitMap::get_prev_one_offset(idx_t l_offset, idx_t r_offset) const {
+  return get_prev_bit_impl<find_ones_flip, false>(l_offset, r_offset);
+}
+
 // Returns a bit mask for a range of bits [beg, end) within a single word.  Each
 // bit in the mask is 0 if the bit is in the range, 1 if not in the range.  The
 // returned mask can be used directly to clear the range, or inverted to set the

src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.hpp

@@ -67,8 +67,12 @@ class ShenandoahMarkingContext : public CHeapObj<mtGC> {
   inline bool is_marked_or_old(oop obj) const;
   inline bool is_marked_strong_or_old(oop obj) const;
 
+  // Return address of the first marked address in the range [addr,limit), or limit if no marked object found
   inline HeapWord* get_next_marked_addr(const HeapWord* addr, const HeapWord* limit) const;
 
+  // Return address of the last marked object in range [limit, start], returning start+1 if no marked object found
+  inline HeapWord* get_prev_marked_addr(const HeapWord* limit, const HeapWord* start) const;
+
   inline bool allocated_after_mark_start(const oop obj) const;
   inline bool allocated_after_mark_start(const HeapWord* addr) const;
 

src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.inline.hpp

@@ -72,6 +72,10 @@ inline HeapWord* ShenandoahMarkingContext::get_next_marked_addr(const HeapWord*
   return _mark_bit_map.get_next_marked_addr(start, limit);
 }
 
+inline HeapWord* ShenandoahMarkingContext::get_prev_marked_addr(const HeapWord* limit, const HeapWord* start) const {
+  return _mark_bit_map.get_prev_marked_addr(limit, start);
+}
+
 inline bool ShenandoahMarkingContext::allocated_after_mark_start(oop obj) const {
   const HeapWord* addr = cast_from_oop<HeapWord*>(obj);
   return allocated_after_mark_start(addr);