Request for review (XS): 7197906: BlockOffsetArray::power_to_cards_back() needs to handle > 32 bit shifts

[John Cuthbertson]

Hi Ramki,

I don't think there will be an issue with back porting this to hs24 
(which I think is going into 7u12). I just have to check the dates. 
There's a couple of other CRs that we're probably going to want to back 
port as well.

I searched the sources for the offending pattern (with and without 
whitespace) and only found the instances fixed by Hal. I think we're OK 
in that respect. But thanks for making sure. :)

JohnC

On 09/13/12 15:21, Srinivas Ramakrishna wrote:
>
> Thanks Bengt; changes look good to me too, Hal, although i just looked 
> at the changes, and did not
> check the code to see if there might be more instances of the fingered 
> anti-pattern. (I am pretty sure
> you and John and other reviewers have covered that front well :-)
>
> Bengt, any chance that the fix will get bkported also to 7uXX soon, 
> for some suitable value of XX?
>
> thanks!
> -- ramki
>
> On Thu, Sep 13, 2012 at 8:27 PM, Bengt Rutisson 
> <bengt.rutisson at oracle.com <mailto:bengt.rutisson at oracle.com>> wrote:
>
>
>     Hi Ramki,
>
>     Thanks for looking at this!
>
>     Here's a webrev based on the second patch that Hal sent out:
>     http://cr.openjdk.java.net/~brutisso/7197906/webrev.02/
>     <http://cr.openjdk.java.net/%7Ebrutisso/7197906/webrev.02/>
>
>     Bengt
>
>
>     On 2012-09-13 18:12, Srinivas Ramakrishna wrote:
>>     Hal, thanks for the catch and the fix!
>>
>>     Bengt, might it be possible to host this as a traditional webrev
>>     in the usual place, to simplify review logistics?
>>
>>     thanks!
>>     -- ramki
>>
>>     On Thu, Sep 13, 2012 at 4:55 PM, Jianhao Mo <mojianhao at gmail.com
>>     <mailto:mojianhao at gmail.com>> wrote:
>>
>>         Hi all,
>>
>>         Attached please find the new patch, modified according
>>         to John's advice.
>>
>>         Thanks Bengt again for the help :)
>>
>>         Regards,
>>
>>         Hal
>>
>>         2012/9/12 Jianhao Mo <mojianhao at gmail.com
>>         <mailto:mojianhao at gmail.com>>
>>
>>             Hi all,
>>
>>             Could I get a couple of reviews for this small patch, please?
>>
>>             7197906: BlockOffsetArray::power_to_cards_back() needs to
>>             handle > 32 bit shifts
>>
>>             CR link:
>>             http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7197906
>>             Webrev:
>>             http://cr.openjdk.java.net/~brutisso/7197906/webrev.01/
>>             <http://cr.openjdk.java.net/%7Ebrutisso/7197906/webrev.01/>
>>             Contributed-by: 
>>             Hal Mo <kungu.mjh at taobao.com
>>             <mailto:kungu.mjh at taobao.com>> from Alibaba Group(with OCA) 
>>
>>             It may take a while before the CR link is publicly
>>             accessible.
>>
>>             In blockOffsetTable.hpp, there is an unexpected data
>>             truncation in power_to_cards_back(uint i), which may lead
>>             to crashing the VM on very large GC heaps.
>>             The problem could be reproduced easily on machines, that
>>             have enough memory, with:
>>             $ java -Xmx135g -XX:+UseConcMarkSweepGC
>>
>>             The proposed patch fixes this problem, and builds on all
>>             OpenJDK supported platforms.
>>
>>             I'd like to thank Bengt Rutisson for the initial review
>>             and help preparing the CR and Webrev.
>>
>>             Regards,
>>             Hal 
>>
>>
>>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20120913/a8d9da2f/attachment.htm>