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

[Bengt Rutisson]

Another FYI: Last week I pushed the fix to the HS24 repository:
http://hg.openjdk.java.net/hsx/hsx24/hotspot/rev/9c930c24f9b0

This will most likely go in to the next 7update release. With the 
current planning this means 7u12.

Bengt


On 2012-09-17 22:08, Bengt Rutisson wrote:
>
> 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/20120925/6897ae0b/attachment.htm>