Static analyzer example
When PVS-Studio reported that they finally released a standalone version that does not require Visual Studio to work, I, of course, could not pass by :) Before that, I had already played with a trial version on the code of one of the old projects . Now there is an opportunity to look at the code of our last project, which is going to be in the development environment AVR Studio (which is eclipse-based).
For work, files are required immediately after the preprocessor. AVR Studio can do this, with one small exception - after turning on the “Preprocessor only” flag, files after the preprocessor really appear on the output - but still with the extension .o instead of the expected .i. Well, a 5-minute Python script solves this misunderstanding, and the analyzer starts fine!
Surprisingly, there are few messages - about two dozen. Most of them are insignificant remarks or false positives (in the embedded record in the register of the same value it occurs twice in a row, the analyzer sees this as a potential problem (and I generally agree with it - it is better to play it safe and check such places)).
In a couple of places, real typos and copy-paste errors are found. For example, a variable of the type of one enum-a is compared with a value from another enum-a. Or one variable is assigned two different values in a row (although, as indicated above, in most cases this was a false positive for registering a sequence in a register).
But the most interesting, because of which I am writing this post, was the one and only line “Possible NULL pointer dereferencing” ...
It so happened that everywhere in the code we used a construction of the form
And literally in several functions the design was a little different:
Until one day after one of the small refactoring in one of the places the following appeared:
Actually on this line the analyzer also cursed. SUCCESS, of course, had a value of 0.
We rewind time a bit back to the moment when this change hit the repository.
After refactoring, a very large set of automated tests continued to run successfully. The code review left this line unnoticed (too often * perr = SUCCESS lines flickered in the code).
About 30 days after that commit, night tests fell for the first time. Play fall did not work.
Then the tests fell again. And further. It was experimentally established that the drop occurs on average once every thirty runs of the test suite.
The team spent about 50 hours searching for the error. To no avail. True, we managed to localize the commit, after which it all started - but the reason was never found.
The reason, by the way, was two steps lower. The function some_fun (perr) inside itself called some_other_fun (perr), and that one called some_third_fun (perr). And already in some_third_fun (perr) there was a code that checked for an error:
Those. despite the fact that in some_action function (which was quite non-trivial, involving a bunch of external peripherals - because of which to localize the problem and was not an easy task), no errors occurred, the continuation of the cycle depended on the value written to 0 address (in embedded the zero address is quite legal in most cases). And in the vast majority of cases, 0 was written to this address.
Summary: an error, the detection of which was unsuccessfully spent about 50 hours, was detected and corrected in less than an hour by running the analyzer once!
A convincing argument for using the analyzer? Alas, not always. In particular, we have just the very case: a project with payment according to the time and material scheme. Since the 50 hours spent on the search are paid by the customer, the analyzer implementation means direct losses for management: (((
Also, by the way: FreeRTOS is used in the project. So, there was not a single warning in its code!
And yes, this post is exclusively out of love toart analyzers.
For work, files are required immediately after the preprocessor. AVR Studio can do this, with one small exception - after turning on the “Preprocessor only” flag, files after the preprocessor really appear on the output - but still with the extension .o instead of the expected .i. Well, a 5-minute Python script solves this misunderstanding, and the analyzer starts fine!
Surprisingly, there are few messages - about two dozen. Most of them are insignificant remarks or false positives (in the embedded record in the register of the same value it occurs twice in a row, the analyzer sees this as a potential problem (and I generally agree with it - it is better to play it safe and check such places)).
In a couple of places, real typos and copy-paste errors are found. For example, a variable of the type of one enum-a is compared with a value from another enum-a. Or one variable is assigned two different values in a row (although, as indicated above, in most cases this was a false positive for registering a sequence in a register).
But the most interesting, because of which I am writing this post, was the one and only line “Possible NULL pointer dereferencing” ...
It so happened that everywhere in the code we used a construction of the form
void fun(error_t * perr)
{
*perr = SUCCESS;
...
if (something)
{
*perr = SOME_ERROR;
}
}
And literally in several functions the design was a little different:
void init(void)
{
error_t err = SUCCESS;
...
fun(&err);
}
Until one day after one of the small refactoring in one of the places the following appeared:
void some_init(void)
{
error_t *perr = SUCCESS;
...
some_fun(perr);
}
Actually on this line the analyzer also cursed. SUCCESS, of course, had a value of 0.
We rewind time a bit back to the moment when this change hit the repository.
After refactoring, a very large set of automated tests continued to run successfully. The code review left this line unnoticed (too often * perr = SUCCESS lines flickered in the code).
About 30 days after that commit, night tests fell for the first time. Play fall did not work.
Then the tests fell again. And further. It was experimentally established that the drop occurs on average once every thirty runs of the test suite.
The team spent about 50 hours searching for the error. To no avail. True, we managed to localize the commit, after which it all started - but the reason was never found.
The reason, by the way, was two steps lower. The function some_fun (perr) inside itself called some_other_fun (perr), and that one called some_third_fun (perr). And already in some_third_fun (perr) there was a code that checked for an error:
for(number_of_loops)
{
some_action(perr);
if (*perr != SUCCESS)
return;
}
Those. despite the fact that in some_action function (which was quite non-trivial, involving a bunch of external peripherals - because of which to localize the problem and was not an easy task), no errors occurred, the continuation of the cycle depended on the value written to 0 address (in embedded the zero address is quite legal in most cases). And in the vast majority of cases, 0 was written to this address.
Summary: an error, the detection of which was unsuccessfully spent about 50 hours, was detected and corrected in less than an hour by running the analyzer once!
A convincing argument for using the analyzer? Alas, not always. In particular, we have just the very case: a project with payment according to the time and material scheme. Since the 50 hours spent on the search are paid by the customer, the analyzer implementation means direct losses for management: (((
Also, by the way: FreeRTOS is used in the project. So, there was not a single warning in its code!
And yes, this post is exclusively out of love to