RFR (S): 8151499: gc/g1/g1OopClosures.inline.hpp: assert(_from->is_in_reserved(p)) failed: p is not in from

[Tom Benson]

Hi Thomas,

It doesn't seem like the test at line 181 is strictly necessary, because 
the comparison on the following line would fail if it was false:

  181           _g1->heap_region_containing(p)->is_humongous() &&
  182           _from->humongous_start_region() == _g1->heap_region_containing(p)->humongous_start_region())

Also, should the test require be expanded to   @requires vm.gc == "G1" | 
vm.gc == null   ?
Tom

On 4/1/2016 9:05 AM, Thomas Schatzl wrote:
> Hi all,
>
>    can I have reviews for the fix of a wrong assert in
> g1OopClosure.inline.hpp?
>
> The assert tried to make sure that for a given pointer to an oop p, the
> "_from" region stored in the closure matches.
>
> This is not true for a humongous non-array of references object: for
> these we mark only the header dirty, and during investigation of a
> particular card/reference, we need to go over all references of that
> object.
>
> Since in this case, other references may not be in the same region as
> p, the assert _from->is_in_reserved(p) fails since the change in JDK
> -8139867.
>
> While trying to think through the code, I added a note about additional
> but benign remembered set entries that might be added while executing
> the method to explain the issue a bit if you dare to think through the
> entire code.
>
> Thanks go to A. Shipilev for the test.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8151499
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8151499/webrev/
> Testing:
> test case runs successfully, jprt, runs of gcbasher with
> -XX:+G1VerifyRememberedSets
>
> Thanks,
>    Thomas
>