Geekly articles weekly

Cómo dispararte en el pie en C y C ++. Haiku OS Cookbook

La historia de cómo se reunieron el analizador estático PVS-Studio y el código Haiku OS se remonta al año 2015. Fue un experimento emocionante y una experiencia útil para los equipos de ambos proyectos. ¿Por qué el experimento? En ese momento, no teníamos el analizador para Linux y no lo tendríamos por otro año y medio. De todos modos, los esfuerzos de los entusiastas de nuestro equipo han sido recompensados: nos reunimos con los desarrolladores de Haiku y aumentamos la calidad del código, ampliamos nuestra base de errores con errores raros hechos por los desarrolladores y refinamos el analizador. Ahora puede verificar el código Haiku en busca de errores de manera fácil y rápida.
Imagen 1


Introduccion


Conoce a los personajes principales de nuestra historia: el Haiku con código fuente abierto y el analizador estático PVS-Studio para C, C ++, C # y Java. Cuando analizamos el análisis del proyecto hace 4.5 años, solo teníamos que lidiar con el archivo analizador ejecutable compilado. Toda la infraestructura para analizar los parámetros del compilador, ejecutar un preprocesador, hacer el análisis en paralelo, etc., se tomó de la utilidad UI de compilación del compilador , escrita en C #. Esa utilidad fue portada en partes a la plataforma Mono para ejecutarse en Linux. El proyecto Haiku se construye utilizando el compilador cruzado en varios sistemas operativos, excepto Windows. Una vez más, me gustaría mencionar la conveniencia y la integridad de la documentación relacionada con la construcción de Haiku. También me gustaría agradecer a los desarrolladores de Haiku por su ayuda en la construcción del proyecto.

Es mucho más sencillo realizar el análisis ahora. Aquí está la lista de todos los comandos para construir y analizar el proyecto:

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 implementó en un contenedor Docker. Recientemente hemos preparado nueva documentación sobre este tema: Ejecutar PVS-Studio en Docker . Esto puede facilitar que algunas empresas apliquen técnicas de análisis estático para sus proyectos.

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 ha inicializado en la declaración, por lo que su comparación con el valor nulo conducirá a un resultado indefinido. Si las circunstancias fallan, el valor incierto de la variable rval puede convertirse en un 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; .... } } .... }

Aquí hay un caso similar de uso de la variable no inicializada, excepto que res es un puntero no inicializado que tiene lugar aquí.

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); }

El programador probablemente necesitaba normalizar el objeto usando una variable intermedia. Pero ahora el puntero de fuente contiene el puntero al objeto normalizado , que se eliminará después de salir del ámbito, donde se creó el 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 del constructor supuestamente para inicializar / anular campos de clase. En este caso, la modificación de los campos de clase no ocurre, pero se crea un nuevo objeto sin nombre de esta clase y luego se destruye inmediatamente. Desafortunadamente, hay muchos lugares de este tipo en el proyecto:

  • 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 de su declaración en la clase misma. En este ejemplo, el campo fInternal será el primero en inicializarse utilizando 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); }

Este 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 toda la razón, si las ramas son idénticas. Pero, ¿dónde están las funciones mtx_lock_spin y mtx_unlock_spin ? Las macros TQ_LOCK , TQ_UNLOCK y la función grouptask_block se declaran en un archivo casi una al lado de la otra, pero, sin embargo, se realizó un reemplazo en algún lugar aquí.

La búsqueda en los archivos solo resultó en mutex.h con el siguiente contenido:

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

Los desarrolladores de proyectos deben verificar si dicho reemplazo es correcto o no. Revisé este 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 el puntero nulo en la función libre , pero dicho uso es definitivamente sospechoso. Por lo tanto, el analizador encontró líneas de código mezcladas. Primero, el autor del código tuvo que liberar la memoria mediante el puntero fVectorIcon , solo después de asignar 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); // <= .... }

Este es otro ejemplo de pasar explícitamente un puntero nulo a la función libre . Esta línea se puede eliminar, ya que la función sale después de obtener el puntero con éxito.

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; }

Alguien cometió un error aquí. Se debe utilizar el operador || en lugar de &&. Solo en este caso se lanzará la excepción std :: bad_alloc () en caso de que falle la asignación de 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 una matriz de caracteres. El operador de eliminación se utiliza para liberar la 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 una función que llama a malloc . Luego, ubicación-nueva construye el objeto aquí. Como 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"); }

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; };

Es un error común usar el operador de eliminación en lugar de eliminar []. Es más fácil cometer un error al escribir una clase, ya que el código del destructor a menudo está lejos de las ubicaciones de memoria. Aquí, el programador libera incorrectamente la memoria almacenada por el puntero fOutBuffer en el destructor.

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 una elección incorrecta entre eliminar / eliminar [] y liberar , también puede encontrarse con un comportamiento indefinido al intentar borrar la memoria con un puntero al tipo de vacío (void *) .

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; }

Un operador de asignación sobrecargado carece de un valor de retorno. En este caso, el operador devolverá un valor aleatorio, que puede conducir a errores extraños.

Aquí hay 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 */ }

