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