Understanding the Sub-expression overflow reasoning

Question

I am trying to understand the reasoning behind this particular suggestion in Visual Studio 2022, as it doesn't seem to make sense to me. Here's the simple code:

static constexpr uint32_t MAX_ELEMENTS = 100000;
const std::vector<int> A{ some random ints };


std::vector<bool> found(MAX_ELEMENTS);
for (int value : A)
{
    if (value > 0 && value <= MAX_ELEMENTS)
        found[value - 1] = true;             // Problem it complains about is the value - 1.
}

It suggests that a "sub-expression may overflow before being added to a wider type". Now obviously the condition-if prevents this from ever happening, but what's the reasoning here?

Now if this was a Spectre thing, I could understand that the compiler would add a memory fence to stop any speculative execution of the statement after the if, but I don't think that's the answer here.

My only thinking is that it has to do with the subscript operator on the vector; that its index is of a type larger than int and is implicitly cast to size_t?

Just seems a little unintuitive that the below is an issue:

found[value - 1]

and its perfectly fine to do this,

int a = value - 1;
found[a];

when the end result is the same. What am I missing here in terms of understanding?

Matthei

@Sebastian: Only signed overflow is UB, unsigned overflow is modular as per the spec. – Fidole

Deleted wrong comment – Circular

Answer 1

In this case, it is a false positive, as you suspected. This is a rule that sometimes gets used in stricter code bases. (This particular warning is an error in MISRA, for example.)

A lot of warnings are like this... the compiler writers are trying to detect a situation where the behavior of the program is unexpected or unintentional, but the warnings are not always correct. For example,

uint64_t x = 0xffffffffu << 16;

On a system with 32-bit int, the standard is very clear what the value of this is supposed to be... it's supposed to be:

uint64_t x = 0xffff0000u;

However, it does look like someone meant to write this instead:

uint64_t x = (uint64_t)0xffff0000 << 16;

That's what the warning is trying to catch. That's the reasoning behind this rule. In your case, the compiler is doing a bad job of it.

You can see a more detailed justification for the warning in the compiler documentation: Warning C26451.

Spectre has nothing to do with it.

It's perfectly fine...

To do this:

int a = value - 1;
found[a];

In this case the "intent" is more obvious, because the programmer has explicitly written out "int" as the type.

Solutions

This is a code style issue, so there are a few different solutions and you pick whatever one you feel comfortable with.

• Leave the warning on (noisy, may distract you from real warnings)

• Assign the expression to an int first (verbose)

• Disable the warning across the code base (if you don't think it's helpful)

• Disable the warning around this location with a #pragma warning (very verbose)

Fidole

Other (often better) solutions for fixing warnings could be: Fix the reason for the warning or rewrite the overall code in a way the compiler does not issue a warning. – Circular

@Circular Whilst I agree with you completely here, the above code is simple enough to understand without rewriting it to get around a warning where there shouldn't be one. – Matthei

Answer 2

I played a little bit with this suggestion, and discovered that the reason why it is generated is because vector<T> size is a 64-bits number, and int is 32-bits.

But the suggestion will never be generated when we assign a single value to a wider type, only an expression. The expression should not be assigned to a narrower type even explicitly, wider type should be used.

For example, this will work:

long long a = 2;
long long a = (int) 2;

but this will generate a suggestion:

long long a = 2 - 1;
long long a = (int) (2 - 1);

The solution is to convert the arithmetic expression to a wider type, or to use a wider operand(s):

long long a = (long long) 2 - 1;

long long b = 2;
long long a = b - 1;

Interestingly, this suggestion will never appear when we deal with types smaller than 64-bits, maybe because all arithmetic operations are at least 32-bits wide...

Here is an article from the official page: https://learn.microsoft.com/en-us/cpp/ide/lnt-arithmetic-overflow?view=msvc-170

Topless