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

[Afshin Zafari]

On Wed, 14 Dec 2022 18:32:40 GMT, Evgeny Astigeevich <eastigeevich at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale
>
> src/hotspot/share/services/memReporter.hpp line 69:
> 
>> 67: 
>> 68:   // Convert diff amount in bytes to current reporting scale
>> 69:   inline int64_t diff_in_current_scale(size_t s1, size_t s2) const {
> 
> Why not to base on the code I provided with ideas from @tstuefe:
> 
> 
> bool is_negative = false;
> if (s1 < s2) {
>   is_negative = true;
>   swap(s1, s2);
> }
> 
> size_t amount = s1 - s2;
> assert(amount <= SIZE_MAX - _scale / 2, "size_t overflow");
> amount = (amount + _scale / 2) / _scale;
> // We assume the valid range for deltas [-SSIZE_MAX, SSIZE_MAX] to simplify the code.
> assert((sizeof(size_t) < sizeof(int64_t)) || (sizeof(size_t) == sizeof(int64_t) && amount <= SSIZE_MAX), "cannot fit scaled diff into size_t");
> return (is_negative) ? -(int64_t)amount : (int64_t)amount;

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;

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

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