Static code analysis

Note from the translator . Initially, this article was published on the AltDevBlogADay website. But the site, unfortunately, ceased to exist. For more than a year this article remained unavailable to readers. We turned to John Carmack , and he said he did not mind if we posted this article on our website. What we gladly did . You can get acquainted with the original article using the Wayback Machine - Internet Archive: Static Code Analysis .

Since all the articles on our website are in Russian and English, we have translated the Static Code Analysis article into Russian. And at the same time they decided to publish it on Habré. There has already been published a retelling of this article . But I am sure that it will be interesting for many to read the translation.

I consider my main achievement as a programmer in recent years to be familiar with the static code analysis technique and its active use. It's not even so much the hundreds of serious bugs that are not allowed into the code thanks to it, but rather the change caused by this experience in my programmer's worldview regarding the reliability and quality of software.

Immediately it should be noted that it is impossible to reduce everything to quality, and admitting this does not mean at all to betray some of our moral principles. The value of the product you create as a whole is of value , and the quality of a code is only one of its components, along with cost, functionality and other characteristics. The world knows many super-successful and respected game projects, stuffed with bugs and without end falling; Yes, and it would be foolish to approach the writing of the game with the same seriousness with which they create software for space shuttles. And yet quality is undoubtedly an important component.
')
I always tried to write good code. By my nature, I am like an artisan who is driven by the desire to continuously improve something. I read piles of books with boring titles like “Strategies, Standards and Quality Plans”, and working at Armadillo Aerospace opened the way for me to a completely different world of software development with different security requirements than my previous experience.

More than ten years ago, when we were developing Quake 3, I bought a license for PC-Lint and tried to use it in my work: I was attracted to the idea of ​​automatically detecting defects in code. However, the need to run from the command line and view long lists of diagnostic messages discouraged my use of this tool, and I soon abandoned it.

Since then, both the number of programmers and the size of the code base have grown by an order of magnitude, and the emphasis in programming has shifted from C to C ++. All this has prepared a much more fertile ground for program errors. Several years ago, after reading a selection of scientific articles on modern static code analysis, I decided to check how the state of affairs in this area has changed in the past ten years since I tried to work with PC-Lint.

At that time, the code was compiled at the fourth level of warnings, while we left off only a few highly specialized diagnostics. With this approach — knowingly treat every warning as an error — programmers were forced to strictly adhere to this policy. And although in our code it was possible to find several dusty corners in which all the “garbage” had accumulated over the years, on the whole it was quite modern. We thought that we had quite a good code base.

Coverity

It all started with the fact that I contacted Coverity and signed up for a test diagnosis of our code with their tool. This is a serious program, the cost of the license depends on the total number of lines of code, and we stopped at a price expressed by a five-digit number. Showing us the results of the analysis, the experts from Coverity noted that our base was one of the cleanest in its “weight category” of all that they had seen (perhaps they say this to all customers to encourage them), but the report that they We were handed over, contained about a hundred problem areas. This approach was very different from my previous experience with PC-Lint. The signal-to-noise ratio in this case turned out to be extremely high: most of the warnings issued by Coverity really pointed to clearly incorrect parts of the code that could have serious consequences.

This case literally opened my eyes to a static analysis, but the high price of the whole pleasure kept me from buying an instrument for a while. We thought that in the code remaining before the release we would have not so many errors.

Microsoft / analyze

It is possible that I would finally decide to buy Coverity, but while I was pondering over this, Microsoft got rid of my doubts by implementing the / analyze function in 360 SDK. / Analyze was previously available as a component of a top-end, insanely expensive version of Visual Studio, and then suddenly got free for every xbox 360 developer. I understand that the quality of games on the 360th Microsoft platform is more than the quality of Windows software. :-)

From a technical point of view, the Microsoft analyzer only conducts local analysis, i.e. it is inferior to Coverity's global analysis; however, when we turned it on, it threw out mountains of messages - much more than Coverity issued. Yes, there were a lot of false positives, but even without them there were a lot of all kinds of scary, really creepy badies.

