Errores que el análisis de código estático no encuentra porque no se usa

De vez en cuando, los lectores de nuestros artículos sobre la verificación de proyectos de código abierto notan que el analizador de código estático PVS-Studio detecta un gran porcentaje de errores que son insignificantes o que no afectan la aplicación en absoluto. Realmente lo es Los errores más importantes ya se han solucionado gracias a las pruebas manuales, las revisiones de los usuarios y otros métodos costosos. Al mismo tiempo, muchos de estos errores podrían encontrarse incluso en la etapa de escritura del código y corregirse con una pérdida mínima de tiempo, reputación y dinero. Este artículo proporcionará algunos ejemplos de errores reales que se corregirían de inmediato si los autores de los proyectos usaran análisis de código estático.

La idea es muy simple. Veamos GitHub para ver ejemplos de solicitudes de extracción, cuyos comentarios indican que se trata de una corrección de errores. E intente encontrar estos errores utilizando el analizador de código estático PVS-Studio. Si el analizador encuentra el error corregido, esto significa que podría corregirse incluso en la etapa de escritura del código. Y cuanto antes se corrija el error, más barato será.

Desafortunadamente, GitHub nos decepcionó y no permitió hacer un gran artículo inteligente sobre este tema. GitHub en sí también tiene una falla (o característica) que no le permite buscar comentarios en solicitudes de extracción en proyectos escritos solo en ciertos lenguajes de programación. Bueno, o no "sé cocinarlo". Independientemente de lo que le diga que busque comentarios en proyectos en C ++, C # o Java, los resultados se muestran en todos los lenguajes, incluidos PHP, Python, JavaScript, etc. Como resultado, buscar casos adecuados fue una tarea extremadamente tediosa, y me limitaré a solo unos pocos ejemplos. Sin embargo, son suficientes para demostrar la utilidad de las herramientas de análisis de código estático cuando se usan regularmente.

¿Qué pasa si el error se detecta temprano? La respuesta es simple: los programadores no tendrían que esperar su manifestación y luego buscar y corregir el código defectuoso.
Veamos los errores que PVS-Studio pudo detectar de inmediato:

El primer ejemplo está tomado del proyecto SatisfactoryModLoader. Antes de corregir el error, el código se veía así:

// gets an API function from the mod handler SML_API PVOID getAPIFunction(std::string name) { bool found = false; for (Registry reg : modHandler.APIRegistry) { if (reg.name == name) { found = true; } } if (!found) { std::string msg = ...; MessageBoxA(NULL, msg.c_str(), "SatisfactoryModLoader Fatal Error", MB_ICONERROR); abort(); } } 

Este código contenía un error que PVS-Studio inmediatamente daría una advertencia:

V591 La función no nula debería devolver un valor. ModFunctions.cpp 44

La función anterior no tiene retorno , por lo que devolverá un valor formalmente indefinido. El programador no usó el analizador de código, por lo que tuvo que buscar un error por su cuenta. Función después de editar:

 // gets an API function from the mod handler SML_API PVOID getAPIFunction(std::string name) { bool found = false; PVOID func = NULL; for (Registry reg : modHandler.APIRegistry) { if (reg.name == name) { func = reg.func; found = true; } } if (!found) { std::string msg = ...; MessageBoxA(NULL, msg.c_str(), "SatisfactoryModLoader Fatal Error", MB_ICONERROR); abort(); } return func; } 

Curiosamente, en el commit, el autor indicó el error como crítico: " error crítico corregido donde las funciones API no se devolvieron ".

En la segunda confirmación del historial del proyecto mc6809, se realizaron correcciones en el siguiente código:

 void mc6809dis_direct( mc6809dis__t *const dis, mc6809__t *const cpu, const char *const op, const bool b16 ) { assert(dis != NULL); assert(op != NULL); addr.b[MSB] = cpu->dp; addr.b[LSB] = (*dis->read)(dis, dis->next++); ... if (cpu != NULL) { ... } } 

El autor corrigió solo una línea. Reemplazó la expresión

 addr.b[MSB] = cpu->dp; 

en expresión

 addr.b[MSB] = cpu != NULL ? cpu->dp : 0; 

En la versión anterior del código, no se realizó ninguna comprobación de CPU para la igualdad del puntero nulo. Si de repente resulta que se pasa un puntero nulo a la función mc6809dis_direct como segundo argumento, entonces se desreferenciará en el cuerpo de la función. El resultado es deplorable e impredecible .

La desreferenciación de puntero nulo es uno de los patrones más comunes de los que se nos informa: “Esto no es un error crítico. Bueno, ella vive en código y vive, y si ocurre la desreferenciación, el programa se bloqueará silenciosamente y eso es todo ". Es extraño y triste escuchar esto de los programadores de C ++, pero la vida es vida.

En cualquier caso, en este proyecto, dicha desreferenciación se convirtió en un error, ya que el encabezado de confirmación nos dice: " Solución de error --- Desreferencia NULL ".

Si el desarrollador del proyecto utilizara PVS-Studio, habría podido verificar su código y detectar una advertencia hace dos meses y medio (es decir, cuánto ha pasado desde que se cometió el error):

