Geekly articles weekly

Cómo dispararte en el pie en C y C ++. Colección de Recetas Haiku OS

La historia de la reunión del analizador estático PVS-Studio con el código del sistema operativo Haiku se remonta a 2015. Fue un experimento interesante y una experiencia útil para los equipos de ambos proyectos. ¿Por qué un experimento? Entonces no había analizador para Linux y no habrá otro año y medio. Pero el trabajo de los entusiastas de nuestro equipo fue recompensado: conocimos a los desarrolladores de Haiku y mejoramos la calidad del código, reabastecemos la base de datos con raros errores de programador y perfeccionamos el analizador. Comprobar el código de Haiku en busca de errores es rápido y fácil ahora.

Cuadro 3


Introduccion


Los héroes de nuestra historia son el sistema operativo Haiku de código abierto y el analizador estático PVS-Studio para C, C ++, C # y Java. Cuando hace 4.5 años comenzamos a analizar el proyecto, solo teníamos que trabajar con el archivo ejecutable del analizador compilado. Toda la infraestructura para analizar parámetros de compilación, iniciar un preprocesador, análisis de paralelización, etc. fue tomado de la utilidad de compilación de la interfaz de usuario de UI en C #, que fue portada en partes a la plataforma Mono para ejecutarse en Linux. El proyecto Haiku en sí se construye utilizando un compilador cruzado en varios sistemas operativos, excepto Windows. Una vez más, quiero señalar la conveniencia y la integridad de la documentación del ensamblaje de Haiku, y también agradecer a los desarrolladores de Haiku por su ayuda en la construcción del proyecto.

Ahora el análisis es mucho más fácil. La lista de todos los comandos para construir y analizar el proyecto se ve así:

cd /opt git clone https://review.haiku-os.org/buildtools git clone https://review.haiku-os.org/haiku cd ./haiku mkdir generated.x86_64; cd generated.x86_64 ../configure --distro-compatibility official -j12 \ --build-cross-tools x86_64 ../../buildtools cd ../../buildtools/jam make all cd /opt/haiku/generated.x86_64 pvs-studio-analyzer trace -- /opt/buildtools/jam/bin.linuxx86/jam \ -q -j1 @nightly-anyboot pvs-studio-analyzer analyze -l /mnt/svn/PVS-Studio.lic -r /opt/haiku \ -C x86_64-unknown-haiku-gcc -o /opt/haiku/haiku.log -j12

Por cierto, el análisis del proyecto se realizó en el contenedor Docker. Recientemente, hemos preparado nueva documentación sobre este tema: Ejecutar PVS-Studio en Docker . Esto puede simplificar enormemente el uso de técnicas de análisis de proyectos estáticos para algunas empresas.

Variables no inicializadas


