RFR: 8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale [v2]

[Evgeny Astigeevich]

On Thu, 15 Dec 2022 11:30:29 GMT, Evgeny Astigeevich <eastigeevich at openjdk.org> wrote:

>> The predicates : `amount + _scale/2 <= SIZE_MAX` and its equivalent `amount <= SIZE_MAX - _scale/2` are always true. We cannot catch wrap-around using these. Because if `amount` is too large, adding `_scale/2` to it may wrap the SIZE_MAX and end up in a very small number, but it is still less than SIZE_MAX.
>> The same is true for `amount - _scale/2` that may wrap the 0 and ends up with a large number, but it is still less than SIZE_MAX.
>> Generally, all `size_t` variables are within `[0, SIZE_MAX]` range but adding or subtracting may wrap around the boundaries while the final values are still in that range. Therefore, checking the final value with the upper/lower boundaries does not indicate that a wrap-around (overflow, underflow) has happened.
>> I used `(a + b) > a` where `b != 0` for finding if an overflow happened or not.
>> Also `(a - b) < a` where `b != 0` is used to find if an underflow happened or not.
>> 
>> I explain myself using this illustration for overflow case:
>> 
>> graph LR;
>> 0 --> a_plus_b;
>> a_plus_b --> a;
>> 0 --> a;
>> a --> SIZE_MAX;
>> 
>> SIZE_MAX --> 0;
>
>> The predicates : `amount + _scale/2 <= SIZE_MAX` and its equivalent `amount <= SIZE_MAX - _scale/2` are always true.
> 
> You are not correct. `amount + _scale/2 <= SIZE_MAX` is always true but `amount <= SIZE_MAX - _scale/2` is not.
> Example:
> Let's SIZE_MAX to be 255 and _scale to be 10. SIZE_MAX, _scale and amount are `unsigned char`.
> When `amount` is 252, the expression `amount + _scale/2 <= SIZE_MAX` is evaluated as:
> - `a = amount + _scale/2`
> - `a <= SIZE_MAX`
> So we have:
> - `a = 252 + 10/2 == 257 % 256 => 1`
> - `1 <= 255 => true`
> 
> Now evaluate `amount <= SIZE_MAX - _scale/2`:
> - `a = SIZE_MAX - _scale/2` => `a = 255 - 5` => 250
> - `amount <= a` => `252 <= 250` => false
> 
> You can see that `amount <= SIZE_MAX - _scale/2` detects overflow.
> 
>> I used `(a + b) > a` where `b != 0` for finding if an overflow happened or not.
> 
> This works for unsigned types in C++ but the result is undefined for signed types. In your code:
> 
> 
>     int64_t amount = s1 - s2;
>     int64_t scale = (int64_t)_scale;
> 
> 
> So according to the C++ standard, the result of `(amount + scale / 2) > amount` is undefined if overflow happens.

My code follows https://wiki.sei.cmu.edu/confluence/display/c/INT30-C.+Ensure+that+unsigned+integer+operations+do+not+wrap for correct handling unsigned wrapping.

I recommend you to read 
- https://wiki.sei.cmu.edu/confluence/display/c/INT31-C.+Ensure+that+integer+conversions+do+not+result+in+lost+or+misinterpreted+data
- https://wiki.sei.cmu.edu/confluence/display/c/INT32-C.+Ensure+that+operations+on+signed+integers+do+not+result+in+overflow

-------------

PR: https://git.openjdk.org/jdk/pull/11514