Seemingly contradictory code in sharedRuntime.cpp

Vladimir Ivanov vladimir.x.ivanov at oracle.com
Thu Oct 17 23:15:32 UTC 2024


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

It looks like _WIN64-specific code in sharedRuntime.cpp is redundant 
now. There's x86-specific implementation of SharedRuntime::frem in 
sharedRuntime_x86.cpp which is not guarded by "!defined(_WIN64)" anymore.

Best regards,
Vladimir Ivanov

> [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
> 


More information about the hotspot-dev mailing list