[PATCH] Adding limitation while checking for cardTable

[Y.S.Ramakrishna at Sun.COM]

Hi Neo -- thanks for finding this problem and
initiating a fix. I will help shepherd this bug fix via:

6559052 [OpenJDK] CardTableRS::do_MemRegion() could attempt 
out-of-bounds translation in addr_for()

-- ramki.

Neo Jia wrote:

> On 5/10/07, Y Srinivas Ramakrishna <Y.S.Ramakrishna at sun.com> wrote:
>
>> 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.
>>
>
> Ramki,
>
> Thanks. Your fix makes more sense than mine.
>
> Neo
>
>>   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);
>>     }
>>   }
>>
>> -- Ramki Ramakrishna
>>
>> Neo Jia wrote On 05/09/07 03:31 PM,:
>>
>> > 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);
>> >
>>
>> -- 
>>
>> ---------------------------------------------------------------------------- 
>>
>> Y. Srinivas Ramakrishna               HotSpot JVM
>> Sun Microsystems, Inc., USCA 14-102   JWS / Software Group
>> 4140 Network Circle                   408 276 7250 (x17250)
>> Santa Clara, CA  95054, U.S.A.        Y dot S dot Ramakrishna at Sun 
>> dot COM
>> ---------------------------------------------------------------------------- 
>>
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 
>>
>>
>> NOTICE: This email message is for the sole use of the intended 
>> recipient(s)
>> and may contain confidential and privileged information. Any 
>> unauthorized
>> review, use, disclosure or distribution is prohibited. If you are not 
>> the
>> intended recipient, please contact the sender by reply email and destroy
>> all copies of the original message.
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 
>>
>>
>>
>
>