New Static Analysis Rule for Bounds Checking

Jordan

We have added a new experimental static analysis rule in Visual Studio 16.10 version Preview 3 – C26458, WARNING_PATH_SENSITIVE_USE_GSL_AT. The new warning is a more precise and less noisy version of warning C26446, WARNING_USE_GSL_AT. Both warnings analyse standard containers for unchecked element access and they both share the warning message: “Prefer to use gsl::at() instead of unchecked subscript operator (bounds.4).” This new warning, however, uses path sensitive analysis to track buffer size validation calls to provide a less noisy, more targeted warning compared to C26446.

Path sensitive analysis is not an inexpensive operation: the complexity and time required to analyze each function depends on the length, number of branching operations, and individual properties that are tracked in each function. The path simulation walks the function and emulates each branch and loop that it encounters, updating an internal state based on various assumptions made in the code. Consider the following code segment:

    int i{};
    // ...
    if (i < v.size()) {
        // ...
    }
    // ...

As the simulation reaches the branch, the analysis forks its state. In one fork it tracks that i is less than v.size(), and in the other i is greater than or equal to v.size(). The analysis does not necessarily know the value of i or the number of elements in v. It will only know the relation between these two due to the comparison. The same branching happens when the analysis encounters a loop.

Example and comparison to C26446:

void function(std::vector<int>& v)
{
    if(v.size())
        v[0]; // C26446
    v[0]; // C26446 and C26458
}

In the path sensitive check, one branch simulated knows that v is not empty proving that it is safe to access the first element, but in the other, v is empty, which is why the second access issues the warning. C26446, on the other hand, will warn on any operator[] call not made by a gsl::span.

Why is this check in experimental and what can it not do?

Currently, C26458 does not track expansion of the container. Meaning that calls to push_back, emplace, insert, etc. are not yet supported. Nor does it track removal of elements. However, unlike the expansion of the container, reduction requires full revalidation of the containers bounds. Support for expansion/reduction of containers will be added in a future update.

void function(std::vector<int>& v)
{
    if (v.size() > 2)
    {
        v[2]; // C26446

        v.push_back(0);
        v[2]; // C26446
        v[3]; // C26446 & C26458

        v.erase(v.begin()); // element removal currently removes all assumptions for the container
        v[2]; // 26446 & C26458
    }
}

How to enable C26458

Enabling the rule in Visual Studio:

In the Project Properties page: Navigate to Configuration Properties -> Code Analysis -> Microsoft and select C++ Core Guidelines Experimental Rules.

Image Picture1

Alternatively, you can configure your current ruleset to include C26458.

We recommend that users disable C26446 when using C26458 to avoid duplicate warnings.

Feedback and follow up:

Let us know your experience with the new checker, we are eager to hear back from all of you. Also, let us know if you would like to see more path sensitive rules like this in the future. Please download the latest Visual Studio 2019 and give it a try! Any feedback is welcome. We can be reached via the comments below, Developer Community, email, and Twitter.

2 comments

Comments are closed. Login to edit/delete your existing comments

  • Mike Diack

    Tried with with the sample code, had no warning on accessing v[0] with warning 26458 enabled – are you sure the software made it into VS 2019 Update 10 Preview 3?
    More than once I’ve seen features announced at MS Blogs, but they didn’t actually make it into the software being blogged about!!!!!

    • Jordan MaplesMicrosoft employee

      Hi Mike,

      Yes, the warning did ship with VS 2019 Update 10 Preview 3. I just checked and you’re correct, there is a bug with using empty that slipped through the test matrix. I’ve updated the first example to use size instead of empty and have filed a bug to fix the issue.

      Thanks,
      Jordan