PVS-Studio olhou para o Bullet Engine do Red Dead Redemption

Quadro 4

Atualmente, não há necessidade de implementar a física dos objetos do zero para o desenvolvimento de jogos, porque existem muitas bibliotecas para esse fim. O Bullet foi usado ativamente em muitos jogos AAA, projetos de realidade virtual, várias simulações e aprendizado de máquina. E ainda é usado, sendo, por exemplo, um dos motores Red Dead Redemption e Red Dead Redemption 2. Então, por que não verificar o Bullet com o PVS-Studio para ver quais erros a análise estática pode detectar em um projeto de simulação de física em larga escala.

Esta biblioteca é distribuída gratuitamente , para que todos possam usá-la em seus próprios projetos, se assim o desejarem. Além do Red Dead Redemption, esse mecanismo de física também é usado na indústria cinematográfica para criar efeitos especiais. Por exemplo, foi usado nas filmagens de "Sherlock Holmes" de Guy Ritchie para calcular colisões.

Se esta é a primeira vez que você encontra um artigo em que o PVS-Studio verifica projetos, farei uma pequena digressão. O PVS-Studio é um analisador de código estático que ajuda a encontrar erros, defeitos e possíveis vulnerabilidades no código fonte dos programas Java em C, C ++, C #. A análise estática é um tipo de processo automatizado de revisão de código.

Aquecer


Exemplo 1:

Vamos começar com um erro engraçado:

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

Um pequeno erro de digitação no valor Pi (3,141592653 ...). Falta o sétimo dígito na parte fracionária - ele deve ser igual a 6.
Quadro 1

Talvez um erro na décima milionésima fração após o ponto decimal não leve a consequências significativas, mas você deve usar as constantes da biblioteca já existentes que não têm erros de digitação. Há uma constante M_PI para o número Pi do cabeçalho math.h.

Copiar e colar


Exemplo 2:

Às vezes, o analisador permite encontrar o erro indiretamente. Por exemplo, três argumentos relacionados halfExtentsX, halfExtentsY, halfExtentsZ são passados ​​para a função aqui, mas o último não é usado em nenhum lugar da função. Você pode perceber que a variável halfExtentsY é usada duas vezes ao chamar o método addVertex . Talvez seja um erro de copipaste e o argumento esquecido deve ser usado aqui.

V751 O parâmetro 'halfExtentsZ' não é usado dentro do corpo da função. TinyRenderer.cpp 375

 void TinyRenderObjectData::createCube(float halfExtentsX, float halfExtentsY, float halfExtentsZ, ....) { .... m_model->addVertex(halfExtentsX * cube_vertices_textured[i * 9], halfExtentsY * cube_vertices_textured[i * 9 + 1], halfExtentsY * cube_vertices_textured[i * 9 + 2], cube_vertices_textured[i * 9 + 4], ....); .... } 

Exemplo 3:

O analisador também detectou o seguinte fragmento interessante e eu o mostrarei primeiro na forma inicial.

Quadro 11

Veja esta linha looooooooooong?

Quadro 12

É muito estranho que o programador tenha decidido escrever uma condição tão longa em uma linha. Mas não é de surpreender que um erro tenha provavelmente caído nele.

O analisador gerou os seguintes avisos nesta linha.

V501 Existem sub-expressões idênticas 'rotmat.Column1 (). Norm () <1.0001' à esquerda e à direita do operador '&&'. LinearR4.cpp 351

V501 Existem sub-expressões idênticas '0.9999 <rotmat.Column1 (). Norm ()' à esquerda e à direita do operador '&&'. LinearR4.cpp 351

Se escrevermos tudo em um formulário "tabular" claro, podemos ver que as mesmas verificações se aplicam à Coluna1 . As duas últimas comparações mostram que há Coluna1 e Coluna2 . Provavelmente, a terceira e quarta comparações deveriam ter verificado o valor da Coluna2 .

  Column1().Norm() < 1.0001 && 0.9999 < Column1().Norm() && Column1().Norm() < 1.0001 && 0.9999 < Column1().Norm() &&(Column1() ^ Column2()) < 0.001 && (Column1() ^ Column2()) > -0.001 

Nesta forma, as mesmas comparações se tornam muito mais visíveis.

Exemplo 4:

Erro do mesmo tipo:

V501 Existem subexpressões idênticas 'cs.m_fJacCoeffInv [0] == 0' à esquerda e à direita do operador '&&'. b3CpuRigidBodyPipeline.cpp 169

 float m_fJacCoeffInv[2]; static inline void b3SolveFriction(b3ContactConstraint4& cs, ....) { if (cs.m_fJacCoeffInv[0] == 0 && cs.m_fJacCoeffInv[0] == 0) { return; } .... } 

