Os 10 principais erros encontrados em projetos C ++ em 2019

Quadro 7

Outro ano está chegando ao fim, e é um momento perfeito para tomar uma xícara de café e reler as revisões de bugs coletados em projetos de código aberto ao longo deste ano. Isso levaria um bom tempo, é claro, por isso, preparamos este artigo para torná-lo mais fácil. Hoje, relembraremos os pontos escuros mais interessantes que encontramos nos projetos C / C ++ de código aberto em 2019.

Não. 10. Em que sistema operacional estamos rodando?


V1040 Possível erro de digitação na ortografia de um nome de macro predefinido. A macro '__MINGW32_' é semelhante a '__MINGW32__'. winapi.h 4112

#if !defined(__UNICODE_STRING_DEFINED) && defined(__MINGW32_) #define __UNICODE_STRING_DEFINED #endif 

Há um erro de digitação no nome da macro __MINGW32 _ (MINGW32 é realmente declarado por __MINGW32__). Em outras partes do projeto, a verificação está escrita corretamente:

Quadro 3


A propósito, esse bug não foi apenas o primeiro a ser descrito no artigo " CMake: o caso em que a qualidade do projeto é imperdoável ", mas o primeiro bug genuíno encontrado pelo diagnóstico da V1040 em um projeto de código aberto real (19 de agosto) , 2019).

Não. 9. quem é o primeiro?


V502 Talvez o operador '?:' Funcione de maneira diferente do esperado. O operador '?:' Tem uma prioridade mais baixa que o operador '=='. mir_parser.cpp 884

 enum Opcode : uint8 { kOpUndef, .... OP_intrinsiccall, OP_intrinsiccallassigned, .... kOpLast, }; bool MIRParser::ParseStmtIntrinsiccall(StmtNodePtr &stmt, bool isAssigned) { Opcode o = !isAssigned ? (....) : (....); auto *intrnCallNode = mod.CurFuncCodeMemPool()->New<IntrinsiccallNode>(....); lexer.NextToken(); if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) { intrnCallNode->SetIntrinsic(GetIntrinsicID(lexer.GetTokenKind())); } else { intrnCallNode->SetIntrinsic(static_cast<MIRIntrinsicID>(....)); } .... } 

Estamos interessados ​​na seguinte parte:

 if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) { .... } 

A precedência do operador '==' é maior que a do operador ternário (? :). Portanto, a expressão condicional é avaliada na ordem errada e é equivalente ao seguinte código:

 if ((o == !isAssigned) ? OP_intrinsiccall : OP_intrinsiccallassigned) { .... } 

Como as constantes OP_intrinsiccall e OP_intrinsiccallassigned são não nulas, a condição retornará verdadeira o tempo todo, o que significa que o corpo da ramificação else é um código inacessível.

Este bug foi descrito no artigo " Verificando o código-fonte aberto recentemente compilado pela Huawei pela Huawei ".

Não. 8. Operações bit a bit perigosas


V1046 Uso inseguro dos tipos bool 'e' int 'juntos na operação' & = '. GSLMultiRootFinder.h 175

 int AddFunction(const ROOT::Math::IMultiGenFunction & func) { ROOT::Math::IMultiGenFunction * f = func.Clone(); if (!f) return 0; fFunctions.push_back(f); return fFunctions.size(); } template<class FuncIterator> bool SetFunctionList( FuncIterator begin, FuncIterator end) { bool ret = true; for (FuncIterator itr = begin; itr != end; ++itr) { const ROOT::Math::IMultiGenFunction * f = *itr; ret &= AddFunction(*f); } return ret; } 

O código sugere que a função SetFunctionList percorre uma lista de iteradores. Se pelo menos um iterador for inválido, a função retornará false ou true, caso contrário.

No entanto, a função SetFunctionList pode retornar false, mesmo para iteradores válidos. Vamos descobrir o porquê . A função AddFunction retorna o número de iteradores válidos na lista fFunctions . Ou seja, adicionar iteradores não nulos fará com que a lista cresça gradualmente em tamanho: 1, 2, 3, 4 e assim por diante. É aqui que o bug entra em jogo:

 ret &= AddFunction(*f); 

Como a função retorna um valor do tipo int em vez de bool , a operação '& =' retornará false para valores pares porque o bit menos significativo de um número par é sempre definido como zero. É assim que um bug sutil pode quebrar o valor de retorno de SetFunctionsList, mesmo quando seus argumentos são válidos.

Se você estivesse lendo o trecho com atenção (e não estava?), Você poderia ter notado que ele veio do projeto ROOT. Sim, também verificamos: " Analisando o código de raiz, estrutura de análise de dados científicos ".

Não. 7. Variáveis ​​misturadas


V1001 [CWE-563] A variável 'Mode' está atribuída, mas não é usada no final da função. SIModeRegister.cpp 48

 struct Status { unsigned Mask; unsigned Mode; Status() : Mask(0), Mode(0){}; Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) { Mode &= Mask; }; .... }; 

