RFR: 8240615: is_power_of_2() has Undefined Behaviour and is inconsistent

[Vladimir Kozlov]

+1

But please test it.

Thanks,
Vladimir

On 3/11/20 9:27 AM, Doerr, Martin wrote:
> Hi Andrew,
> 
> I think it's worth fixing as you suggest.
> Current usages don't have a problem, but who knows about future usages?
> 
> Thanks and best regards,
> Martin
> 
> 
>> -----Original Message-----
>> From: Andrew Haley <aph at redhat.com>
>> Sent: Mittwoch, 11. März 2020 16:47
>> To: Doerr, Martin <martin.doerr at sap.com>; Stefan Karlsson
>> <stefan.karlsson at oracle.com>; John Rose <john.r.rose at oracle.com>
>> Cc: hotspot-dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
>> Subject: Re: RFR: 8240615: is_power_of_2() has Undefined Behaviour and is
>> inconsistent
>>
>> On 3/9/20 3:56 PM, Doerr, Martin wrote:
>>> it doesn't run into the UB case, but log2_jint(x) returns 63 for negative x.
>>> I'd treat this as bug. A jint has 32 bit and log2 should never return 63.
>>
>> I guess this bug dates from the days when uintptr_t was 32 bits.
>>
>> I think you're right, but this has never been a problem because all uses
>> are either guarded by is_power_of_2() or they occur in a context (such as,
>> say, os::active_processor_count()) where > 2**31 is extremely unlikely.
>>
>> I'm happy to fix this, though, if anyone thinks I should. The fix is:
>>
>> diff -r f9893c227e12 src/hotspot/share/utilities/globalDefinitions.hpp
>> --- a/src/hotspot/share/utilities/globalDefinitions.hpp	Wed Mar 11
>> 15:02:09 2020 +0000
>> +++ b/src/hotspot/share/utilities/globalDefinitions.hpp	Wed Mar 11
>> 15:45:26 2020 +0000
>> @@ -1022,12 +1022,12 @@
>>
>>   inline int log2_int(int x) {
>>     STATIC_ASSERT(sizeof(int) <= sizeof(uintptr_t));
>> -  return log2_intptr((uintptr_t)x);
>> +  return log2_intptr((uintptr_t)(unsigned int)x);
>>   }
>>
>>   inline int log2_jint(jint x) {
>>     STATIC_ASSERT(sizeof(jint) <= sizeof(uintptr_t));
>> -  return log2_intptr((uintptr_t)x);
>> +  return log2_intptr((uintptr_t)(juint)x);
>>   }
>>
>>   inline int log2_uint(uint x) {
>>
>> --
>> Andrew Haley  (he/him)
>> Java Platform Lead Engineer
>> Red Hat UK Ltd. <https://www.redhat.com>
>> https://keybase.io/andrewhaley
>> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>