Nesse caso, um e o mesmo elemento da matriz é verificado duas vezes. Provavelmente, a condição deve ter se parecido com isto: cs.m_fJacCoeffInv [0] == 0 && cs.m_fJacCoeffInv [1] == 0 . Este é um exemplo clássico de um erro de copiar e colar.

Exemplo 5:

Também foi descoberto que havia esse defeito:

V517 O uso do padrão 'if (A) {...} else if (A) {...}' foi detectado. Há uma probabilidade de presença de erro lógico. Verifique as linhas: 79, 112. main.cpp 79

 int main(int argc, char* argv[]) { .... while (serviceResult > 0) { serviceResult = enet_host_service(client, &event, 0); if (serviceResult > 0) { .... } else if (serviceResult > 0) { puts("Error with servicing the client"); exit(EXIT_FAILURE); } .... } .... } 

A função enet_host_service , cujo resultado é atribuído a serviceResult , retorna 1 em caso de conclusão bem-sucedida e -1 em caso de falha. Provavelmente, a ramificação else if deveria ter reagido ao valor negativo de serviceResult , mas a condição de verificação foi duplicada. Provavelmente, também é um erro de copiar e colar.

Há um aviso semelhante do analisador, mas não faz sentido analisá-lo mais de perto neste artigo.

V517 O uso do padrão 'if (A) {...} else if (A) {...}' foi detectado. Há uma probabilidade de presença de erro lógico. Verifique as linhas: 151, 190. PhysicsClientUDP.cpp 151

Por cima: excedendo os limites da matriz


Exemplo 6:

Um dos erros desagradáveis ​​para procurar é a saturação da matriz. Esse erro geralmente ocorre devido a uma indexação complexa em um loop.

Aqui, na condição de loop, o limite superior da variável dofIndex é 128 e o dof ' é 4, inclusive. Mas m_desiredState também contém apenas 128 itens. Como resultado, o índice [dofIndex + dof] pode causar uma saturação da matriz.

A saturação da matriz V557 é possível. O valor do índice 'dofIndex + dof' pode chegar a 130. PhysicsClientC_API.cpp 968

 #define MAX_DEGREE_OF_FREEDOM 128 double m_desiredState[MAX_DEGREE_OF_FREEDOM]; B3_SHARED_API int b3JointControl(int dofIndex, double* forces, int dofCount, ....) { .... if ( (dofIndex >= 0) && (dofIndex < MAX_DEGREE_OF_FREEDOM ) && dofCount >= 0 && dofCount <= 4) { for (int dof = 0; dof < dofCount; dof++) { command->m_sendState.m_desiredState[dofIndex+dof] = forces[dof]; .... } } .... } 

Exemplo 7:

Um erro semelhante, mas agora é causado pela soma não ao indexar uma matriz, mas em uma condição. Se o arquivo tiver um nome com tamanho máximo, o zero do terminal será gravado fora da matriz ( Erro Off-by-one ). Obviamente, a variável len será igual a MAX_FILENAME_LENGTH apenas em casos excepcionais, mas não elimina o erro, mas simplesmente o torna raro.

A saturação da matriz V557 é possível. O valor do índice 'len' pode chegar a 1024. PhysicsClientC_API.cpp 5223

 #define MAX_FILENAME_LENGTH MAX_URDF_FILENAME_LENGTH 1024 struct b3Profile { char m_name[MAX_FILENAME_LENGTH]; int m_durationInMicroSeconds; }; int len = strlen(name); if (len >= 0 && len < (MAX_FILENAME_LENGTH + 1)) { command->m_type = CMD_PROFILE_TIMING; strcpy(command->m_profile.m_name, name); command->m_profile.m_name[len] = 0; } 

Meça uma vez e corte sete vezes


Exemplo 8:

Nos casos em que você precisa usar o resultado do trabalho de alguma função várias vezes ou usar uma variável que exija passar por toda a cadeia de chamadas para obter acesso, use variáveis ​​temporárias para otimização e melhor legibilidade do código. O analisador encontrou mais de 100 lugares no código onde você pode fazer essa correção.

V807 desempenho reduzido. Considere criar um ponteiro para evitar o uso da expressão 'm_app-> m_renderer-> getActiveCamera ()' repetidamente. InverseKinematicsExample.cpp 315

 virtual void resetCamera() { .... if (....) { m_app->m_renderer->getActiveCamera()->setCameraDistance(dist); m_app->m_renderer->getActiveCamera()->setCameraPitch(pitch); m_app->m_renderer->getActiveCamera()->setCameraYaw(yaw); m_app->m_renderer->getActiveCamera()->setCameraPosition(....); } } 

