PVS-Studio investigó el motor de bala del Red Dead Redemption

Hoy en día no hay necesidad de implementar la física de los objetos desde cero para el desarrollo del juego porque hay muchas bibliotecas para este propósito. Bullet se utilizó activamente en muchos juegos AAA, proyectos de realidad virtual, diversas simulaciones y aprendizaje automático. Y todavía se usa, siendo, por ejemplo, uno de los motores Red Dead Redemption y Red Dead Redemption 2. Entonces, ¿por qué no consultar Bullet con PVS-Studio para ver qué errores puede detectar el análisis estático en un proyecto de simulación de física a gran escala?

Esta biblioteca se distribuye libremente , por lo que todos pueden usarla en sus propios proyectos si lo desean. Además de Red Dead Redemption, este motor de física también se usa en la industria del cine para crear efectos especiales. Por ejemplo, se utilizó en el rodaje de "Sherlock Holmes" de Guy Ritchie para calcular colisiones.

Si es la primera vez que se encuentra con un artículo en el que PVS-Studio verifica proyectos, haré una pequeña digresión. PVS-Studio es un analizador de código estático que le ayuda a encontrar errores, defectos y vulnerabilidades potenciales en el código fuente de C, C ++, C #, programas Java. El análisis estático es un tipo de proceso de revisión de código automatizado.

Calentar

Ejemplo 1:

Comencemos con un error gracioso:

V624 Probablemente haya un error de imprenta en la constante '3.141592538'. Considere usar la 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); .... } 

Un pequeño error tipográfico en el valor Pi (3.141592653 ...). Falta el séptimo dígito en la parte fraccionaria: debe ser igual a 6.
Tal vez, un error en la fracción diez millonésima después del punto decimal no tendrá consecuencias significativas, pero aún así debe usar las constantes de biblioteca ya existentes que no tienen errores tipográficos. Hay una constante M_PI para el número Pi del encabezado math.h.

Copiar y pegar

Ejemplo 2

Algunas veces el analizador le permite encontrar el error indirectamente. Por ejemplo, tres argumentos relacionados halfExtentsX, halfExtentsY, halfExtentsZ se pasan a la función aquí, pero este último no se utiliza en ninguna parte de la función. Puede observar que la variable halfExtentsY se usa dos veces al llamar al método addVertex . Entonces, tal vez sea un error de copia y el argumento olvidado debería usarse aquí.

El parámetro V751 'halfExtentsZ' no se usa dentro del cuerpo de la función. 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], ....); .... } 

Ejemplo 3

El analizador también detectó el siguiente fragmento interesante y lo mostraré primero en la forma inicial.


¿Ves esta línea muuuuuuy larga?


Es muy extraño que el programador haya decidido escribir una condición tan larga en una línea. Pero no es sorprendente que lo más probable es que se haya introducido un error.

El analizador generó las siguientes advertencias en esta línea.

V501 Hay subexpresiones idénticas 'rotmat.Column1 (). Norm () <1.0001' a la izquierda y a la derecha del operador '&&'. LinearR4.cpp 351

V501 Hay subexpresiones idénticas '0.9999 <rotmat.Column1 (). Norm ()' a la izquierda y a la derecha del operador '&&'. LinearR4.cpp 351

Si lo escribimos todo en forma clara "tabular", podemos ver que todas las mismas verificaciones se aplican a la Columna1 . Las dos últimas comparaciones muestran que hay Column1 y Column2 . Lo más probable es que la tercera y cuarta comparaciones deberían haber verificado el valor de Column2 .

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

De esta forma, las mismas comparaciones se vuelven mucho más notables.

Ejemplo 4

Error del mismo tipo:

V501 Hay subexpresiones idénticas 'cs.m_fJacCoeffInv [0] == 0' a la izquierda y a la derecha del 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; } .... } 

En este caso, el mismo elemento de matriz se verifica dos veces. Lo más probable es que la condición se haya visto así: cs.m_fJacCoeffInv [0] == 0 && cs.m_fJacCoeffInv [1] == 0 . Este es un ejemplo clásico de un error de copiar y pegar.

Ejemplo 5:

También se descubrió que había tal defecto:

V517 El uso del patrón 'if (A) {...} else if (A) {...}' fue detectado. Hay una probabilidad de presencia de error lógico. Líneas de verificación: 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); } .... } .... } 

