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

[Doerr, Martin]

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