One day in the life of a PVS-Studio developer, or how I debugged diagnostics that turned out to be more attentive than three programmers
The main purpose of static analyzers is to find errors that have gone unnoticed by the developer. And recently the PVS-Studio team again came across an interesting example of the power of this technique.
Working with static code analysis tools requires care. Often the code pointed to by the analyzer seems to be correct. In such cases, you want to consider the warning as a false positive. The other day we ourselves fell into such a trap. Here’s how it happened.
Recently we modified the analyzer core, and a colleague who looked at the new positives found a false one among them. He wrote out a trigger for the team lead, who skimmed the code and created a task. I took the task. In such a simple way, a chain of three programmers was formed.
Analyzer warning: V645 The ‘strncat’ function call could lead to the ‘a.consoleText’ buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold.
Snippet of code:
struct A
{
char consoleText[512];
};
void foo(A a)
{
char inputBuffer[1024];
....
strncat(a.consoleText, inputBuffer, sizeof(a.consoleText) –
strlen(a.consoleText) - 5);
....
}
Before exploring the example, let’s remember what the function does strncat:
char *strncat(
char *strDest,
const char *strSource,
size_t count
);
Where:
‘destination’ – destination string;
‘source’ – source string;
‘count’ is the maximum number of characters that can be added.
At first glance, it seems that everything is fine with the code. It calculates the amount of empty space in the buffer. At the same time, there even seems to be a margin of 4 bytes … And precisely because of this feeling that everything is fine with the code, it was written out as an example where the analyzer generates a false warning.
Let’s see how things really are. In the expression:
sizeof(a.consoleText) – strlen(a.consoleText) – 5
the maximum value can be reached with the minimum value of the second operand:
strlen(a.consoleText) = 0
Then the result will be 507 and no overflow will happen. What is PVS-Studio talking about then? Let’s go a little deeper into the internal mechanisms of the analyzer and try to figure it out.
In a static analyzer, the data-flow analysis mechanism is responsible for counting such expressions. In most cases, when an expression consists of compile-time constants, data-flow will return the strict value of the expression. In other cases, as in the case of a warning, it will generate only the range of possible values of the expression.
In this case, the value of the operand strlen (a.consoleText) unknown to us at compile time. Let’s see the range.
After a few minutes of debugging, we get as many as 2 ranges from the data-flow:
[0, 507] U [0xFFFFFFFFFFFFFFFC, 0xFFFFFFFFFFFFFFFF]
At first glance, it seems that the second range is superfluous. However, it is not. We just forgot that the expression can result in a negative number. For example, this can happen when strlen (a.consoleText) = 508… In this case, an unsigned number overflow will occur, and the result of the expression will be the maximum value of the resulting type, in this case – size_t…
It turns out that the analyzer is right! In this expression, to the field consoleText many more characters can be added than its size, which will lead to buffer overflow and as a consequence, undefined behavior… The reason for the unexpected warning is that there is no false positive!
This is how we again received confirmation that the key advantage of static analysis over a person is his attentiveness. Thoughtful study of the analyzer’s warnings helps the developer to save time during debugging and nerves, saves him from mistakes and hasty conclusions.
If you want to share this article with an English-speaking audience, please use the translation link: Vladislav Stolyarov. One Day in the Life of PVS-Studio Developer, or How I Debugged Diagnostic That Surpassed Three Programmers.
