How to add a new diagnostic rule in PVS-Studio? Weekdays developers ...

We are often asked how to add our own diagnostic rule to our PVS-Studio static analyzer. And we always answer that it is very simple to do this: "You just need to write us a letter, and we will add this rule to the analyzer." Such an interface for adding new rules is convenient for the user. This is the best and most convenient interface. Doing the job yourself is not as easy as it sounds. In the note, I will show the underwater part of the iceberg, which is hidden behind the concept of "added this simple rule."
Let's get clean right away. In PVS-Studio there is no mechanism for the user to write his own rules. This is, first of all, an architectural limitation. And some users may be upset. However, in reality, such users do not understand what they want. Because in life, the process of adding a new rule is quite complicated.
Here is, for example, rule V536 : Be advised that the utilized constant value is represented by an octal form ("Check the correct use of the octal constant"). It is designed to detect errors such as this one in Miranda IM:
static const struct _tag_cpltbl
{
unsigned cp;
const char * mimecp;
} cptbl [] =
{
{037, "IBM037"}, // IBM EBCDIC US-Canada
{437, "IBM437"}, // OEM United States
{500, "IBM500"}, // IBM EBCDIC International
{708, "ASMO-708"}, // Arabic (ASMO 708)
...
}It says 037 instead of 37 just for alignment, although it turned out to be decimal 31, since 037 is an octal notation according to C ++ rules. The error is quite simple to diagnose, right? In fact, we are looking for a constant, see if it starts with 0 and issue a diagnostic message. Such a diagnostic rule can be implemented in an hour. And the user of a static analyzer (a professional in programming, but new to the development of such tools) is ready in theory to implement such a rule.
However, he will not be able to do this. Rather, it will turn out, but he will not be satisfied with the result due to the large number of false positives.
We (PVS-Studio developers) do the following. Before implementing the rule, we first write unit tests on it, where we mark out in a special way the lines on which it should be triggered, and also leave the lines in the unit test where the rule should not produce anything. Then the first version of the code for diagnostics is written. Having achieved the passage of unit tests written for this new rule, we run already complete unit tests. After all, the new rule could break something. Complete unit tests take several tens of minutes. Of course, the first time “nothing can be broken” is far from always possible, therefore iterations are repeated until the unit tests pass completely.
After unit tests, the next step is to launch on real projects. In our test base there are about 70 projects (for 2011). These are well-known and not very open source projects on which we test our analyzer. We have our own self-made launcher, which opens projects in Visual Studio, launches the analyzer, saves the log, compares it with the standard and shows the differences (which messages disappeared, which were added, and which changed). One run of all projects for Visual Studio 2005 lasts 4 hours on a machine with four cores and eight gigabytes of memory (the analysis is parallel). According to the results of the analysis, we see the results of the implementation of the new rule. Usually, we start to look at the differences without waiting for the analysis to complete, as sometimes it is immediately clear that "everything has broken." However, even if everything works and nothing is broken, often tests stop earlier. The reason for this is false positives. If we see that the diagnostic rule has too many positives, then exceptions begin to appear in the rule.
Let's return to the search for incorrect octal numbers - rule V536. It has a number of exceptions. Here are the situations when the diagnosis is not issued:
- The constant value is less than 8.
- The number is declared via #define.
- In one block more than two octal numbers (the rule worked more than two times), except 0, of type char.
- This number is used as an argument in the functions: _open, mknod, open, wxMkdir, wxFileName :: Mkdir, wxFileName :: AppendDir, chmod.
- This is a nested block of numbers. We check only the topmost one. Example: {{090}, {091}, {092}, {093}}.
- This number is <= 0777 and is part of the statement where there is a word: file, File.
- A number is an argument to a function, one of the parameters of which contains the string "% o".
It is not always possible to predict exceptions before launching on real projects. Therefore, the iteration “exception - test run - analysis of results” can be repeated many times.
Then, when all exceptions are implemented, and the number of false positives is adequate, test results (stored log files of detected errors) are declared as reference ones. After that, the same tests are run for other versions of Visual Studio. Now (2011) we support three versions of Visual Studio 2005/2008/2010, the tests in them are performed accordingly 4 / 4.5 / 5 hours, which gives a total of 14-15 hours of time. According to the results of a run in another version of Visual Studio, it may turn out that a different number of diagnostic messages are issued for the same code, depending on the version of Visual Studio. This is usually due to differences in the header files of various SDKs. We are trying to eliminate the differences so that the test run in all versions of Visual Studio gives the same results.
Why is it so important to run all these tedious tests? Really only because of false positives? No, it's not just about false positives. The point is that when implementing the rules it is very difficult to take into account all possible types of structures and write the rule so that it works correctly and moreover does not lead to the analyzer crashing. This is what all this huge amount of tests are needed for!
What strange designs are we talking about? Let me give you a couple of examples.
Did you know that you can declare a variable, for example, somehow like this:
int const unsigned static a22 = 0;
Do you know __int64 could be a variable name?
int __identifier (__ int64);
You know that after switch braces are optional?
switch (0) if (X) Foo ();
Even if you yourself do not use a similar construction in your code, they are in the libraries used.
Finally, after passing all the tests, you can take on the documentation. For each diagnostic rule, we write a certificate in Russian and English. Translation into English also takes time. Now (when the code is debugged, the tests are passed, the documentation is written), the rule will go to the next release of PVS-Studio.
So, the development cycle of a new diagnostic rule can be briefly represented as follows:
- The idea of a new rule. Either we invent, or users prompt, or we manage to peek somewhere.
- Formulation of the rule.
- The implementation of the rule.
- All kinds of testing.
- Analysis of the results. The options are:
- Finalization of the wording of the rule, introduction of exceptions, transition to paragraph 2
- The implementation of the rule is considered established, you can write and translate documentation.
Now back to the question of how to add a rule to the user. As you can see, doing it yourself is difficult for two simple reasons. The user does not have sufficient qualifications to come up with successful exceptions and there is no good test base to drive the rule through tests.
Then the question arises: “Why, in some well-known and respected static analyzers, are there ways to set your own custom rules?” Maybe we just can’t do this? Partly yes. But only the mechanism for setting your own rules serves one simple purpose - to create “matrices for comparing with competitors”:

And so, the reader will ask, in PVS-Studio it will never be possible to create a custom rule? Someday we will have enough strength to implement this and not lose in the comparison matrices :-). In the meantime, we will everyone who wants to create a new rule to give a link to this text and say: “We have the best interface for adding custom rules - just write us an email!”