I slowly began to edit the code - first of all, I started on my own, then on the system one, and finally on the game board. I had to work in fragments in my spare time, so the whole process was delayed for a couple of months. However, this delay had its side effect as well: we were convinced that / analyze does catch important defects. The fact is that at the same time with my corrections, our developers staged a big multi-day hunt for bugs, and it turned out that every time they attacked a trace of some error, already marked / analyze, but not yet corrected by me. In addition, there were other, less dramatic, cases where debugging led us to code already marked / analyze. All these were real mistakes.

In the end, I ensured that all the code used was compiled into a launch file for the 360th platform without a single warning when / analyze was enabled, and set this compilation mode as standard for 360 assemblies. After that, the code of each programmer working on the same platform was checked for errors every time it was compiled, so that he could immediately correct the bugs as they were added to the program instead of being used by me. Of course, because of this, the compilation process has slowed down somewhat, but / analyze is definitely the fastest tool I've ever dealt with, and, believe me, it's worth it.

Once we in some project accidentally turned off static analysis. A few months passed, and when I noticed this and turned it on again, the tool gave out a bunch of new warnings about errors made to the code during that time. Similarly, programmers working only under a PC or PS3, enter the code with errors into the repository and remain in the dark until they receive a letter with a report about the “unsuccessful 360 build”. These examples vividly demonstrate that in the course of their daily activities, developers repeatedly make mistakes of certain kinds, and / analyze reliably kept us from most of them.

Bruce Dawson has repeatedly mentioned in his blog about working with / analysis .

PVS-Studio

Since we could only use / analyze on 360-code, a large amount of our codebase was still not covered by static analysis - this concerned the code for the PC and PS3 platforms, as well as all programs running only on a PC.

The next tool I met was PVS-Studio . It easily integrates into Visual Studio and offers a convenient demo mode (try it yourself!). Compared to / analyze, PVS-Studio is terribly slow, but it managed to catch a number of new critical bugs, even in the code that was already completely cleaned from the point of view of / analyze. In addition to the obvious mistakes, PVS-Studio catches a lot of other defects, which are erroneous programmer cliches, even if seemingly seemingly normal code. Because of this, a certain percentage of false positives is almost inevitable, but, hell, such patterns were found in our code, and we corrected them.

On the PVS-Studio website you can find a large number of wonderful articles about the tool, and many of them contain examples from real open-source projects, illustrating specifically the types of errors that are covered in the article. I wondered if I could put in here some significant diagnostic messages issued by PVS-Studio, but much more interesting examples have already appeared on the site. So visit the page and see for yourself. And yes - when you read these examples, do not grin and say that you would never write.

PC-Lint

In the end, I returned to the version using PC-Lint in conjunction with Visual Lint for integration into the development environment. In accordance with the legendary tradition of the Unix world, the tool can be configured to perform almost any task, but its interface is not very friendly and cannot simply be “taken and run”. I acquired a set of five licenses, but its development turned out to be so laborious that, as far as I know, all other developers refused it as a result. Flexibility does have its advantages - for example, I managed to configure it to check all of our code for the PS3 platform, although it took me a lot of time and effort.

And again, in the code that was already clean from the point of view of / analyze and PVS-Studio, there were new important errors. I honestly tried to clean it up so that lint did not swear, but failed. I corrected the entire system code, but I gave up when I saw how many warnings he issued for the game code. I sorted out the errors into classes and dealt with the most critical of them, ignoring a lot of others related more to stylistic flaws or potential problems.

I believe that the attempt to correct the enormous amount of code to the maximum from the point of view of PC-Lint is doomed to failure. I wrote a certain amount of code from scratch in those places where I dutifully tried to get rid of every annoying “lint” comment, but for most experienced C / C ++ programmers this approach to working on bugs is too much. I still have to tinker with PC-Lint settings in order to pick the most appropriate set of warnings and squeeze the maximum benefit out of the tool.

findings

I learned a lot by going through it all. I am afraid that some of my conclusions will be hardly perceived by people who did not have to personally analyze hundreds of error messages in a short time and each time feel dizzy, starting to edit them, and the standard reaction to my words will be “well, then everything is in order "or" everything is not so bad. "

The first step on this path is to honestly admit to yourself that your code is teeming with bugs. For most programmers, this is a bitter pill, but without swallowing it, you will inevitably take any suggestion to change and improve the code with irritation, or even undisguised hostility. You should want to criticize your code.

