De nos jours, il n'est pas nécessaire d'implémenter la physique des objets à partir de zéro pour le développement de jeux car il existe de nombreuses bibliothèques à cet effet. Bullet a été activement utilisé dans de nombreux jeux AAA, des projets de réalité virtuelle, diverses simulations et l'apprentissage automatique. Et il est toujours utilisé, étant, par exemple, l'un des moteurs Red Dead Redemption et Red Dead Redemption 2. Alors pourquoi ne pas vérifier la puce avec PVS-Studio pour voir quelles erreurs l'analyse statique peut détecter dans un projet de simulation physique à si grande échelle.
Cette bibliothèque est
librement distribuée , donc chacun peut l'utiliser dans ses propres projets s'il le souhaite. En plus de Red Dead Redemption, ce moteur physique est également utilisé dans l'industrie cinématographique pour créer des effets spéciaux. Par exemple, il a été utilisé dans le tournage du "Sherlock Holmes" de Guy Ritchie pour calculer les collisions.
Si c'est la première fois que vous rencontrez un article où PVS-Studio vérifie des projets, je vais faire une petite digression.
PVS-Studio est un analyseur de code statique qui vous aide à trouver des erreurs, des défauts et des vulnérabilités potentielles dans le code source des programmes C, C ++, C #, Java. L'analyse statique est une sorte de processus automatisé de révision de code.
Réchauffer
Exemple 1:Commençons par une erreur amusante:
V624 Il y a probablement une
faute d' impression dans la constante '3.141592538'. Pensez à utiliser 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); .... }
Une petite faute de frappe dans la valeur Pi (3.141592653 ...). Le 7e chiffre de la partie fractionnaire est manquant - il doit être égal à 6.
Peut-être qu'une erreur dans la dix millionième fraction après la virgule décimale n'entraînera aucune conséquence significative, mais vous devez toujours utiliser les constantes de bibliothèque déjà existantes qui n'ont pas de fautes de frappe. Il existe une constante
M_PI pour le nombre Pi de l'en-tête
math.h.Copier coller
Exemple 2:Parfois, l'analyseur vous permet de trouver l'erreur indirectement. Par exemple, trois arguments connexes halfExtentsX, halfExtentsY, halfExtentsZ sont passés ici dans la fonction mais ce dernier n'est utilisé nulle part dans la fonction. Vous pouvez remarquer que la variable halfExtentsY est utilisée deux fois lors de l'appel de la méthode
addVertex . Il s'agit donc peut-être d'une erreur de copypaste et l'argument oublié doit être utilisé ici.
V751 Le paramètre 'halfExtentsZ' n'est pas utilisé dans le corps de la fonction. 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], ....); .... }
Exemple 3:L'analyseur a également détecté le fragment intéressant suivant et je vais le montrer d'abord dans la forme initiale.
Voir cette ligne looooooooooong?
Il est très étrange que le programmeur ait décidé d'écrire une condition aussi longue sur une seule ligne. Mais il n'est pas surprenant qu'une erreur s'y soit glissée.
L'analyseur a généré les avertissements suivants sur cette ligne.
V501 Il existe des sous-expressions identiques 'rotmat.Column1 (). Norm () <1.0001' à gauche et à droite de l'opérateur '&&'. LinearR4.cpp 351
V501 Il existe des sous-expressions identiques '0.9999 <rotmat.Column1 (). Norm ()' à gauche et à droite de l'opérateur '&&'. LinearR4.cpp 351
Si nous écrivons tout cela sous une forme "tabulaire" claire, nous pouvons voir que les mêmes vérifications s'appliquent à
Colonne1 . Les deux dernières comparaisons montrent qu'il y a
Colonne1 et
Colonne2 . Très probablement, les troisième et quatrième comparaisons auraient dû vérifier la valeur de
Colonne2 .
Column1().Norm() < 1.0001 && 0.9999 < Column1().Norm() && Column1().Norm() < 1.0001 && 0.9999 < Column1().Norm() &&(Column1() ^ Column2()) < 0.001 && (Column1() ^ Column2()) > -0.001
Sous cette forme, les mêmes comparaisons deviennent beaucoup plus visibles.
Exemple 4:Erreur du même type:
V501 Il existe des sous-expressions identiques «cs.m_fJacCoeffInv [0] == 0» à gauche et à droite de l'opérateur «&&». 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; } .... }
Dans ce cas, un même élément de tableau est vérifié deux fois. Très probablement, la condition doit ressembler à ceci:
cs.m_fJacCoeffInv [0] == 0 && cs.m_fJacCoeffInv [1] == 0 . Il s'agit d'un exemple classique d'une erreur de copier-coller.
Exemple 5:Il a également été découvert qu'il y avait un tel défaut:
V517 L'utilisation du
modèle 'if (A) {...} else if (A) {...}' a été détectée. Il y a une probabilité de présence d'erreur logique. Vérifiez les lignes: 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 fonction
enet_host_service , dont le résultat est affecté à
serviceResult , renvoie 1 en cas de réussite et -1 en cas d'échec. Très probablement, la branche
else if aurait dû réagir à la valeur négative de
serviceResult , mais la condition de vérification a été dupliquée. C'est probablement aussi une erreur de copier-coller.
Il existe un avertissement similaire de l'analyseur, mais il est inutile de l'examiner de plus près dans cet article.
V517 L'utilisation du
modèle 'if (A) {...} else if (A) {...}' a été détectée. Il y a une probabilité de présence d'erreur logique. Vérifiez les lignes: 151, 190. PhysicsClientUDP.cpp 151
Au-dessus: dépasser les limites du tableau
Exemple 6:L'une des erreurs désagréables à rechercher est le dépassement de la baie. Cette erreur se produit souvent en raison d'une indexation complexe dans une boucle.
Ici, dans la condition de boucle, la limite supérieure de la variable
dofIndex est de 128 et celle de
dof est de 4 inclus. Mais
m_desiredState ne contient également que 128 éléments. Par conséquent, l'
index [dofIndex + dof] peut provoquer un dépassement de tableau.
V557 Le dépassement de matrice est possible. La valeur de l'indice «dofIndex + dof» pourrait atteindre 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]; .... } } .... }
Exemple 7:Une erreur similaire, mais maintenant, elle est causée par la sommation non pas lors de l'indexation d'un tableau mais dans une condition. Si le fichier a un nom avec une longueur maximale, le zéro terminal sera écrit en dehors du tableau (
erreur Off-by-one ). Bien sûr, la variable
len sera égale à
MAX_FILENAME_LENGTH uniquement dans des cas exceptionnels, mais elle n'élimine pas l'erreur mais la rend simplement rare.
V557 Le dépassement de matrice est possible. La valeur de l'index 'len' pourrait atteindre 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; }
Mesurez-le une fois, coupez-le sept fois
Exemple 8:Dans les cas où vous devez utiliser le résultat du travail de certaines fonctions plusieurs fois ou utiliser une variable qui nécessite de traverser toute la chaîne d'appels pour y accéder, vous devez utiliser des variables temporaires pour l'optimisation et une meilleure lisibilité du code. L'analyseur a trouvé plus de 100 emplacements dans le code où vous pouvez effectuer une telle correction.
V807 Performances
réduites . Envisagez de créer un pointeur pour éviter d'utiliser à plusieurs reprises l'expression 'm_app-> m_renderer-> getActiveCamera ()'. 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(....); } }
La même chaîne d'appel est utilisée plusieurs fois ici et peut être remplacée par un seul pointeur.
Exemple 9:Performances
réduites du V810 . La fonction 'btCos (euler_out.pitch)' a été appelée plusieurs fois avec des arguments identiques. Le résultat devrait éventuellement être enregistré dans une variable temporaire, qui pourrait ensuite être utilisée lors de l'appel de la fonction 'btAtan2'. btMatrix3x3.h 576
Performances
réduites du V810 . La fonction 'btCos (euler_out2.pitch)' a été appelée plusieurs fois avec des arguments identiques. Le résultat devrait éventuellement être enregistré dans une variable temporaire, qui pourrait ensuite être utilisée lors de l'appel de la fonction '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)); } .... }
Dans ce cas, vous pouvez créer deux variables et enregistrer les valeurs renvoyées par la fonction
btCos pour
euler_out.pitch et
euler_out2.pitch en elles au lieu d'appeler la fonction quatre fois pour chaque argument.
Fuite
Exemple 10:De nombreuses erreurs du type suivant ont été détectées dans le projet:
V773 La portée de visibilité du pointeur «importateur» a été fermée sans libérer la mémoire. Une fuite de mémoire est possible. SerializeSetup.cpp 94
void SerializeSetup::initPhysics() { .... btBulletWorldImporter* importer = new btBulletWorldImporter(m_dynamicsWorld); .... fclose(file); m_guiHelper->autogenerateGraphicsObjects(m_dynamicsWorld); }
La mémoire n'a pas été libérée du pointeur d'
importation ici. Cela peut entraîner une fuite de mémoire. Et pour le moteur physique, cela peut être une mauvaise tendance. Pour éviter une fuite, il suffit d'ajouter un
importateur de suppression lorsque la variable devient inutile. Mais, bien sûr, il est préférable d'utiliser des pointeurs intelligents.
C ++ vit par son propre code
Exemple 11:L'erreur suivante apparaît dans le code car les règles C ++ ne coïncident pas toujours avec les règles mathématiques ou le "bon sens". Allez-vous remarquer où ce petit fragment de code contient une erreur?
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; .... } .... }
L'analyseur génère l'avertissement suivant:
V709 Comparaison suspecte trouvée: 'f0 == f1 == m_fractureBodies.size ()'. N'oubliez pas que «a == b == c» n'est pas égal à «a == b && b == c». btFractureDynamicsWorld.cpp 483
Il semblerait que la condition vérifie que
f0 est égal à
f1 et est égal au nombre d'éléments dans
m_fractureBodies . Il semble que cette comparaison aurait dû vérifier si
f0 et
f1 sont situés à la fin du tableau
m_fractureBodies , car ils contiennent la position de l'objet trouvée par la méthode
findLinearSearch () . Mais en fait, cette expression se transforme en une vérification pour voir si
f0 et
f1 sont égaux à
m_fractureBodies.size () , puis une vérification pour voir si
m_fractureBodies.size () est égale au résultat
f0 == f1 . Par conséquent, le troisième opérande est ici comparé à 0 ou 1.
Belle erreur! Et, heureusement, assez rare. Jusqu'à présent, nous ne l'avons
rencontré que dans deux projets open source, et il est intéressant de noter que tous étaient des moteurs de jeu.
Exemple 12:Lorsque vous travaillez avec des chaînes, il est souvent préférable d'utiliser les fonctionnalités fournies par la classe de
chaînes . Ainsi, pour les deux cas suivants, il est préférable de remplacer respectivement
strlen (MyStr.c_str ()) et val = "" par
MyStr.length () et
val.clear () .
Performances
réduites du V806 . L'expression du type strlen (MyStr.c_str ()) peut être réécrite en 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 Performances
réduites . Pensez à remplacer l'expression 'val = ""' par 'val.clear ()'. b3CommandLineArgs.h 40
void addArgs(int argc, char **argv) { .... std::string val; .... val = ""; .... }
Il y a eu d'autres avertissements, mais je pense que nous pouvons nous arrêter ici. Comme vous le voyez, l'analyse de code statique peut détecter un large éventail d'erreurs diverses.
Il est intéressant de lire sur les vérifications de projet ponctuelles, mais ce n'est pas la bonne façon d'utiliser des analyseurs de code statiques. Et nous en parlerons ci-dessous.
Erreurs trouvées devant nous
Il était intéressant d'essayer de trouver des bogues ou des défauts qui ont déjà été corrigés mais qu'un analyseur statique pouvait détecter à la lumière de l'article récent "
Erreurs qui ne sont pas trouvées par l'analyse de code statique parce qu'il n'est pas utilisé ".
Il n'y avait pas beaucoup de demandes d'extraction dans le référentiel et beaucoup d'entre elles sont liées à la logique interne du moteur. Mais il y avait aussi des erreurs que l'analyseur pouvait détecter.
Exemple 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) { .... } }
Le commentaire de la demande indique que vous devez vérifier que le tableau n'est pas vide, mais qu'une vérification de pointeur sans signification a été effectuée, qui renvoyait toujours true. Voici ce que l'avertissement de PVS-Studio concernant la vérification d'origine vous indique:
V600 Envisagez d'inspecter l'état. Le pointeur 'info.m_deviceExtensions' n'est toujours pas égal à NULL. b3OpenCLUtils.cpp 551
Exemple 14:Pouvez-vous découvrir quel est le problème avec la fonction suivante?
inline void Matrix4x4::SetIdentity() { m12 = m13 = m14 = m21 = m23 = m24 = m13 = m23 = m41 = m42 = m43 = 0.0; m11 = m22 = m33 = m44 = 1.0; }
L'analyseur génère les avertissements suivants:
V570 La même valeur est affectée deux fois à la variable 'm23'. LinearR4.h 627
V570 La même valeur est affectée deux fois à la variable 'm13'. LinearR4.h 627
Les affectations répétées dans cette forme d'enregistrement sont difficiles à suivre à l'œil nu et, par conséquent, certains éléments de la matrice n'ont pas obtenu la valeur initiale. Cette erreur a été corrigée par la forme tabulaire d'enregistrement des affectations:
m12 = m13 = m14 = m21 = m23 = m24 = m31 = m32 = m34 = m41 = m42 = m43 = 0.0;
Exemple 15:L'erreur suivante dans l'une des conditions de la
fonction btSoftBody :: addAeroForceToNode () a conduit à un bogue évident. Selon le commentaire dans la demande de traction, les forces ont été appliquées aux objets du mauvais côté.
struct eAeroModel { enum _ { V_Point, V_TwoSided, .... END }; }; void btSoftBody::addAeroForceToNode(....) { .... if (....) { if (btSoftBody::eAeroModel::V_TwoSided) { .... } .... } .... }
PVS-Studio peut également trouver cette erreur et générer l'avertissement suivant:
V768 La constante d'énumération 'V_TwoSided' est utilisée comme variable de type booléen. btSoftBody.cpp 542
Le contrôle fixe ressemble à ceci:
if (m_cfg.aeromodel == btSoftBody::eAeroModel::V_TwoSided) { .... }
Au lieu de l'équivalence de la propriété d'un objet à l'un des énumérateurs, l'énumérateur
V_TwoSided lui-même a été vérifié.
Il est clair que je n'ai pas examiné toutes les demandes de tirage, car ce n'était pas le but. Je voulais juste vous montrer que l'utilisation régulière d'un analyseur de code statique peut détecter des erreurs à un stade très précoce. C'est la bonne façon d'utiliser l'analyse de code statique. L'analyse statique doit être intégrée au processus DevOps et être le filtre de bogues principal. Tout cela est bien décrit dans l'article «
Introduire l'analyse statique dans le processus, ne vous contentez pas de rechercher des bogues avec elle ».
Conclusion
A en juger par certaines pull-requests, un projet est parfois vérifié à travers divers outils d'analyse de code mais les corrections ne sont pas faites progressivement mais en groupe et à grands intervalles. Dans certaines demandes, le commentaire indique que les modifications ont été apportées uniquement pour supprimer les avertissements. Cette approche de l'utilisation de l'analyse réduit considérablement son utilité car ce sont les vérifications régulières du projet qui vous permettent de corriger les erreurs immédiatement plutôt que d'attendre l'apparition de bogues explicites.
Suivez-nous et abonnez-vous à nos comptes et canaux de médias sociaux:
Instagram ,
Twitter ,
Facebook ,
Telegram . Nous serions ravis d'être avec vous où que vous soyez et de vous tenir au courant.