En nuestros artículos, repetimos regularmente un pensamiento importante: un analizador estático debe usarse con regularidad. En este caso, muchos errores se detectan en la etapa más temprana y su corrección es lo más barata posible. Sin embargo, la teoría es una cosa, pero es mucho mejor respaldar las palabras con ejemplos prácticos. Echemos un vistazo a algunos errores recientes que han aparecido en el nuevo código del proyecto Blender.
Recientemente, establecimos una verificación periódica del proyecto Blender , del que me contó mi colega en el artículo " Solo por diversión: al equipo de PVS-Studio se le ocurrió la idea de monitorear la calidad de algunos proyectos de código abierto ". En el futuro, planeamos comenzar a monitorear algunos proyectos más interesantes.
, . ( ), . , , PVS-Studio, .
, , Blender.
: double-checked locking
typedef struct bNodeTree {
....
struct NodeTreeUIStorage *ui_storage;
} bNodeTree;
static void ui_storage_ensure(bNodeTree &ntree)
{
/* As an optimization, only acquire a lock if the UI storage doesn't exist,
* because it only needs to be allocated once for every node tree. */
if (ntree.ui_storage == nullptr) {
std::lock_guard<std::mutex> lock(global_ui_storage_mutex);
/* Check again-- another thread may have allocated the storage
while this one waited. */
if (ntree.ui_storage == nullptr) {
ntree.ui_storage = new NodeTreeUIStorage();
}
}
}
PVS-Studio. V1036: Potentially unsafe double-checked locking. node_ui_storage.cc 46
. "C++ and the Perils of Double-Checked Locking", Scott Meyers Andrei Alexandrescu 2004 . , , , . , PVS-Studio :). , :
Consider again the line that initializes pInstance: pInstance = newSingleton;
This statement causes three things to happen:
Step 1: Allocate memory to hold a Singleton object.
Step 2: Construct a Singleton object in the allocated memory.
Step 3: Make pInstance point to the allocated memory.
Of critical importance is the observation that compilers are not constrained to perform these steps in this order! In particular, compilers are sometimes allowed to swap steps 2 and 3. Why they might want to do that is a question we'll address in a moment. For now, let's focus on what happens if they do.
Consider the following code, where we've expanded pInstance's initialization line into the three constituent tasks we mentioned above and where we've merged steps 1 (memory allocation) and 3 ( pInstance assignment) into a single statement that precedes step 2 ( Singleton construction). The idea is not that a human would write this code. Rather, it's that a compiler might generate code equivalent to this in response to the conventional DCLP source code (shown earlier) that a human would write.
, , . .
! . , . , . . , 1000 , , PVS-Studio .
1. , , . , , , . / .
2. , . C++17 , new T (operator =). , C++17, " , ". , , . , : std::atomic<NodeTreeUIStorage *> ui_storage\.
: realloc
static void icon_merge_context_register_icon(struct IconMergeContext *context,
const char *file_name,
struct IconHead *icon_head)
{
context->read_icons = realloc(context->read_icons,
sizeof(struct IconInfo) * (context->num_read_icons + 1));
struct IconInfo *icon_info = &context->read_icons[context->num_read_icons];
icon_info->head = *icon_head;
icon_info->file_name = strdup(path_basename(file_name));
context->num_read_icons++;
}
PVS-Studio , . .
: V701: realloc() possible leak: when realloc() fails in allocating memory, original pointer 'context->read_icons' is lost. Consider assigning realloc() to a temporary pointer. datatoc_icon.c 252
, realloc NULL. context->read_icons, . , , . .
: V522: There might be dereferencing of a potential null pointer 'context->read_icons'. Check lines: 255, 252. datatoc_icon.c
– - . , . . , , , . , . , . , .
. - . , - . , , , . - , . . " , malloc".
:
static int node_link_invoke(bContext *C, wmOperator *op, const wmEvent *event)
{
....
bNodeLinkDrag *nldrag = node_link_init(bmain, snode, cursor, detach);
nldrag->last_picked_multi_input_socket_link = NULL;
if (nldrag) {
op->customdata = nldrag;
....
}
PVS-Studio: V595: The 'nldrag' pointer was utilized before it was verified against nullptr. Check lines: 1037, 1039. node_relationships.c
(proof). nldrag . , .
. , , , , , .
, - , . : V595: The 'seq' pointer was utilized before it was verified against nullptr. Check lines: 373, 385. strip_add.c
, : Andrey Karpov. PVS-Studio, Blender: Series of Notes on Advantages of Regular Static Analysis of Code.