El comentario de un usuario NOTREACHED no significa nada aquí. Debe anotar funciones como noreturn para escribir correctamente el código para tales escenarios. Para hacer esto, hay atributos de retorno: estándar y específicos del compilador. En primer lugar, los compiladores tienen en cuenta estos atributos para generar un código adecuado o notificar ciertos tipos de errores mediante 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; }

El lanzamiento de la palabra clave fue olvidado accidentalmente aquí. Por lo tanto, la excepción ParseException no se generará mientras que el objeto de esta clase simplemente se destruirá al salir del ámbito. Después de eso, la función continuará funcionando 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ó la excepción IOException lanzada por el puntero. Lanzar un puntero lleva al hecho de que la excepción no será atrapada. Entonces, la excepción finalmente se toma por referencia. Además, el uso de un puntero obliga al lado de captura a llamar al operador de eliminación para destruir el objeto creado, lo que no se había hecho.

Un par de otros fragmentos de código con problemas:

  • 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 ha detectado código sospechoso, destinado a la eliminación segura de datos privados. Desafortunadamente, la macro SAFE_FREE que se expande en el memset , las llamadas gratuitas y la asignación NULL no hacen que el código sea más seguro, ya que el compilador lo elimina todo al optimizar con O2 .

Por cierto, no es más que CWE-14 : eliminación del compilador de código para borrar buffers.

Aquí está la lista de lugares, donde la limpieza de los buffers no se realiza de hecho:

  • 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 con variables sin signo.


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 la variable sin signo con valores negativos. Quizás, uno debería comparar la variable restante solo con nulo 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 obtener el punto principal del error, abordemos 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 el valor 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; }

Un objeto de dispositivo es liberado por el operador de eliminación . Es bastante lógico para la función FreeDevice . Pero, por alguna razón, para liberar otros recursos, se aborda el objeto ya eliminado.

Dicho código es extremadamente peligroso y se puede cumplir en 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ó el caso cuando NULL se pasa a la función y el puntero de datos con dicho valor finalmente se desreferencia 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 . En este ejemplo, en caso de error, la memoria no se liberará. Alguien podría pensar que, en caso de errores, no debería preocuparse por la liberación de la memoria, ya que el programa aún finalizará. Pero no siempre es así. Es un requisito para muchos programas manejar los errores correctamente y continuar trabajando.

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; }

Es un error muy común desreferenciar los punteros antes de verificarlos. El diagnóstico V595 casi siempre prevalece en el número de advertencias en un proyecto. Este fragmento de código incluye el uso peligroso del puntero fReply .

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 varias líneas antes de que se compruebe si es nulo. Hay muchos lugares similares en el proyecto. En algunos fragmentos, el uso y la comprobación del puntero están bastante alejados entre sí, por lo que en este artículo encontrará solo algunos ejemplos. Los desarrolladores pueden consultar otros ejemplos en el informe completo del analizador.

Misceláneo


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 muy obvia para alguien que no está familiarizado con la descripción de estas funciones. La función strlcat espera el tamaño de todo el búfer como tercer argumento, mientras que la función strncat , el tamaño del espacio libre en un búfer, que requiere evaluar un valor necesario antes de llamar a la función. Pero los desarrolladores a menudo lo olvidan o no lo saben. Pasar todo el tamaño del búfer a la función strncat puede provocar un desbordamiento del búfer, ya que la función considerará este valor como un número aceptable de caracteres para copiar. La función strlcat no tiene ese problema. Pero debe pasar cadenas, que terminan en terminal nulo para que funcione correctamente.

Aquí está la lista completa de lugares peligrosos con cadenas:

  • 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'. Los límites no deben contener el tamaño del búfer, sino una cantidad de caracteres que puede contener. NamespaceDump.cpp 107
  • 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 110
  • 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 113
  • 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 118
  • 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 119
  • 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 120
  • 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 123
  • 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 126
  • 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 129
  • 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 132
  • 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 135
  • 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 138
  • 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 141
  • 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 144
  • 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 283
  • 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 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 '|' y '||' los operadores estaban confundidos. Este error conduce a llamadas innecesarias de 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, el autor debería haber evaluado el tamaño de un 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 el valor de tipo bool . Al hacerlo, el resultado de la función se compara con el número 0x12 en la condición.

Conclusión


Nos perdimos el lanzamiento de la primera beta de Haiku el otoño pasado, ya que estábamos ocupados lanzando PVS-Studio para Java. Aún así, la naturaleza de los errores de programación es tal que no desaparecen si no los busca y no presta atención a la calidad del código. Los desarrolladores de proyectos utilizaron Coverity Scan , pero la última ejecución fue hace casi dos años. Esto debe ser molesto para los usuarios de Haiku. Aunque el análisis se configuró en 2014 usando Coverity, no nos impidió escribir dos largos artículos sobre revisión de errores en 2015 ( parte 1 , parte 2 )

Pronto saldrá otra revisión de errores de Haiku para aquellos que leen esta publicación hasta el final. El informe completo del analizador se enviará a los desarrolladores antes de publicar esta revisión de errores, por lo que es posible que algunos errores se solucionen cuando esté leyendo esto. Para pasar el tiempo entre los artículos, sugiero descargar y probar PVS-Studio para su proyecto.

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

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

More articles:


All Articles