PING: RFR: 8194249: SA: G1HeapRegionTable#getByAddress() returns incorrect HeapRegion
Thomas Schatzl
thomas.schatzl at oracle.com
Wed Jan 31 08:57:12 UTC 2018
Hi,
On Wed, 2018-01-31 at 09:49 +0900, Yasumasa Suenaga wrote:
> Hi Thomas,
>
> > > looks good to me - however there is another (pre-existing) bug:
> > > the
> > > shift in that code should be a logical shift, not an arithmetic
> > > shift.
> > >
> > > I.e. ">>" instead of ">>>".
> > >
> > > I will run the patch through testing and report back in a few
> > > hours.
> > > Should be okay.
> >
> > is good. Do you want to fix the issue with the shift operator too
> > here, or use another CR?
>
> Thanks!
>
> If the use of ">>>" is the bug, I want to fix it in new bug ticket.
> I do not think the use of ">>>" is not a bug.
>
> I g1BiasedArray.hpp, G1BiasedMappedArray::get_by_address() uses ">>"
> operator to calculate biased_index:
> http://hg.openjdk.java.net/jdk/hs/file/ee513596f3ee/src/hotspot/sha
> re/gc/g1/g1BiasedArray.hpp#l134
>
> idx_t is defined as size_t. So it is calculated as unsigned value.
> In JLS 15.19, ">>>" is for unsigned. If we use ">>", it might remain
> MSB in some cases.
> https://docs.oracle.com/javase/specs/jls/se9/html/jls-15.html#jls-1
> 5.19
>
> Thus I think this is not a bug.
You are right, I mixed them up.
I will push the change then, with jgeorge and me as reviewers.
Thomas
More information about the serviceability-dev
mailing list