RFR: 8328934: Assert that ABS input and output are legal [v3]

[Aleksey Shipilev]

On Fri, 12 Apr 2024 09:59:09 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Only assert integral type arguments
>
> src/hotspot/share/utilities/globalDefinitions.hpp line 1112:
> 
>> 1110:   assert(!std::is_integral<T>::value || x != std::numeric_limits<T>::min(),
>> 1111:          "ABS: argument should not allow overflow");
>> 1112:   T res = (x > 0) ? x : -x;
> 
> Beware! If x is MIN_INT, GCC might delete the assertion because the negation below is UB. I'd only do the negation if it won't overflow.

This is why we can't have nice things. So the fix would be to check for overflow first, and the do the abs, like this?


template<class T> inline T asserted_abs(T x, const char* file, int line) {
  if (std::is_integral<T>::value && x == std::numeric_limits<T>::min()) {
#ifdef ASSERT    
    report_vm_error(file, line, "ABS: argument should not allow overflow");
#endif   
    // Do not allow UB, return the overflowed value.
    return x; 
  } else {
    T res = (x > 0) ? x : -x;
#ifdef ASSERT
    if (res < 0) {
      report_vm_error(file, line, "ABS: result should be non-negative");
    }
#endif
    return res;
  }
}

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18751#discussion_r1562337745