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

[Andrew Haley]

On Fri, 12 Apr 2024 10:01:46 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

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

Or not, because GCC knows that assert may be `noreturn`? In that case never mind.

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

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