My C programming, and especially my attempts at using C++ “as a better C”, can usually use a lot of help. I admit it. My code quality can be inconsistent. Sometimes it’s because I’m not fully dedicated to a specific coding convention to use: Do I want to continue using NULL or should I switch to nullptr? Sometimes it’s because I’m initially focused on getting my device to work, and I take some messy shortcuts. That’s fine, except when those shortcuts get accidentally left in the code at Final Release. Stuff like this:
// // Set the BME280 into NORMAL mode, with 16x oversampling for both // temp and pressure. // writeBuffer[0] = BME280_REGISTER_CONTROL; writeBuffer[1] = 0xB7; status = Bme280WriteData(DevContext, writeBuffer, sizeof(writeBuffer));
OK, that’s not the worst code ever written. But WTF does 0xB7 mean?? Yes, the comment implies it means “16x oversampling for both temp and pressure” but shouldn’t we have used some sort of a symbolic constant instead of a literal hex value?
I’m also sort of famous for coding or cut/pasting a call to some function and then neglecting to check the return status. Yeah, I know: sloppy. I’m not proud of it. Sometimes my mind just starts working faster than my fingers.
Because of these things, I’ve learned to stop worrying and love static analysis tools.
What Tools Are Available?
Visual Studio 2019 incorporates multiple, highly configurable, static analysis tools that can be useful to driver developers. At OSR, we’ve found that the most success lies in tuning each of them properly and using them in combination.
The primary static analysis tool that Visual Studio provides is Microsoft Code Analysis, which everyone just refers to as Code Analysis or “CA.” This is the tool that started out as PREfast, was run via OACR, and eventually was integrated into the “new” C/C++ compiler. CA has a huge number of settings and options, including some that are specific to Windows drivers. In fact, the vast number of ever-changing settings is both one of the tool’s biggest advantages and biggest sources of frustration. We’ll talk more about these later.
A new static analysis option that’s become available starting with VS 2019 is Clang-Tidy. Clang-Tidy is part of the LLVM tool suite. This tool, like Microsoft Code Analysis, examines your code for a wide variety of issues. Some of its checks have to do with common errors, others have to do with what it considers “best practices”, there are some checks that are purely stylistic in nature, and still others are focused on enforcing system-specific coding conventions (for example Clang-Tidy supports a set of checks for adherence to Zircon kernel coding conventions).
There is, of course, one more static analysis tool available to driver devs in the WDK: This is The Mother of All Static Analysis Tools, Static Driver Verifier (SDV). SDV aims at reviewing driver code for functional correctness, by simulating its interactions with the OS. We’re not going to discuss SDV in this article because (a) it provides a very different type of analysis from that provided by Code Analysis and Clang-Tidy, and (b) nobody at OSR has been able to get it to work – at all – in VS 2019. So, we’re going to ignore SDV for now.
SideBar:
Real-Time Squiggles, Dots, Light Bulbs, Wrenches, Colors and Whatnot
It seems there are two very different types of devs: Those who revel in “interactive help as you type” and those that find the pop-ups, squiggles, and extreme chroma-coding distracting and annoying as hell. I am in the former category. I can use all the help I can get, and if I can get it sooner – like, as I’m typing – I figure that just makes my life easier.
Many years ago, when working on an enormous C# project, I learned the value of ReSharper. I discovered that, using it, I could write pretty respectable C# code. Since that time, I’ve tried to up my driver coding game by alternately using ReSharper for C++ and Visual Assist for C/C++.
I used Visual Assist for my C/C++ projects happily for several years. I even wrote an article about why it was my VS extension of choice. But I recently switched to using ReSharper for C++. It’s purely personal preference, but in my opinion the ReSharper people have really upped their game in the last couple of versions when it comes to C++ code inspection and refactoring. No, the C++ support in ReSharper isn’t anywhere nearly as good as ReSharper’s support for C# (where ReSharper practically writes – or re-writes – your code for you). But it’s darn good.
Tools like ReSharper and Visual Assist can feel instantly overwhelming and frustrating to some users. In my experience, the key to happiness with using these tools lies in taking the time to configure and tune them the way you want them. This is also true for the VS IDE, of course. The problem can be that there are so many choices for formatting and code inspection, often with each choice having multiple options, that the tuning and configuring can be overwhelming (or, at least, tiresome). This is especially true if you’re forced to do this tuning and configuring by navigating a long list of check boxes and drop-down lists: Let’s see… do I want a space before and after member operators? Yes? No? Do I even know what this means? How many such options can I really consider in a row before I just quit and get coffee?
One thing that I really value about ReSharper for C++ is that it allows you to tune and configure many options from the code editor, in real-time. This saves you the torture of navigating that long list of names and settings. The way this works is, when an issue is reported you right click it and ReSharper will allow you to tune its “severity” (and thus the type of visual annotation that it displays), or even to disable it (for a given occurrence with a comment, in the whole file with a comment, or even globally).
One thing I love most about ReSharper for C++ is that, in addition to its own real-time inspections and formatting tricks, it has real-time support for Clang-Tidy static analysis inspections. Getting Clang-Tidy feedback at compile time (the way VS provides it) can be useful. But I love getting those hints in real-time in the IDE.
So, for me, more squiggly lines and icons and funny chroma-coding the better. When the COVID Pandemic started and it became clear that I’d need to write actual code while at home and not merely spend countless hours surfing the internet, one of the first things I had to do was get ReSharper installed on my home system (I already had VS and the WDK) and get my config imported from my desktop system at work. If you’ve never used tools like ReSharper or Visual Assist, I urge you to give them a try.
For the avoidance of doubt: Since quitting the Microsoft MVP program last year, I pay the retail price for ReSharper. Neither I nor anyone at OSR has any relationship with, nor receive any compensation from, JetBrains (the company that makes ReSharper) or from Whole Tomato (the company that makes Visual Assist).
Selecting Code Analysis Rules and Options
You can tell VS to run Code Analysis automatically during your builds, by right-clicking on your project, and changing the settings under – not surprisingly – Code Analysis. You can see this in Figure 1 (next page).
In this same page, you select whether you want VS to automatically run Microsoft Code Analysis, Clang-Tidy, or both. Of course, the same caveats apply to Code Analysis settings that apply to any project settings that you make in Visual Studio: Be aware of the Configuration and the Platform for which you’re changing your settings.
Finally, note that in the Code Analysis settings section there are individual pages for indicating the specific configurations for Microsoft Code Analysis and Clang-Tidy. We’ll talk about those in the sections describing each tool.
About Microsoft Code Analysis
One of the biggest frustrations that we encounter with Microsoft Code Analysis is that it changes, sometimes dramatically, with each release of Visual Studio. Sure, it’s “always” been the case that as new versions of the compiler (or new versions of PREfast) are released, old spurious errors get fixed and new and different checks get implemented. But for the last several versions of VS (since the C/C++ compiler was rewritten, in fact) Microsoft Code Analysis has been decidedly temperamental. Sometimes it reports errors when compiling a given project. Sometimes it does not report errors when compiling that same project, with exactly the same configuration and platform. Sometimes it reports errors on one system, but not another. It’s unpredictable. It’s seemingly random. It’s usually maddening.
On the other hand, with each release of VS, CA continues to add more, and sometimes even more useful, inspections. And false positives that existed in previous version of CA usually disappear. This alone is a good argument for keeping updated with the latest version of VS and the WDK.
The way you tell CA which inspections you want to be applied to your code is by selecting a set of rules. Microsoft provides a (large and unwieldy) set of rules collections, that provide various checks and levels of scrutiny.
For years we preached that you should make your drivers build “clean” (with /W4 and) selecting the CA rule set “Microsoft All Rules.” We even had a slide that said this in our WDF seminar. Then, with some update of something or other, “All Rules” suddenly included checks for compliance with the C++ Core Guidelines. While I’m certainly in favor of the C++ Core Guidelines as a general principle, I’m not a believer in every one of the rules that these constructs advocate. Not to mention the fact that many of the rules set forth by the Guidelines simply can’t be applied to writing drivers for Windows (“I.10: Use [C++] exceptions to signal a failure to perform a required task” is the most obvious example).
So, how can you best use Microsoft Code Analysis and its vast array of rule sets in driver projects? Let me try to provide you some guidance that you may find helpful.
Selecting a Microsoft CA Rule Set to Start With
In the project property pages, under the Code Analysis section, in the Microsoft sub-section, you’ll find a drop-down box that contains the known rule sets that you can select. See Figure 2.
Note that you can select one or more rule sets here, and you can even create your very own rule set with your own, customized, sets of CA inspections to apply. If you do that… good luck. And remember to check it in someplace with your project.
Here at OSR, by default we set our driver projects to “Enable Code Analysis on Build” for every Configuration and Property that the project supports, and we select the “Microsoft Driver Recommended Rules” rule set. If you don’t see this rule set as an option, choose “Browse…” in the rule selection dialog and look in the “%ProgramFiles(x86)%\Windows Kits\10\CodeAnalysis” directory. You’ll find the driver-specific rule sets there.
It’s worth noting that the driver-specific inspections that are enabled by the “Microsoft Driver Recommended Rules” and “Microsoft Driver Minimum Rules” are identical. These rule sets only differ in terms in terms of which C/C++ “native” rule sets are also applied. The “Driver Minimum” rule set uses the C/C++ native “Minimum” rules. The “Driver Recommended” rule set uses the C/C++ native “recommended” rules.
In general, Microsoft CA doesn’t usually spew a lot of false positives, and your driver should be able to pass the “Driver Recommended” rule set inspections without any warnings. But saying this is definitely not the same as saying that you should never suppress any warnings that Microsoft CA displays. You can see an example where CA gets it wrong in Figure 3.
I was too lazy to include the code that creates the SPB_TRANSFER_LIST_AND_ENTRIES in the code clip, and unless you write SPB drivers you won’t care what it says in any case, but you can trust me what I tell you that it allocates two entries for the list and the CA warning is just flat-out wrong.
Fortunately, when CA gives you a spurious warning, you can right-click the error (directly in the VS Error List dialog) and tell VS to suppress the warning by putting a comment in the source code. When you suppress a CA warning, do yourself a favor and put a comment that includes the version of the WDK being used and the error type. The WDK version implies a version of VS, so you shouldn’t need to note both (Figure 4).
When we change the tool chain that we use for a given project (for example, if we update from VS 2017 to VS 2019 for a given release of a product) we always (try to) go back to the projects, remove all the warning suppressions, and run CA again on the project. We usually find that when a legitimately spurious error has been suppressed, CA will eventually get fixed and the suppression comment is no longer needed.
So, that’s what we do here at OSR by default on every build. And it’s what we recommend you do as well. But much there’s more you can do. Before releasing code for a project, we’ll usually do some builds with additional rule sets selected.
Going Further with Microsoft CA
Before we release our code to the world, we’ll often do a few Microsoft CA builds using the “Microsoft All Rules” rule set. This can be useful for finding problems that won’t otherwise be flagged, or for pointing out things (like marking parameters or variables const) that you might want to at least think about. Note that many of the inspections that Microsoft CA provides are duplicated by Clang-Tidy, and often in a more effective or useful way. We’ll talk Clang-Tidy later in this article.
When we run “Microsoft All Rules” on our code, we usually find the Core Guidelines checks overwhelming unless we disable a few basic checks for things like C-style casts and uninitialized variables. We do this with #pragma warning(disable : xxxx) statements in the driver’s master header file. You can see the suppressions that we start with for drivers in Figure 5.
#pragma warning(disable: 26438) // avoid "goto" #pragma warning(disable: 26440) // Function can be delcared "noexcept" #pragma warning(disable: 26485) // No array to pointer decay #pragma warning(disable: 26493) // C-style casts #pragma warning(disable: 26494) // Variable is uninitialized
Figure 5 – Suppressions we enable by default when running CA with “Microsoft All Rules”
You may like or you may hate some of these suppressions (note, for example, that we embrace the use of goto). No matter, you’ll almost certainly want to customize them to your chosen coding style, preferences, and practices.
In general, we do not bother trying to get our driver code to the point where “Microsoft All Rules” runs clean. Rather, we use the warnings that are displayed as suggestions. And once we’ve satisfied ourselves with any changes that we’ve made as a result of these suggestions, we switch the project back to running “Microsoft Driver Recommended Rules” on every build by default.
Even Better Code with Clang-Tidy
While running Microsoft CA with “All Rules” can be interesting, running Clang-Tidy can be both interesting and often extremely useful. Clang-Tidy is a very powerful, very comprehensive, and very smart static analyzer that is capable of inspections that enforce things ranging from coding style to correctness.
Unfortunately, VS doesn’t provide any default Clang- Tidy inspections. Instead, you need to enter a list of the Clang-Tidy checks or groups of checks that you want to enable or disable. You do this in the Clang-Tidy subsection of the Code Analysis section of the project property dialog. See Figure 6.
Fortunately, the Clang-Tidy docs are pretty good at explaining the various possible checks, and often provide examples of things the inspection checks for. Because these inspections can be so wide-ranging, your coding style will have a significant impact on the checks you want performed. To give you someplace to start, Figure 7 has the list of checks that we usually enable by default at OSR.
porability-*, readability-*, cert-*, clang-analyzer-*, bugprone- *, modernize-*, misc-*,-modernize-use-trailing-return-type, -modernize-use-using, -modernize-avoid-c-arrays, -modernize-use-auto, -readability-implicit-bool-conversion, -readability-delete-null-pointer, -readability-simplify-boolean-expr, -readability-redundant-control-flow
Figure 7 – The Clang-Tidy settings that we use at OSR
When you enable Clang-Tidy inspections, the outputs and warnings from those inspections appear in the Visual Studio Output window, not in the Error List window (as with Microsoft CA).
Like with Microsoft CA’s “All Rules” rule set, when we run Clang-Tidy on (even) a well-written driver project we usually wind-up with a few dozen warnings. As a result, we almost never aim for zero warnings from Clang-Tidy. Rather, we run with Clang-Tidy analysis one or two times each release cycle, and take under advisement the suggestions it provides. I often find Clang-Tidy’s observations particularly helpful when I’m modernizing an older or an inherited driver project, while I try to bring that driver’s code up to a bit of a higher standard. Given that cross-platform code is becoming more important these days, one check that I love is that it flags non-portable path specifications for include files, where the case of a file name is different from the case that appears in the #include directive. You can see some examples of the type of warnings Clang-Tidy displays in Figure 8.
Go Boldly
Clang-Tidy and “Microsoft All Rules” giving you a bunch of warnings that you will not want to fix is definitely not a good reason for you to choose not to run these analyses. Consider running these checks like a mini in-office code review, but without the insults, rude remarks, and potential to impact your annual bonus. Who wouldn’t want that?
Static Analysis, FTW
It used to be that once you wrote and debugged your code, the only hope for improving it was either a thorough code review by your colleagues or your own code inspection undertaken when you return to a module after some time. Those activities still provide unmatched value. But static analysis tools, whether the ones that are built-in to Visual Studio or those provided by third party VS extensions, can go a long way toward improving code quality, readability, and maintainability.