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

[Thomas Stuefe]

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