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.


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?


On Sep 21, 2016, at 11:57 PM, Jamil Nimeh <> 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?


Reply via email to