RFR: 8346914: UB issue in scalbnA

[Axel Boldt-Christmas]

On Thu, 5 Jun 2025 14:34:38 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> I think that is equivalent, but I did not want to disrupt the general control flow of the algorithm so that it still closely resembles the original fdlibm code.
>> 
>> I also initially misread your suggestion and now I am wondering if I can actually simplify it to a simple two line change:
>> 
>> unsigned u_k = k + n; // avoid UB signed integer overflow
>> k = (int) u_k;  // safely assign to k
>> 
>> does that bypass any UB?
>
> @dholmes-ora , if `k = (int) u_k;` does not say to the compiler that it can assume that `0 <= u_k < 2**31 - 1`, then this seems like a good changeset. Frankly, I would not trust myself in this matter, I find this very finicky.

Initially I thought it would be nice if we could have expressed this as simply `k = wrapping_add(k, n);`. But it would require implementing such a method correctly, which seems non-trivial. (Some compilers have intrinsics for this I think).

Might it is the case that casting the number here is also UB (if the unsigned number is to large). What we really need is a bit_cast (and use 2s complement, which I think is only guaranteed in C++20, but I think we already make assumptions about this, or we could use int32_t which at least should be 2s complement, not 100%).

But then again looking at this code we also read from none active members of a union, which I think is a compiler language extension. (Only valid in C). 

If we keep the structure as it is, I would find the code more readable if it was `u_k <= (unsigned)std::numeric_limits<int>::max()`

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

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