É muito perigoso usar os mesmos nomes para argumentos de função e para membros da classe, porque você corre o risco de confundi-los. E foi exatamente o que aconteceu aqui. A seguinte expressão não faz sentido:

 Mode &= Mask; 

O argumento da função muda, e é isso. Este argumento não é usado de forma alguma depois disso. O que o programador realmente queria escrever era provavelmente o seguinte:

 Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) { this->Mode &= Mask; }; 

Este bug foi encontrado no LLVM . Temos uma tradição para verificar este projeto de vez em quando. Este ano, verificamos mais uma vez.

Não. 6. C ++ tem suas próprias leis


Esse bug decorre do fato de que as regras C ++ nem sempre seguem regras matemáticas ou "senso comum". Veja o pequeno trecho abaixo e tente encontrar o bug você mesmo.

V709 Comparação suspeita encontrada: 'f0 == f1 == m_fractureBodies.size ()'. Lembre-se de que 'a == b == c' não é igual a 'a == b && b == c'. btFractureDynamicsWorld.cpp 483

 btAlignedObjectArray<btFractureBody*> m_fractureBodies; void btFractureDynamicsWorld::fractureCallback() { for (int i = 0; i < numManifolds; i++) { .... int f0 = m_fractureBodies.findLinearSearch(....); int f1 = m_fractureBodies.findLinearSearch(....); if (f0 == f1 == m_fractureBodies.size()) continue; .... } .... } 

A condição parece estar verificando se f0 é igual a f1 e é igual ao número de elementos em m_fractureBodies . Provavelmente foi feito para verificar se f0 e f1 estão localizados no final da matriz m_fractureBodies, pois eles contêm uma posição de objeto encontrada pelo método findLinearSearch () . Mas, na realidade, essa expressão condicional verifica se f0 é igual a f1 e se m_fractureBodies.size () é igual ao resultado da expressão f0 == f1 . Ou seja, o terceiro operando aqui é verificado contra 0 ou 1.

Isso é um bom bug! E, felizmente, bem raro. Até agora, vimos isso apenas em três projetos de código aberto e, curiosamente, todos os três eram motores de jogos. Este não é o único bug encontrado no Bullet; os mais interessantes foram descritos no artigo " PVS-Studio olhou para o Bullet Engine do Red Dead Redemption ".

Não. 5. O que há no final da linha?


Este é fácil se você souber um detalhe complicado.

O V739 EOF não deve ser comparado com um valor do tipo 'char'. O 'ch' deve ser do tipo 'int'. json.cpp 762

 void JsonIn::skip_separator() { signed char ch; .... if (ch == ',') { if( ate_separator ) { .... } .... } else if (ch == EOF) { .... } 

Esse é um daqueles erros que você não consegue identificar facilmente se não souber que o EOF está definido como -1. Portanto, se você tentar compará-lo com uma variável do tipo char assinado , a condição quase sempre será falsa . A única exceção é o caractere codificado como 0xFF (255). Quando comparado com EOF , esse caractere se transformará em -1, tornando a condição verdadeira.

Muitos bugs no Top 10 deste ano foram encontrados em softwares de jogos de computador: mecanismos ou jogos de código aberto. Como você já adivinhou, esse também veio dessa área. Mais erros são descritos no artigo " Cataclysm Dark Days Ahead: Static Analysis and Roguelike Games ".

Não. 4. A constante mágica Pi


V624 Provavelmente, há uma impressão incorreta na constante '3.141592538'. Considere usar a constante M_PI de <math.h>. PhysicsClientC_API.cpp 4109

 B3_SHARED_API void b3ComputeProjectionMatrixFOV(float fov, ....) { float yScale = 1.0 / tan((3.141592538 / 180.0) * fov / 2); .... } 


Há um pequeno erro de digitação no número Pi (3,141592653 ...): o número "6" está ausente na 7ª casa decimal.

Quadro 4
Um dígito decimal incorreto de um milionésimo dificilmente causaria danos visíveis, mas ainda é melhor usar constantes existentes de bibliotecas, cuja correção é garantida. O número Pi, por exemplo, é representado pela constante M_PI do cabeçalho math.h.

Você já leu sobre esse bug no artigo " PVS-Studio olhou para o Bullet Engine do Red Dead Redemption ", onde foi colocado em sexto. Se você ainda não leu, esta é sua última chance.

Uma pequena diversão


Estamos nos aproximando dos 3 principais bugs mais interessantes. Como você já deve ter notado, estou classificando os bugs não pelo impacto, mas pelo esforço que um revisor humano precisa para encontrá-los. Afinal, a vantagem da análise estática sobre as revisões de código é basicamente a incapacidade das ferramentas de software de se cansarem ou esquecerem as coisas. :)

Agora, vamos ver o que temos em nossos 3 principais.

Quadro 8


Não. 3. Uma exceção indescritível


