Os 10 principais erros nos projetos C ++ para 2018

Já se passaram três meses desde o final de 2018. Para muitos, ele voou quase imperceptivelmente, mas para nós, os desenvolvedores do PVS-Studio, ficou muito saturado. Nós trabalhamos duro, lutamos sem medo pelo avanço da análise estática para as massas e procuramos novos erros em projetos de código aberto escritos em C, C ++, C # e Java. Os dez mais interessantes que reunimos para você neste artigo!



Procuramos lugares interessantes usando o analisador de código estático PVS-Studio . Ele pode detectar erros e possíveis vulnerabilidades no código escrito nos idiomas mencionados acima.

Se você estiver interessado em procurar erros, pode sempre fazer o download e experimentar o nosso analisador. Fornecemos uma versão gratuita do analisador para estudantes e programadores entusiasmados, uma licença gratuita para desenvolvedores de projetos de código aberto, bem como uma versão de teste para tudo-tudo-tudo. Quem sabe, talvez até o próximo ano você consiga o seu top 10? :)

Nota: sugiro que o leitor verifique a si próprio e, antes de observar o aviso do analisador, tente identificar as anomalias. Quantos erros você consegue encontrar?

Décimo lugar


Fonte: E novamente no espaço: como o unicórnio Stellarium visitou

Este erro foi descoberto ao verificar o planetário virtual do Stellarium.

O trecho de código acima, embora pequeno, está repleto de um erro bastante complicado:

Plane::Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3) : distance(0.0f), sDistance(0.0f) { Plane(v1, v2, v3, SPolygon::CCW); } 

Você encontrou?

PVS-Studio Warning: V603 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> Plane :: Plane (....)' deve ser usado. Plane.cpp 29

O autor do código queria inicializar parte dos campos do objeto usando outro construtor aninhado no principal. É verdade que ele apenas conseguiu criar um objeto temporário que seria destruído ao sair de sua área de visibilidade. Assim, vários campos do objeto permanecerão não inicializados.

Em vez de uma chamada de construtor aninhado, você deve usar o construtor de delegação apresentado no C ++ 11. Por exemplo, você pode fazer isso:

 Plane::Plane(Vec3f& v1, Vec3f& v2, Vec3f& v3) : Plane(v1, v2, v3, SPolygon::CCW) { distance = 0.0f; sDistance = 0.0f; } 

Todos os campos obrigatórios seriam inicializados corretamente. Não é maravilhoso?

Nono lugar


Fonte: Perl 5: Como ocultaram os erros de macro

Em nono lugar, exibe uma macro notável do código-fonte Perl 5.

Ao coletar erros para escrever o artigo, meu colega Svyatoslav encontrou um aviso emitido pelo analisador sobre o uso de uma macro. Aqui está:

 PP(pp_match) { .... MgBYTEPOS_set(mg, TARG, truebase, RXp_OFFS(prog)[0].end); .... } 

Para descobrir qual era o problema, Svyatoslav se aprofundou. Ele abriu a definição de macro e viu que ela continha várias macros aninhadas, algumas das quais também tinham macros aninhadas. Era tão difícil descobrir que eu tinha que usar um arquivo pré-processado. Mas, infelizmente, isso não ajudou. No lugar da linha de código anterior, Svyatoslav descobriu o seguinte:

 (((targ)->sv_flags & 0x00000400) && (!((targ)->sv_flags & 0x00200000) || S_sv_only_taint_gmagic(targ)) ? (mg)->mg_len = ((prog->offs)[0].end), (mg)->mg_flags |= 0x40 : ((mg)->mg_len = (((targ)->sv_flags & 0x20000000) && !__builtin_expect(((((PL_curcop)->cop_hints + 0) & 0x00000008) ? (_Bool)1 :(_Bool)0),(0))) ? (ssize_t)Perl_utf8_length( (U8 *)(truebase), (U8 *)(truebase)+((prog->offs)[0].end)) : (ssize_t)((prog->offs)[0].end), (mg)->mg_flags &= ~0x40)); 

