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

[Afshin Zafari]

On Wed, 7 Dec 2022 14:04:36 GMT, Evgeny Astigeevich <eastigeevich at openjdk.org> wrote:

>> 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;
> }

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;
  }

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

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