Search code examples
c++portability

c++ portable address encoding


I'm writing a software that, at some point must write internal addresses into a buffer. I wrote the following code which works. But produce warnings when cross-compiling to a target device with an address size smaller than 64 bits.

How can I make this portable without generating errors? I would have expected gcc to ignore the warning due to the condition-always-false around the problematic statement. I get the same behaviour when doing this with a template (where i feed sizeof(void*()) as address_size).

uint8_t decode_address_big_endian(uint8_t* buf, uintptr_t* addr)
    {
        constexpr unsigned int addr_size = sizeof(void*);
        static_assert(addr_size == 1 || addr_size == 2 || addr_size == 4 || addr_size == 8, "Unsupported address size");

        uintptr_t computed_addr = 0;
        unsigned int i = 0;

        if (addr_size >= 8)
        {
            computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 56));
            computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 48));
            computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 40));
            computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 32));
        }

        if (addr_size >= 4)
        {
            computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 24));
            computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 16));
        }

        if (addr_size >= 2)
        {
            computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 8));
        }

        if (addr_size >= 1)
        {
            computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 0));
        }
        
        *addr = computed_addr;

        return static_cast<uint8_t>(addr_size);
    }

The code works. MSVC is able to optimize that to a single x86_64 instruction. It also works on a AtMega328p, but avr-gcc do throw these warnings.

/home/py/scrutiny-embedded/lib/src/protocol/scrutiny_protocol_tools.cpp:32:72: warning: left shift count >= width of type [-Wshift-count-overflow]
                 computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 56));
                                                                        ^
/home/py/scrutiny-embedded/lib/src/protocol/scrutiny_protocol_tools.cpp:33:72: warning: left shift count >= width of type [-Wshift-count-overflow]
                 computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 48));
                                                                        ^
/home/py/scrutiny-embedded/lib/src/protocol/scrutiny_protocol_tools.cpp:34:72: warning: left shift count >= width of type [-Wshift-count-overflow]
                 computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 40));
                                                                        ^
/home/py/scrutiny-embedded/lib/src/protocol/scrutiny_protocol_tools.cpp:35:72: warning: left shift count >= width of type [-Wshift-count-overflow]
                 computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 32));
                                                                        ^
/home/py/scrutiny-embedded/lib/src/protocol/scrutiny_protocol_tools.cpp:40:72: warning: left shift count >= width of type [-Wshift-count-overflow]
                 computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 24));
                                                                        ^
/home/py/scrutiny-embedded/lib/src/protocol/scrutiny_protocol_tools.cpp:41:72: warning: left shift count >= width of type [-Wshift-count-overflow]
                 computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << 16));
                                                                        ^

In this case, I can make a static_assert on address_size to prove that it is indeed equal to 2. But still get the warnings.


Solution

  • Try

    if constexpr (addr_size >= 8)
    

    If C++17 is not available to you, you can suppress the warning

    if (addr_size >= 8) {
       computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << ((addr_size >= 8) * 56)));
       computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << ((addr_size >= 8) * 48)));
       computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << ((addr_size >= 8) * 40)));
       computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << ((addr_size >= 8) * 32)));
    }
    

    If addr_size >= 8 is true, it evaluates to 1 in the multiplication arguments. Otherwise, it would evaluate to 0, but the branch is not executed.

    Shorter with @RaymondChen's hint.

    if (addr_size >= 8) {
       computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << (56 % (addr_size * 8));
       computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << (48 % (addr_size * 8));
       computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << (40 % (addr_size * 8));
       computed_addr |= ((static_cast<uintptr_t>(buf[i++]) << (32 % (addr_size * 8));
    }
    // Will be shorter with constexpr unsigned int addr_size = sizeof(void*) * 8;
    

    Alternative code

    if (addr_size >= 8) {
       uint64_t tmp;
       tmp = static_cast<uintptr_t>(buf[i++]) << 28;
       computed_addr |= tmp << 28;
       tmp = static_cast<uintptr_t>(buf[i++]) << 24;
       computed_addr |= tmp << 24;
       tmp = static_cast<uintptr_t>(buf[i++]) << 20;
       computed_addr |= tmp << 20;
       tmp = static_cast<uintptr_t>(buf[i++]) << 16;
       computed_addr |= tmp << 16;
    }