Geekly articles weekly

Como dar um tiro no pé em C e C ++. Haiku OS Cookbook

A história de como o analisador estático PVS-Studio e o código do Haiku OS se encontraram remonta ao ano de 2015. Foi uma experiência emocionante e uma experiência útil para as equipes dos dois projetos. Por que o experimento? Naquele momento, não tínhamos o analisador para Linux e não o teríamos por mais um ano e meio. De qualquer forma, os esforços dos entusiastas de nossa equipe foram recompensados: nos reunimos com os desenvolvedores do Haiku e aumentamos a qualidade do código, ampliamos nossa base de erros com bugs raros feitos pelos desenvolvedores e refinamos o analisador. Agora você pode verificar o código do Haiku de forma fácil e rápida.
Quadro 1


1. Introdução


Conheça os principais personagens da nossa história - o Haiku com código-fonte aberto e o analisador estático PVS-Studio para C, C ++, C # e Java. Quando analisamos a análise do projeto há 4,5 anos, tivemos que lidar apenas com o arquivo do analisador executável compilado. Toda a infraestrutura para analisar parâmetros do compilador, executando um pré-processador, paralelizando a análise e assim por diante foi obtida da interface do usuário do Compiler Monitoring , escrita em C #. Esse utilitário foi portado em partes para a plataforma Mono para ser executada no Linux. O projeto Haiku é construído usando o compilador cruzado em vários sistemas operacionais, exceto Windows. Mais uma vez, gostaria de mencionar a conveniência e a integridade da documentação relacionadas ao edifício Haiku. Também gostaria de agradecer aos desenvolvedores do Haiku por sua ajuda na construção do projeto.

É muito mais simples executar a análise agora. Aqui está a lista de todos os comandos para criar e analisar o projeto:

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

A propósito, a análise do projeto foi implementada em um contêiner Docker. Recentemente, preparamos nova documentação sobre este tópico: Executando o PVS-Studio no Docker . Isso pode facilitar para algumas empresas a aplicação de técnicas de análise estática em seus projetos.

Variáveis ​​não inicializadas


V614 Variável não inicializada 'rval' usada. 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); } .... }

A variável rval não foi inicializada na declaração; portanto, a comparação com o valor nulo levará a resultados indefinidos. Se as circunstâncias falharem, o valor incerto da variável rval pode se tornar um valor de retorno da função auto_fetch .

V614 Ponteiro não inicializado 'res' usado. commands.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; .... } } .... }

Aqui está um caso semelhante de usar a variável não inicializada, exceto que res é um ponteiro não inicializado que ocorre aqui.

V506 O ponteiro para a variável local 'normalizada' é armazenado fora do escopo dessa variável. Esse ponteiro se tornará 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); }

O programador provavelmente precisava normalizar o objeto usando uma variável intermediária. Mas agora o ponteiro da fonte contém o ponteiro para o objeto normalizado , que será removido após a saída do escopo, onde o objeto temporário foi criado.

V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 27

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

Um erro muito comum entre os programadores de C ++ é usar a chamada do construtor para inicializar / anular campos de classe. Nesse caso, a modificação dos campos da classe não ocorre, mas um novo objeto não nomeado dessa classe é criado e imediatamente destruído. Infelizmente, existem muitos desses lugares no projeto:

  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 37
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 49
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 58
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 67
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 77
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 89
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 103
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 115
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 126
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 142
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 152
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 163
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 186
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 196
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 206
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 214
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 222
  • V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> BUnicodeChar :: BUnicodeChar (....)' deve ser usado. UnicodeChar.cpp 230

V670 O membro da classe não inicializado 'fPatternHandler' é usado para inicializar o membro 'fInternal'. Lembre-se de que os membros são inicializados na ordem de suas declarações dentro de uma classe. 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; };

Outro exemplo de inicialização incorreta. Os campos de classe são inicializados na ordem de sua declaração na própria classe. Neste exemplo, o campo fInternal será o primeiro a inicializar usando o valor fPatternHandler não inicializado.

#Define suspeito


V523 A instrução 'then' é equivalente à instrução '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); }

Esse trecho de código não parece suspeito até que você observe o resultado do pré-processador:

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

