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

[Evgeny Astigeevich]

On Wed, 7 Dec 2022 09:31:35 GMT, Afshin Zafari <duke at openjdk.org> wrote:

>> src/hotspot/share/services/memReporter.hpp line 82:
>> 
>>> 80:       amount -= _scale / 2;
>>> 81:     }
>>> 82:     return amount / _scale;
>> 
>> I think the correct code should be like the following:
>> 
>> 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;
>> if (is_negative) {
>>   assert(amount - 1  <= LONG_MAX, "cannot fit scaled diff into long);
>>   return (long)(-(long long)amount);
>> } else {
>>   assert(amount <= LONG_MAX, "cannot fit scaled diff into long);
>>   return (long)amount;
>> }
>
> New ways of handling wrap-around of size_t variables are implemented and are under tests.

@afshin-zafari,
I am sorry my code for the negative case will have UB on LP64 systems. It assumes `sizeof(long long) > sizeof(long)`.
On LP64 systems: `sizeof(size_t) == sizeof(long) == sizeof(long long) == 8`.
On LP64 systems if `amount == LONG_MAX+1`, then `amount == LLONG_MAX+1`. So `(long long)amount` will have UB.

The correct code for the negative case:


if (is_negative) {
  assert(amount - 1  <= LONG_MAX, "cannot fit scaled diff into long);
  return (amount - 1 == LONG_MAX) ? LONG_MIN : -(long)amount;
}

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

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