RFR: 8346914: UB issue in scalbnA

[David Holmes]

On Tue, 17 Jun 2025 20:04:18 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> I'm not seeing the full suggestion here. In the original code this line:
>> 
>> if (k > 0x7fe) return hugeX*copysignA(hugeX,x); /* overflow  */
>> 
>> is defining a logical overflow, not the wrapping overflow that we are trying to deal with. The wrapping overflow results in a negative value, which is the third case that gets handled. So AFAICS we need to use `u_k` all the way through until the end.
>
> Here is the diff for what I'm suggesting.
> 
> diff --git a/src/hotspot/share/runtime/sharedRuntimeMath.hpp b/src/hotspot/share/runtime/sharedRuntimeMath.hpp
> index 91dda2a4fe8..321f3be580a 100644
> --- a/src/hotspot/share/runtime/sharedRuntimeMath.hpp
> +++ b/src/hotspot/share/runtime/sharedRuntimeMath.hpp
> @@ -111,16 +111,23 @@ static double scalbnA(double x, int n) {
>      if (n< -50000) return tiny*x;       /*underflow*/
>    }
>    if (k==0x7ff) return x+x;             /* NaN or Inf */
> -  k = k+n;
> -  if (k > 0x7fe) return hugeX*copysignA(hugeX,x); /* overflow  */
> +  // If the high (sign) bit of u_k is set, then either
> +  // n is positive and k+n would overflow, or
> +  // n is negative and |n| > k.
> +  unsigned u_k = (unsigned)k + (unsigned)n;
> +  if ((u_k > 0x7fe) && (n > 0)) {
> +    // Either (k+n > 0x7fe && k+n <= INT_MAX) or k+n would overflow.
> +    return hugeX*copysignA(hugeX,x); /* overflow  */
> +  }
> +  // Set k to k+n, now that we know k+n wouldn't overflow.
> +  // We know that k+n <= (int)0x7fe, and might be negative if n is negative.
> +  k = (int)u_k;
>    if (k > 0) {                          /* normal result */
>      set_high(&x, (hx&0x800fffff)|(k<<20));
>      return x;
>    }
>    if (k <= -54) {
> -    if (n > 50000)      /* in case integer overflow in n+k */
> -      return hugeX*copysignA(hugeX,x);  /*overflow*/
> -    else return tiny*copysignA(tiny,x); /*underflow*/
> +    return tiny*copysignA(tiny,x); /*underflow*/
>    }
>    k += 54;                              /* subnormal result */
>    set_high(&x, (hx&0x800fffff)|(k<<20));

+  if ((u_k > 0x7fe) && (n > 0)) {
+    // Either (k+n > 0x7fe && k+n <= INT_MAX) or k+n would overflow.
+    return hugeX*copysignA(hugeX,x); /* overflow  */
+  }

But that is not correct - we only take this "overflow" path for 0x7fe < k+n < INT_MAX

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25656#discussion_r2153604667