Geekly articles weekly

Como dar um tiro no pé em C e C ++. Coleção de receitas do sistema operacional Haiku

A história da reunião do analisador estático PVS-Studio com o código do sistema operacional Haiku remonta ao distante ano de 2015. Foi um experimento interessante e uma experiência útil para as equipes dos dois projetos. Por que um experimento? Não havia analisador para Linux na época e não haverá mais um ano e meio. Mas o trabalho dos entusiastas de nossa equipe foi recompensado: conhecemos os desenvolvedores do Haiku e melhoramos a qualidade do código, reabastecemos o banco de dados com raros erros de programadores e refinamos o analisador. Verificar o código do Haiku quanto a erros é rápido e fácil agora.

Quadro 3


1. Introdução


Os heróis de nossa história são o sistema operacional Haiku de código aberto e o analisador estático PVS-Studio para C, C ++, C # e Java. Quando, há 4,5 anos, começamos a analisar o projeto, tivemos que trabalhar apenas com o arquivo executável do analisador compilado. Toda a infraestrutura para analisar parâmetros de compilação, iniciar um pré-processador, analisar paralelamente, etc. foi retirado do utilitário de interface do usuário do Compiler Monitoring em C #, que foi portado em partes para a plataforma Mono para execução no Linux. O projeto Haiku em si é construído usando um compilador cruzado em vários sistemas operacionais, exceto o Windows. Mais uma vez, quero observar a conveniência e a integridade da documentação do assembly do Haiku e também agradecer aos desenvolvedores do Haiku por sua ajuda na construção do projeto.

Agora a análise é muito mais fácil. A lista de todos os comandos para criar e analisar o projeto é assim:

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 realizada no contêiner Docker. Recentemente, preparamos nova documentação sobre este tópico: Executando o PVS-Studio no Docker . Isso pode simplificar bastante o uso de técnicas de análise estática de projetos para algumas empresas.

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 quando declarada; portanto, compará-la com um valor nulo levará a um resultado indefinido. Se as circunstâncias falharem, o valor indefinido da variável rval pode se tornar o 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; .... } } .... }

Um caso semelhante de usar uma variável não inicializada, somente aqui está o ponteiro não inicializado res .

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

Provavelmente, o programador precisava normalizar o objeto através de uma variável intermediária. Mas agora, no ponteiro da fonte , o ponteiro para o objeto temporário normalizado foi salvo , que será destruído após sair do escopo no qual esse 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 de construtor supostamente para inicializar / zerar os campos da classe. Nesse caso, não há modificação nos campos da classe, mas um novo objeto não nomeado dessa classe é criado e imediatamente destruído. Infelizmente, existem muitos desses lugares:

  • 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 em que são declarados na própria classe. Neste exemplo, o campo fInternal será inicializado primeiro usando o valor não inicializado fPatternHandler .

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

O 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 - os ramos if e else são idênticos. Mas para onde foram 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 no mesmo arquivo e quase lado a lado, no entanto, houve uma substituição em algum lugar.

Uma pesquisa no conteúdo dos arquivos encontrou apenas 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)

Se essa substituição está correta ou não, vale a pena verificar os desenvolvedores do projeto. Eu verifiquei o 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 um ponteiro nulo para a função livre , mas essa entrada é claramente suspeita. Então, o analisador encontrou as linhas de código confusas. Primeiro, você precisava liberar a memória usando o ponteiro fVectorIcon e, em seguida, defini-lo como 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); // <= .... }

Outro exemplo de passagem explícita de um ponteiro nulo para a função livre . Esta linha pode ser excluída porque após o recebimento bem-sucedido do ponteiro, a função sai.

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

Aqui está um erro. Em vez do operador &&, o operador || deve ser usado. Somente nesse caso será lançada uma exceção std :: bad_alloc () se não for possível alocar memória usando a função malloc .

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 armazenar uma matriz de caracteres, e o operador delete é usado para liberar memória, em vez de delete [] .

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 é a função que faz malloc . E, em seguida, posicionamento-novo constrói o objeto aqui. E 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 destruidores de objetos dessa classe não serão chamados devido ao uso da função free 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; };

Usar o operador de exclusão em vez de excluir [] é um erro muito comum. A maneira mais fácil de cometer um erro ao escrever uma aula é porque O código destruidor geralmente está localizado longe dos locais de alocação de memória. Aqui, o programador libera incorretamente a memória no destruidor pelo ponteiro fOutBuffer .

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 da escolha errada entre excluir / excluir [] e livre , você pode adicionar um comportamento indefinido ao programa de outra maneira - tente limpar a memória com um ponteiro para um tipo indefinido (vazio *) .

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

