Hoy en día, por ejemplo, para desarrollar juegos, ya no es necesario implementar de forma independiente la física de los objetos desde cero, ya que hay una gran cantidad de bibliotecas para esto. Bullet se usó activamente en muchos juegos AAA, proyectos de realidad virtual, varias simulaciones y aprendizaje automático. Sí, y todavía se usa hoy en día, siendo, por ejemplo, uno de los motores Red Dead Redemption y Red Dead Redemption 2. Entonces, ¿por qué no ver Bullet con PVS-Studio para ver qué errores puede detectar el análisis estático en un proyecto a gran escala? relacionado con la simulación física.
Esta biblioteca se
distribuye libremente , de modo que todos puedan usarla opcionalmente en sus proyectos. 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, estuvo involucrado en la filmación de Sherlock Holmes por Guy Ritchie para calcular las colisiones.
Si este es su primer encuentro con un artículo donde PVS-Studio verifica proyectos, entonces haré una pequeña digresión.
PVS-Studio es un analizador de código estático que ayuda a encontrar errores, deficiencias 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.
Para el calentamiento
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 número Pi (3.141592653 ...), falta el número "6" en la séptima posición en la parte fraccionaria.
Tal vez el error en la décima millonésima posición decimal no tendrá consecuencias tangibles, pero aún debe usar las constantes de biblioteca existentes sin errores tipográficos. Para Pi, hay una constante
M_PI del encabezado
math.h.Copiar y pegar
Ejemplo 2Algunas veces el analizador le permite encontrar el error indirectamente. Entonces, por ejemplo, aquí se
pasan tres argumentos relacionados
halfExtentsX, halfExtentsY, halfExtentsZ a la función, pero este último no se usa en ninguna parte de la función. Puede notar que al llamar al método
addVertex , la variable
halfExtentsY se usa dos veces. Entonces, tal vez sea un error de copiar y pegar, y aquí debería usarse un argumento olvidado.
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 3El analizador también encontró el siguiente fragmento interesante, lo traeré primero en su forma original.
¿Ves esta última línea?
Es muy extraño que el programador haya decidido escribir una condición tan larga en una línea. Pero el hecho de que lo más probable es que se haya introducido un error no es del todo sorprendente.
El analizador emitió 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 escribimos todo en forma visual "tabular", quedará claro que 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, la misma comparación se vuelve mucho más notable.
Ejemplo 4Error de un tipo similar:
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 debería haberse visto así:
cs.m_fJacCoeffInv [0] == 0 && cs.m_fJacCoeffInv [1] == 0 . Este es un ejemplo clásico de error de copiar y pegar.
Ejemplo 5:Se descubrió otra deficiencia:
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 uno si tiene
éxito y -1 si falla. Lo más probable es que la rama si hubiera respondido al valor negativo de
serviceResult , pero la condición de verificación se duplicó. Lo más probable es que esto también sea un error de copiar y pegar.
Una advertencia similar del analizador, que no tiene sentido describir en el 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
Más allá de los límites: ir más allá de los límites de la matriz
Ejemplo 6Una de las cosas desagradables para buscar errores es salir de la matriz. A menudo, este error ocurre debido a una indexación compleja en el bucle.
Aquí, en la condición de bucle, la variable
dofIndex está limitada a 128 desde arriba, y
dof a 4, inclusive. Pero
m_desiredState también contiene solo 128 elementos. Como resultado, el índice
[dofIndex + dof] puede conducir a la
salida 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 7Un error similar, pero ahora la suma no lo lleva a indexar una matriz, sino a una condición. Si el nombre del archivo es lo más largo posible, el terminal cero se escribirá fuera de la matriz (
Error Off-by-one ). Naturalmente, la variable
len será igual a
MAX_FILENAME_LENGTH solo en casos excepcionales, pero esto no elimina el error, sino que simplemente hace que su ocurrencia sea rara.
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; }
Mida una vez - corte siete veces
Ejemplo 8En los casos en que necesite usar el resultado de una determinada función muchas veces o usar repetidamente una variable a la que necesita acceder a través de una serie de llamadas, debe usar variables temporales para optimizar y leer mejor el código. El analizador encontró más de 100 lugares en el código donde se 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 reutiliza la misma cadena de llamadas, que se puede reemplazar con un solo puntero.
Ejemplo 9V810 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 almacenar los valores devueltos por la función
btCos para
euler_out.pitch y
euler_out2.pitch en ellas , en lugar de llamar a la función cuatro veces para cada argumento.
Fuga
Ejemplo 10Se encontraron 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); }
No se ha liberado memoria del puntero del
importador aquí. Esto puede causar una pérdida de memoria. Y para el motor físico, esta puede ser una mala tendencia. Para evitar fugas, es suficiente después de que la variable ya no sea necesaria para agregar un
importador de eliminación . Pero, por supuesto, es mejor usar punteros inteligentes.
Aquí están tus leyes
Ejemplo 11El siguiente error aparece en el código debido al hecho de que las reglas de C ++ no siempre coinciden con las reglas matemáticas o el "sentido común". ¿Te das cuenta de dónde está el error en un pequeño fragmento de código?
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 e igual al número de elementos en
m_fractureBodies . Parece que esta comparación debería haber verificado si
f0 y
f1 están al final de la matriz
m_fractureBodies , ya que contienen la posición del objeto encontrado por el método
findLinearSearch () . Sin embargo, de hecho, esta expresión se convierte en una prueba para ver si
f0 y
f1 son iguales, y luego para verificar si
m_fractureBodies.size () es igual al resultado de
f0 == f1 . Como resultado, el tercer operando se compara con 0 o 1 aquí.
Hermoso error! Y, afortunadamente, bastante raro. Hasta ahora la hemos
conocido solo en dos proyectos abiertos, y curiosamente, todos ellos eran solo motores de juegos.
Ejemplo 12Cuando se trabaja con cadenas, a menudo es mejor usar las facilidades proporcionadas por la clase de
cadena . Entonces, para los siguientes 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 puedes parar. Como puede ver, el análisis de código estático puede revelar una amplia gama de diversos errores.
Leer sobre verificaciones de proyectos por única vez es interesante, pero esta no es la forma correcta de usar analizadores de código estático. Y sobre esto hablaremos a continuación.
Errores encontrados ante nosotros
Fue interesante en el espíritu del reciente artículo "
Errores que el análisis de código estático no encuentra porque no se usa " para tratar de encontrar errores o deficiencias que ya se han solucionado, pero que el analizador estático podría detectar.
No había tantas 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 edición dice que fue necesario verificar la matriz por el hecho de que no está vacía, sino que se realizó una comprobación de puntero sin sentido, que siempre devolvió verdadero. Esto se indica mediante la advertencia PVS-Studio en el tipo de verificación inicial:
V600 Considere inspeccionar la condición. El puntero 'info.m_deviceExtensions' no siempre es igual a NULL. b3OpenCLUtils.cpp 551
Ejemplo 14¿Puedes averiguar inmediatamente cuál es el problema en la próxima 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 con esta forma de grabación son difíciles de rastrear a simple vista y, como resultado, algunos de los elementos de la matriz no recibieron el valor inicial. Este error se corrigió en la forma tabular del registro de asignación:
m12 = m13 = m14 = m21 = m23 = m24 = m31 = m32 = m34 = m41 = m42 = m43 = 0.0;
Ejemplo 15El siguiente error en una de las condiciones de la función
btSoftBody :: addAeroForceToNode () condujo a un error explícito. Según el comentario en la solicitud de extracción, se aplicaron fuerzas a los objetos del 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 mostrar la siguiente advertencia:
V768 La constante de enumeración 'V_TwoSided' se usa como una variable de tipo booleano. btSoftBody.cpp 542
La verificación corregida es la siguiente:
if (m_cfg.aeromodel == btSoftBody::eAeroModel::V_TwoSided) { .... }
En lugar de la equivalencia de la propiedad del objeto con uno de los enumeradores, se
verificó el enumerador
V_TwoSided .
Está claro que no vi todas las solicitudes de extracción, ya que no me puse esa tarea. Solo quería mostrar que el uso regular de un analizador de código estático puede detectar errores en una etapa muy temprana. Esta es la forma correcta de usar el análisis de código estático. El análisis estático debe integrarse en el proceso DevOps y ser el filtro principal de errores. Todo esto está bien descrito en el artículo "
Incruste el análisis estático en el proceso y no busque errores ".
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, sin embargo, las ediciones no se realizan gradualmente, sino en grupos y a grandes intervalos de tiempo. En algunas solicitudes, un comentario indica que se hicieron cambios solo para suprimir las advertencias. Este enfoque para el uso del análisis reduce significativamente su utilidad, ya que es una verificación constante del proyecto que le permite corregir los errores de inmediato y no esperar hasta que aparezcan errores obvios.
Si quiere estar siempre al tanto de las noticias y eventos de nuestro equipo, suscríbase a nuestros servicios sociales. Redes:
Instagram ,
Twitter ,
Vkontakte ,
Telegram .
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace de traducción:
PVS-Studio examinó el motor de bala de Red Dead Redemption