Erros que a análise de código estático não encontra porque não é usada

Os leitores de nossos artigos ocasionalmente observam que o analisador de código estático do PVS-Studio detecta um grande número de erros que são insignificantes e não afetam o aplicativo. É realmente assim. Na maioria das vezes, erros importantes já foram corrigidos devido a testes manuais, feedback do usuário e outros métodos caros. Ao mesmo tempo, muitos desses erros foram encontrados no estágio de escrita do código e corrigidos com perda mínima de tempo, reputação e dinheiro. Este artigo fornecerá vários exemplos de erros reais, que poderiam ter sido corrigidos imediatamente, se os autores do projeto tivessem usado a análise de código estático.


A ideia é muito simples. Procuraremos exemplos de solicitações pull no GitHub que especificam que um problema é uma correção de bug. Em seguida, tentaremos encontrar esses erros usando o analisador de código estático do PVS-Studio. Se um erro puder ser encontrado pelo analisador, é um erro que pode ter sido encontrado no estágio de escrita do código. Quanto mais cedo o bug for corrigido, mais barato ele será.

Infelizmente, o GitHub nos decepcionou e não conseguimos fazer um grande artigo fino sobre o assunto. O próprio GitHub possui uma falha (ou um recurso) que não permite procurar comentários de solicitações pull em projetos escritos apenas em determinadas linguagens de programação. Ou não sei cozinhar. Apesar de eu especificar a pesquisa de comentários em projetos em C, C ++, C #, os resultados são fornecidos para todos os idiomas, incluindo PHP, Python, JavaScript e outros. Como resultado, procurar casos adequados provou ser extremamente tedioso, e vou dar apenas alguns exemplos. No entanto, eles são suficientes para demonstrar a utilidade das ferramentas de análise de código estático quando usadas regularmente.

E se o bug tivesse sido detectado na fase inicial? A resposta é simples: os programadores não precisariam esperar para aparecer, depois procurar e corrigir o código com defeito.

Vejamos os erros que o PVS-Studio poderia ter detectado imediatamente:

O primeiro exemplo é retirado do projeto SatisfactoryModLoader. Antes de corrigir o erro, o código era o seguinte:

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

Este código continha um erro, que o PVS-Studio emitia imediatamente um aviso para:

V591 A função não nula deve retornar um valor. ModFunctions.cpp 44

A função acima não possui declaração de retorno , portanto retornará um valor formalmente indefinido. O programador não usou o analisador de código, então ele teve que procurar o bug sozinho. A função após a edição:

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

Curiosamente, no commit, o autor marcou o erro como crítico: " corrigiu o erro crítico em que as funções da API não foram retornadas ".

No segundo commit do histórico do projeto mc6809, as edições foram introduzidas no seguinte código:

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

O autor corrigiu apenas uma linha. Ele substituiu a expressão

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

para o seguinte

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

Na versão antiga do código, não havia nenhuma verificação para um ponteiro nulo. Se isso acontecer, para que um ponteiro nulo seja passado para a função mc6809dis_direct como o segundo argumento, sua desreferência ocorrerá no corpo da função. O resultado é deplorável e imprevisível .

A dereferência de ponteiro nulo é um dos padrões mais comuns sobre os quais somos informados: "Não é um bug crítico. Quem se importa com o sucesso do código? Se ocorrer desreferência, o programa travará silenciosamente e é isso. ” É estranho e triste ouvir isso dos programadores de C ++, mas a vida acontece.

De qualquer forma, neste projeto, essa desreferência se transformou em um bug, como o assunto do commit nos diz: " Correção de bug --- desreferência NULL ".

Se o desenvolvedor do projeto tivesse usado o PVS-Studio, ele poderia ter verificado e encontrado o aviso dois meses e meio atrás. Foi quando o bug foi introduzido. Aqui está o aviso:

V595 O ponteiro 'cpu' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 1814, 1821. mc6809dis.c 1814

Assim, o bug teria sido corrigido no momento de sua aparência, o que economizaria o tempo e os nervos do desenvolvedor :).

Um exemplo de outra correção interessante foi encontrado no projeto libmorton.

Código a ser corrigido:

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

Em sua edição, um programador substitui a expressão " firstbit_location + = 32 " por " * firstbit_location + = 32 ". O programador esperava que 32 fossem adicionados ao valor da variável referida pelo ponteiro firstbit_location , mas 32 foi adicionado ao próprio ponteiro. O valor alterado do ponteiro não foi mais usado em nenhum lugar e o valor variável esperado permaneceu inalterado.

O PVS-Studio emitirá um aviso para este código:

V1001 A variável 'firstbit_location' é atribuída, mas não é usada no final da função. morton_common.h 22

Bem, o que há de tão ruim na expressão modificada, mas ainda não utilizada? O diagnóstico V1001 não parece destinado a detectar bugs particularmente perigosos. Apesar disso, encontrou um erro importante que influenciou a lógica do programa.

Além disso, o erro não foi tão fácil de encontrar! Ele não só está no programa desde que o arquivo foi criado, mas também passou por muitas edições em linhas vizinhas e existe no projeto por até 3 (!) Anos! Todo esse tempo, a lógica do programa foi quebrada e não funcionou da maneira que os desenvolvedores esperavam. Se eles tivessem usado o PVS-Studio, o bug teria sido detectado muito antes.

No final, vejamos outro bom exemplo. Enquanto eu colecionava correções no GitHub, me deparei com uma correção com o seguinte conteúdo várias vezes. O erro corrigido estava aqui:

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

O PVS-Studio emitiu um aviso para este trecho de código:

V629 Considere inspecionar a expressão 'vma-> vm_pgoff << 12'. Mudança de bit do valor de 32 bits com uma expansão subsequente para o tipo de 64 bits. mmu.c 1795

Fiz check-out de declarações de variáveis, usadas na expressão " phys_addr_t pa = (vma-> vm_pgoff << PAGE_SHIFT) + vm_start - vma-> vm_start; " e descobri que o código fornecido acima é igual ao seguinte exemplo sintético:

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

Se o valor de uma variável de 32 bits for maior que 0xFFFFF , os 12 bits mais altos terão pelo menos um valor não nulo. Depois de mudar esta variável para a esquerda, esses bits significativos serão perdidos, resultando em informações incorretas escritas em x.

Para eliminar a perda de bits altos, precisamos primeiro converter um para o tipo long long sem sinal e somente após esse deslocamento a variável:

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

Dessa forma, um valor correto sempre será escrito em pa.

Tudo bem, mas esse bug, igual ao primeiro exemplo do artigo, também se mostrou crítico. É autor escreveu sobre isso no comentário. Além disso, esse erro chegou a um enorme número de projetos. Para apreciar completamente a escala da tragédia, sugiro observar o número de resultados ao procurar esse bugfix no GitHub. Assustador, não é?



Portanto, adotei uma nova abordagem para demonstrar os benefícios de um uso regular do analisador de código estático. Espero que tenham gostado. Faça o download e experimente o analisador de código estático do PVS-Studio para verificar seus próprios projetos. No momento da redação deste artigo, havia cerca de 700 regras de diagnóstico implementadas para detectar uma variedade de padrões de erro. Suporta C, C ++, C # e Java.

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


All Articles