Seemingly contradictory code in sharedRuntime.cpp

[Vladimir Kozlov]

Thank you, Julian, for reporting this.

The issue was introduced by [1] changes. I think it is incorrectly removed `defined(_WIN64)` from `#if` in sharedRuntime.cpp

Thanks,
Vladimir K

[1] 
https://github.com/openjdk/jdk/commit/ce2a7ea40a22c652e5f8559c91d5eea197e2d708#diff-808315795bdfbd3d48d1665603fc2fe0db416fa299394371f24dbed2cb32caa4

On 10/17/24 8:28 AM, Julian Waters wrote:
> Hi all,
> 
> Recently while compiling the Windows JDK, gcc caught a very strange
> case in sharedRuntime.cpp. The SharedRuntime::frem and
> SharedRuntime::drem methods both contain code that is only compiled if
> _WIN64 is defined. The problem is this is then surrounded by an ifndef
> check for X86. HotSpot defines X86 if AMD64 or IA32 is defined. Since
> IA32 is not a supported architecture, only AMD64 is of interest to us
> here. But AMD64 and _WIN64 will always both be defined at the same
> time, as _WIN64 is an AMD64 platform. So by extension, X86 and _WIN64
> will always be defined together. But remember that the Windows
> specific code in frem and drem will only be compiled if X86 is not
> defined and _WIN64 is defined. This means the block of code that is
> guarded by the _WIN64 define is never ever compiled. To add to that,
> sharedRuntime_x86.cpp also contains alternate method definitions for
> SharedRuntime::frem and SharedRuntime::drem. This all leads me to
> believe it is a bug where the code meant for Windows x64 was
> erroneously added to the implementation for non x86 platforms instead
> of the correct one inside sharedRuntime_x86.cpp. I just thought I'd
> give everyone a heads up, and also to confirm that this is indeed a
> bug, before I start working on a fix
> 
> The code in question:
> 
> #if !defined(X86) // <----- This will always be defined if _WIN64 is defined!
> JRT_LEAF(jfloat, SharedRuntime::frem(jfloat x, jfloat y))
> #ifdef _WIN64 // <----- This will always be defined if X86 is defined!
>    // 64-bit Windows on amd64 returns the wrong values for
>    // infinity operands.
>    juint xbits = PrimitiveConversions::cast<juint>(x);
>    juint ybits = PrimitiveConversions::cast<juint>(y);
>    // x Mod Infinity == x unless x is infinity
>    if (((xbits & float_sign_mask) != float_infinity) &&
>         ((ybits & float_sign_mask) == float_infinity) ) {
>      return x;
>    }
>    return ((jfloat)fmod_winx64((double)x, (double)y));
> #else
>    return ((jfloat)fmod((double)x,(double)y));
> #endif
> JRT_END
> 
> JRT_LEAF(jdouble, SharedRuntime::drem(jdouble x, jdouble y))
> #ifdef _WIN64 <----- This will always be defined if X86 is defined!
>    julong xbits = PrimitiveConversions::cast<julong>(x);
>    julong ybits = PrimitiveConversions::cast<julong>(y);
>    // x Mod Infinity == x unless x is infinity
>    if (((xbits & double_sign_mask) != double_infinity) &&
>         ((ybits & double_sign_mask) == double_infinity) ) {
>      return x;
>    }
>    return ((jdouble)fmod_winx64((double)x, (double)y));
> #else
>    return ((jdouble)fmod((double)x,(double)y));
> #endif
> JRT_END
> #endif // !X86
> 
> best regards,
> Julian