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

De tempos em tempos, os leitores de nossos artigos sobre verificação de projetos de código aberto percebem que o analisador de código estático do PVS-Studio detecta uma grande porcentagem de erros que são insignificantes ou não afetam o aplicativo. É mesmo. Os bugs mais importantes já foram corrigidos graças a testes manuais, análises de usuários e outros métodos caros. Ao mesmo tempo, muitos desses erros podem ser encontrados mesmo na fase de escrever o código e corrigidos com perda mínima de tempo, reputação e dinheiro. Este artigo fornecerá alguns exemplos de erros reais que seriam corrigidos imediatamente se os autores dos projetos usassem análise de código estático.

A ideia é muito simples. Vamos dar uma olhada no GitHub para exemplos de solicitações pull, cujos comentários indicam que esta é uma correção de bug. E tente encontrar esses erros usando o analisador de código estático do PVS-Studio. Se o erro corrigido for encontrado pelo analisador, isso significa que ele poderá ser corrigido mesmo no estágio de gravação do código. E quanto mais cedo o erro for corrigido, mais barato será.

Infelizmente, o GitHub nos decepcionou e não permitiu criar um ótimo artigo inteligente sobre esse tópico. O próprio GitHub também possui uma falha (ou recurso) que não permite pesquisar comentários em solicitações pull em projetos escritos apenas em determinadas linguagens de programação. Bem, ou eu não "sei cozinhar". Independentemente do que eu digo para você procurar comentários em projetos em C ++, C # ou Java, os resultados são exibidos em todas as linguagens, incluindo PHP, Python, JavaScript e assim por diante. Como resultado, procurar casos adequados foi uma tarefa extremamente tediosa e vou me limitar a 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 erro fosse detectado mais cedo? A resposta é simples: os programadores não precisariam esperar pela manifestação e procurar e corrigir o código com defeito.
Vejamos os erros que o PVS-Studio pôde detectar imediatamente:

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

// 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:

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

A função acima não tem retorno , portanto, retornará um valor formalmente indefinido. O programador não usou o analisador de código, portanto, ele teve que procurar um erro por conta própria. 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 indicou 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, foram feitas correções 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; 

na expressão

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

Na versão antiga do código, nenhuma verificação de cpu foi feita quanto à igualdade com o ponteiro nulo. Se, de repente, acontecer que um ponteiro nulo é passado para a função mc6809dis_direct como o segundo argumento, será desreferenciado no corpo da função. O resultado é deplorável e imprevisível .

A desreferenciação de ponteiro nulo é um dos padrões mais comuns sobre os quais somos informados: “Este não é um erro crítico. Bem, ela vive em código e vive, e se a desreferenciação acontecer, o programa falhará silenciosamente e é tudo. " É estranho e triste ouvir isso dos programadores de C ++, mas a vida é vida.

De qualquer forma, neste projeto, essa desreferenciação ainda se transformou em um bug, como o cabeçalho de confirmação nos diz: " Correção de bug --- desreferencia NULL ".

Se o desenvolvedor do projeto usasse o PVS-Studio, ele poderia verificar seu código e detectar um aviso há dois meses e meio (isto é, quanto passou desde que o erro foi cometido):

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

Assim, o erro seria eliminado mesmo no momento de sua aparência, o que economizaria tempo e, possivelmente, os nervos do desenvolvedor :).

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

O código 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, o programador substitui a expressão " firstbit_location + = 32 " por " * firstbit_location + = 32 ". O programador esperava que o número 32 fosse adicionado ao valor da variável à qual o ponteiro firstbit_location está vinculado , mas foi adicionado diretamente ao ponteiro. O valor alterado do ponteiro não foi usado em nenhum outro lugar e o valor esperado da variável 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

Parece que o que poderia ser terrível em uma expressão modificada, mas ainda não utilizada? O diagnóstico do V1001 obviamente não parece ter como objetivo identificar bugs especialmente perigosos. Apesar disso, ela descobriu um erro importante que afetava a lógica do programa.

Além disso, esse erro não foi tão fácil de encontrar! Ela não estava apenas no programa desde o momento em que o arquivo foi criado, ela também sobreviveu a muitas edições em linhas adjacentes e permaneceu no projeto por até 3 ( ! ) Anos! Todo esse tempo, a lógica do programa foi quebrada e não funcionou exatamente como os desenvolvedores esperavam. Se eles usassem o PVS-Studio, o erro teria sido detectado muito antes.

No final, considere outro exemplo bonito. Enquanto eu colecionava correções de erros no GitHub, me deparei com várias vezes uma correção com esse conteúdo . O bug foi corrigido 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); ... } 

PVS-Studio emitiu um aviso para esta seção 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

Examinei as declarações de variáveis ​​usadas na expressão " phys_addr_t pa = (vma-> vm_pgoff << PAGE_SHIFT) + vm_start - vma-> vm_start; " e constatei que o código acima é semelhante ao seguinte exemplo sintético:

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

Se o valor da variável de 32 bits a for maior que 0xFFFFF , os 12 bits mais significativos terão pelo menos um valor diferente de zero. Após aplicar a operação de deslocamento à esquerda a essa variável, esses bits significativos serão perdidos, como resultado das quais informações incorretas serão gravadas em x .

Para eliminar a perda de bits altos, primeiro você deve converter a para o tipo sem assinatura por muito tempo e somente depois executar a operação de deslocamento:

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

Então pa sempre escreverá o valor correto.

Tudo ficaria bem, mas esse bug, como o primeiro exemplo do artigo, também se mostrou crítico, como o autor do commit escreveu separadamente em seu comentário. Além disso, esse erro acabou de entrar em um grande 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 a utilidade do uso regular de um analisador de código estático. Espero que tenham gostado. Faça o download e experimente o analisador de código estático PVS-Studio para testar seus próprios projetos. No momento da redação deste artigo, ele implementou cerca de 700 regras de diagnóstico para detectar uma variedade de padrões de erro. C, C ++, C # e Java são suportados.

Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: George Gribkov. Erros que a análise de código estático não encontra porque não é usada