RFR: 8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale
Evgeny Astigeevich
eastigeevich at openjdk.org
Wed Dec 7 14:06:53 UTC 2022
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
More information about the hotspot-runtime-dev
mailing list