[aarch64] assert(is_power_of_2(x)) failed: x must be a power of 2: 0xffffffff80000000

[Andrew Haley]

On 2/25/20 2:52 PM, Schmidt, Lutz wrote:
>
>
> On 25.02.20, 10:16, "Andrew Haley" <aph at redhat.com> wrote:
>
>     On 2/24/20 4:27 PM, Schmidt, Lutz wrote:
>     >
>         >     > A possible fix would include:
>     >     >  - make is_power_of_2() and exact_log_2() sign-aware.
>     >     >  - call is_power_of_2() and exact_log_2() with arguments of proper
>     >     > type (juint in this case).
>     >
>     >     Or fix exact_log2(jint) by specializing it appropriately.
>     >
>     > Yes, by treating the jint argument as unsigned.
>
>     OK, I'll look at that. However, studying the code, we have at least
>     one more problem, and it's a dangerous one.
>
>     template <typename T>
>     bool is_power_of_2_t(T x) {
>       return (x != T(0)) && ((x & (x - 1)) == T(0));
>     }
>
>     // long version of is_power_of_2
>     inline bool is_power_of_2_long(jlong x) {
>       return ((x != NoLongBits) && (mask_long_bits(x, x - 1) == NoLongBits));
>     }
>
>     These overflow when x is the largest negative integer. This is
>     Undefined Behaviour, so we have to fix it somehow. We'd do much better
>     converting to an unsigned type and then performing the calculation on
>     that. Maybe we should provide just one definition of this function,
>     casting its argument to uint64_t.
>
> Isn't it such that unsigned arithmetic on today's processors differs
> from signed arithmetic only by interpretation of the MSB (unsigned:
> most significant bit, signed: sign bit), and possibly by indication
> of overflow?

Please forgive me for not wording the reply myself, but
https://blog.regehr.org/archives/213 addresses this directly:

   It is very common for people to say — or at least think — something
   like this:

       The x86 ADD instruction is used to implement C’s signed add
       operation, and it has two’s complement behavior when the result
       overflows. I’m developing for an x86 platform, so I should be
       able to expect two’s complement semantics when 32-bit signed
       integers overflow.

   THIS IS WRONG.

It's worth reading the rest of that blog.

> No matter how (signed or unsigned) you calculate (0x8000_0000 - 1),
> you will always get 0x7fff_ffff as result.

https://blog.regehr.org/archives/213 again:

   If any step in a program’s execution has undefined behavior, then
   the entire execution is without meaning. This is important: it’s
   not that evaluating (1<<32) has an unpredictable result, but rather
   that the entire execution of a program that evaluates this
   expression is meaningless. Also, it’s not that the execution is
   meaningful up to the point where undefined behavior happens: the
   bad effects can actually precede the undefined operation.

Not only is this possible in theory, it happens in practice. We're
particularly exposed to this risk right now because we've just
switched to a new version of GCC.

> That behavior makes is_power_of_2((jint)0x8000_0000)) return "true".
>
> Viewed from close to the hardware, there is no UB. It is introduced
> by a higher level in the stack.  Up there, you don't want to know
> too much about H/W characteristics. On the other hand, don't the
> upper levels rely on integers being stored in 2's complement format?
> With all the arithmetic implications, where the signed/unsigned
> equivalence is one of?
>
> All the confusion/unexpected behavior originates from the mixed
> interpretation (signed/unsigned) of operands and, as a consequence,
> unexpected/unwanted sign extensions.

Among other things, yes.

>     UB problems keep occurring. It'd be good to test HotSpot changes with
>     -fsanitize=undefined, but we still have so much UB in HotSpot that
>     it's impractical to do so because there's so much noise.
>
> That's a compelling dream!

We have a lot of accumulated technical debt in HotSpot which we need
to clean up. We should get it done eventually, IMO. But we certainly
should not be introducing UB now.

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