O operador de atribuição substituído não possui valor de retorno suficiente. Nesse caso, o operador retornará um valor aleatório, o que pode levar a erros estranhos.

Problemas semelhantes em outros trechos de código desta 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 */ }

Um comentário de usuário NÃO ALCANCE aqui não significa nada. Para escrever corretamente o código para esses cenários, você deve marcar funções como noreturn. Existem atributos noreturn para isso: padrão e específico do compilador. Antes de tudo, esses atributos são levados em consideração pelos compiladores para a geração correta do código ou a notificação de alguns tipos de erros com a ajuda de warings. Várias ferramentas de análise estática também levam em consideração os atributos para melhorar a qualidade da análise.

Manipulação de exceção


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 é acidentalmente esquecida aqui. Assim, uma ParseException não será lançada e um objeto dessa classe será simplesmente destruído quando sair do escopo. Após o qual a função continuará seu trabalho como se nada tivesse acontecido, como se o número correto fosse 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 uma IOException lançada pelo ponteiro. Lançar um ponteiro faz com que a exceção não seja capturada; portanto, a exceção é capturada por referência. Além disso, o uso de um ponteiro força o interceptador a chamar o operador de exclusão para destruir o objeto criado, o que também não é feito.

Mais duas áreas problemáticas do código:

  • 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 projetado para limpar com segurança dados particulares. Infelizmente, a macro SAFE_FREE , expandida para memset , chamadas gratuitas e atribuição NULL , não torna o código mais seguro, porque tudo isso é removido pelo compilador durante a otimização do O2 .

A propósito, isso não se parece em nada com CWE-14 : Remoção de código por compilador para limpar buffers.

A lista completa de locais onde os buffers não são realmente limpos:

  • 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 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 de uma variável não assinada com valores negativos. Talvez você deva comparar a variável restante com apenas zero 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 qual é o erro, passemos às 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 um valor de um tipo 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 é limpo pelo operador de exclusão . Esta é uma ação lógica para uma função chamada FreeDevice . Mas, por algum motivo, para liberar outros recursos no código, há novamente um apelo a um objeto já excluído.

Esse código é extremamente perigoso e ocorre em vários outros lugares:

  • 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 uma situação em que NULL é passado para a função e o ponteiro de dados com esse valor é posteriormente 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 . A memória para inputFileFile não é liberada aqui em caso de algum tipo de erro. Alguém acredita que, em caso de erros, você não pode se preocupar em liberar memória - o programa ainda terminará. Mas nem sempre é esse o caso. Manuseie corretamente os erros e continue a trabalhar - um requisito para muitos programas.

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

Com que frequência os desenvolvedores desreferem os ponteiros antes de verificá-los. O Diagnostics V595 é quase sempre um líder no número de avisos em um projeto. O uso perigoso do ponteiro fReply neste pedaço de código.

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 algumas linhas antes da verificação de um valor nulo. Existem muitos lugares semelhantes no projeto. Em alguns lugares, o uso e a verificação de ponteiros estão longe um do outro, portanto, dois exemplos estão incluídos no artigo. O restante poderá ver os desenvolvedores no relatório completo do analisador.

Erros 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 é totalmente óbvia para uma pessoa nova na descrição dessas funções. A função strlcat usa o tamanho de todo o buffer como terceiro argumento, e a função strncat usa o tamanho do espaço livre no buffer , o que requer o cálculo do valor desejado antes de chamar a função. Mas os desenvolvedores geralmente esquecem ou não sabem disso. Passar a função strncat para o tamanho de todo o buffer pode levar a um estouro de buffer, porque a função considerará esse valor como o número de caracteres permitido para cópia. A função strlcat não tem esse problema, mas para que funcione corretamente, é necessário passar as seqüências que terminam no terminal zero.

Toda a lista de lugares perigosos com linhas:

  • 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'. 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 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 285

V792 The 'SetDecoratorSettings' function located to the right of the operator '|' will be called regardless of the value of the left operand. Perhaps, it is better to use '||'. 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; }

, '|' '||'. SetDecoratorSettings .

V627 Consider inspecting the expression. The argument of sizeof() is the macro which expands to a number. 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; .... }

sizeof 0x0c . , - , , data .

V562 It's odd to compare a bool type value with a value of 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(); } .... }

IsProfessionalSpdif bool , 0x12 .

Conclusão


Haiku , .. PVS-Studio Java. , , . Coverity Scan , . Haiku. Coverity 2014 , 2015 ( 1 , 2 ).

, , Haiku . , . , PVS-Studio .

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



Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Svyatoslav Razmyslov. Como dar um tiro no pé em C e C ++. Haiku OS Cookbook

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

More articles:


All Articles