This is one of my posts on how PVS-Studio makes programs safer. That is where and what types of errors it detects. This time it is samples demonstrating handling of the IPP 7.0 library (Intel Performance Primitives Library) we are going to examine.

Intel Parallel Studio 2011 includes the Performance Primitives Library. This library in its turn includes a lot of primitives which allow you to create efficient video and audio codecs, signal processing software, image rendering mechanisms, archivers and so on. Sure, it is rather difficult to handle such a library. That is why Intel created a lot of demonstration programs based on it. You may see descriptions of samples and download them here: Code Samples for the Intel Integrated Performance Primitives (Intel IPP) Library.

All the samples are arranged into four groups:

IPP Samples for Windows

IPP UIC Demo for Windows

IPP DMIP Samples for Windows

IPP Cryptography Samples for Windows

Each set contains many projects, so, for a start, I took only the first set IPP Samples for Windows for the check. I used PVS-Studio 4.10 to perform the analysis.

I want to show you in this post that static analysis is useful regardless of programmers' skill and level of a solution being developed. The idea "you must employ experts and write code without errors right away" does not work. Even highly skilled developers cannot be secure from all the errors and misprints while writing code. Errors in samples for IPP show this very well.

I want you to note that IPP Samples for Windows is a high-quality project. But due to its size, 1.6 millions of code lines, it cannot but contain various errors. Let's examine some of them.

The programmer copied the code fragment several times and changed the arrays' indexes. But in the end his hand shook and he typed number 3 but forgot to delete 0. As a result, we have got index 30 and there is an overrun far outside the array's boundaries.

Identical code branches

Since we started with code copying, let's examine one more example related to it:

The "nLeftBord" pointer returns values from the "sbrencConflictResolution" function. At first, it is the value "nBordNext - 1" which is written by the specified address. On certain conditions, this value must be decremented by one. To decrement the value, the programmer used this code:

*nLeftBord--;

The error is that it is the pointer itself which is decremented instead of the value. The correct code looks this way:

(*nLeftBord)--;

More confusion with "++" increment operation and "*" pointer's dereferencing

I cannot understand the following code at all. I do not know how to fix it to make it meaningful. Perhaps something is missing here.

Here, the loop from the sample above is equivalent to the following code:

tbl += num_tbl;

The PVS-Studio analyzer supposed that parentheses might be missing here and there must be this code: "(*tbl)++;". But this variant is meaningless too. In this case, the loop is equivalent to this code:

*tbl += num_tbl;

So, this loop is a rather strange one. The error does exist but only the code's author seems to know how to fix it.

Loss of error flag

The code has the function "GetTrackByPidOrCreateNew" that returns "-1" if an error occurs.

Remove that does not remove anything

The PVS-Studio's diagnostic message: V530 The return value of function 'remove' is required to be utilized. h264_dec umc_h264_thread.cpp 226

This is quite an interesting combination. On the one hand, everything is cool. We have mutex to correctly remove items in a multithreaded application. On the other hand, the developers simply forgot that the std::remove function does not remove items from the array but only rearranges them. Actually, this code must look this way:

Comparing structures' fields to themselves

I was looking through the errors and noticed that implementation of the H264 video compression standard is somewhat defective. A lot of errors we have found relate to this very project. For instance, the programmer was in a hurry and used two wrong variable names at once.

This double assignment alerts me much more than in the previous sample. It seems as if the programmer was not confident. Or as if he decided to try "nCoef-1" first and then "nCoef". It is also called "programming through experiment method". Anyway, it is that very case when you should stop for a while and think it over on encountering such a fragment.

What is unpleasant about such errors is that the code "almost works". The error occurs only if the minimum item is stored in "m_cur.AcRate[3]". Such errors like to hide during testing and show up on users' computers at user input data.

This is a nice example of code that works slower than it could due to an error. The algorithm must normalize only those items which are specified in the mask array. But this code normalizes all the items. The error is located in the "if(mask<0)" condition. The programmer forgot to use the "i" index. The "mask" pointer will be almost all the time above or equal to zero and therefore we will process all the items.

Processing of situation when the buffer's size is not sufficient is implemented incorrectly. The program will continue working instead of returning the error's code and most likely will crash. The point is that the "memSize" variable has the "unsigned int" type. So, the "memSize < 0" condition is always false and we go on working with a buffer overflow.

I think it is a good example of software attack vulnerability. You may cause a buffer overflow by feeding incorrect data into the program and use it for your own purposes. By the way, we found about 10 such vulnerabilities in the code. I will not describe them here not to overload the text.

The "m_iCurrMBIndex" variable has the "unsigned" type. Because of it, the "m_iCurrMBIndex - x" expression also has the "unsigned" type. Therefore, the "m_iCurrMBIndex - x < 0" condition is always false. Let's see what consequences it has.

Let the "m_iCurrMBIndex" variable amount to 5 and "x" variable amount to 10.

The "m_iCurrMBIndex - x" expression equals 5u - 10i = 0xFFFFFFFBu.

The "m_iCurrMBIndex - x < 0" condition is false.

The "m_MBInfo[row][0xFFFFFFFBu]" expression is executed and an overrun occurs.

Error of using '?:' ternary operator

The ternary operator is rather dangerous because you may easily make an error using it. Nevertheless, programmers like to write code as short as possible and use the interesting language construct. The C++ language punishes them for this.

Summary

I described only some of the errors detected in IPP Samples for Windows in this article. I have not listed some errors because they are twins with those I have discussed in the article, so it would not be interesting to read about them. I also have not given inessential errors here. For instance, take assert() which always has a true condition because of a misprint. I skipped many code fragments because I simply did not know if there were errors or just poor code. But I think I have described enough defects to show you how difficult it is to write large projects even for skilled developers.

Let me once again formulate the idea I have mentioned in the beginning of the article. Even a good programmer is not secure from misprints, absent-mindedness, urge to use Copy-Paste and logical errors. I think this article will be a good answer for those people who believe that the phrase "you must write correct code" will protect them against any errors.

I wish you luck in all your C/C++/C++0x projects. May you find as many errors as possible using the static analysis methodology I love so much!

Use PVS-Studio to search for bugs in C, C++, C# and Java

We offer you to check your project code with PVS-Studio. Just one bug found in the project will show you the benefits of the static code analysis methodology better than a dozen of the articles.