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

[Bengt Rutisson]

FYI, I pushed this earlier today so the fix is now available in hotspot-gc:
http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/859cd1a76f8a

I've also filed a sub CR to backport the fix to hs24/7u12. I hope we get 
approval to push it there soon.

Thanks again for the patch Hal!
Bengt

On 2012-09-14 01:26, Srinivas Ramakrishna wrote:
>
> Great, thanks John, all!
>
> -- ramki
>
> On Fri, Sep 14, 2012 at 12:19 AM, John Cuthbertson 
> <john.cuthbertson at oracle.com <mailto:john.cuthbertson at oracle.com>> wrote:
>
>     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/20120917/b7eeec85/attachment.htm>