O analisador está realmente certo - se e que outros ramos são idênticos. Mas onde estão as funções mtx_lock_spin e mtx_unlock_spin ? As macros TQ_LOCK , TQ_UNLOCK e a função grouptask_block são declaradas em um arquivo quase ao lado do outro, mas mesmo assim uma substituição ocorreu em algum lugar aqui.

A pesquisa nos arquivos resultou apenas em mutex.h com o seguinte conteúdo:

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

Os desenvolvedores do projeto devem verificar se essa substituição está correta ou não. Eu verifiquei este projeto no Linux e essa substituição parecia suspeita para mim.

Erros com a função livre


V575 O ponteiro nulo é passado para a função 'livre'. Inspecione o primeiro 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(); }

Você pode passar o ponteiro nulo na função livre , mas esse uso é definitivamente suspeito. Assim, o analisador encontrou linhas de código misturadas. Primeiro, o autor do código teve que liberar a memória pelo ponteiro fVectorIcon , somente após a atribuição de NULL .

V575 O ponteiro nulo é passado para a função 'livre'. Inspecione o primeiro 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 é outro exemplo de passagem explícita de um ponteiro nulo para a função livre . Esta linha pode ser excluída, pois a função sai após a obtenção do ponteiro com sucesso.

V575 O ponteiro nulo é passado para a função 'livre'. Inspecione o primeiro argumento. PackageFileHeapWriter.cpp 166

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

Alguém cometeu um erro aqui. O operador || deve ser usado em vez de &&. Somente nesse caso a exceção std :: bad_alloc () será lançada caso a alocação de memória (usando a função malloc ) falhe.

Erros com o operador de exclusão


V611 A memória foi alocada usando o operador 'new T []', mas foi liberada usando o operador 'delete'. Considere inspecionar este código. Provavelmente é melhor 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); } }

O ponteiro fMsg é usado para alocar memória para uma matriz de caracteres. O operador de exclusão é usado para liberar a memória em vez de excluir [] .

V611 A memória foi alocada usando o operador 'novo', mas foi liberada usando a função 'livre'. Considere inspecionar as lógicas de operação por trás da variável '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; } .... }

Aqui malloc_flags é uma função que chama malloc . Posicionamento-novo constrói o objeto aqui. Como a classe PageWriteWrapper é implementada da seguinte maneira:

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

os objetos destruidores dessa classe não serão chamados devido ao uso da função livre para liberar memória.

V611 A memória foi alocada usando o operador 'new T []', mas foi liberada usando o operador 'delete'. Considere inspecionar este código. Provavelmente é melhor usar 'delete [] fOutBuffer;'. Verifique as linhas: 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; };

É um erro comum usar o operador de exclusão em vez de excluir []. É mais fácil cometer um erro ao escrever uma classe, pois o código do destruidor geralmente está longe dos locais da memória. Aqui, o programador libera incorretamente a memória armazenada pelo ponteiro fOutBuffer no destruidor.

V772 Chamar um operador 'delete' para um ponteiro nulo causará um comportamento 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; } .... }

Além de uma escolha incorreta entre excluir / excluir [] e livre , você também pode ter um comportamento indefinido ao tentar limpar a memória por um ponteiro para o tipo de nulo (nulo *) .

Funções sem valor de retorno


V591 A função não nula deve retornar um valor. Referenciável.h 228

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

Um operador de atribuição sobrecarregado não possui um valor de retorno. Nesse caso, o operador retornará um valor aleatório, o que pode levar a erros estranhos.

Aqui estão os problemas semelhantes em outros fragmentos de código dessa classe:

  • V591 A função não nula deve retornar um valor. Referenciável.h 233
  • V591 A função não nula deve retornar um valor. Referenciável.h 239

V591 A função não nula deve retornar um 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 */ }

O comentário de um usuário NOTREACHED não significa nada aqui. Você precisa anotar funções como retorno normal para escrever corretamente o código para esses cenários. Para fazer isso, existem atributos noreturn: padrão e específicos do compilador. Antes de tudo, esses atributos são levados em consideração pelos compiladores para geração apropriada de código ou notificação de certos tipos de bugs usando avisos. Várias ferramentas de análise estática também levam em consideração os atributos para melhorar a qualidade da análise.

Manipulando exceções


V596 O objeto foi criado, mas não está sendo usado. A palavra-chave 'throw' pode estar ausente: 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; }

