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

[Afshin Zafari]

On Mon, 19 Dec 2022 14:01:34 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 82:
> 
>> 80:     assert(amount <= SIZE_MAX - _scale / 2, "size_t overflow");
>> 81:     amount = (amount + _scale / 2) / _scale;
>> 82:     // We assume the valid range for deltas [INT64_MIN, INT64_MAX] to simplify the code.
> 
> In the assert the comment should be:
> 
> We assume the valid range for deltas [-INT64_MAX, INT64_MAX] to simplify the code.
> 
> 
> I excluded the `INT64_MIN` case to simplify the code.
> The `INT64_MIN` case is when `sizeof(size_t) == sizeof(int64_t) && amount == INT64_MAX + 1 && is_negative == true`.
> For this case `-(int64_t)amount` is UB which needs proper handling.
> The code instead of
> 
> return (is_negative) ? -(int64_t)amount : (int64_t)amount;
> 
> would be more complex:
> 
> if (!is_negative) {
>   return (int64_t)amount;
> } else {
>   return (amount != INT64_MAX+1) ? -(int64_t)amount : INT64_MIN;
> }
> 
> 
> This is why the assert part is `sizeof(size_t) == sizeof(int64_t) && amount <= INT64_MAX)`
> 
> Maybe the better comment is
> 
> // We assume the valid range for deltas [-INT64_MAX, INT64_MAX].
> // We excluded the `INT64_MIN` case to simplify the code.

I couldn't follow your idea.
The `INT64_MAX + 1` results in compiler overflow error.  Isn't it equal to `INT64_MIN`?

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

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