Analysis of static C ++ code analysis tools
Analysis of the following utilities:Everything you need can be found by following the links, and we immediately get down to business.
Carefully review this code, how many problems can you identify?
Of course, all the necessary headers are included ( iostream, vector, cassert ).
Before we discuss the professional versions of this code, we will perform an analysis of the known utilities for static source code analysis. The following is the result of the above utilities.
RATS : nothing
Cppcheck : nothing
Graudit : nothing
g ++ (with -Wall flag) : nothing
')
Maybe at first glance, everything is not so bad, or is there nothing at all bad? Do not say this with Sutter (among other things, an example is taken from his book “New Challenges in C ++. 40 New Puzzles with Solutions” ).
On the first line in the body of the function main () we declare a vector v consisting of integers. Then reserve a place for 2 items. Before we move on to the next line, take a look at the C ++ standard. What does she tell us about the vector :: reserve method? (warning: free translation)
"
While this is enough, let's go further. The string assert (v.capacity () == 2); bad at best, in the first reserve () guarantees increasing the capacity and in this case assert is just an extra line of code (which “programmers” like to insert, who blindly believe the assertion after each operation and consider it a professional tone). Secondly, as we learned from the standard itself, reserve () can increase the capacity of the container more than the specified argument, so Satter recommends assert (v.capacity ()> = 2) . I recommend removing it altogether.
Further worse. “ Vector <T> :: operator [] can, but is not required to perform range checking. The standard does not say anything about this, so the developer of the standard library has the full right to both add such a check and do without it. ” - Sutter . The reason is efficiency, verification takes some time, numerous such checks will require a significant part of the program’s work time. But this does not mean that your effective program is better than a reliable Vasya program. Try to replace the line v [0]; on v.at (0); The program will take off with an exception. As in most cases, you need a reliable code. vector <T> :: at checks the range of the index value. Strong recommendation to use this method in such and not only situations.
Strings v [0] = 1; v [1] = 2; quite work, you can make sure you compile the example. But we must remember the difference between size / resize and reserve / capacity . resize changes the number of elements in the container to the specified argument value by adding or removing them from the end of the container. We learned about the reserve method from the standard, we add that it increases the size of the internal buffer so that it can accommodate the specified number of elements (at a minimum). " We can safely use operator [] (or the at method) only to change the elements that are actually contained in the container, i.e. really are taken into account in size. " Is that clear? So the static analysis tools are not clear either. But the problem is significant. Sutter and many other pros write in their books about the subtle points in coding on your favorite C ++. At first glance it may seem that the ability to solve problems and knowledge of the syntax of the language is more than enough to develop. But we know with you that we are deceiving ourselves, how many of you inserted cout- s on each line of code in order to identify the problem? How many of you could not tolerate debugging that you have been doing for the third day? So, there are people who are not inherent in such things, they include people who possess professional programming skills, who are not lazy to go further and study the books of such gurus as Sutter and Myers (and many others) in a row.
And yet, in example 1, we are waiting for the last logical error, what do you think that cout << v [0]; after reserve (100) ? That's right, at best, zero! Note that prior to reservation, we gave v [0] a nonzero value. “Problem” is again in the backup function.
So what is the general case? In that I need a utility that tells you that in the case of v.reserve (2); v [0] = 1; better to use v.resize (2); v [0] = 1; or v.reserve (2); v.push_back (1) ! And the utilities reviewed by us are cute and silent. I also need a utility that checks for irregular bounds, and in the case of v [0]; recommend using v.at (0). And our reviewed utilities are again silently silent. Hence the moral, in subtle situations do not rely on utilities such as RATS, Cppcheck, and even more so on Graudit. Even the compiler with the highest level of warnings enabled can sometimes be viewed through the eyes of a paranoid coder.
Well, the utilities do not occupy a place on your screws for nothing, and it was not for nothing that the developers worked on it.
Let's go further, increase the error rate in the code. Consider the same code, with the new prettyFormat function (an example again from Sutter). It gets a number and a buffer, then inserts that number into the resulting buffer using sprintf.
As you can see, prettyFormat was added, and the string char buf [10] . Although we do not need the code from the previous example anymore, but let it remain, maybe one of the utilities will see something? Here is the result:
RATS: on the string char buf [5]; "It is recommended that you take care of character arrays, they are helpless when using character arrays. buffer overflow attacks.
Cppcheck : nothing
Graudit : nothing
g ++ (with -Wall option) : nothing
Much has been said about buffer overflow, so we will be brief and only criticize. I have the impression that the static analysis utilities are intended only for finding possible cases of buffer overflows. However, several of them are silent even in such cases (as Cppcheck or Graudit). I don’t blame the compiler, he didn’t tell me that he would find problems, but the utilities specifically designed for such things are pretty much silent in most cases. In such situations, we need to get warnings and recommendations from utilities to use alternatives to sprintf (well, for other possible functions). You can read about alternative situations in Sutter (we are talking about std :: stringstream, std :: strstream, boost :: lexical_cast, but the latter is based on std :: stringstream , and finally snprintf , which allows you to set the size of the buffer, thereby ensuring protection from buffer overflow).
Add an uninitialized pointer to the prettyFormat function.
Ostolnom code has not changed compared with previous tests. This is not something that may look like a serious test, but it will give a lot of interesting information about the utilities used. In particular, it was said about Cppcheck that it does not duplicate the compiler warnings, but as we will see shortly, this is a bluff. But other utilities simply do not see the problem or indeed do not duplicate the compiler warnings. Below is the result.
RATS : the same as in the previous test (this is the string char buf [5];)
Cppcheck : on the line int * a; gives (error) Uninitialized variable: a.
Fine, but ...
g ++ (with the -Wall option) : In function 'int * prettyFormat (int, char *)':
warning: 'a' is used uninitialized in this function.
Something familiar isn't it? The same warning that Cppcheck finally gave.
Graudit is generally silent.
Add humor, change the prettyFormat function like this:
Added fopen, which opens I don’t know what, a local buffer buf2 was added, into which buf received as an argument is copied to strcpy. Let's see how much one gives.
RATS: For strings char buf2 [5]; char buf [10] the same warning about the use of fixed-size character buffers.
And finally: “High: strcpy
Call will not copy
more data than can be handled, resulting in a buffer overflow. ”
Check if the second argument does not copy more data than buf2 can contain.
Not bad, in many situations would help. Let's go further.
Cppcheck : (error) Unitialized variable: a.
Something familiar, the same as in the previous example, which is a pity.
g ++ (with the -Wall option) : Same as in the previous test.
The task was more or less handled by the RATS, the rest - to shoot! Why is everything so bad? Firstly, the guys who “wrote” these utilities are not really C ++ gurus themselves, many of them most of all like to just publish the program and that's it. Secondly, many subtle errors are difficult to detect, and they are mostly handled by commercial utilities, although I have not tried it myself yet. Thirdly, read books, magazines, in particular Habr and you will not need utilities like these.
Thanks for attention!
PS I wanted to publish on the “Code Revision” blog, there is not enough karma. No, it’s not that I want to increase karma in such a way, just in case, so that the crowd doesn’t scream - “why not there-there -...”.
PPS Thank you so much to the media community - keep it up!
Test 1:
int main() { vector<int> v; v.reserve(2); assert(v.capacity() == 2); v[0]; v[0] = 1; v[1] = 2; cout << v[0] << endl; v.reserve(100); cout << v[0] << endl; return 0; } Carefully review this code, how many problems can you identify?
Of course, all the necessary headers are included ( iostream, vector, cassert ).
Before we discuss the professional versions of this code, we will perform an analysis of the known utilities for static source code analysis. The following is the result of the above utilities.
RATS : nothing
Cppcheck : nothing
Graudit : nothing
g ++ (with -Wall flag) : nothing
')
Maybe at first glance, everything is not so bad, or is there nothing at all bad? Do not say this with Sutter (among other things, an example is taken from his book “New Challenges in C ++. 40 New Puzzles with Solutions” ).
On the first line in the body of the function main () we declare a vector v consisting of integers. Then reserve a place for 2 items. Before we move on to the next line, take a look at the C ++ standard. What does she tell us about the vector :: reserve method? (warning: free translation)
"
void reserve(size_type n); It reports a planned resizing, possibly a corresponding management of memory allocation. After reserve (), capacity () (capacity) is greater than or equal to the argument passed to reserve () in the case of redistribution of memory. Otherwise, it is equal to the previous value of capacity () . Memory redistribution occurs if and only if the current value of capacity () is less than the value of the reserve () argument. It does not change the size of the container and in the worst case requires linear execution time. If the argument is greater than max_size () , it throws a long_error exception. In the case of redistribution, invalidates all references, pointers and iterators referring to the elements of the container. " While this is enough, let's go further. The string assert (v.capacity () == 2); bad at best, in the first reserve () guarantees increasing the capacity and in this case assert is just an extra line of code (which “programmers” like to insert, who blindly believe the assertion after each operation and consider it a professional tone). Secondly, as we learned from the standard itself, reserve () can increase the capacity of the container more than the specified argument, so Satter recommends assert (v.capacity ()> = 2) . I recommend removing it altogether.
Further worse. “ Vector <T> :: operator [] can, but is not required to perform range checking. The standard does not say anything about this, so the developer of the standard library has the full right to both add such a check and do without it. ” - Sutter . The reason is efficiency, verification takes some time, numerous such checks will require a significant part of the program’s work time. But this does not mean that your effective program is better than a reliable Vasya program. Try to replace the line v [0]; on v.at (0); The program will take off with an exception. As in most cases, you need a reliable code. vector <T> :: at checks the range of the index value. Strong recommendation to use this method in such and not only situations.
Strings v [0] = 1; v [1] = 2; quite work, you can make sure you compile the example. But we must remember the difference between size / resize and reserve / capacity . resize changes the number of elements in the container to the specified argument value by adding or removing them from the end of the container. We learned about the reserve method from the standard, we add that it increases the size of the internal buffer so that it can accommodate the specified number of elements (at a minimum). " We can safely use operator [] (or the at method) only to change the elements that are actually contained in the container, i.e. really are taken into account in size. " Is that clear? So the static analysis tools are not clear either. But the problem is significant. Sutter and many other pros write in their books about the subtle points in coding on your favorite C ++. At first glance it may seem that the ability to solve problems and knowledge of the syntax of the language is more than enough to develop. But we know with you that we are deceiving ourselves, how many of you inserted cout- s on each line of code in order to identify the problem? How many of you could not tolerate debugging that you have been doing for the third day? So, there are people who are not inherent in such things, they include people who possess professional programming skills, who are not lazy to go further and study the books of such gurus as Sutter and Myers (and many others) in a row.
And yet, in example 1, we are waiting for the last logical error, what do you think that cout << v [0]; after reserve (100) ? That's right, at best, zero! Note that prior to reservation, we gave v [0] a nonzero value. “Problem” is again in the backup function.
So what is the general case? In that I need a utility that tells you that in the case of v.reserve (2); v [0] = 1; better to use v.resize (2); v [0] = 1; or v.reserve (2); v.push_back (1) ! And the utilities reviewed by us are cute and silent. I also need a utility that checks for irregular bounds, and in the case of v [0]; recommend using v.at (0). And our reviewed utilities are again silently silent. Hence the moral, in subtle situations do not rely on utilities such as RATS, Cppcheck, and even more so on Graudit. Even the compiler with the highest level of warnings enabled can sometimes be viewed through the eyes of a paranoid coder.
Well, the utilities do not occupy a place on your screws for nothing, and it was not for nothing that the developers worked on it.
Test 2:
Let's go further, increase the error rate in the code. Consider the same code, with the new prettyFormat function (an example again from Sutter). It gets a number and a buffer, then inserts that number into the resulting buffer using sprintf.
void prettyFormat(int i, char* buf) { sprintf(buf, "%4d", i); } int main() { vector<int> v; v.reserve(2); //.... char buf[5]; prettyFormat(10, buf); return 0; } As you can see, prettyFormat was added, and the string char buf [10] . Although we do not need the code from the previous example anymore, but let it remain, maybe one of the utilities will see something? Here is the result:
RATS: on the string char buf [5]; "It is recommended that you take care of character arrays, they are helpless when using character arrays. buffer overflow attacks.
Cppcheck : nothing
Graudit : nothing
g ++ (with -Wall option) : nothing
Much has been said about buffer overflow, so we will be brief and only criticize. I have the impression that the static analysis utilities are intended only for finding possible cases of buffer overflows. However, several of them are silent even in such cases (as Cppcheck or Graudit). I don’t blame the compiler, he didn’t tell me that he would find problems, but the utilities specifically designed for such things are pretty much silent in most cases. In such situations, we need to get warnings and recommendations from utilities to use alternatives to sprintf (well, for other possible functions). You can read about alternative situations in Sutter (we are talking about std :: stringstream, std :: strstream, boost :: lexical_cast, but the latter is based on std :: stringstream , and finally snprintf , which allows you to set the size of the buffer, thereby ensuring protection from buffer overflow).
Test 3:
Add an uninitialized pointer to the prettyFormat function.
int* prettyFormat(int i, char* buf) { sprintf(buf, "%4d", i); int* a; return a; } int main() { ... Ostolnom code has not changed compared with previous tests. This is not something that may look like a serious test, but it will give a lot of interesting information about the utilities used. In particular, it was said about Cppcheck that it does not duplicate the compiler warnings, but as we will see shortly, this is a bluff. But other utilities simply do not see the problem or indeed do not duplicate the compiler warnings. Below is the result.
RATS : the same as in the previous test (this is the string char buf [5];)
Cppcheck : on the line int * a; gives (error) Uninitialized variable: a.
Fine, but ...
g ++ (with the -Wall option) : In function 'int * prettyFormat (int, char *)':
warning: 'a' is used uninitialized in this function.
Something familiar isn't it? The same warning that Cppcheck finally gave.
Graudit is generally silent.
Test 4:
Add humor, change the prettyFormat function like this:
int* prettyFormat(int i, char* buf) { sprintf(buf, "%4d", i); int* a; fopen("filename", "r"); char buf2[5]; strcpy(buf2, buf); return a; } Added fopen, which opens I don’t know what, a local buffer buf2 was added, into which buf received as an argument is copied to strcpy. Let's see how much one gives.
RATS: For strings char buf2 [5]; char buf [10] the same warning about the use of fixed-size character buffers.
And finally: “High: strcpy
Call will not copy
more data than can be handled, resulting in a buffer overflow. ”
Check if the second argument does not copy more data than buf2 can contain.
Not bad, in many situations would help. Let's go further.
Cppcheck : (error) Unitialized variable: a.
Something familiar, the same as in the previous example, which is a pity.
g ++ (with the -Wall option) : Same as in the previous test.
The task was more or less handled by the RATS, the rest - to shoot! Why is everything so bad? Firstly, the guys who “wrote” these utilities are not really C ++ gurus themselves, many of them most of all like to just publish the program and that's it. Secondly, many subtle errors are difficult to detect, and they are mostly handled by commercial utilities, although I have not tried it myself yet. Thirdly, read books, magazines, in particular Habr and you will not need utilities like these.
Thanks for attention!
PS I wanted to publish on the “Code Revision” blog, there is not enough karma. No, it’s not that I want to increase karma in such a way, just in case, so that the crowd doesn’t scream - “why not there-there -...”.
PPS Thank you so much to the media community - keep it up!
Source: https://habr.com/ru/post/128534/
All Articles