As classes V702 sempre devem ser derivadas de std :: exception (e similares) como 'public' (nenhuma palavra-chave foi especificada, então o compilador o padroniza como 'private'). CalcManager CalcException.h 4

 class CalcException : std::exception { public: CalcException(HRESULT hr) { m_hr = hr; } HRESULT GetException() { return m_hr; } private: HRESULT m_hr; }; 

O analisador detectou uma classe derivada da classe std :: exception usando o modificador privado (que é usado por padrão, se não for especificado de outra forma). O problema com esse código é que uma tentativa de capturar uma std :: exception genérica fará com que o programa perca uma exceção do tipo CalcException . Esse comportamento decorre do fato de que a herança privada proíbe a conversão implícita de tipos.

Você definitivamente não gostaria de ver o seu programa travar devido a um modificador público ausente. A propósito, aposto que você usou esse aplicativo pelo menos uma vez na vida porque é a boa e velha calculadora do Windows , que também verificamos no início deste ano.

Não. 2. Tags HTML não fechadas


V735 Possivelmente um HTML incorreto. A tag de fechamento "</body>" foi encontrada, enquanto a tag "</div>" era esperada. book.cpp 127

 static QString makeAlgebraLogBaseConversionPage() { return BEGIN INDEX_LINK TITLE(Book::tr("Logarithmic Base Conversion")) FORMULA(y = log(x) / log(a), log<sub>a</sub>x = log(x) / log(a)) END; } 

Como geralmente acontece, o código-fonte C / C ++ não diz muito por si só, então vamos dar uma olhada no código pré-processado gerado a partir do snippet acima:

Quadro 6


O analisador encontrou uma tag <div> não fechada. Existem muitos fragmentos de código html aqui, então os autores precisam revisá-lo.

Surpreso, podemos diagnosticar esse tipo de erro? Fiquei impressionado também quando vi isso pela primeira vez. Então, sim, sabemos algo sobre a análise de código html. Bem, apenas se estiver dentro do código C ++. :)

Este bug não é apenas o segundo colocado, mas é uma segunda calculadora em nossa lista dos 10 melhores. Para saber quais outros erros encontramos neste projeto, consulte o artigo " Seguindo os passos das calculadoras: SpeedCrunch ".

Não. 1. Funções padrão indescritíveis


Aqui está o bug colocado em primeiro lugar. Este é um bug impressionantemente estranho, que conseguiu passar pela revisão de código.

Tente encontrar você mesmo:

 static int EatWhitespace (FILE * InFile) /* ----------------------------------------------------------------------- ** * Scan past whitespace (see ctype(3C)) and return the first non-whitespace * character, or newline, or EOF. * * Input: InFile - Input source. * * Output: The next non-whitespace character in the input stream. * * Notes: Because the config files use a line-oriented grammar, we * explicitly exclude the newline character from the list of * whitespace characters. * - Note that both EOF (-1) and the nul character ('\0') are * considered end-of-file markers. * * ----------------------------------------------------------------------- ** */ { int c; for (c = getc (InFile); isspace (c) && ('\n' != c); c = getc (InFile)) ; return (c); } /* EatWhitespace */ 

Agora vamos ver o que o analisador tem a dizer:

V560 Uma parte da expressão condicional é sempre verdadeira: ('\ n'! = C). params.c 136.

Estranho, não é? Vamos dar uma olhada em outro ponto curioso, mas em um arquivo diferente (charset.h):

 #ifdef isspace #undef isspace #endif .... #define isspace(c) ((c)==' ' || (c) == '\t') 

Hum, isso é realmente estranho ... Então, se a variável c for igual a '\ n', a função aparentemente inofensiva isspace (c) retornará falsa , impedindo a execução da segunda parte da verificação devido à avaliação de curto-circuito. E se isspace (c) for executado, a variável c será igual a '' ou '\ t', que obviamente não é igual a '\ n' .

Você pode argumentar que essa macro é semelhante a #define true false e código como esse nunca passaria por uma revisão de código. Mas esse trecho específico foi - e estava no repositório esperando para ser descoberto.

Para comentários mais detalhados sobre esse bug, consulte o artigo " Deseja interpretar um detetive? Encontre o bug em uma função do Midnight Commander ".

Conclusão


Quadro 9


Encontramos toneladas de bugs durante este ano. Esses eram erros comuns de copiar e colar, constantes imprecisas, tags não fechadas e muitos outros defeitos. Mas nosso analisador está evoluindo e aprendendo a diagnosticar cada vez mais tipos de problemas, portanto, certamente não iremos diminuir a velocidade e publicaremos novos artigos sobre bugs encontrados em projetos com a mesma regularidade de antes.

Caso você não tenha lido nossos artigos antes, todos esses erros foram encontrados usando o analisador estático PVS-Studio, do qual você pode baixar e experimentar seus próprios projetos. Ele detecta bugs em programas escritos em C, C ++, C # e Java.

Você finalmente chegou à linha de chegada! Se você perdeu os dois primeiros níveis, sugiro que você aproveite a oportunidade e complete esses níveis conosco: C # e Java .

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


All Articles