Static Analysis Fixes, Improvements, and Updates in Visual Studio 2019 16.9

Avatar

Jordan

The C++ static analysis team’s goal is to make your C++ coding experience as safe as possible. We are adding richer code safety checks and addressing high impact customer feedback bugs posted on the C++ Developer Community page. Thank you for engaging with us and giving us great feedback on the past releases and early previews leading to this point. Going forward, the C++ team will provide a list of code analysis bug and crash fixes with every GA release of Visual Studio. Below is the compilation of improvements and bug fixes that were made from VS 2019 16.8 to 16.9 for code analysis and Cpp Core Check.

Analysis crash fixes:

  • Using an index operator on the address of a non-address and non-array object.
void function() {
    int buf{};
    ((unsigned char*)&buf)[3] = 1;
}
  • Functions with more than 255 arguments would cause a crash during analysis.
  • Array member field addresses were incorrectly converted in dynamic initializer function.
  • Fixed internal compiler error for aggregate initialization in /analyze.
char c[]{'1', {}};
  • Fixed a crash caused during analysis of bitfields and enums.
struct TestStruct {
public:
    enum TestEnum : char { Dummy };
    TestEnum    m1 : 1;
    TestEnum    m2 : 1;
    short       m3;
}

TestStruct Test() {
    return{ TestStruct::Dummy, TestStruct::Dummy, {} };
}
  • Specifying an array of three elements but only providing two elements in the initializer list.
#include <array>
#include <string>
using namespace std;
void function() {
    array<string, 3> arr {"one", "two"};
}
  • Fixed crash on empty KMDF projects.

Bug fixes:

  • Addressed noisy warnings in an object’s destructor when a function that would have initialized or updated the object fails.
  • Support for the GSL functions gsl::as_bytes and gsl::as_writable_bytes was added to prevent C26710 warnings from being issued against otherwise valid buffer accesses.
#include <gsl/span>
void fn1(gsl::span<int, 5> view, byte val) {
    auto bview = as_writable_bytes(view);
    bview[19] = val;  // OK
    bview[20] = val;  // C26710 & C26000
}
  • Fixed ruleset loading failures that occurred when a relative path of a ruleset was used in combination with the exact path of a ruleset directory. E.g: /analyze:rulesetdirectory f:\customRuleset /analyze:ruleset myrules.ruleset
  • Fixed false positives of C6237 and C6285 on if constexpr expressions.
constexpr bool get_condition_a() { return false; }
constexpr bool some_other_check() { return true; }
constexpr void f1() {
    constexpr bool some_condition = get_condition_a();
    if constexpr (some_condition && some_other_check()) {  //Previously issued C6237
        //...
    }
}

constexpr void f2() {
    constexpr int print_debug = false;
    constexpr int headers_debug = false;
    if constexpr (print_debug == true || headers_debug == true) { //Previously issued C6285
        //...
    }
}
  • Fixed false positive of C26444 when returning upon construction.
struct Test {
    int i{};
};

Test foo() {
    return Test(); //Previously issued C26444
}
  • Fixed issue where casts with the same source and destination types were being misidentified as reinterpret cast, which would produce C26490 instead of C26473.
struct S{};
void foo(S* s) {
    S* s2 = static_cast<S*>(s); //Previously C26490, now C26473
}
  • Fixed an incorrect C26465 warning when attempting to cast away const. C26492 will now be issued instead.
struct S{};
void foo(const S& s) {
    const S* pS = &s;
    S* s2 = const_cast<S*>(pS); //Previously C26465, now C26492
}
  • Fixed false positive for C26814 that would be issued on const member variables.
  • Fixed corner case where PREFast entered an infinite loop while examining buffer extents.
  • Fixed false positive of C26815 that fired when assigning a value to a std::optional that is passed by reference into a function.
  • Fixed false positive C26816 when returning a pointer from a vector of pointers.
  • Fixed false positive of C26485 which appeared when calls to printf used string literals chosen by a ternary operator.

Additional changes:

  • Updated support for SARIF format to conform to the version 2.1 specification.
  • Added SARIF support for additional rule action levels for ruleset files. The rule actions can now be specified as “None”, “Default”, “Info”, “Warning”, and “Error”.
  • Removed C26443 – The enforcement for C.128 has changed making C26443 obsolete.
<?xml version="1.0" encoding="utf-8"?>
<RuleSet Name="Example For Warning Levels" Description="" ToolsVersion="16.0">
  <IncludeAll Action="Info" />
  <Rules AnalyzerId="Microsoft.Analyzers.NativeCodeAnalysis"
         RuleNamespace="Microsoft.Rules.Native">
    <Rule Id="C6001" Action="Error" />
    <Rule Id="C6011" Action="Warning" />
    <Rule Id="C6101" Action="Info" />
    <Rule Id="C6201" Action="Default" />
    <Rule Id="C6386" Action="None" />
  </Rules>
</RuleSet>
  • Using a C-style void cast to ignore return values decorated with [[nodiscard]] previously issued C26493 urging developers to not use C-style casts. The new rule C26457 will be issued in its place, guiding the developer to assign the return value to std::ignore if they intend to discard the return value.