V614 Variable no inicializada 'rval' utilizada. fetch.c 1727

 int auto_fetch(int argc, char *argv[]) { volatile int argpos; int rval; // <= argpos = 0; if (sigsetjmp(toplevel, 1)) { if (connected) disconnect(0, NULL); if (rval > 0) // <= rval = argpos + 1; return (rval); } .... }

La variable rval no se inicializó cuando se declaró, por lo que compararla con un valor nulo generará un resultado indefinido. Si las circunstancias fallan, el valor indefinido de la variable rval puede convertirse en el valor de retorno de la función auto_fetch .

V614 Puntero no inicializado 'res' utilizado. comandos.c 2873

 struct addrinfo { int ai_flags; int ai_family; int ai_socktype; int ai_protocol; socklen_t ai_addrlen; char *ai_canonname; struct sockaddr *ai_addr; struct addrinfo *ai_next; }; static int sourceroute(struct addrinfo *ai, char *arg, char **cpp, int *lenp, int *protop, int *optp) { static char buf[1024 + ALIGNBYTES]; char *cp, *cp2, *lsrp, *ep; struct sockaddr_in *_sin; #ifdef INET6 struct sockaddr_in6 *sin6; struct ip6_rthdr *rth; #endif struct addrinfo hints, *res; // <= int error; char c; if (cpp == NULL || lenp == NULL) return -1; if (*cpp != NULL) { switch (res->ai_family) { // <= case AF_INET: if (*lenp < 7) return -1; break; .... } } .... }

Un caso similar de uso de una variable no inicializada, solo aquí está el puntero res no inicializado.

El puntero V506 a la variable local 'normalizado' se almacena fuera del alcance de esta variable. Tal puntero se volverá inválido. TextView.cpp 5596

 void BTextView::_ApplyStyleRange(...., const BFont* font, ....) { if (font != NULL) { BFont normalized = *font; _NormalizeFont(&normalized); font = &normalized; } .... fStyles->SetStyleRange(fromOffset, toOffset, fText->Length(), mode, font, color); }

Probablemente, el programador necesitaba normalizar el objeto a través de una variable intermedia. Pero ahora, en el puntero de fuente , se guardó el puntero al objeto temporal normalizado , que se destruirá después de abandonar el ámbito en el que se creó este objeto temporal.

V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 27

 int8 BUnicodeChar::Type(uint32 c) { BUnicodeChar(); return u_charType(c); }

Un error muy común entre los programadores de C ++ es usar la llamada al constructor supuestamente para inicializar / cero los campos de clase. En este caso, no hay modificación de los campos de la clase, pero se crea un nuevo objeto sin nombre de esta clase, que se destruye inmediatamente. Desafortunadamente, hay muchos de esos lugares:

  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 37
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 49
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 58
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 67
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 77
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 89
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 103
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 115
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 126
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 142
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 152
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 163
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 186
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 196
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 206
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 214
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 222
  • V603 El objeto fue creado pero no se está utilizando. Si desea llamar al constructor, debe usar 'this-> BUnicodeChar :: BUnicodeChar (....)'. UnicodeChar.cpp 230

V670 El miembro de clase no inicializado 'fPatternHandler' se usa para inicializar el miembro 'fInternal'. Recuerde que los miembros se inicializan en el orden de sus declaraciones dentro de una clase. Painter.cpp 184

 Painter::Painter() : fInternal(fPatternHandler), .... fPatternHandler(), .... { .... }; class Painter { .... private: mutable PainterAggInterface fInternal; // line 336 bool fSubpixelPrecise : 1; bool fValidClipping : 1; bool fDrawingText : 1; bool fAttached : 1; bool fIdentityTransform : 1; Transformable fTransform; float fPenSize; const BRegion* fClippingRegion; drawing_mode fDrawingMode; source_alpha fAlphaSrcMode; alpha_function fAlphaFncMode; cap_mode fLineCapMode; join_mode fLineJoinMode; float fMiterLimit; PatternHandler fPatternHandler; // line 355 mutable AGGTextRenderer fTextRenderer; };

Otro ejemplo de inicialización incorrecta. Los campos de clase se inicializan en el orden en que se declaran en la clase misma. En este ejemplo, el campo fInternal se inicializará primero usando el valor no inicializado fPatternHandler .

#Define sospechoso


V523 La declaración 'then' es equivalente a la declaración 'else'. subr_gtaskqueue.c 191

 #define TQ_LOCK(tq) \ do { \ if ((tq)->tq_spin) \ mtx_lock_spin(&(tq)->tq_mutex); \ else \ mtx_lock(&(tq)->tq_mutex); \ } while (0) #define TQ_ASSERT_LOCKED(tq) mtx_assert(&(tq)->tq_mutex, MA_OWNED) #define TQ_UNLOCK(tq) \ do { \ if ((tq)->tq_spin) \ mtx_unlock_spin(&(tq)->tq_mutex); \ else \ mtx_unlock(&(tq)->tq_mutex); \ } while (0) void grouptask_block(struct grouptask *grouptask) { .... TQ_LOCK(queue); gtask->ta_flags |= TASK_NOENQUEUE; gtaskqueue_drain_locked(queue, gtask); TQ_UNLOCK(queue); }

El fragmento de código no parece sospechoso hasta que observe el resultado del preprocesador:

 void grouptask_block(struct grouptask *grouptask) { .... do { if ((queue)->tq_spin) mtx_lock(&(queue)->tq_mutex); else mtx_lock(&(queue)->tq_mutex); } while (0); gtask->ta_flags |= 0x4; gtaskqueue_drain_locked(queue, gtask); do { if ((queue)->tq_spin) mtx_unlock(&(queue)->tq_mutex); else mtx_unlock(&(queue)->tq_mutex); } while (0); }

El analizador tiene mucha razón: las ramas if y else son idénticas. Pero, ¿a dónde fueron las funciones mtx_lock_spin y mtx_unlock_spin ? Las macros TQ_LOCK , TQ_UNLOCK y la función grouptask_block se declaran en el mismo archivo y casi una al lado de la otra, sin embargo, hubo una sustitución en alguna parte.

Una búsqueda en el contenido de los archivos encontró solo mutex.h con los siguientes contenidos:

 /* on FreeBSD these are different functions */ #define mtx_lock_spin(x) mtx_lock(x) #define mtx_unlock_spin(x) mtx_unlock(x)

Vale la pena verificar si los cambios del proyecto son correctos o no. Revisé el proyecto en Linux, y esa sustitución me pareció sospechosa.

Errores con la función libre


V575 El puntero nulo se pasa a la función 'libre'. Inspecciona el primer argumento. setmime.cpp 727

 void MimeType::_PurgeProperties() { fShort.Truncate(0); fLong.Truncate(0); fPrefApp.Truncate(0); fPrefAppSig.Truncate(0); fSniffRule.Truncate(0); delete fSmallIcon; fSmallIcon = NULL; delete fBigIcon; fBigIcon = NULL; fVectorIcon = NULL; // <= free(fVectorIcon); // <= fExtensions.clear(); fAttributes.clear(); }

Puede pasar un puntero nulo a la función libre , pero dicha entrada es claramente sospechosa. Entonces, el analizador encontró las líneas de código confusas. Primero, tenía que liberar la memoria usando el puntero fVectorIcon , y solo luego establecerlo en NULL .

V575 El puntero nulo se pasa a la función 'libre'. Inspecciona el primer argumento. driver_settings.cpp 461

 static settings_handle * load_driver_settings_from_file(int file, const char *driverName) { .... handle = new_settings(text, driverName); if (handle != NULL) { // everything went fine! return handle; } free(handle); // <= .... }

Otro ejemplo de pasar explícitamente un puntero nulo a la función libre . Esta línea se puede eliminar porque Al recibir con éxito el puntero, la función se cierra.

V575 El puntero nulo se pasa a la función 'libre'. Inspecciona el primer argumento. PackageFileHeapWriter.cpp 166

 void* _GetBuffer() { .... void* buffer = malloc(fBufferSize); if (buffer == NULL && !fBuffers.AddItem(buffer)) { free(buffer); throw std::bad_alloc(); } return buffer; }

Aquí hay un error. En lugar del operador &&, se debe utilizar el operador ||. Solo en este caso se lanzará una excepción std :: bad_alloc () si no fue posible asignar memoria usando la función malloc .

Errores con el operador de eliminación


V611 La memoria se asignó usando el operador 'nuevo T []' pero se liberó usando el operador 'borrar'. Considere inspeccionar este código. Probablemente sea mejor usar 'delete [] fMsg;'. Err.cpp 65

 class Err { public: .... private: char *fMsg; ssize_t fPos; }; void Err::Unset() { delete fMsg; // <= fMsg = __null; fPos = -1; } void Err::SetMsg(const char *msg) { if (fMsg) { delete fMsg; // <= fMsg = __null; } if (msg) { fMsg = new(std::nothrow) char[strlen(msg)+1]; // <= if (fMsg) strcpy(fMsg, msg); } }

El puntero fMsg se usa para asignar memoria para almacenar una matriz de caracteres, y el operador de eliminación se usa para liberar memoria, en lugar de eliminar [] .

V611 La memoria fue asignada usando el operador 'nuevo' pero fue liberada usando la función 'libre'. Considere inspeccionar las lógicas de operación detrás de la variable 'wrapperPool'. vm_page.cpp 3080

 status_t vm_page_write_modified_page_range(....) { .... PageWriteWrapper* wrapperPool = new(malloc_flags(allocationFlags)) PageWriteWrapper[maxPages + 1]; PageWriteWrapper** wrappers = new(malloc_flags(allocationFlags)) PageWriteWrapper*[maxPages]; if (wrapperPool == NULL || wrappers == NULL) { free(wrapperPool); // <= free(wrappers); // <= wrapperPool = stackWrappersPool; wrappers = stackWrappers; maxPages = 1; } .... }

Aquí malloc_flags es la función que hace malloc . Y luego ubicación-nueva construye el objeto aquí. Y dado que la clase PageWriteWrapper se implementa de la siguiente manera:

 class PageWriteWrapper { public: PageWriteWrapper(); ~PageWriteWrapper(); void SetTo(vm_page* page); bool Done(status_t result); private: vm_page* fPage; struct VMCache* fCache; bool fIsActive; }; PageWriteWrapper::PageWriteWrapper() : fIsActive(false) { } PageWriteWrapper::~PageWriteWrapper() { if (fIsActive) panic("page write wrapper going out of scope but isn't completed"); }

entonces los destructores de objetos de esta clase no serán llamados debido al uso de la función libre para liberar memoria.

V611 La memoria se asignó usando el operador 'nuevo T []' pero se liberó usando el operador 'borrar'. Considere inspeccionar este código. Probablemente sea mejor usar 'delete [] fOutBuffer;'. Verifique las líneas: 26, 45. PCL6Rasterizer.h 26

 class PCL6Rasterizer : public Rasterizer { public: .... ~PCL6Rasterizer() { delete fOutBuffer; fOutBuffer = NULL; } .... virtual void InitializeBuffer() { fOutBuffer = new uchar[fOutBufferSize]; } private: uchar* fOutBuffer; int fOutBufferSize; };

Usar el operador de eliminación en lugar de eliminar [] es un error muy común. La forma más fácil de cometer un error al escribir una clase es porque El código del destructor a menudo se encuentra lejos de las ubicaciones de asignación de memoria. Aquí, el programador libera incorrectamente memoria en el destructor mediante el puntero fOutBuffer .

V772 Llamar a un operador 'eliminar' para un puntero nulo causará un comportamiento indefinido. Hashtable.cpp 207

 void Hashtable::MakeEmpty(int8 keyMode,int8 valueMode) { .... for (entry = fTable[index]; entry; entry = next) { switch (keyMode) { case HASH_EMPTY_DELETE: // TODO: destructors are not called! delete (void*)entry->key; break; case HASH_EMPTY_FREE: free((void*)entry->key); break; } switch (valueMode) { case HASH_EMPTY_DELETE: // TODO: destructors are not called! delete entry->value; break; case HASH_EMPTY_FREE: free(entry->value); break; } next = entry->next; delete entry; } .... }

Además de la elección incorrecta entre eliminar / eliminar [] y liberar , puede agregar un comportamiento indefinido al programa de otra manera: intente borrar la memoria con un puntero a un tipo indefinido (nulo *) .

Funciones sin valor de retorno


V591 La función no nula debería devolver un valor. Referenciable.h 228

 BReference& operator=(const BReference<const Type>& other) { fReference = other.fReference; }

El operador de asignación anulado no tiene suficiente valor de retorno. En este caso, el operador devolverá un valor aleatorio, que puede conducir a errores extraños.

Problemas similares en otros fragmentos de código de esta clase:

  • V591 La función no nula debería devolver un valor. Referenciable.h 233
  • V591 La función no nula debería devolver un valor. Referenciable.h 239

V591 La función no nula debería devolver un valor. main.c 1010

 void errx(int, const char *, ...) ; char * getoptionvalue(const char *name) { struct option *c; if (name == NULL) errx(1, "getoptionvalue() invoked with NULL name"); c = getoption(name); if (c != NULL) return (c->value); errx(1, "getoptionvalue() invoked with unknown option '%s'", name); /* NOTREACHED */ }

Un comentario de usuario NO ALCANZADO aquí no significa nada. Para escribir correctamente el código para tales escenarios, debe marcar funciones como noreturn. No hay atributos de retorno para esto: estándar y específico del compilador. En primer lugar, los compiladores tienen en cuenta dichos atributos para la generación correcta de códigos o la notificación de algunos tipos de errores con la ayuda de advertencias. Varias herramientas de análisis estático también tienen en cuenta los atributos para mejorar la calidad del análisis.

Manejo de excepciones


V596 El objeto fue creado pero no se está utilizando. Podría faltar la palabra clave 'throw': throw ParseException (FOO); Response.cpp 659

 size_t Response::ExtractNumber(BDataIO& stream) { BString string = ExtractString(stream); const char* end; size_t number = strtoul(string.String(), (char**)&end, 10); if (end == NULL || end[0] != '\0') ParseException("Invalid number!"); return number; }

Aquí, la palabra clave throw se olvida accidentalmente. Por lo tanto, no se lanzará una ParseException , y un objeto de esta clase simplemente se destruirá cuando salga del alcance. Después de lo cual la función continuará su trabajo como si nada hubiera sucedido, como si se hubiera ingresado el número correcto.

V1022 El puntero arrojó una excepción. Considere lanzarlo por valor en su lugar. gensyscallinfos.cpp 316

 int main(int argc, char** argv) { try { return Main().Run(argc, argv); } catch (Exception& exception) { // <= fprintf(stderr, "%s\n", exception.what()); return 1; } } int Run(int argc, char** argv) { .... _ParseSyscalls(argv[1]); .... } void _ParseSyscalls(const char* filename) { ifstream file(filename, ifstream::in); if (!file.is_open()) throw new IOException(string("Failed to open '") + filename + "'."); // <= .... }

El analizador detectó una IOException lanzada por el puntero. Lanzar un puntero hace que la excepción no se capture, por lo que la excepción finalmente se captura por referencia. Además, el uso de un puntero obliga al interceptor a llamar al operador de eliminación para destruir el objeto creado, lo que tampoco se hace.

Dos áreas problemáticas más del código:

  • V1022 El puntero arrojó una excepción. Considere lanzarlo por valor en su lugar. gensyscallinfos.cpp 347
  • V1022 El puntero arrojó una excepción. Considere lanzarlo por valor en su lugar. gensyscallinfos.cpp 413

Seguridad formal


V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el objeto 'f_key'. La función memset_s () debe usarse para borrar los datos privados. dst_api.c 1018

 #ifndef SAFE_FREE #define SAFE_FREE(a) \ do{if(a != NULL){memset(a,0, sizeof(*a)); free(a); a=NULL;}} while (0) .... #endif DST_KEY * dst_free_key(DST_KEY *f_key) { if (f_key == NULL) return (f_key); if (f_key->dk_func && f_key->dk_func->destroy) f_key->dk_KEY_struct = f_key->dk_func->destroy(f_key->dk_KEY_struct); else { EREPORT(("dst_free_key(): Unknown key alg %d\n", f_key->dk_alg)); } if (f_key->dk_KEY_struct) { free(f_key->dk_KEY_struct); f_key->dk_KEY_struct = NULL; } if (f_key->dk_key_name) SAFE_FREE(f_key->dk_key_name); SAFE_FREE(f_key); return (NULL); }

El analizador detectó un código sospechoso diseñado para limpiar de forma segura los datos privados. Desafortunadamente, la macro SAFE_FREE , que se expande en memset , llamadas gratuitas y asignación NULL , no hace que el código sea más seguro, porque el compilador elimina todo esto durante la optimización de O2 .

Por cierto, esto no se parece en nada a CWE-14 : Eliminación del código del compilador para borrar buffers.

La lista completa de lugares donde los buffers no se borran realmente:

  • V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el búfer 'encoded_block'. La función memset_s () debe usarse para borrar los datos privados. dst_api.c 446
  • V597 El compilador podría eliminar la llamada a la función 'memset', que se utiliza para vaciar el objeto 'key_st'. La función memset_s () debe usarse para borrar los datos privados. dst_api.c 685
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el búfer 'in_buff'. La función memset_s () debe usarse para borrar los datos privados. dst_api.c 916
  • V597 El compilador podría eliminar la llamada de función 'memset', que se utiliza para vaciar el objeto 'ce'. La función memset_s () debe usarse para borrar los datos privados. fs_cache.c 1078

Comparaciones sin firmar


V547 La expresión 'restante <0' siempre es falsa. El valor de tipo sin signo nunca es <0. DwarfFile.cpp 1947

 status_t DwarfFile::_UnwindCallFrame(....) { .... uint64 remaining = lengthOffset + length - dataReader.Offset(); if (remaining < 0) return B_BAD_DATA; .... }

El analizador encontró una comparación explícita de una variable sin signo con valores negativos. Quizás debería comparar la variable restante con solo cero o implementar una verificación de desbordamiento.

V547 La expresión 'suspensión ((sin signo) segundos) <0' siempre es falsa. El valor de tipo sin signo nunca es <0. Misc.cpp 56

 status_t snooze(bigtime_t amount) { if (amount <= 0) return B_OK; int64 secs = amount / 1000000LL; int64 usecs = amount % 1000000LL; if (secs > 0) { if (sleep((unsigned)secs) < 0) // <= return errno; } if (usecs > 0) { if (usleep((useconds_t)usecs) < 0) return errno; } return B_OK; }

Para entender cuál es el error, pasemos a las firmas de las funciones sleep y usleep :

 extern unsigned int sleep (unsigned int __seconds); extern int usleep (__useconds_t __useconds);

Como podemos ver, la función de suspensión devuelve un valor de un tipo sin signo y su uso en el código es incorrecto.

Punteros peligrosos


V774 El puntero 'dispositivo' se usó después de liberar la memoria. xhci.cpp 1572

 void XHCI::FreeDevice(Device *device) { uint8 slot = fPortSlots[device->HubPort()]; TRACE("FreeDevice() port %d slot %d\n", device->HubPort(), slot); // Delete the device first, so it cleans up its pipes and tells us // what we need to destroy before we tear down our internal state. delete device; DisableSlot(slot); fDcba->baseAddress[slot] = 0; fPortSlots[device->HubPort()] = 0; // <= delete_area(fDevices[slot].trb_area); delete_area(fDevices[slot].input_ctx_area); delete_area(fDevices[slot].device_ctx_area); memset(&fDevices[slot], 0, sizeof(xhci_device)); fDevices[slot].state = XHCI_STATE_DISABLED; }

El operador de eliminación borra un objeto de dispositivo . Esta es una acción lógica para una función llamada FreeDevice . Pero, por alguna razón, para liberar otros recursos en el código, nuevamente existe una apelación a un objeto ya eliminado.

Tal código es extremadamente peligroso y ocurre en varios otros lugares:

  • V774 El puntero 'self' se usó después de liberar la memoria. TranslatorRoster.cpp 884
  • V774 El puntero 'string' se usó después de liberar la memoria. RemoteView.cpp 1269
  • V774 El puntero 'bs' se usó después de liberar la memoria. mkntfs.c 4291
  • V774 El puntero 'bs' se usó después de liberar la memoria. mkntfs.c 4308
  • V774 El puntero 'al' se usó después de reasignar la memoria. inode.c 1155

V522 Puede tener lugar la desreferenciación del puntero nulo 'datos'. El puntero nulo se pasa a la función 'malo_hal_send_helper'. Inspeccione el tercer argumento. Verifique las líneas: 350, 394. if_malohal.c 350

 static int malo_hal_fwload_helper(struct malo_hal *mh, char *helper) { .... /* tell the card we're done and... */ error = malo_hal_send_helper(mh, 0, NULL, 0, MALO_NOWAIT); // <= NULL .... } static int malo_hal_send_helper(struct malo_hal *mh, int bsize, const void *data, size_t dsize, int waitfor) { mh->mh_cmdbuf[0] = htole16(MALO_HOSTCMD_CODE_DNLD); mh->mh_cmdbuf[1] = htole16(bsize); memcpy(&mh->mh_cmdbuf[4], data , dsize); // <= data .... }

El análisis interprocedural reveló una situación en la que NULL se pasa a la función y el puntero de datos con este valor se desreferencia posteriormente en la función memcpy .

V773 Se salió de la función sin soltar el puntero 'inputFileFile'. Una pérdida de memoria es posible. command_recompress.cpp 119

 int command_recompress(int argc, const char* const* argv) { .... BFile* inputFileFile = new BFile; error = inputFileFile->SetTo(inputPackageFileName, O_RDONLY); if (error != B_OK) { fprintf(stderr, "Error: Failed to open input file \"%s\": %s\n", inputPackageFileName, strerror(error)); return 1; } inputFile = inputFileFile; .... }

PVS-Studio puede detectar pérdidas de memoria . La memoria para inputFileFile no se libera aquí en caso de algún tipo de error. Alguien cree que en caso de errores, no puede molestarse en liberar memoria: el programa aún finalizará. Pero este no es siempre el caso. Maneje los errores correctamente y continúe trabajando, un requisito para tantos programas.

V595 El puntero 'fReply' se utilizó antes de que se verificara contra nullptr. Líneas de verificación: 49, 52. ReplyBuilder.cpp 49

 RPC::CallbackReply* ReplyBuilder::Reply() { fReply->Stream().InsertUInt(fStatusPosition, _HaikuErrorToNFS4(fStatus)); fReply->Stream().InsertUInt(fOpCountPosition, fOpCount); if (fReply == NULL || fReply->Stream().Error() == B_OK) return fReply; else return NULL; }

¿Con qué frecuencia los desarrolladores desreferencian los punteros antes de verificarlos? Diagnostics V595 es casi siempre un líder en la cantidad de advertencias en un proyecto. El uso peligroso del puntero fReply en este fragmento de código.

V595 El puntero 'mq' se utilizó antes de verificarlo con nullptr. Líneas de verificación: 782, 786. oce_queue.c 782

 static void oce_mq_free(struct oce_mq *mq) { POCE_SOFTC sc = (POCE_SOFTC) mq->parent; struct oce_mbx mbx; struct mbx_destroy_common_mq *fwcmd; if (!mq) return; .... }

Un ejemplo similar El puntero mg se desreferencia unas pocas líneas antes de que se verifique un valor nulo. Hay muchos lugares similares en el proyecto. En algunos lugares, el uso y la verificación de punteros están muy alejados entre sí, por lo que se incluyen dos ejemplos en el artículo. El resto podrá ver a los desarrolladores en el informe completo del analizador.

Errores varios


V645 La llamada a la función 'strncat' podría conducir al desbordamiento del búfer 'salida'. Los límites no deben contener el tamaño del búfer, sino una cantidad de caracteres que puede contener. NamespaceDump.cpp 101

 static void dump_acpi_namespace(acpi_ns_device_info *device, char *root, int indenting) { char output[320]; char tabs[255] = ""; .... strlcat(tabs, "|--- ", sizeof(tabs)); .... while (....) { uint32 type = device->acpi->get_object_type(result); snprintf(output, sizeof(output), "%s%s", tabs, result + depth); switch(type) { case ACPI_TYPE_INTEGER: strncat(output, " INTEGER", sizeof(output)); break; case ACPI_TYPE_STRING: strncat(output, " STRING", sizeof(output)); break; .... } .... } .... }

La diferencia entre las funciones strlcat y strncat no es del todo obvia para una persona nueva en la descripción de estas funciones. La función strlcat toma el tamaño de todo el búfer como tercer argumento, y la función strncat toma el tamaño del espacio libre en el búfer , lo que requiere calcular el valor deseado antes de llamar a la función. Pero los desarrolladores a menudo lo olvidan o no lo saben. Pasar la función strncat al tamaño de todo el búfer puede provocar un desbordamiento del búfer, porque la función considerará este valor como la cantidad de caracteres permitidos para copiar. La función strlcat no tiene este problema, pero para que funcione correctamente, es necesario pasar cadenas que terminen en la terminal cero.

La lista completa de lugares peligrosos con líneas:

  • V645 La llamada a la función 'strncat' podría conducir al desbordamiento del búfer 'salida'. Los límites no deben contener el tamaño del búfer, sino una cantidad de caracteres que puede contener. NamespaceDump.cpp 104
  • V645 La llamada a la función 'strncat' podría conducir al desbordamiento del búfer 'salida'. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 107
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 110
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 113
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 118
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 119
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 120
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 123
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 126
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 129
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 132
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 135
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 138
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 141
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 144
  • V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 283
  • V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 284
  • V645 La llamada a la función 'strncat' podría conducir al desbordamiento del búfer 'features_string'. Los límites no deben contener el tamaño del búfer, sino una cantidad de caracteres que puede contener. VirtioDevice.cpp 285

V792 La función 'SetDecoratorSettings' ubicada a la derecha del operador '|' se llamará independientemente del valor del operando izquierdo. Quizás, es mejor usar '||'. DesktopListener.cpp 324

 class DesktopListener : public DoublyLinkedListLinkImpl<DesktopListener> { public: .... virtual bool SetDecoratorSettings(Window* window, const BMessage& settings) = 0; .... }; bool DesktopObservable::SetDecoratorSettings(Window* window, const BMessage& settings) { if (fWeAreInvoking) return false; InvokeGuard invokeGuard(fWeAreInvoking); bool changed = false; for (DesktopListener* listener = fDesktopListenerList.First(); listener != NULL; listener = fDesktopListenerList.GetNext(listener)) changed = changed | listener->SetDecoratorSettings(window, settings); return changed; }

Lo más probable es que los operadores '|' y '||'. Tal error lleva a llamadas innecesarias a la función SetDecoratorSettings .

V627 Considere inspeccionar la expresión. El argumento de sizeof () es la macro que se expande a un número. device.c 72

 #define PCI_line_size 0x0c /* (1 byte) cache line size in 32 bit words */ static status_t wb840_open(const char* name, uint32 flags, void** cookie) { .... data->wb_cachesize = gPci->read_pci_config(data->pciInfo->bus, data->pciInfo->device, data->pciInfo->function, PCI_line_size, sizeof(PCI_line_size)) & 0xff; .... }

Pasar el valor 0x0c al operador sizeof parece sospechoso. Quizás debería calcular el tamaño de algún objeto, por ejemplo, datos . V562 Es extraño comparar un valor de tipo bool con un valor de 18: 0x12 == IsProfessionalSpdif (). CEchoGals_mixer.cpp 533



 typedef bool BOOL; virtual BOOL IsProfessionalSpdif() { ... } #define ECHOSTATUS_DSP_DEAD 0x12 ECHOSTATUS CEchoGals::ProcessMixerFunction(....) { .... if ( ECHOSTATUS_DSP_DEAD == IsProfessionalSpdif() ) // <= { Status = ECHOSTATUS_DSP_DEAD; } else { pMixerFunction->Data.bProfSpdif = IsProfessionalSpdif(); } .... }

La función IsProfessionalSpdif devuelve un valor de tipo bool , mientras que en la condición el resultado de la función se compara con el número 0x12 .

Conclusión


Nos perdimos el lanzamiento de la primera beta de Haiku el otoño pasado, ya que estaban ocupados lanzando PVS-Studio para el lenguaje Java. Pero la naturaleza de los errores del programador es tal que no van a ninguna parte a menos que los busque y no preste atención a la calidad del código. Los desarrolladores intentaron usar Coverity Scan , pero la última ejecución del análisis fue hace casi dos años. Esto debería molestar a los usuarios de Haiku. Aunque el análisis con Coverity se estableció en 2014, esto no nos impidió escribir dos grandes artículos con revisiones de errores en 2015 ( parte 1 , parte 2 ).

Aquellos que han leído hasta el final están esperando otra revisión del código Haiku de no menos volumen con nuevos tipos de errores. Se enviará un informe completo del analizador a los desarrolladores antes de publicar revisiones de código, por lo que se pueden corregir algunos errores. Para pasar el tiempo entre publicaciones, sugiero descargar y probar PVS-Studio en su proyecto.

¿Quieres probar Haiku y tienes alguna pregunta? Los desarrolladores de Haiku te invitan al canal de telegramas .



Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: Svyatoslav Razmyslov. Cómo dispararte en el pie en C y C ++. Haiku OS Cookbook

Source: https://habr.com/ru/post/461255/

More articles:


All Articles