RFR (s) 8225397 : Integer value miscalculation in toString() method of BitSet

[Ivan Gerasimov]

Thank you Andrew!

The multiplier 6 was pre-existent, and I don't really want to change 
behavior in this legacy class.

The high limit of (Integer.MAX_VALUE - 20) was arbitrarily chosen.

I don't think it matters much in this case, but I can change it to 
(Integer.MAX_VALUE - 8), which is used in several other places.

I've just updated the code in-place to make the intent of the fix more 
clear.

Could you please check it if it looks better now?

http://cr.openjdk.java.net/~igerasim/8225397/00/webrev/

Thanks in advance!

Ivan



On 6/6/19 3:57 AM, Andrew Haley wrote:
> On 6/6/19 10:18 AM, Ivan Gerasimov wrote:
>> Hello!
>>
>> It is yet another instance of integer overflow under certain extreme
>> circumstances.
>>
>> This time it is when calculating the initial capacity of a StringBuilder
>> in BitSet.toString.
>>
>> If there are too many elements in the set, we can't do much anyway.
>>
>> The best effort is to avoid confusing NegativeArraySizeException and let
>> the method throw OOM.
>>
>> Would you please help review the fix?
>>
>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8225397
>> WEBREV: http://cr.openjdk.java.net/~igerasim/8225397/00/webrev/
> @@ -1184,7 +1184,9 @@
>
>           int numBits = (wordsInUse > 128) ?
>               cardinality() : wordsInUse * BITS_PER_WORD;
> -        StringBuilder b = new StringBuilder(6*numBits + 2);
> +        int sizeHint = (numBits <= (Integer.MAX_VALUE - 22) / 6) ?
> +            6 * numBits + 2 : Integer.MAX_VALUE - 20;
> +        StringBuilder b = new StringBuilder(sizeHint);
>           b.append('{');
>
>           int i = nextSetBit(0);
>
> This needs a comment. What is significant about 6 and 22?
>

-- 
With kind regards,
Ivan Gerasimov