#include <tuple>

struct S{};
[[nodiscard]] S fn1();

void function() {
    (void)fn1(); //Previously C26493, now C26457
    std::ignore = fn1();
}
  • The text for C26496 was updated from “The variable '%variable%' is assigned only once, mark it as const (con.4)” to “The variable '%variable%' does not change after construction, mark it as const (con.4)”.

As mentioned earlier, the work that we do is heavily influenced by feedback we receive on the Developer Community so thank you again for your participation. Please continue to file feedback and let us know if there is a checker or rule that you would like to see added to C++ Core Check.

Stay tuned for more C++ static analysis blogs as we work towards 16.10. Coming soon are posts on improvements to C++ Core Check rules, improved diagnostics, and an update on the Microsoft/GSL GitHub project. In the meanwhile, do not hesitate to reach out to us. We can be reached via the comments below or @VisualC on Twitter.

17 comments

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

  • Avatar
    Krzysztof Kawa

    Static analysis can be very noisy with large 3rd party libraries. Is support for /external anywhere on the roadmap or has that been completely dropped? It’s a useful feature that, sadly, never got integrated with static analysis.

    • Avatar
      Sunny ChatterjeeMicrosoft employee

      Krzysztof, support for /external in static analysis is very much in our radar and we will be making some announcements for this feature soon. Please stay tuned. In the mean time, you may continue to use the feature as an experimental option, where you can specify the list of external includes that should not be analyzed using the `CAExcludePath` environment variable. For example:

      /external:env:CAExcludePath
  • Avatar
    Hristo Hristov

    I had an static analysis issue in a CMake based project, which was completely ignored: After running static analysis on a file with capital letters, and if I selected to fix it and then saving it, the file was saved with small letters. The answer was: that it wasn’t a static analysis issue. I tried to report it again after a while to the “proper” team and it was ignored again because it weren’t a static analysis issue. In the output the file name was reported with small letter like so: `somefilename.cpp` instead of `SomeFileName.cpp`. If it isn’t really a static analysis issue (yet caused by running it), how about instead of closing it, to forward it the the appropriate team to fix it?

    I gave up on using the built-in static analysis because of this issue.

    • Avatar
      Sunny ChatterjeeMicrosoft employee

      Hristo, I am sorry to hear that your feedback was closed before reaching a satisfactory resolution. I was able to look up your feedback ticket in the community and will re-visit the resolution with appropriate feature teams.

      • Avatar
        Hristo Hristov

        Thank you for taking your time to look into this. I wonder if there was any resolution since I last tried using the static analysis tools.

        • Avatar
          Sunny ChatterjeeMicrosoft employee

          Hi Hristo,

          We took a look at the issue and were unable to repro this. We believe this may have been fixed by a change in the compiler toolset, which now honors the original file name in the code analysis logs as opposed to converting them to lower-case. Can you please give it another try and let me know if it works for your needs?

          Thank you,
          Sunny

  • Avatar
    Lambert Clara

    Nice! Is there a way to get cl.exe to output “regular” warnings and errors in SARIF format as well?
    Would be cool to have that also for the rest of the tools in the toolchain like link.exe.

  • Avatar
    Serhiy Malokhatko

    Great tool, but just yesterday switched it off due to slow build.
    Of course, we will use it but not every build now.

    • Avatar
      Sunny ChatterjeeMicrosoft employee

      Serhiy, thank you for the feedback and glad you found the tool useful. By default, we apply the “Microsoft Native Recommended Rules” for C++ and want to make sure it is not slowing down projects too much, although 2-3x slowdown is not unexpected. I am curious to learn more about the rules you were using and would be happy to work with you on a set of rules that would add value without slowing down the build. Please feel free to reach out to us by filing a feedback ticket in the developer community. In the future, we may consider adding performance data during rule selection if it makes it easier for our customers to choose the right rules for their project. For example, in general, you could expect syntactic checks to be much faster than path sensitive dataflow checks.

  • Avatar
    Mike Diack

    I have to say – I criticise Microsoft a lot for a lot of stuff about Visual Studio, but credit where it’s due – the improvements and work you have done for C++ static analysis (particularly during VS 2019’s ongoing updates) have been absolutely excellent – thanks to everyone involved.

    • Avatar
      Hristo Hristov

      I remember the days when Microsoft nearly tried to bury C++. Now it looks like they will probably have full C++20 support even before GCC! What an amazing transformation 🙂 From the Poor C++03 -> horrorible C++/CX -> C++20 and they’ll even support C17 soon!

      • Avatar
        Paulo Pinto

        Yes, I remember when Microsoft finally was able to release something that could match C++ Builder and QtDesigner for creating advanced GUIs in C++, but the Windows team decided we were being too productive.

        Now it is back to .NET for designing Windows GUIs, while pushing customers to move into Qt or Firemonkey/VCL, as I am definitly not writing IDL files by hand without any tooling support and then manually copying and merging them just for satisfaction of some ISO purists.