La función enet_host_service , cuyo resultado se asigna a serviceResult , devuelve 1 en caso de finalización exitosa y -1 en caso de falla. Lo más probable es que, si no, la rama debería haber reaccionado al valor negativo de serviceResult , pero la condición de verificación se duplicó. Probablemente también sea un error de copiar y pegar.

Hay una advertencia similar del analizador, pero no tiene sentido mirarlo más de cerca en este artículo.

V517 El uso del patrón 'if (A) {...} else if (A) {...}' fue detectado. Hay una probabilidad de presencia de error lógico. Líneas de verificación: 151, 190. PhysicsClientUDP.cpp 151

En la parte superior: exceder los límites de la matriz

Ejemplo 6

Uno de los errores desagradables para buscar es el desbordamiento de la matriz. Este error a menudo ocurre debido a una indexación compleja en un bucle.

Aquí, en la condición de bucle, el límite superior de la variable dofIndex es 128 y el dof es 4 inclusive. Pero m_desiredState también contiene solo 128 elementos. Como resultado, el índice [dofIndex + dof] puede causar un desbordamiento de la matriz.

V557 Array overrun es posible. El valor del índice 'dofIndex + dof' podría alcanzar 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]; .... } } .... } 

Ejemplo 7

Un error similar, pero ahora se produce al resumir no al indexar una matriz, sino en una condición. Si el archivo tiene un nombre con longitud máxima, el terminal cero se escribirá fuera de la matriz ( Error Off-by-one ). Por supuesto, la variable len será igual a MAX_FILENAME_LENGTH solo en casos excepcionales, pero no elimina el error sino que simplemente lo hace raro.

V557 Array overrun es posible. El valor del índice 'len' podría llegar 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; } 

Mídelo una vez, córtalo siete veces

Ejemplo 8

En los casos en que necesite usar el resultado del trabajo de alguna función muchas veces o usar una variable que requiera pasar por toda la cadena de llamadas para obtener acceso, debe usar variables temporales para la optimización y una mejor legibilidad del código. El analizador ha encontrado más de 100 lugares en el código donde puede realizar dicha corrección.

V807 Disminución del rendimiento. Considere crear un puntero para evitar usar la expresión '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(....); } } 

Aquí se usa la misma cadena de llamadas muchas veces y se puede reemplazar con un solo puntero.

Ejemplo 9

V810 Disminución del rendimiento. La función 'btCos (euler_out.pitch)' se llamó varias veces con argumentos idénticos. El resultado posiblemente debería guardarse en una variable temporal, que luego podría usarse mientras se llama a la función 'btAtan2'. btMatrix3x3.h 576

V810 Disminución del rendimiento. La función 'btCos (euler_out2.pitch)' se llamó varias veces con argumentos idénticos. El resultado posiblemente debería guardarse en una variable temporal, que luego podría usarse mientras se llama a la función '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)); } .... } 

En este caso, puede crear dos variables y guardar los valores devueltos por la función btCos para euler_out.pitch y euler_out2.pitch en lugar de llamar a la función cuatro veces para cada argumento.

Fuga

Ejemplo 10

Se detectaron muchos errores del siguiente tipo en el proyecto:

V773 El alcance de visibilidad del puntero 'importador' se salió sin liberar la memoria. Una pérdida de memoria es posible. SerializeSetup.cpp 94

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

La memoria no se ha liberado del puntero del importador aquí. Esto puede provocar una pérdida de memoria. Y para el motor físico puede ser una mala tendencia. Para evitar una fuga, es suficiente agregar un importador de eliminación después de que la variable se vuelva innecesaria. Pero, por supuesto, es mejor usar punteros inteligentes.

C ++ vive por su propio código

Ejemplo 11

El siguiente error aparece en el código porque las reglas de C ++ no siempre coinciden con las reglas matemáticas o el "sentido común". ¿Notarás dónde este pequeño fragmento de código contiene un error?

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

El analizador genera la siguiente advertencia:

V709 Comparación sospechosa encontrada: 'f0 == f1 == m_fractureBodies.size ()'. Recuerde que 'a == b == c' no es igual a 'a == b && b == c'. btFractureDynamicsWorld.cpp 483

Parece que la condición comprueba que f0 es igual a f1 y es igual al número de elementos en m_fractureBodies . Parece que esta comparación debería haber verificado si f0 y f1 se encuentran al final de la matriz m_fractureBodies , ya que contienen la posición del objeto encontrada por el método findLinearSearch () . Pero, de hecho, esta expresión se convierte en una comprobación para ver si f0 y f1 son iguales a m_fractureBodies.size () y luego una comprobación para ver si m_fractureBodies.size () es igual al resultado f0 == f1 . Como resultado, el tercer operando aquí se compara con 0 o 1.

