Search code examples
c++type-conversioncompiler-warningsstatic-analysis

C++ warn when storing 32 bit value in a 64 bit variable


I recently discovered a hard to find bug in a project I am working on. The problem was that we did a calculation that had a uint32_t result and stored that in a uint64_t variable. We expected the result to be a uint64_t because we knew that the result can be too big for a 32 bit unsigned integer.

My question is: is there a way to make the compiler (or a static analysis tool like clang-tidy) warn me when something like this happens?

An example:

#include <iostream>

constexpr uint64_t MUL64 { 0x00000000ffffffff };
constexpr uint32_t MUL32 { 0xffffffff };

int main() {
    const uint32_t value { 0xabababab };

    const uint64_t value1 { MUL64 * value }; // the result is a uint64_t because
                                             // MUL64 is a uint64_t
    const uint64_t value2 { MUL32 * value }; // i'd like to have a warning here

    if (value1 == value2) {
        std::cout << "Looks good!\n";
        return EXIT_SUCCESS;
    }
    std::cout << "Whoopsie\n";
    return EXIT_FAILURE;
}

Edit:

The overflow was expected, i.e. we knew that we would need an uint64_t to store the calculated value. We also know how to fix the problem and we changed it later to something like:

const uint64_t value2 { static_cast<uint64_t>(MUL32) * value };

That way the upper 32 bits aren't cut off during the calculation. But things like that may still happen from time to time, and I just want to know whether there is way to detect this kind of mistakes.

Thanks in advance!

Greetings,
Sebastian


Solution

  • The multiplication behavior for unsigned integral types is well-defined to wrap around modulo 2 to the power of the width of the integer type. Therefore there isn't anything here that the compiler could be warning about. The behavior is expected and may be intentional. Warning about it would give too many false positives.

    Also, in general the compiler cannot test for overflow at compile-time outside a constant expression evaluation. In this specific case the values are obvious enough that it could do that though.

    Warning about any widening conversion after arithmetic would very likely also be very noisy.

    I am not aware of any compiler flag that would add warnings for the reasons given above.


    Clang-tidy does have a check named bugprone-implicit-widening-of-multiplication-result specifically for this case of performing a multiplication in a narrower type which is then implicitly widened. It seems the check is present since LLVM 13. I don't think there is an equivalent for addition though.

    This check works here as expected:

    <source>:11:29: warning: performing an implicit widening conversion to type 'const uint64_t' (aka 'const unsigned long') of a multiplication performed in type 'unsigned int' [bugprone-implicit-widening-of-multiplication-result]
        const uint64_t value2 { MUL32 * value }; // i'd like to have a warning here
                                ^
    <source>:11:29: note: make conversion explicit to silence this warning
        const uint64_t value2 { MUL32 * value }; // i'd like to have a warning here
                                ^~~~~~~~~~~~~
                                static_cast<const uint64_t>( )
    <source>:11:29: note: perform multiplication in a wider type
        const uint64_t value2 { MUL32 * value }; // i'd like to have a warning here
                                ^~~~~
                                static_cast<const uint64_t>()
    

    Clang's undefined behavior sanitizer also has a check that flags all unsigned integer overflows at runtime, which is not normally included in -fsanitize=undefined. It can be included with -fsanitize=unsigned-integer-overflow. That will very likely require adding suppressions for intended wrap-around behavior. See https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html for details.

    It seems however that this check isn't applied here since the arithmetic is performed by the compiler at compile-time. If you remove the const on value2, UBSan does catch it:

    /app/example.cpp:11:29: runtime error: unsigned integer overflow: 4294967295 * 2880154539 cannot be represented in type 'unsigned int'
    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /app/example.cpp:11:29 in 
    Whoopsie
    

    GCC does not seem to have an equivalent option.


    If you want consistent warnings for overflow in unsigned arithmetic, you need to define your own wrapper classes around the integer types that perform the overflow check and e.g. throw an exception if it fails or alternatively you can implement functions for overflow-safe addition/multiplication which you would then have to use instead of the + and * operators.