Aviso PVS-Studio: V502 Talvez o operador '?:' Funcione de maneira diferente do esperado. O operador '?:' Tem uma prioridade mais baixa que o operador '&&'. pp_hot.c 3036

Eu acho que será difícil encontrar esse erro com meus olhos. Honestamente, meditamos sobre esse código por um longo tempo e chegamos à conclusão de que, de fato, não há erro aqui. Mas, em qualquer caso, este é um exemplo bastante divertido de código ilegível.

Dizem que as macros são más. Obviamente, há momentos em que eles se tornam indispensáveis, mas se você pode substituir a macro por uma função, definitivamente deve fazê-lo.

Macros aninhadas são especialmente difíceis. Não apenas porque são difíceis de entender, mas também porque podem dar resultados imprevisíveis. Se o autor de uma macro cometer um erro acidental nessa macro, será muito mais difícil encontrá-lo do que em uma função.

Oitavo lugar


Fonte: Crómio: Outros Erros

O exemplo a seguir foi retirado de uma série de artigos sobre a análise do projeto Chromium. Ela se cobriu na biblioteca WebRTC.

 std::vector<SdpVideoFormat> StereoDecoderFactory::GetSupportedFormats() const { std::vector<SdpVideoFormat> formats = ....; for (const auto& format : formats) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } } return formats; } 

PVS-Studio Warning: V789 CWE-672 Iteradores para o contêiner de 'formatos', usado no loop for baseado em intervalo, tornam-se inválidos quando a função 'push_back' é chamada. stereocodecfactory.cc 89

O erro é que o tamanho do vetor de formatos muda dentro do loop for baseado em intervalo. Os loops baseados em intervalo são baseados em iteradores, portanto, redimensionar o contêiner dentro desses loops pode levar à invalidação desses iteradores.

Este erro persistirá se você reescrever o loop usando iteradores explícitos. Portanto, para maior clareza, você pode trazer este código:

 for (auto format = begin(formats), __end = end(formats); format != __end; ++format) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } } 

Por exemplo, ao usar o método push_back , um vetor pode ser liberado - e os iteradores apontarão para uma área de memória inválida.

Para evitar esses erros, você deve seguir a regra: nunca redimensione o contêiner dentro do loop, cujas condições estão vinculadas a esse contêiner. Isso se aplica a loops e loops baseados em intervalo usando iteradores. Você pode ler sobre quais operações podem levar à invalidação de iteradores nas discussões no StackOverflow.

Sétimo lugar


Fonte: Godot: Uso Regular de Analisadores de Código Estático

O primeiro exemplo da indústria de videogames será um trecho de código que descobrimos no mecanismo de jogos Godot. Você pode ter que suar para descobrir o erro com seus olhos, mas tenho certeza que nossos leitores sofisticados podem lidar com isso:

 void AnimationNodeBlendSpace1D::add_blend_point( const Ref<AnimationRootNode> &p_node, float p_position, int p_at_index) { ERR_FAIL_COND(blend_points_used >= MAX_BLEND_POINTS); ERR_FAIL_COND(p_node.is_null()); ERR_FAIL_COND(p_at_index < -1 || p_at_index > blend_points_used); if (p_at_index == -1 || p_at_index == blend_points_used) { p_at_index = blend_points_used; } else { for (int i = blend_points_used - 1; i > p_at_index; i++) { blend_points[i] = blend_points[i - 1]; } } .... } 

Aviso do PVS-Studio: V621 CWE-835 Considere inspecionar o operador 'for'. É possível que o loop seja executado incorretamente ou não seja executado. animation_blend_space_1d.cpp 113

Vamos considerar a condição do ciclo em mais detalhes. A variável do contador é inicializada com o valor blend_points_used - 1 . Ao mesmo tempo, com base nas duas verificações anteriores (em ERR_FAIL_COND e em if ), fica claro que, no momento em que o loop é executado, blend_points_used sempre será maior que p_at_index . Portanto, a condição do loop sempre será verdadeira ou o loop não será executado.

Se blend_points_used for 1 == p_at_index , o loop não será executado.

