Klocwork complains about unsigned to zero comparison that is always true -- WHY?

Question

Klocwork says that "comparison of unsigned value against 0 is always true", on the following condition line:

#define MAX_VALUE 8589934592 //2^33
...
uint64_t val = get_val();
if (val >= MAX_VALUE)
{
  return ERROR_INVALID_VALUE;
}

Why? MAX_VALUE isn't 0...

Please advise.

Thank you in advance.

Answer 1

#define MAX_VALUE 8589934592 //2^33

That literal might be a problem, depending on which version of C++ you're using and how your compiler chooses to treat it.

In the 2003 standard, an unsuffixed decimal literal is of type int if it's in the range of int, or of type long int if it's in the range of long int; otherwise, the behavior is undefined. If your version of gcc conforms to the 2003 standard, the undefinedness of the behavior could likely manifest itself as 8589934592 evaluating to 0, which would explain the problem you're seeing. (But I'd expect a warning about the integer literal being out of range; did you get such a warning?)

In the 2011 standard, such a literal can be of type int, long, or long long. (C++ 2003 doesn't have long long; it was borrowed from C99.)

(Note that ^ is the bitwise xor operator; C++ has no exponentation operator. That doesn't matter much in a comment, but 2**33 might be clearer, even though that's not a valid C++ expression either.)

...
uint64_t val = get_val();

What type does get_val() return? That shouldn't affect the warning, but it would be interesting to know.

if (val >= MAX_VALUE)

If long is 64 bits on your system, then this should be ok. If long is 32 bits, then uint64_t would have to be something wider than long / unsigned long -- which implies that your compiler is not operating in conforming C++ 2003 mode.

What output do you get for the following program?

#include <stdint.h>
#include <limits.h>
#include <iostream>
#include <ctime>

uint64_t get_val();

const char *type_name(int i) { return "int"; }
const char *type_name(long i) { return "long"; }
const char *type_name(unsigned long i) { return "unsigned long"; }
const char *type_name(long long i) { return "long long"; }
const char *type_name(unsigned long long i) { return "unsigned long long"; }

int main() {
    std::cout << "UINT_MAX   = " << UINT_MAX << "\n";
    std::cout << "ULONG_MAX  = " << ULONG_MAX << "\n";
    std::cout << "8589934592 = " << 8589934592 << "\n";
    std::cout << "8589934592 is of type " << type_name(8589934592) << "\n";

    uint64_t val = time(0);
    if (val >= 8589934592) {
        std::cout << "It's getting late!\n";
    }
}

and what warning, if any, does Klocwork give you on the comparison? If Klockwork gives you a warning that's inconsistent with the output of the program, this could be a bug in Klocwork (which you should report) -- or you might just need to invoke it with different options. (I've never used Klocwork myself.)

You might be able to solve (or at least work around) the problem by invoking g++ with -std=c++0x.

Answer 2

Tools like Klocwork has a certain rate of "False Positive", usually 15%, but up to 50% in some cases, actually a large part of my job is to look at output of Static Code Analysis Tools and decide if the error they reporting are False Positive(or if there is False Negatives )

To cut a long story short, show us the prototype of get_val() and tell me what types it should return and I will see why it reports that message and if it is a FP.

Answer 3

Alternative solution without big numbers (avoiding the size-of-literal problem)

const int MAX_VALUE_LOG2 = 33;
...
uint64_t val = get_val();
if ( (val >> MAX_MAX_VALUE_LOG2) > 0)
{
  return ERROR_INVALID_VALUE;
}