RFR: JDK-8049516: sun.security.provider.SeedGenerator throws ArrayIndexOutOfBoundsException

[Xuelei Fan]

Looks fine to me.  Thanks!

Xuelei

On 9/22/2016 4:05 PM, Jamil Nimeh wrote:
> That's a very good suggestion.  I've made that change and updated the
> webrev:
>
> http://cr.openjdk.java.net/~jnimeh/reviews/8049516/webrev.02/
>
> Thanks again,
> --Jamil
>
> On 09/21/2016 05:34 PM, Xuelei Fan wrote:
>> > latch = (latch + 1) & 0x7FFFFFFF;  // Mask the sign bit
>> I'm fine with it.
>>
>> BTW, if you like, I may suggest to use a little bit small number for
>> the mask, for example 0x1FFFFFFF, so that the counter variable does
>> not overflow either.  It works if counter overflow, but better not if
>> we want the counter meaningful.
>>
>> Xuelei
>>
>>
>> On 9/22/2016 7:29 AM, Jamil Nimeh wrote:
>>> So here are a couple ideas related to your suggestion.
>>>
>>> We can leave a simple increment on latch and let it overflow, then in
>>> the array access do this:
>>>
>>> v ^= rndTab[(latch & 0x7FFFFFFF) % 255];
>>>
>>> That clears the high order bit and the mod will keep it in the range of
>>> 0-254.  the lvalue for the mod operation will never go negative.  The
>>> downside of that is counter will still get assigned a large negative
>>> value (which as I said earlier could cause another iteration of the main
>>> loop...not the end of the world since it can only happen 6 times max).
>>>
>>> The other approach would be to increment latch and mask off the high
>>> order bit:
>>>
>>> latch = (latch + 1) & 0x7FFFFFFF;  // Mask the sign bit
>>>
>>> Then the v^=[latch % 255] is safe as-is and counter doesn't get some
>>> massively negative value (though it can still overflow across multiple
>>> iterations of the outer loop).
>>>
>>> I like the second approach a little better, personally. Up-front use of
>>> the bitwise-and is a little more clear that we're forcing the value to
>>> be non-negative before we use it as an array index input.  Let me know
>>> what you think and I'll update the webrev accordingly.
>>>
>>> Thanks,
>>> --Jamil
>>>
>>> On 09/21/2016 09:05 AM, Wang Weijun wrote:
>>>> I am OK with your fix, but I found "(latch + 1) % Integer.MAX_VALUE" a
>>>> little difficult to understand. Does rndTab[latch & 0xff] also work?
>>>>
>>>> Thanks
>>>> Max
>>>>
>>>>
>>>>> On Sep 21, 2016, at 11:57 PM, Jamil Nimeh <jamil.j.nimeh at oracle.com>
>>>>> wrote:
>>>>>
>>>>> Hi Max and Xuelei,
>>>>>
>>>>> Yesterday I also reached out to the SQE engineer who submitted the
>>>>> bug, asking if this is an issue he's seen going forward from the
>>>>> original instance in 8u20.  He said that he hasn't seen this issue
>>>>> come up since the original bug submission.  Since the simple overflow
>>>>> condition is easily solved with my fix, and the code has been
>>>>> otherwise stable I'd like to suggest that we keep the fix as-is.  The
>>>>> loop timing as it stands now is not the source of the bug, other than
>>>>> that latch can overflow and that is solved in one line.  If we want
>>>>> to revisit this and improve the overall performance (though I haven't
>>>>> seen evidence that there is a perf issue with this at all) then maybe
>>>>> an RFE is in order.  What do you think?
>>>>>
>>>>> --Jamil
>>>
>