Hermoso error! Y, afortunadamente, bastante raro. Hasta ahora, solo lo hemos cumplido en dos proyectos de código abierto, y es interesante que todos ellos fueran motores de juegos.

Ejemplo 12

Al trabajar con cadenas, a menudo es mejor usar las características proporcionadas por la clase de cadena . Por lo tanto, para los próximos dos casos es mejor reemplazar strlen (MyStr.c_str ()) y val = "" con MyStr.length () y val.clear () , respectivamente.

V806 Disminución del rendimiento. La expresión de strlen (MyStr.c_str ()) puede reescribirse 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 Disminución del rendimiento. Considere reemplazar la expresión 'val = ""' con 'val.clear ()'. b3CommandLineArgs.h 40

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

Hubo otras advertencias, pero creo que podemos parar aquí. Como puede ver, el análisis de código estático puede detectar una amplia gama de diversos errores.

Es interesante leer sobre verificaciones de proyectos por única vez, pero no es la forma correcta de usar analizadores de código estático. Y hablaremos de eso a continuación.

Errores encontrados ante nosotros

Fue interesante tratar de encontrar errores o defectos que ya se han solucionado pero que un analizador estático podría detectar a la luz del reciente artículo " Errores que no se encuentran en el análisis de código estático porque no se está utilizando ".

No había muchas solicitudes de extracción en el repositorio y muchas de ellas están relacionadas con la lógica interna del motor. Pero también hubo errores que el analizador pudo detectar.

Ejemplo 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) { .... } } 

El comentario de la solicitud dice que necesita verificar la matriz por el hecho de que no estaba vacía, sino que se realizó una comprobación de puntero sin sentido, que siempre devolvió verdadero. Esto es lo que le dice la advertencia de PVS-Studio sobre el cheque original:

V600 Considere inspeccionar la condición. El puntero 'info.m_deviceExtensions' no siempre es igual a NULL. b3OpenCLUtils.cpp 551

Ejemplo 14

¿Puedes averiguar cuál es el problema con la siguiente función?

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

El analizador genera las siguientes advertencias:

V570 Se asigna el mismo valor dos veces a la variable 'm23'. LinealR4.h 627

V570 Se asigna el mismo valor dos veces a la variable 'm13'. LinealR4.h 627

Las tareas repetidas en esta forma de grabación son difíciles de rastrear a simple vista y, como resultado, algunos de los elementos de la matriz no obtuvieron el valor inicial. Este error fue corregido por la forma tabular de registro de asignación:

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

Ejemplo 15

El siguiente error en una de las condiciones de la función btSoftBody :: addAeroForceToNode () condujo a un error obvio. Según el comentario en la solicitud de extracción, las fuerzas se aplicaron a los objetos desde el lado equivocado.

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

PVS-Studio también podría encontrar este error y generar la siguiente advertencia:

V768 La constante de enumeración 'V_TwoSided' se usa como una variable de tipo booleano. btSoftBody.cpp 542

El cheque fijo se ve así:

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

En lugar de la equivalencia de la propiedad de un objeto con uno de los enumeradores, se verificó el enumerador V_TwoSided .

Está claro que no examiné todas las solicitudes de extracción, porque ese no era el punto. Solo quería mostrarle que el uso regular de un analizador de código estático puede detectar errores en las primeras etapas. Esta es la forma correcta de utilizar el análisis de código estático. El análisis estático debe integrarse en el proceso DevOps y ser el filtro de error primario. Todo esto está bien descrito en el artículo " Introducir análisis estático en el proceso, no solo busque errores con él ".

Conclusión

A juzgar por algunas solicitudes de extracción, un proyecto a veces se verifica a través de varias herramientas de análisis de código, pero las correcciones no se realizan gradualmente sino en grupos y con grandes intervalos. En algunas solicitudes, el comentario indica que los cambios se realizaron solo para suprimir las advertencias. Este enfoque para usar el análisis reduce significativamente su utilidad porque son las verificaciones periódicas del proyecto las que le permiten corregir los errores de inmediato en lugar de esperar a que aparezcan errores explícitos.

Síganos y suscríbase a nuestras cuentas y canales de redes sociales: Instagram , Twitter , Facebook , Telegram . Nos encantaría estar contigo dondequiera que estés y mantenerte informado.