Workaround for Spectre warning MSVC C5040

Question

MSVC just released an update which has added a new warning about some code the compiler will inject to mitigate (apparently some small bit) of Spectre:

https://blogs.msdn.microsoft.com/vcblog/2018/01/15/spectre-mitigations-in-msvc/

Here's a little MCVE derived from their example of "problematic" code:

#include <stdio.h>

int main(int argc, char *argv) {
    unsigned char array1[1] = {0};
    int array1_length = 1;
    unsigned char array2[1] = {99};
    int untrusted_index = 0; /* in this MCVE, I trust it, compiler doesn't */
    for (; untrusted_index < array1_length; ++untrusted_index) {
        unsigned char value = array1[untrusted_index];
        unsigned char value2 = array2[value * 64];
        printf("Picked value %d\n", value2);
    }
    return 0;
}

"In the above example, the code performs an array-bounds check to ensure that untrusted_index is less than the length of array1. This is needed to ensure that the program does not read beyond the bounds of the array. While this appears to be sound as written, it does not take into account microarchitectural behaviors of the CPU involving speculative execution."

So you now get a warning:

Warning C5045: Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified

Which is its way of telling you that this code could end up being slower than you might like it to be (if compiled /Qspectre), because it's going to put in some extra protections.

Since it doesn't seem you can take anything for granted, I'm suspicious of making changes that "just make the warning go away". For instance, changing untrusted_index < array1_length to untrusted_index != array1_length seems to do it, for the specific instance of the MCVE code I give here. But is that a viable patch, or is their warning just incomplete--and in the next update, it will complain about that too?

I know I can disable the warning, with /wd5040, or otherwise. But I'm interested in making sure that if the code is compiled with /Qspectre that there are not slowdowns, and that there are no warnings if it is not compiled with /Qspectre. I don't want to go around touching files changing < to != in loop conditions--or whatever--if that's just churn.

So a bigger question would be if there are legitimate workaround patterns that are this basic, why aren't there some mentions of them? For instance the case I describe is an iteration where I control the index, and don't have to worry about it coming from an "untrusted source". Yet I got a warning, and switching from < to != made it go away. Why? Should it have?

Answer 1

From the article itself:

It is important to note that there are limits to the analysis that MSVC and compilers in general can perform when attempting to identify instances of variant 1. As such, there is no guarantee that all possible instances of variant 1 will be instrumented under /Qspectre.

You've probably encountered one of the cases where the current implementation of /Qspectre won't mitigate the vulnerability by design. This is reasonable because excessive use of LFENCE may significantly reduce performance. Mitigating every single instance of variant 1 that appears in the code is just too expensive to be done fully in software (using LFENCE).

In the comments, someone asked:

Can you characterize for developers what MSVC’s limits are, and what more developers need to do to protect themselves from “variant 1”?

The author of the article replied:

We’re not going into the details MSVC’s implementation. A lot of people and companies rely upon our tools so we’re going to err on the side of caution with regards to what we discuss publicly.

So Microsoft doesn't seem to want to disclose exactly which instances of variant 1 won't be mitigated by /Qspectre.

Answer 2

If you don't want the warning just use #pragma warning(disable :5040), or disable it in the project property page.

And note that your offered change to "untrusted_index != array1_length" is not sufficient as it leaves the entire range greater than the size open to abuse.

Remember, this diagnostic simply tells you that the compiler would do something different than before with the specter mitigation enabled, it's not really telling you that you necessarily need to do anything to the code.