The following blog post, unless otherwise noted, was written by a member of Gamasutra’s community.
The thoughts and opinions expressed are those of the writer and not Gamasutra or its parent company.

Abstract

We go on analyzing open source projects and making the software world better. This time we have checked the Blender 2.62 package intended for creating 3D computer graphics.

Introduction

We regularly check various open source projects in C/C++ and make reports on check results. It allows the world of open source programs to become better and us to tell programmers about the PVS-Studio tool. Reports usually contain far not all the issues we find: since we are not familiar with projects, it might be difficult for us to tell if certain fragments are real errors or just intricate code. It's OK. We always give the authors of open source projects a free registration key for some time so that they could analyze their source code more thoroughly. If a project is small, the trial version of PVS-Studio will be quite enough to check it, since it provides the full functionality.

Readers often say in comments that checking open source projects is just advertisement of our tool. They also give Coverity as an example of a tool that supports open source projects much more intensively.

This comparison is not fair. Improving quality of open source products' codes has become a result of realizing the Vulnerability Discovery and Remediation Open Source Hardening Project campaign. Within the framework of this initiative, the Coverity company was granted $297,000 to support open source projects [1]. That's not too much, of course, but if we were sponsored at least a bit too, we could be more active analyzing open source projects.

About the Blender project

Blender is an open source package for creating 3D computer graphics which includes tools of designing, animation, rendering, video postprocessing and also tools of making interactive games. Starting with 2002, Blender is an open source project (GNU GPL) and develops under active support by Blender Foundation [2].

The Blender package is written in C, C++ and Python. We naturally checked parts in C and C++. The size of the source code together with additional libraries is 68 Mbytes (2105 KLOC).

In this project, by the way, I seem to have met a function with the highest cyclomatic complexity I've ever seen. This is the fast9_corner_score() function which can be found in the fast_9.c file. Its cyclomatic complexity is 1767. But the function is actually simple, so you won't see anything incredible here.

Analysis was performed by the PVS-Studio static analyzer version 4.60.

False positives

The programming style used in Blender causes the PVS-Studio analyzer to generate a lot of false positives among which real messages get lost. As a result, you cannot start working with Blender without preliminarily customizing the analyzer. It's not that bad, however, as it may seem at first. It will take you few efforts to greatly simplify your work when reading the report.

Let me clarify the above stated idea using numerical data. All in all, PVS-Studio generates 574 first-level warnings referring to general analysis diagnostic rules. Just glancing through the report helps you understand that most of the false positives refer to macros BLI_array_append, BLI_array_growone and other macros starting with "BLI_array_".

These macros are safe but they are used quite often. The analyzer generates warnings V514 and V547 for the places where they are used. To get rid of these warnings you can add a special comment in the BLI_array.h file that contains definitions of all these macros:

//-V:BLI_array_:514,547

This comment can be added anywhere in the text. After that you'll have to relaunch analysis but the result will be quite noticeable: about 280 false positives will be eliminated.

All in all, the amount of first-level messages will be cut from 574 to 294 after adding one single comment! This example shows very well that presence of a large number of false positives doesn't mean that the report is difficult to analyze. The most part of noise can be often removed through quite little effort.

To learn more about methods of suppressing false alarms, please read the corresponding documentation section about suppression of false alarms.

Defects and odd code fragments we've found

Error in a macro

The sample given above shows how one can significantly reduce the number of false positives suppressing warnings related to certain macros. But before suppressing a warning, make sure that there is no real error. I know it from my own experience that when some warning concerns a macro, you feel an urge not to investigate the reasons and ignore it right away. But don't hurry.

For example, consider the DEFAULT_STREAM macro that is used more than once in the Blender project. It is long, so we'll cite only part of it here:

The 'tpart' pointer in the render_new_particle_system() function is initialized by zero and never changed until the moment of dereferencing. The function is quite complex and contains variables with similar names. This is most likely a misprint and a different pointer should be used.

Identical functions