Em todos os outros casos, a verificação i> p_at_index sempre será verdadeira, pois o contador i aumenta a cada iteração do loop.

Pode parecer que o ciclo vai durar para sempre, mas não é.

Primeiro, haverá um estouro inteiro da variável i , que é um comportamento indefinido. Portanto, confiar nisso não vale a pena.

Se eu fosse do tipo unsigned int , depois que o contador atingir o valor máximo possível, o operador i ++ o transformaria em 0 . Esse comportamento é definido pelo padrão e é chamado "Invólucro não assinado". No entanto, você deve estar ciente de que usar esse mecanismo também não é uma boa ideia .

Isso foi o primeiro, mas ainda existe o segundo! O fato é que ele nem atingirá o excesso de número inteiro. Onde antes a matriz vai para o exterior. Isso significa que será feita uma tentativa de acessar a área de memória fora do bloco alocado para a matriz. E isso também é um comportamento vago. Exemplo clássico :)

Para tornar mais fácil evitar esses erros, posso fazer apenas algumas recomendações:

  1. Escreva código mais simples e intuitivo
  2. Faça uma revisão de código mais completa e escreva mais testes para código recém-escrito
  3. Use analisadores estáticos;)


Sexto lugar


Fonte: Amazon Lumberyard: O Grito da Alma