A mesma cadeia de chamadas é usada muitas vezes aqui e pode ser substituída por um único ponteiro.

Exemplo 9:

V810 desempenho reduzido. A função 'btCos (euler_out.pitch)' foi chamada várias vezes com argumentos idênticos. O resultado deve ser salvo em uma variável temporária, que pode ser usada ao chamar a função 'btAtan2'. btMatrix3x3.h 576

V810 desempenho reduzido. A função 'btCos (euler_out2.pitch)' foi chamada várias vezes com argumentos idênticos. O resultado deve ser salvo em uma variável temporária, que pode ser usada ao chamar a função 'btAtan2'. btMatrix3x3.h 578

 void getEulerZYX(....) const { .... if (....) { .... } else { .... euler_out.roll = btAtan2(m_el[2].y() / btCos(euler_out.pitch), m_el[2].z() / btCos(euler_out.pitch)); euler_out2.roll = btAtan2(m_el[2].y() / btCos(euler_out2.pitch), m_el[2].z() / btCos(euler_out2.pitch)); euler_out.yaw = btAtan2(m_el[1].x() / btCos(euler_out.pitch), m_el[0].x() / btCos(euler_out.pitch)); euler_out2.yaw = btAtan2(m_el[1].x() / btCos(euler_out2.pitch), m_el[0].x() / btCos(euler_out2.pitch)); } .... } 

Nesse caso, você pode criar duas variáveis ​​e salvar os valores retornados pela função btCos para euler_out.pitch e euler_out2.pitch nelas, em vez de chamar a função quatro vezes para cada argumento.

Vazamento


Exemplo 10:

Muitos erros do seguinte tipo foram detectados no projeto:

V773 O escopo de visibilidade do ponteiro 'importador' foi encerrado sem liberar a memória. É possível um vazamento de memória. SerializeSetup.cpp 94

 void SerializeSetup::initPhysics() { .... btBulletWorldImporter* importer = new btBulletWorldImporter(m_dynamicsWorld); .... fclose(file); m_guiHelper->autogenerateGraphicsObjects(m_dynamicsWorld); } 

A memória não foi liberada do ponteiro do importador aqui. Isso pode resultar em vazamento de memória. E para o mecanismo físico, pode ser uma tendência ruim. Para evitar vazamentos, basta adicionar o importador de exclusão depois que a variável se torna desnecessária. Mas, é claro, é melhor usar ponteiros inteligentes.

C ++ vive por seu próprio código


Exemplo 11:

O próximo erro aparece no código porque as regras do C ++ nem sempre coincidem com as regras matemáticas ou com o "senso comum". Você notará onde esse pequeno fragmento de código contém um erro?

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

O analisador gera o seguinte aviso:

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

Parece que a condição verifica se f0 é igual a f1 e é igual ao número de itens em m_fractureBodies . Parece que essa comparação deveria ter verificado se f0 e f1 estão localizados no final da matriz m_fractureBodies , pois contêm a posição do objeto encontrada pelo método findLinearSearch () . Mas, na verdade, essa expressão se transforma em uma verificação para ver se f0 e f1 são iguais a m_fractureBodies.size () e, em seguida, uma verificação para ver se m_fractureBodies.size () é igual ao resultado f0 == f1 . Como resultado, o terceiro operando aqui é comparado a 0 ou 1.

Belo erro! E, felizmente, bastante raro. Até agora, só o encontramos em dois projetos de código aberto, e é interessante que todos eles fossem motores de jogos.

Exemplo 12:

Ao trabalhar com strings, geralmente é melhor usar os recursos fornecidos pela classe de strings . Portanto, para os próximos dois casos, é melhor substituir strlen (MyStr.c_str ()) e val = "" por MyStr.length () e val.clear () , respectivamente.

V806 desempenho reduzido. A expressão do tipo strlen (MyStr.c_str ()) pode ser reescrita como MyStr.length (). RobotLoggingUtil.cpp 213

 FILE* createMinitaurLogFile(const char* fileName, std::string& structTypes, ....) { FILE* f = fopen(fileName, "wb"); if (f) { .... fwrite(structTypes.c_str(), strlen(structTypes.c_str()), 1, f); .... } .... } 

V815 Desempenho Diminuído. Considere substituir a expressão 'val = ""' por 'val.clear ()'. b3CommandLineArgs.h 40

 void addArgs(int argc, char **argv) { .... std::string val; .... val = ""; .... } 