A palavra-chave throw foi acidentalmente esquecida aqui. Portanto, a exceção ParseException não será gerada enquanto o objeto desta classe será simplesmente destruído ao sair do escopo. Depois disso, a função continuará funcionando como se nada tivesse acontecido, como se o número correto tivesse sido inserido.

V1022 Uma exceção foi lançada pelo ponteiro. Considere jogá-lo por valor. 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 + "'."); // <= .... }

O analisador detectou a exceção IOException lançada pelo ponteiro. Jogar um ponteiro leva ao fato de que a exceção não será capturada. Portanto, a exceção é capturada por referência. Além disso, o uso de um ponteiro força o lado de captura a chamar o operador de exclusão para destruir o objeto criado, o que não havia sido feito.

Alguns outros fragmentos de código com problemas:

  • V1022 Uma exceção foi lançada pelo ponteiro. Considere jogá-lo por valor. gensyscallinfos.cpp 347
  • V1022 Uma exceção foi lançada pelo ponteiro. Considere jogá-lo por valor. gensyscallinfos.cpp 413

Segurança formal


V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'f_key'. A função memset_s () deve ser usada para apagar os dados 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); }

O analisador detectou um código suspeito, destinado à limpeza segura de dados privados. Infelizmente, a macro SAFE_FREE que se expande para o memset , chamadas gratuitas e atribuição NULL não torna o código mais seguro, pois tudo é removido pelo compilador ao otimizar com o O2 .

A propósito, nada mais é que CWE-14 : Remoção de código pelo compilador para limpar buffers.

Aqui está a lista de locais em que a limpeza de buffers não é realizada de fato:

  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'encoded_block'. A função memset_s () deve ser usada para apagar os dados privados. dst_api.c 446
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'key_st'. A função memset_s () deve ser usada para apagar os dados privados. dst_api.c 685
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'in_buff'. A função memset_s () deve ser usada para apagar os dados privados. dst_api.c 916
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o objeto 'ce'. A função memset_s () deve ser usada para apagar os dados privados. fs_cache.c 1078

Comparações com variáveis ​​não assinadas


V547 A expressão 'restante <0' é sempre falsa. O valor do tipo não assinado nunca é <0. DwarfFile.cpp 1947

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

O analisador encontrou uma comparação explícita da variável não assinada com valores negativos. Talvez deva comparar a variável restante apenas com nula ou implementar uma verificação de estouro.

V547 A expressão 'suspensão ((sem sinal) segundos)) <0' é sempre falsa. O valor do tipo não assinado nunca é <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 o ponto principal do erro, vamos abordar as assinaturas das funções sleep e usleep :

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

Como podemos ver, a função sleep retorna o valor não assinado e seu uso no código está incorreto.

Indicadores perigosos


V774 O ponteiro 'dispositivo' foi usado após o lançamento da memória. 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; }

Um objeto de dispositivo é liberado pelo operador de exclusão . É bastante lógico para a função FreeDevice . Mas, por algum motivo, para liberar outros recursos, o objeto já removido é endereçado.

Esse código é extremamente perigoso e pode ser encontrado em outros locais:

  • V774 O ponteiro 'self' foi usado após a liberação da memória. TranslatorRoster.cpp 884
  • V774 O ponteiro 'string' foi usado após o lançamento da memória. RemoteView.cpp 1269
  • V774 O ponteiro 'bs' foi usado após o lançamento da memória. mkntfs.c 4291
  • V774 O ponteiro 'bs' foi usado após o lançamento da memória. mkntfs.c 4308
  • V774 O ponteiro 'al' foi usado após a realocação da memória. inode.c 1155

V522 A desreferenciação do ponteiro nulo 'dados' pode ocorrer. O ponteiro nulo é passado para a função 'malo_hal_send_helper'. Inspecione o terceiro argumento. Verifique as linhas: 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 .... }

A análise interprocedural revelou o caso quando NULL é passado para a função e o ponteiro de dados com esse valor é eventualmente desreferenciado na função memcpy .

V773 A função foi encerrada sem liberar o ponteiro 'inputFileFile'. É possível um vazamento de memória. 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; .... }