V595 El puntero 'cpu' se utilizó antes de que se verificara contra nullptr. Líneas de verificación: 1814, 1821. mc6809dis.c 1814

Por lo tanto, el error se eliminaría incluso en el momento de su aparición, lo que ahorraría tiempo y, posiblemente, los nervios del desarrollador :).

Se encontró un ejemplo de otra edición interesante en el proyecto libmorton.

El código corregido:

 template<typename morton> inline bool findFirstSetBitZeroIdx(const morton x, unsigned long* firstbit_location) { #if _MSC_VER && !_WIN64 // 32 BIT on 32 BIT if (sizeof(morton) <= 4) { return _BitScanReverse(firstbit_location, x) != 0; } // 64 BIT on 32 BIT else { *firstbit_location = 0; if (_BitScanReverse(firstbit_location, (x >> 32))) { // check first part firstbit_location += 32; return true; } return _BitScanReverse(firstbit_location, (x & 0xFFFFFFFF)) != 0; } #elif _MSC_VER && _WIN64 .... #elif __GNUC__ .... #endif } 

En su edición, el programador reemplaza la expresión " firstbit_location + = 32 " con " * firstbit_location + = 32 ". El programador esperaba que el número 32 se agregara al valor de la variable a la que está enlazado el puntero firstbit_location , pero se agregó directamente al puntero. El valor modificado del puntero no se usó en ningún otro lugar, y el valor esperado de la variable se mantuvo sin cambios.

PVS-Studio emitiría una advertencia para este código:

V1001 Se asigna la variable 'firstbit_location' pero no se usa al final de la función. morton_common.h 22

Parece que lo que podría ser terrible en una expresión modificada, pero aún no utilizada? El diagnóstico de V1001 obviamente no parece que esté destinado a identificar errores especialmente peligrosos. A pesar de esto, descubrió un error importante que afectó la lógica del programa.

¡Además, resultó que este error no era tan fácil de encontrar! ¡No solo estuvo en el programa desde el momento en que se creó el archivo, sino que también sobrevivió a muchas ediciones en líneas adyacentes y existió en el proyecto durante 3 ( ! ) Años! Todo este tiempo, la lógica del programa se rompió y no funcionó exactamente como lo esperaban los desarrolladores. Si usaran PVS-Studio, entonces el error se habría detectado mucho antes.

Al final, considere otro hermoso ejemplo. Mientras recopilaba correcciones de errores en GitHub, encontré varias veces una solución con este contenido . El error corregido aquí:

 int kvm_arch_prepare_memory_region(...) { ... do { struct vm_area_struct *vma = find_vma(current->mm, hva); hva_t vm_start, vm_end; ... if (vma->vm_flags & VM_PFNMAP) { ... phys_addr_t pa = (vma->vm_pgoff << PAGE_SHIFT) + vm_start - vma->vm_start; ... } ... } while (hva < reg_end); ... } 

PVS-Studio emitió una advertencia a esta sección de código:

V629 Considere inspeccionar la expresión 'vma-> vm_pgoff << 12'. Desplazamiento de bits del valor de 32 bits con una expansión posterior al tipo de 64 bits. mmu.c 1795

Miré las declaraciones de variables utilizadas en la expresión " phys_addr_t pa = (vma-> vm_pgoff << PAGE_SHIFT) + vm_start - vma-> vm_start; ", y descubrí que el código anterior es similar al siguiente ejemplo sintético:

 void foo(unsigned long a, unsigned long b) { unsigned long long x = (a << 12) + b; } 

Si el valor de la variable a de 32 bits a es mayor que 0xFFFFF , entonces los 12 bits más significativos tendrán al menos un valor distinto de cero. Después de aplicar la operación de desplazamiento a la izquierda a esta variable, estos bits significativos se perderán, como resultado de lo cual se escribirá información incorrecta en x .

Para eliminar la pérdida de bits altos, primero debe emitir un tipo sin signo largo largo , y solo después de eso realizar la operación de cambio:

 pa = (phys_addr_t)vma->vm_pgoff << PAGE_SHIFT; pa += vm_start - vma->vm_start; 

Entonces pa siempre escribirá el valor correcto.

Todo estaría bien, pero este error, como el primer ejemplo del artículo, también resultó ser crítico, ya que el autor del commit escribió por separado en su comentario. Además, este error acaba de entrar en una gran cantidad de proyectos. Para apreciar completamente la magnitud de la tragedia, sugiero que mire la cantidad de resultados cuando busque esta corrección de errores en GitHub. Miedo, ¿no es así?

Entonces, tomé un nuevo enfoque para demostrar la utilidad de usar un analizador de código estático regularmente. Espero que lo hayas disfrutado. Descargue y pruebe el analizador de código estático PVS-Studio para probar sus propios proyectos. Al momento de escribir, ha implementado alrededor de 700 reglas de diagnóstico para detectar una variedad de patrones de error. C, C ++, C # y Java son compatibles.

Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: George Gribkov. Errores que el análisis de código estático no encuentra porque no se usa