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

Evgeny Astigeevich eastigeevich at openjdk.org
Tue Dec 20 15:05:53 UTC 2022


On Tue, 20 Dec 2022 08:56:42 GMT, Afshin Zafari <duke at openjdk.org> wrote:

>> I did not know there was a defect report [#1313](https://cplusplus.github.io/CWG/issues/1313.html) fixed in C++11.
>> Now the standard prohibits to use constant expressions like `INT64_MAX + 1` in code.
>> 
>>> I couldn't follow your idea.
>> 
>> The idea is to show that if you want to handle -(2^63), you need more code. Your comment is not correct. The code does not handle INT64_MIN.
>> 
>> If we limit cases to [-(2^63 - 1), 2^63 - 1] the code would be simpler. IMHO one byte does not worth to increase the code complexity. In most cases -(2^63) would be a result of some wrong calculations.
>
> What is the replacement for `INT64_MAX + 1` in our code?

Ok, if we want to support [INT64_MIN, INT64_MAX] here is the solution:

```c++
inline int64_t diff_in_current_scale(size_t s1, size_t s2) const {
  // _scale should not be 0, otherwise division by zero at return.
  assert(_scale != 0, "wrong scale");

  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 [INT64_MIN, INT64_MAX].
   assert(sizeof(size_t) <= sizeof(int64_t) && amount - (int)is_negative <= INT64_MAX), "cannot fit scaled diff into int64_t");
   if (is_negative) {
     return (sizeof(size_t) == sizeof(int64_t) && amount - 1 == INT64_MAX)
                 ? INT64_MIN : -(int64_t)amount;
   } else {
     return (int64_t)amount;
   }
}


If `sizeof(size_t) < sizeof(int64_t)`, a compiler will optimise  `(sizeof(size_t) == sizeof(int64_t) && amount - 1 == INT64_MAX) ? INT64_MIN : -(int64_t)amount` into `-(int64_t)amount`.

If `sizeof(size_t) == sizeof(int64_t)`, a compiler will optimise  `(sizeof(size_t) == sizeof(int64_t) && amount - 1 == INT64_MAX) ? INT64_MIN : -(int64_t)amount` into `(amount - 1 == INT64_MAX) ? INT64_MIN : -(int64_t)amount`.

The C++ standard guarantees `(int)is_negative` to be either 0 or 1. So the check `amount - (int)is_negative <= INT64_MAX` will be `amount <= INT64_MAX` for positive diffs and `amount - 1 <= INT64_MAX` for negative diffs.

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

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


More information about the hotspot-runtime-dev mailing list