The analyzer has found a lot of functions with identical bodies. I didn't investigate these messages too closely but I seemed to have found at least one error. Perhaps if Blender's authors use PVS-Studio, they can find other similar fragments.

PVS-Studio's warning: V524 It is odd that the body of 'uiLayoutGetScaleY' function is fully equivalent to the body of 'uiLayoutGetScaleX' function (interface_layout.c, line 2410). bf_editor_interface interface_layout.c 2415

Intuition tells me that the uiLayoutGetScaleY() function should return the second item of the 'scale' array:

If you look attentively, you can notice an error occurring when assigning a new value to the 'y0' variable. At the very end of the line, a member of the 'tilec->x0' class is used instead of 'tilec->y0'.

This code was most likely created through the Copy-Paste technology and the programmer forgot to change the name of one variable during editing. This is the correct code:

According to the C++ language standard, right shift of a negative value leads to unspecified behavior. In practice this method is often used but you shouldn't do that: it cannot be guaranteed that the code will always work as intended. This issue was discussed in the article "Wade not in unknown waters. Part three".

Incorrect array filling

PVS-Studio's warning: V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. bf_imbuf tiff.c 442

The analyzer generates one warning, but the programmer actually managed to make 2 mistakes at once in one line. We have noted it down for ourselves to implement a rule to find the second error - it should be easy.

The first error. The 'fbuf' variable is a pointer, which means that sizeof(fbuf) will return the pointer size instead of the array size. As a result, the memset() function will fill only the first several bytes in the array.

The second error. The array consisting of items of the float type was intended to be filled with ones. But the memset function handles bytes, so the array will be filled with trash.

A similar error can be found here:

V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. bf_imbuf tiff.c 450

Misprint in code clearing an array

PVS-Studio's warning: V586 The 'clear' function is called twice for deallocation of the same resource. Check lines: 176, 177. bf_intern_elbeem ntl_geometrymodel.cpp 177

I find it meaningless to clear an array in just now created objects. But I'm not familiar with the project, so perhaps there is some sense in this operation. A misprint causes one and the same array to be cleared both times. This is the correct code:

Double check

In Blender's code, we have found two identical checks written besides each other. The second condition should be probably replaced with another. Or maybe this code is correct and the second check is extraneous.

According to the comment, if the file descriptor equals NULL, a new file shall be created. However, before the fwrite() function is called, the 'filxe_' variable is not used anywhere. As a result, a null pointer will be passed into the fwrite() function as a descriptor.

Using a pointer before verifying it against being a null pointer

PVS-Studio has an interesting rule V595. This diagnostic rule can be briefly posed in this way:

V595 is generated if:

1) a pointer is dereferenced;

2) the pointer is not changed anywhere further;

3) the pointer is compared to 0.

There are some exceptions to this rule, but let's not go into details.

This rule has both its advantage and disadvantage. The former is that you can find interesting errors with its help. The latter is that it produces quite many false positives.

False positives are in most cases determined by the presence of unnecessary checks in macros. We cannot fight this issue yet. Here is a typical example where a false positive is generated:

The 'p' pointer is always not equal to NULL. But the code contains a check and the analyzer gets suspicious about it.

We've made such a long introduction because the V595 warning is generated very often in Blender. All in all, PVS-Studio has produced 119 warnings of this type. More than half of them are most likely to be false positives. But authors should study the PVS-Studio-generated report themselves.

The 'surface' pointer is used in the beginning to initialize the 'sData' variable. And only then the 'surface' pointer is verified against being a null pointer.

Conclusions

1) Static analyzers are useful. Don't forget that they are most useful when you use them regularly. They help you detect a lot of errors at the earliest stage and therefore avoid numerous painful debuggings, but-reports from testers and users' complaints.

2) PVS-Studio sometimes produces a whole lot of false positives. But they usually can be eliminated through quite little effort.

3) The trial version of PVS-Studio that can be downloaded from the website has the full functionality. It will be sufficient to check small projects. Developers of large free open source programs will receive a free key for some time from us.