RFR: 8281213: Unsafe uses of long and size_t in MemReporterBase::diff_in_current_scale
Thomas Stuefe
stuefe at openjdk.org
Wed Dec 7 14:33:58 UTC 2022
On Wed, 7 Dec 2022 14:16:57 GMT, Afshin Zafari <duke at openjdk.org> wrote:
>> @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;
>> }
>
> 1) When amount is negative, it is a very large positive number as it is unsigned.
> 2) amount +/- _scale/2 may wrap-around the boundaries.
> 3) In some cases, the actual negative amount should be returned. Expected by some tests.
>
> My new code under test (All NMT tests passed locally, mach5 tier1-5 ongoing) is as follows:
>
> inline long 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");
>
> // Since size_t is unsigned, the amount >= 0 is always true.
> // Therefore, shifting the amount half the scale down/up is
> // decided based on the s1 and s2. Thus, s1 >= s2 means amount >= 0.
> // ((a + b) > a) where b != 0, is used to check wrap-around of adding two size_t variables.
> // ((a - b) < a) where b != 0, is used to check wrap-around of subtracting two size_t variables.
> size_t amount = s1 - s2;
> if (amount == 0){
> return 0L;
> }
>
> if (s1 >= s2) {
> if ((_scale / 2) != 0) {
> assert((amount + _scale / 2) > amount, "difference is greater than the upper limit.");
> amount += _scale / 2;
> }
> return (long)(amount / _scale);
> }
>
> if ((_scale / 2) != 0){
> assert((amount - _scale / 2) < amount, "difference is less than the lower limit.");
> amount -= _scale / 2;
> }
> return (long) amount / (long) _scale;
> }
I don't get how this helps for deltas > 4gb if long is 4 and size_t is 8 bytes?
-------------
PR: https://git.openjdk.org/jdk/pull/11514
More information about the hotspot-runtime-dev
mailing list