Houve outros avisos, mas acho que podemos parar por aqui. Como você vê, a análise de código estático pode detectar uma ampla gama de vários erros.

É interessante ler sobre verificações únicas de projetos, mas não é a maneira correta de usar analisadores de código estático. E falaremos sobre isso abaixo.

Erros encontrados antes de nós


Foi interessante tentar encontrar bugs ou defeitos que já foram corrigidos, mas que um analisador estático pôde detectar à luz do artigo recente " Erros que não são encontrados pela análise estática de código porque não está sendo usada ".
Quadro 2

Não havia muitas solicitações pull no repositório e muitas delas estão relacionadas à lógica interna do mecanismo. Mas também houve erros que o analisador pôde detectar.

Exemplo 13:

 char m_deviceExtensions[B3_MAX_STRING_LENGTH]; void b3OpenCLUtils_printDeviceInfo(cl_device_id device) { b3OpenCLDeviceInfo info; b3OpenCLUtils::getDeviceInfo(device, &info); .... if (info.m_deviceExtensions != 0) { .... } } 

O comentário para a solicitação diz que você precisava verificar a matriz pelo fato de ela não estar vazia, mas, em vez disso, foi executada uma verificação sem ponteiro, que sempre retornava verdadeira. É o que o aviso do PVS-Studio sobre a verificação original informa:

V600 Considere inspecionar a condição. O ponteiro 'info.m_deviceExtensions' nem sempre é igual a NULL. b3OpenCLUtils.cpp 551

Exemplo 14:

Você pode descobrir qual é o problema da próxima função?

 inline void Matrix4x4::SetIdentity() { m12 = m13 = m14 = m21 = m23 = m24 = m13 = m23 = m41 = m42 = m43 = 0.0; m11 = m22 = m33 = m44 = 1.0; } 

O analisador gera os seguintes avisos:

V570 O mesmo valor é atribuído duas vezes à variável 'm23'. LinearR4.h 627

V570 O mesmo valor é atribuído duas vezes à variável 'm13'. LinearR4.h 627

Tarefas repetidas nessa forma de gravação são difíceis de rastrear a olho nu e, como resultado, alguns dos elementos da matriz não obtiveram o valor inicial. Este erro foi corrigido pela forma tabular de gravação de atribuição:

 m12 = m13 = m14 = m21 = m23 = m24 = m31 = m32 = m34 = m41 = m42 = m43 = 0.0; 

Exemplo 15:

O erro a seguir em uma das condições da função btSoftBody :: addAeroForceToNode () levou a um erro óbvio. De acordo com o comentário na solicitação de extração, as forças foram aplicadas aos objetos do lado errado.

 struct eAeroModel { enum _ { V_Point, V_TwoSided, .... END }; }; void btSoftBody::addAeroForceToNode(....) { .... if (....) { if (btSoftBody::eAeroModel::V_TwoSided) { .... } .... } .... } 

O PVS-Studio também pode encontrar esse erro e gerar o seguinte aviso:

V768 A constante de enumeração 'V_TwoSided' é usada como uma variável de um tipo booleano. btSoftBody.cpp 542

O cheque fixo fica assim:

 if (m_cfg.aeromodel == btSoftBody::eAeroModel::V_TwoSided) { .... } 

Em vez da equivalência da propriedade de um objeto a um dos enumeradores, o próprio enumerador V_TwoSided foi verificado.

É claro que eu não olhei para todas as solicitações de recebimento, porque esse não era o ponto. Eu só queria mostrar que o uso regular de um analisador de código estático pode detectar erros desde o início. Este é o caminho certo para usar a análise de código estático. A análise estática deve ser incorporada ao processo do DevOps e ser o principal filtro de bug. Tudo isso está bem descrito no artigo " Introduzir análise estática no processo, não basta procurar por erros ".

Conclusão


Quadro 6

A julgar por algumas solicitações de recebimento, às vezes um projeto é verificado através de várias ferramentas de análise de código, mas as correções são feitas não gradualmente, mas em grupos e com grandes intervalos. Em algumas solicitações, o comentário indica que as alterações foram feitas apenas para suprimir avisos. Essa abordagem para usar a análise reduz significativamente sua utilidade, pois são as verificações regulares do projeto que permitem corrigir erros imediatamente, em vez de aguardar o aparecimento de erros explícitos.

Siga-nos e assine nossas contas e canais de mídia social: Instagram , Twitter , Facebook , Telegram . Adoraríamos estar com você onde quer que você esteja e mantê-lo informado.

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


All Articles