Response from ###@###.###:
-------------------------------

...

Thanks. Your fix makes more sense than mine.
Response from ###@###.###:
--------------------------------------

Hi Neo,

Thanks for the heads-up. This seems worth fixing, at least
for hygiene, How about the following instead, where we avoid the
addr_for() translation until we are sure it will be safe,
provided that the mr argument to the method is backed by
card table entries. This avoids the need for the
extra compare-and-branch inside the loop in your patch.

  void do_MemRegion(MemRegion mr) {
    HeapWord* end_of_non_clean = mr.end();
    HeapWord* start_of_non_clean = end_of_non_clean;
    jbyte* entry = _ct->byte_for(mr.last());
    jbyte* first_entry = _ct->byte_for(mr.start());
    while (entry >= first_entry) {
      jbyte entry_val = *entry;
      HeapWord* cur = _ct->addr_for(entry);
      if (!clear_card(entry)) {
        if (start_of_non_clean < end_of_non_clean) {
          MemRegion mr2(start_of_non_clean, end_of_non_clean);
          _dirty_card_closure->do_MemRegion(mr2);
        }
        end_of_non_clean = cur;
        start_of_non_clean = end_of_non_clean;
      }
      start_of_non_clean = cur;
      entry--;
    }
    if (start_of_non_clean < end_of_non_clean) {
      MemRegion mr2(start_of_non_clean, end_of_non_clean);
      _dirty_card_closure->do_MemRegion(mr2);
    }
  }
Email from ###@###.###: [OpenJDK]
----------------------------
hi,

Acturally, the existing code works well although it misses the
limitation checking, because the current heap layout will make sure
the memory region checking failed before it goes beyond the limitation
of byte map.

The reason is that the nursery space is on the lower address, which
will make its committed card table also on the lower address of inside
the byte maps. While walking through the card table of the mature
space and keeping reduce the entry, we will finally hit the boundary
of its byte map before doing the check of memory region, which will
fire the assert of the addr_for, if it is on the lower address.

So, just adding more safety for this function.

Thanks,
Neo
Index: memory/cardTableRS.cpp
===================================================================
--- memory/cardTableRS.cpp (revision 86)
+++ memory/cardTableRS.cpp (working copy)
@@ -163,6 +163,9 @@
    start_of_non_clean = end_of_non_clean;
       }
       entry--;
+ // Adding a limit checking for safety.
+ if (entry < _ct->base_map())
+ break;
       start_of_non_clean = cur;
       cur = _ct->addr_for(entry);
     }
Index: memory/cardTableRS.hpp
===================================================================
--- memory/cardTableRS.hpp (revision 86)
+++ memory/cardTableRS.hpp (working copy)
@@ -101,6 +101,8 @@

   CardTableModRefBS* ct_bs() { return &_ct_bs; }

+ jbyte * base_map() { return &(_ct_bs._byte_map[0]); }
+
   // Override.
   void prepare_for_younger_refs_iterate(bool parallel);