O PVS-Studio pode detectar vazamentos de memória . Neste exemplo, em caso de erro, a memória não será liberada. Alguém pode pensar que, no caso de erros, você não deve se preocupar com o lançamento da memória, pois o programa ainda terminará. Mas nem sempre é assim. É um requisito para muitos programas lidar com erros corretamente e continuar trabalhando.

V595 O ponteiro 'fReply' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 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; }

É um erro muito comum remover referências de ponteiros antes de verificá-las. O diagnóstico V595 quase sempre prevalece no número de avisos em um projeto. Esse fragmento de código inclui o uso perigoso do ponteiro fReply .

V595 O ponteiro 'mq' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 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; .... }

Um exemplo semelhante. O ponteiro mg é desreferenciado várias linhas antes do que é verificado como nulo. Existem muitos lugares semelhantes no projeto. Em alguns trechos, o uso e a verificação do ponteiro estão bastante distantes um do outro, portanto, neste artigo, você encontrará apenas alguns exemplos. Os desenvolvedores podem verificar outros exemplos no relatório completo do analisador.

Diversos


V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. 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; .... } .... } .... }

A diferença entre as funções strlcat e strncat não é muito óbvia para quem não está familiarizado com a descrição dessas funções. A função strlcat espera o tamanho de todo o buffer como terceiro argumento, enquanto a função strncat - o tamanho do espaço livre em um buffer, que requer a avaliação de um valor necessário antes de chamar a função. Mas os desenvolvedores geralmente esquecem ou não sabem disso. Passar todo o tamanho do buffer para a função strncat pode levar ao buffer overflow, pois a função considerará esse valor como um número aceitável de caracteres para copiar. A função strlcat não tem esse problema. Mas você precisa passar as strings, terminando com terminal nulo para que funcione corretamente.

Aqui está a lista completa de lugares perigosos com cadeias de caracteres:

  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 104
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 107
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 110
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 113
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 118
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 119
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 120
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 123
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 126
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 129
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 132
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 135
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 138
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 141
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer da 'saída'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. NamespaceDump.cpp 144
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer 'features_string'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. VirtioDevice.cpp 283
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer 'features_string'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. VirtioDevice.cpp 284
  • V645 A chamada de função 'strncat' pode levar ao estouro de buffer 'features_string'. Os limites não devem conter o tamanho do buffer, mas vários caracteres que ele pode conter. VirtioDevice.cpp 285

V792 A função 'SetDecoratorSettings' localizada à direita do operador '|' será chamado independentemente do valor do operando esquerdo. Talvez seja melhor 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; }

Provavelmente, '|' e '||' operadores estavam confusos. Este erro leva a chamadas desnecessárias da função SetDecoratorSettings .

V627 Considere inspecionar a expressão. O argumento sizeof () é a macro que se expande para um 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; .... }

Passar o valor 0x0c para o operador sizeof parece suspeito. Talvez o autor deva ter avaliado o tamanho de um objeto, por exemplo, dados .

V562 É estranho comparar um valor do tipo bool com um 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(); } .... }

A função IsProfessionalSpdif retorna o valor do tipo bool . Ao fazer isso, o resultado da função é comparado com o número 0x12 na condição.

Conclusão


Perdemos o lançamento do primeiro beta do Haiku no outono passado, pois estávamos ocupados lançando o PVS-Studio for Java. Ainda assim, a natureza dos erros de programação é tal que eles não desaparecem se você não procurá-los e não presta atenção à qualidade do código. Os desenvolvedores do projeto usaram o Coverity Scan , mas a última execução foi há quase dois anos. Isso deve ser perturbador para os usuários do Haiku. Embora a análise tenha sido configurada em 2014 usando o Coverity, ela não nos impediu de escrever dois longos artigos sobre revisão de erros em 2015 ( parte 1 , parte 2 )

Outra revisão de erros do Haiku será publicada em breve para quem ler este post até o final. O relatório completo do analisador será enviado aos desenvolvedores antes de publicar esta revisão de erros; portanto, alguns erros podem ser corrigidos no momento em que você estiver lendo isso. Para passar o tempo entre os artigos, sugiro fazer o download e experimentar o PVS-Studio para o seu projeto.

Deseja experimentar o Haiku e tiver dúvidas? Os desenvolvedores do Haiku convidam você para o canal de telegrama .

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

More articles:


All Articles