Automation is required. When you see reports of monstrous failures in automatic systems, it is impossible not to experience such gloating, but for every error in automation there is a legion of human errors. Calls to “write better code”, good intentions to conduct more code inspection sessions, pair programming and so on simply do not work, especially when dozens of people are involved in the project and have to work in a wild rush. The tremendous value of static analysis lies in the possibility of finding at least small portions of errors available with this technique every time you start .

I noticed that with each update, PVS-Studio found new and new errors in our code due to new diagnostics. From this we can conclude that when the code base reaches a certain size, it seems that all permissible errors from the point of view of the syntax appear. In large projects, the quality of the code is subject to the same statistical laws as the physical properties of the substance - defects in it are common everywhere, and you can only try to minimize their impact on users.

Static analysis tools have to work “with one hand tied behind their backs”: they have to draw conclusions based on the analysis of languages, which do not necessarily provide information for such conclusions, and in general make very careful assumptions. Therefore, you should help your analyzer as far as possible - prefer indexing over pointer arithmetic, keep the call graph in a single source file, use explicit annotations, etc. Anything that may seem unclear to a static analyzer will almost certainly confuse your fellow programmers. The characteristic "hacker" aversion to languages ​​with strict static typing ("bondage and discipline languages") turns out to be short-sighted in practice: the needs of large, long-lived projects, which involve large teams of programmers, drastically differ from the small and fast tasks performed for themselves.

Null pointers are the most pressing problem in C / C ++, at least for us. The possibility of dual use of a single value as both a flag and an address leads to an incredible number of critical errors. Therefore, whenever there is a possibility for this, in C ++ one should prefer links rather than pointers. Although the reference “in fact” is nothing but the same pointer, it is tied by the implicit obligation that the equality is not equal to zero. Check the pointers to zero when they turn into links — this will allow you to forget about the problem later. In the field of game development, there are many deep-rooted programmer patterns that carry potential danger, but I don’t know how to completely and safely go from zero checks to links.

The second most important problem in our code base was errors with printf functions. It was further aggravated by the fact that the transfer of idStr instead of idStr :: c_str () almost every time ended with the fall of the program. However, when we began to use the / analyze annotations for functions with a variable number of arguments, so that type checking was performed correctly, the problem was solved once and for all. In the useful warnings of the analyzer, we encountered dozens of such defects that could lead to a fall, happen to some erroneous condition to launch the corresponding branch of the code - this, incidentally, also says how small was the percentage of coverage of our code with tests.

Many serious bugs reported by the analyzer were related to code modifications made a long time after it was written. An incredibly common example is when the ideal code, in which earlier pointers were checked for zero before performing an operation, was subsequently changed in such a way that pointers suddenly began to be used without checking. If we consider this problem in isolation, we could complain about the high cyclomatic complexity of the code, but if we look into the history of the project, it turns out that the reason is most likely that the author of the code could not clearly convey the prerequisites to the programmer who was later responsible for refactoring.

A person, by definition, is not able to hold attention to everything at once, so first of all focus on the code that you will deliver to customers, and pay less attention to the code for internal needs. Actively transfer the code from the database for sale to internal projects. Recently, an article appeared where it was told that all the code quality metrics in all their diversity almost perfectly correlate with the code size, as well as the error rate, which allows one to predict the number of errors with high accuracy by the size of the code. So reduce the part of your code that is critical in terms of quality.

If you are not scared to the bottom of your heart all the additional difficulties that parallel programming carries with you, it seems that you just haven’t understood how to do this.

It is impossible to carry out reliable control tests in software development, but our success in using code analysis was so distinct that I can simply say: not to use code analysis is irresponsible! Automatic console logs about falls contain objective data that clearly show that Rage, even being a pioneer in many ways, turned out to be much more stable and healthier than most of the most modern games. Unfortunately, the launch of Rage on a PC failed - I bet that AMD does not use static analysis when developing its graphics drivers.

Here's a recipe for you: if your version of Visual Studio has built-in / analyze, enable it and try to work like this. If I were asked to choose one from a variety of tools, I would dwell on this solution from Microsoft. To everyone else who works in Visual Studio, I advise you to at least try PVS-Studio in demo mode. If you are developing commercial software, acquiring static analysis tools will be one of the best ways to invest.

And finally, a comment from Twitter:

Dave Revell @dave_revell The more I use static analysis on my code, the more I wonder how computers start up at all.