Outro exemplo da indústria gamedev, a saber, do código-fonte do mecanismo AAA Amazon Lumberyard.

 void TranslateVariableNameByOperandType(....) { // Igor: yet another Qualcomm's special case // GLSL compiler thinks that -2147483648 is // an integer overflow which is not if (*((int*)(&psOperand->afImmediates[0])) == 2147483648) { bformata(glsl, "-2147483647-1"); } else { // Igor: this is expected to fix // paranoid compiler checks such as Qualcomm's if (*((unsigned int*)(&psOperand->afImmediates[0])) >= 2147483648) { bformata(glsl, "%d", *((int*)(&psOperand->afImmediates[0]))); } else { bformata(glsl, "%d", *((int*)(&psOperand->afImmediates[0]))); } } bcatcstr(glsl, ")"); .... } 

PVS-Studio Warning: V523 A instrução 'then' é equivalente à instrução 'else'. toglsloperand.c 700

O Amazon Lumberyard está sendo desenvolvido como um mecanismo de plataforma cruzada. Portanto, os desenvolvedores estão tentando oferecer suporte ao maior número possível de compiladores. O programador Igor encontrou o compilador Qualcomm, como nos dizem os comentários.

Não se sabe se Igor conseguiu concluir sua tarefa e lidar com as verificações "paranóicas" do compilador, mas ele deixou para trás um código muito estranho. É estranho que tanto os ramos então - quanto os outros - da instrução if contenham código absolutamente idêntico. Provavelmente, esse erro foi cometido como resultado de um Copy-Paste desleixado.

Eu nem sei o que pode ser recomendado aqui. Portanto, desejo aos desenvolvedores do Amazon Lumberyard sucesso na correção de bugs e boa sorte ao programador Igor!

Quinto lugar


Fonte: Mais uma vez, o analisador PVS-Studio mostrou-se mais atento do que uma pessoa

Uma história interessante aconteceu com o exemplo a seguir. Meu colega Andrei Karpov estava preparando um artigo sobre o próximo teste do framework Qt. Ao escrever erros notáveis, ele encontrou um aviso do analisador, que considerou falso. Aqui está o trecho de código e o aviso relevantes:

 QWindowsCursor::CursorState QWindowsCursor::cursorState() { enum { cursorShowing = 0x1, cursorSuppressed = 0x2 }; CURSORINFO cursorInfo; cursorInfo.cbSize = sizeof(CURSORINFO); if (GetCursorInfo(&cursorInfo)) { if (cursorInfo.flags & CursorShowing) // <= V616 .... } 

Aviso do PVS-Studio: V616 CWE-480 A constante nomeada 'CursorShowing' com o valor 0 é usada na operação bit a bit. qwindowscursor.cpp 669

Ou seja, o PVS-Studio jurou em um lugar onde, obviamente, não havia erro! Não é possível que a constante CursorShowing seja 0 , porque literalmente algumas linhas acima dela são inicializadas como 1 .

Como uma versão instável do analisador foi usada para verificação, Andrei duvidou da exatidão do aviso. Ele examinou cuidadosamente essa seção do código várias vezes e ainda não encontrou um erro. Como resultado, ele escreveu esse falso positivo para o rastreador de bugs, para que outros colegas pudessem corrigir a situação.

E somente com uma análise detalhada ficou claro que o PVS-Studio estava novamente mais atento do que uma pessoa. O valor 0x1 é atribuído à constante nomeada cursorShowing , e a operação constante de bit “and” envolve a constante nomeada CursorShowing . Essas são constantes completamente diferentes, porque a primeira começa com uma letra minúscula e a segunda com uma letra maiúscula.

O código é compilado com êxito, porque a classe QWindowsCursor realmente contém uma constante com esse nome. Aqui está a definição dela:

 class QWindowsCursor : public QPlatformCursor { public: enum CursorState { CursorShowing, CursorHidden, CursorSuppressed }; .... } 

Se você não atribuir explicitamente uma constante enum nomeada, ela será inicializada por padrão. Como CursorShowing é o primeiro elemento de uma enumeração, ele será definido como 0 .

Para evitar esses erros, você não deve dar nomes muito semelhantes às entidades. Você deve seguir esta regra com cuidado especial se essas entidades forem do mesmo tipo ou puderem ser implicitamente convertidas uma para a outra. De fato, nesses casos, será quase impossível detectar um erro a olho nu, e o código incorreto será compilado com sucesso e viverá feliz dentro do seu projeto.

Quarto lugar


Fonte: Atiramos no pé, processando os dados de entrada

Estamos nos aproximando dos três principais finalistas, e o erro do projeto FreeSWITCH é o próximo.

 static const char *basic_gets(int *cnt) { .... int c = getchar(); if (c < 0) { if (fgets(command_buf, sizeof(command_buf) - 1, stdin) != command_buf) { break; } command_buf[strlen(command_buf)-1] = '\0'; /* remove endline */ break; } .... } 

Aviso do PVS-Studio: V1010 CWE-20 Dados contaminados não verificados são usados ​​no índice: 'strlen (command_buf)'.

O analisador avisa que a expressão strlen (command_buf) - 1 usa dados não verificados. E realmente: se command_buf estiver vazio do ponto de vista da string da linguagem C (contendo um único caractere - '\ 0'), então strlen (command_buf) retornará 0 . Nesse caso, command_buf [-1] será chamado , o que representa um comportamento indefinido. Problema!

O próprio suco desse erro nem é por que isso acontece, mas como acontece. Este erro é um daqueles exemplos agradáveis ​​que você pode "tocar" sozinho, reproduzir. Você pode iniciar o FreeSwitch, executar algumas ações que levarão à execução da seção de código acima e passar ao programa uma linha vazia para entrada.

Como resultado, com um movimento do pulso, um programa de trabalho se transforma (não, não em shorts elegantes ) em um programa que não funciona! Detalhes sobre como reproduzir esse erro podem ser encontrados no artigo de origem no link acima, mas por enquanto darei um resultado claro:



Lembre-se de que a entrada pode ser qualquer coisa e você deve sempre verificá-la. Então o analisador não xingará e o programa será mais confiável.

Agora é hora de lidar com nossos vencedores: estamos chegando à final!



Terceiro lugar


Fonte: NCBI Genome Workbench: Pesquisa em Perigo

Os três vencedores são abertos por um código do projeto NCBI Genome Workbench - um conjunto de ferramentas para estudar e analisar dados genéticos. Embora não seja necessário ser um super-homem geneticamente modificado para encontrar um erro aqui, poucos estão cientes dessa possibilidade.

 /** * Crypt a given password using schema required for NTLMv1 authentication * @param passwd clear text domain password * @param challenge challenge data given by server * @param flags NTLM flags from server side * @param answer buffer where to store crypted password */ void tds_answer_challenge(....) { .... if (ntlm_v == 1) { .... /* with security is best be pedantic */ memset(hash, 0, sizeof(hash)); memset(passwd_buf, 0, sizeof(passwd_buf)); ... } else { .... } } 

Avisos do PVS-Studio:

  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'hash'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 365
  • V597 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'passwd_buf'. A função memset_s () deve ser usada para apagar os dados privados. challenge.c 366

Conseguiu encontrar um erro? Se sim, então você - bem feito! ... bem, ou ainda um super-homem geneticamente modificado.

O fato é que os compiladores de otimização modernos podem fazer muito para tornar o programa montado mais rápido. Em particular, os compiladores conseguem rastrear que o buffer passado para o memset não é usado em nenhum outro lugar.

Nesse caso, eles podem excluir a chamada memset "desnecessária" e têm todo o direito de fazê-lo. Em seguida, um buffer que armazena dados importantes pode permanecer na memória para o deleite dos invasores.

Nesse contexto, o comentário letrado “com segurança é bom ser pedante” parece ainda mais engraçado. A julgar pelo número muito pequeno de avisos emitidos para este projeto, os desenvolvedores tentaram muito ser cuidadosos e escrever um código seguro. No entanto, como podemos ver, pular essa falha de segurança é muito simples. De acordo com a Common Weakness Enumeration, um defeito é classificado como CWE-14 : Remoção de código do compilador para limpar buffers.

Para limpar a limpeza da memória, use a função memset_s () . Não é apenas mais seguro que memset () , mas também não pode ser "ignorado" pelo compilador.

Segundo lugar


Fonte: Como o PVS-Studio se mostrou mais atento do que três programadores e meio

O medalhista de prata deste top foi enviado a nós por um de nossos clientes. Ele tinha certeza de que o analisador gerou avisos falsos.

Eugene recebeu a carta, a digitalizou brevemente e a enviou a Svyatoslav. Svyatoslav olhou pensativamente para a seção de código enviada pelo cliente e pensou: "O analisador pode estar tão descaradamente errado?" Portanto, ele foi fazer uma consulta com Andrei. Ele também verificou o site e decidiu: de fato, o analisador fornece falsos positivos.

O que você pode fazer, você precisa corrigi-lo. E somente quando Svyatoslav começou a fazer exemplos sintéticos para formalizar a tarefa como rastreador de bugs, ele percebeu o que estava acontecendo.

Os erros estavam realmente presentes no código, mas nenhum dos programadores conseguiu detectá-los. Honestamente, o autor deste artigo também não teve sucesso.

E isso apesar do fato de o analisador ter emitido avisos claramente para os lugares errados!

Você pode encontrar um erro tão astuto? Teste a si mesmo em busca de vigilância e atenção.


Aviso do PVS-Studio:
  • V560 Uma parte da expressão condicional é sempre falsa: (ch> = 0x0FF21). decodew.cpp 525
  • V560 Uma parte da expressão condicional é sempre verdadeira: (ch <= 0x0FF3A). decodew.cpp 525
  • V560 Uma parte da expressão condicional é sempre falsa: (ch> = 0x0FF41). decodew.cpp 525
  • V560 Uma parte da expressão condicional é sempre verdadeira: (ch <= 0x0FF5A). decodew.cpp 525

Se você for bem-sucedido, não terá meu respeito!

O erro está no fato de que o operador de negação lógica (!) Não se aplica a toda a condição, mas apenas à sua primeira subexpressão:

 !((ch >= 0x0FF10) && (ch <= 0x0FF19)) 

Se essa condição for atendida, o valor da variável ch estará no intervalo [0x0FF10 ... 0x0FF19]. Assim, as quatro comparações adicionais não fazem mais sentido: elas sempre serão verdadeiras ou falsas.

Para evitar esses erros, vale a pena seguir algumas regras. Primeiro, é muito conveniente e claro alinhar o código com uma tabela. Em segundo lugar, não sobrecarregue expressões com parênteses. Por exemplo, esse código pode ser reescrito assim:

 const bool isLetterOrDigit = (ch >= 0x0FF10 && ch <= 0x0FF19) // 0..9 || (ch >= 0x0FF21 && ch <= 0x0FF3A) // A..Z || (ch >= 0x0FF41 && ch <= 0x0FF5A); // a..z if (!isLetterOrDigit) 

Então, primeiro, o número de parênteses se torna muito menor e, segundo, aumenta a probabilidade de "pegar" um erro cometido pelos olhos.

E agora - cereja: estamos nos movendo para o primeiro lugar!

Primeiro lugar


Fonte: Sistema em estado de choque: erros interessantes nos códigos-fonte do lendário System Shock

Então, o finalista do top de hoje é um erro do lendário System Shock! Este jogo, lançado em 1994, tornou-se o progenitor e inspirador de jogos icônicos como Dead Space, BioShock e Deus Ex.

Mas primeiro tenho que admitir algo. O que vou mostrar agora não contém nenhum erro. De modo geral, nem sequer é um trecho de código, mas não resisti em não compartilhar isso com você!

O fato é que, no processo de análise do código fonte do jogo, minha colega Victoria encontrou muitos comentários interessantes. Aqui e ali, de repente, houve comentários engraçados e irônicos, e até versos:

 // I'll give you fish, I'll give you candy, // I'll give you, everything I have in my hand // that kid from the wrong side came over my house again, // decapitated all my dolls // and if you bore me, you lose your soul to me // - "Gepetto", Belly, _Star_ // And here, ladies and gentlemen, // is a celebration of C and C++ and their untamed passion... // ================== TerrainData terrain_info; // Now the actual stuff... // ======================= // this is all outrageously horrible, as we dont know what // we really need to deal with here // And if you thought the hack for papers was bad, // wait until you see the one for datas... - X // Returns whether or not in the humble opinion of the // sound system, the sample should be politely obliterated // out of existence // it's a wonderful world, with a lot of strange men // who are standing around, and they all wearing towels 

Para nossos leitores de língua russa, fiz uma tradução gratuita aproximada:
 //    ,    , //    ,       //      //      //     //     ,      // - "Gepetto", Belly, _Star_ //  ,   , //    C  C++     // ================== TerrainData terrain_info; //    ... // ======================= //    ,     , //         //    ,          //      ,      ... - X //       , //         //   ,     //   ,     

Esses comentários foram deixados pelos desenvolvedores do jogo no início dos anos 90 ... Aliás, Doug Church - o designer-chefe do System Shock - também estava escrevendo código. Quem sabe, talvez algum desses comentários tenha sido escrito por ele pessoalmente? Espero que os homens de toalhas não sejam da sua conta :)

Conclusão


Concluindo, quero agradecer aos meus colegas por procurarem novos erros e escrever artigos sobre eles. Obrigado pessoal! Sem você, este artigo não seria tão interessante.

Também quero falar um pouco sobre nossas realizações, porque durante um ano inteiro nos envolvemos mais do que apenas em busca de erros. Também desenvolvemos e aprimoramos o analisador, como resultado do qual ele sofreu alterações significativas.

Por exemplo, adicionamos suporte a vários novos compiladores e expandimos a lista de regras de diagnóstico. Também fornecemos suporte inicial aos padrões MISRA C e MISRA C ++ . A inovação mais importante e demorada foi o suporte de um novo idioma. Sim, agora podemos analisar o código Java ! E nós atualizamos o ícone:)

Também quero agradecer aos nossos leitores. Obrigado por ler nossos artigos e escrever para nós! Seu feedback é muito agradável e importante para nós.

Com isso, nossos 10 principais erros de C ++ para o ano de 2018 chegaram ao fim. De quais lugares você mais gostou e por quê? Você encontrou exemplos interessantes em 2018? Conte-nos nos comentários!

Até a próxima!



Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: George Gribkov. Os 10 principais erros de projetos C ++ encontrados em 2018

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


All Articles