Redis bitfield Command Use (uint64_t)-1 << 64，this is undefined behavior

127.0.0.1:6380> bitfield hello set i64 0 -2
1) (integer) 0
127.0.0.1:6380> bitfield hello get i64 0
1) (integer) -1
127.0.0.1:6380> get hello
"\xff\xff\xff\xff\xff\xff\xff\xfe"
127.0.0.1:6380> bitfield hi set i64 0 9223372036854775807
1) (integer) 0
127.0.0.1:6380> bitfield hi incrby i64 0 1
1) (integer) -1
127.0.0.1:6380> get hi
"\xff\xff\xff\xff\xff\xff\xff\xff"

When use i64, Redis will use (uint64_t)-1 << 64，this behavior is undefined. On my compiler, ((uint64_t)-1 << 64) == ((uint64_t)-1).

In function getSignedBitfield, the value will always be -1:

    if (value & ((uint64_t)1 << (bits-1)))
        //  value will be -1 if bit == 64
        value |= ((uint64_t)-1) << bits;

In function checkSignedBitfieldOverflow, c will always be -1:

    {
        uint64_t mask = ((uint64_t)-1) << bits;
        uint64_t msb = (uint64_t)1 << (bits-1);
        uint64_t a = value, b = incr, c;
        c = a+b; /* Perform addition as unsigned so that's defined. */

        /* If the sign bit is set, propagate to all the higher order
         * bits, to cap the negative value. If it's clear, mask to
         * the positive integer limit. */
        if (c & msb) {
            c |= mask;   // c will be -1 if bit == 64
        } else {
            c &= ~mask;
        }
        *limit = c;
    }

In function checkUnsignedBitfieldOverflow，Because bits can't be 64, it is OK.

Comment From: antirez

Thanks @huangzhw! Very good catch. Do you agree with the proposed fix in 746297314? The idea of the fix is that, when -1 << 64 would overflow, is because we don't need to do any masking at all.

Comment From: huangzhw

@antirez I agree with the proposed fix in 7462973. It is simple and correct!

Comment From: antirez

Thanks @huangzhw! Closing.