Heutzutage ist es nicht erforderlich, die Physik von Objekten für die Spieleentwicklung von Grund auf neu zu implementieren, da es zu diesem Zweck viele Bibliotheken gibt. Bullet wurde in vielen AAA-Spielen, Virtual-Reality-Projekten, verschiedenen Simulationen und maschinellem Lernen aktiv eingesetzt. Und es wird immer noch verwendet, zum Beispiel als eine der Red Dead Redemption- und Red Dead Redemption 2-Engines. Überprüfen Sie das Bullet mit PVS-Studio, um festzustellen, welche Fehler die statische Analyse in einem so großen Physiksimulationsprojekt erkennen kann.
Diese Bibliothek ist
frei verteilt , sodass jeder sie auf Wunsch in seinen eigenen Projekten verwenden kann. Neben Red Dead Redemption wird diese Physik-Engine auch in der Filmindustrie verwendet, um Spezialeffekte zu erzeugen. Zum Beispiel wurde es bei der Aufnahme von Guy Ritchies "Sherlock Holmes" verwendet, um Kollisionen zu berechnen.
Wenn Sie zum ersten Mal auf einen Artikel stoßen, in dem PVS-Studio Projekte überprüft, werde ich einen kleinen Exkurs machen.
PVS-Studio ist ein statischer Code-Analysator, mit dem Sie Fehler, Defekte und potenzielle Schwachstellen im Quellcode von C-, C ++ -, C # - und Java-Programmen finden können. Die statische Analyse ist eine Art automatisierter Codeüberprüfungsprozess.
Aufwärmen
Beispiel 1:Beginnen wir mit einem lustigen Fehler:
V624 In der Konstante '3.141592538' liegt wahrscheinlich ein Druckfehler vor. Erwägen Sie die Verwendung der M_PI-Konstante aus <math.h>. PhysicsClientC_API.cpp 4109
B3_SHARED_API void b3ComputeProjectionMatrixFOV(float fov, ....) { float yScale = 1.0 / tan((3.141592538 / 180.0) * fov / 2); .... }
Ein kleiner Tippfehler im Pi-Wert (3.141592653 ...). Die 7. Ziffer im Bruchteil fehlt - sie muss gleich 6 sein.
Möglicherweise führt ein Fehler im zehnmillionsten Bruch nach dem Dezimalpunkt nicht zu signifikanten Konsequenzen. Sie sollten jedoch die bereits vorhandenen Bibliothekskonstanten verwenden, die keine Tippfehler enthalten. Es gibt eine
M_PI- Konstante für die Pi-Nummer aus dem
math.h- Header.
Kopieren Einfügen
Beispiel 2:Manchmal können Sie mit dem Analysegerät den Fehler indirekt finden. Beispielsweise werden hier drei verwandte Argumente halfExtentsX, halfExtentsY, halfExtentsZ an die Funktion übergeben, wobei letzteres jedoch nirgendwo in der Funktion verwendet wird. Möglicherweise stellen Sie fest, dass die Variable halfExtentsY beim Aufrufen der Methode
addVertex zweimal
verwendet wird. Vielleicht handelt es sich also um einen Copypaste-Fehler, und das vergessene Argument sollte hier verwendet werden.
V751 Der Parameter 'halfExtentsZ' wird im Funktionskörper nicht verwendet. 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], ....); .... }
Beispiel 3:Der Analysator hat auch das folgende interessante Fragment entdeckt und ich werde es zuerst in der Ausgangsform zeigen.
Sehen Sie diese looooooooooong Linie?
Es ist sehr seltsam, dass der Programmierer beschlossen hat, eine so lange Bedingung in eine Zeile zu schreiben. Es ist jedoch nicht verwunderlich, dass höchstwahrscheinlich ein Fehler aufgetreten ist.
Der Analysator hat in dieser Zeile die folgenden Warnungen generiert.
V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke 'rotmat.Column1 (). Norm () <1.0001'. LinearR4.cpp 351
V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke '0.9999 <rotmat.Column1 (). Norm ()'. LinearR4.cpp 351
Wenn wir alles in einer klaren "tabellarischen" Form aufschreiben, können wir sehen, dass für
Spalte 1 dieselben Überprüfungen gelten. Die letzten beiden Vergleiche zeigen, dass es
Spalte1 und
Spalte2 gibt . Höchstwahrscheinlich hätte der dritte und vierte Vergleich den Wert von
Spalte 2 überprüfen
müssen .
Column1().Norm() < 1.0001 && 0.9999 < Column1().Norm() && Column1().Norm() < 1.0001 && 0.9999 < Column1().Norm() &&(Column1() ^ Column2()) < 0.001 && (Column1() ^ Column2()) > -0.001
In dieser Form werden die gleichen Vergleiche viel deutlicher.
Beispiel 4:Fehler der gleichen Art:
V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke 'cs.m_fJacCoeffInv [0] == 0'. 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; } .... }
In diesem Fall wird ein und dasselbe Array-Element zweimal überprüft. Höchstwahrscheinlich muss die Bedingung folgendermaßen ausgesehen haben:
cs.m_fJacCoeffInv [0] == 0 && cs.m_fJacCoeffInv [1] == 0 . Dies ist ein klassisches Beispiel für einen Fehler beim Kopieren und Einfügen.
Beispiel 5:Es wurde auch festgestellt, dass es einen solchen Defekt gab:
V517 Die Verwendung des
Musters 'if (A) {...} else if (A) {...}' wurde erkannt. Es besteht die Wahrscheinlichkeit eines logischen Fehlers. Überprüfen Sie die Zeilen: 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); } .... } .... }
Die Funktion
enet_host_service , deren Ergebnis
serviceResult zugewiesen
ist , gibt bei erfolgreichem Abschluss 1 und bei Fehler -1 zurück. Höchstwahrscheinlich
hätte der Zweig
else if auf den negativen Wert von
serviceResult reagieren sollen , aber die
Prüfbedingung wurde dupliziert. Wahrscheinlich ist es auch ein Fehler beim Kopieren und Einfügen.
Es gibt eine ähnliche Warnung des Analysators, aber es macht keinen Sinn, sie in diesem Artikel genauer zu betrachten.
V517 Die Verwendung des
Musters 'if (A) {...} else if (A) {...}' wurde erkannt. Es besteht die Wahrscheinlichkeit eines logischen Fehlers. Überprüfen Sie die Zeilen: 151, 190. PhysicsClientUDP.cpp 151
Over the top: Arraygrenzen überschreiten
Beispiel 6:Einer der unangenehmen Fehler bei der Suche ist der Array-Überlauf. Dieser Fehler tritt häufig aufgrund einer komplexen Indizierung in einer Schleife auf.
In der Schleifenbedingung
beträgt die
Obergrenze der Variablen
dofIndex 128 und die von
dof 4 einschließlich.
M_desiredState enthält aber auch nur 128 Elemente. Infolgedessen kann der Index
[dofIndex + dof] einen Array-Überlauf verursachen.
V557 Array-Überlauf ist möglich. Der Wert des Index 'dofIndex + dof' könnte 130 erreichen. 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]; .... } } .... }
Beispiel 7:Ein ähnlicher Fehler, der jetzt jedoch durch Summieren nicht beim Indizieren eines Arrays, sondern in einem Zustand verursacht wird. Wenn die Datei einen Namen mit maximaler Länge hat, wird das Terminal Null außerhalb des Arrays geschrieben (
Off-by-One-Fehler ). Natürlich ist die Variable
len nur in Ausnahmefällen gleich
MAX_FILENAME_LENGTH , aber sie beseitigt den Fehler nicht, sondern macht ihn einfach selten.
V557 Array-Überlauf ist möglich. Der Wert des 'len'-Index könnte 1024 erreichen. 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; }
Einmal messen, siebenmal schneiden
Beispiel 8:In Fällen, in denen Sie das Ergebnis der Arbeit einiger Funktionen mehrmals verwenden oder eine Variable verwenden müssen, die die gesamte Aufrufkette durchlaufen muss, um Zugriff zu erhalten, sollten Sie temporäre Variablen zur Optimierung und besseren Lesbarkeit des Codes verwenden. Der Analysator hat mehr als 100 Stellen im Code gefunden, an denen Sie eine solche Korrektur vornehmen können.
V807 Leistungsminderung. Erstellen Sie einen Zeiger, um zu vermeiden, dass der Ausdruck 'm_app-> m_renderer-> getActiveCamera ()' wiederholt verwendet wird. 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(....); } }
Dieselbe Aufrufkette wird hier häufig verwendet und kann durch einen einzelnen Zeiger ersetzt werden.
Beispiel 9:V810 Leistungsminderung. Die Funktion 'btCos (euler_out.pitch)' wurde mehrmals mit identischen Argumenten aufgerufen. Das Ergebnis sollte möglicherweise in einer temporären Variablen gespeichert werden, die dann beim Aufrufen der Funktion 'btAtan2' verwendet werden kann. btMatrix3x3.h 576
V810 Leistungsminderung. Die Funktion 'btCos (euler_out2.pitch)' wurde mehrmals mit identischen Argumenten aufgerufen. Das Ergebnis sollte möglicherweise in einer temporären Variablen gespeichert werden, die dann beim Aufrufen der Funktion 'btAtan2' verwendet werden kann. 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)); } .... }
In diesem Fall können Sie zwei Variablen erstellen und die von der Funktion
btCos für
euler_out.pitch und
euler_out2.pitch zurückgegebenen Werte
darin speichern , anstatt die Funktion für jedes Argument viermal aufzurufen.
Leck
Beispiel 10:Im Projekt wurden viele Fehler der folgenden Art festgestellt:
V773 Der Sichtbarkeitsbereich des 'Importer'-Zeigers wurde beendet, ohne den Speicher freizugeben. Ein Speicherverlust ist möglich. SerializeSetup.cpp 94
void SerializeSetup::initPhysics() { .... btBulletWorldImporter* importer = new btBulletWorldImporter(m_dynamicsWorld); .... fclose(file); m_guiHelper->autogenerateGraphicsObjects(m_dynamicsWorld); }
Der Speicher wurde hier nicht vom
Importerzeiger freigegeben. Dies kann zu einem Speicherverlust führen. Und für den physischen Motor kann es ein schlechter Trend sein. Um ein Leck zu vermeiden, reicht es aus, den
Löschimporter hinzuzufügen, nachdem die Variable nicht mehr benötigt wird. Aber natürlich ist es besser, intelligente Zeiger zu verwenden.
C ++ lebt von seinem eigenen Code
Beispiel 11:Der nächste Fehler erscheint im Code, da C ++ - Regeln nicht immer mit mathematischen Regeln oder dem "gesunden Menschenverstand" übereinstimmen. Werden Sie feststellen, wo dieses kleine Codefragment einen Fehler enthält?
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; .... } .... }
Der Analysator generiert die folgende Warnung:
V709 Verdächtiger Vergleich gefunden: 'f0 == f1 == m_fractureBodies.size ()'. Denken Sie daran, dass 'a == b == c' nicht gleich 'a == b && b == c' ist. btFractureDynamicsWorld.cpp 483
Es scheint, dass die Bedingung prüft,
ob f0 gleich
f1 und gleich der Anzahl der Elemente in
m_fractureBodies ist . Es scheint, dass dieser Vergleich hätte prüfen müssen, ob sich
f0 und
f1 am Ende des Arrays
m_fractureBodies befinden , da sie die Objektposition enthalten, die von der Methode
findLinearSearch () gefunden wurde . Tatsächlich wird dieser Ausdruck jedoch zu einer Überprüfung, ob
f0 und
f1 gleich
m_fractureBodies.size () sind, und dann zu einer Überprüfung, ob
m_fractureBodies.size () gleich dem Ergebnis
f0 == f1 ist . Infolgedessen wird der dritte Operand hier mit 0 oder 1 verglichen.
Schöner Fehler! Und zum Glück ziemlich selten. Bisher haben wir es nur in zwei Open-Source-Projekten
kennengelernt , und es ist interessant, dass alle Game-Engines waren.
Beispiel 12:Wenn Sie mit Zeichenfolgen arbeiten, ist es häufig besser, die von der
Zeichenfolgenklasse bereitgestellten Funktionen zu verwenden. Für die nächsten beiden Fälle ist es daher besser,
strlen (MyStr.c_str ()) und val = "" durch
MyStr.length () bzw.
val.clear () zu ersetzen.
V806 Verminderte Leistung. Der Ausdruck der Art strlen (MyStr.c_str ()) kann als MyStr.length () umgeschrieben werden. 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 Leistungsminderung. Ersetzen Sie den Ausdruck 'val = ""' durch 'val.clear ()'. b3CommandLineArgs.h 40
void addArgs(int argc, char **argv) { .... std::string val; .... val = ""; .... }
Es gab andere Warnungen, aber ich denke, wir können hier aufhören. Wie Sie sehen, kann die statische Code-Analyse eine Vielzahl verschiedener Fehler erkennen.
Es ist interessant, über einmalige Projektprüfungen zu lesen, aber es ist nicht die richtige Art, statische Code-Analysatoren zu verwenden. Und wir werden weiter unten darüber sprechen.
Vor uns gefundene Fehler
Es war interessant zu versuchen, Fehler oder Defekte zu finden, die bereits behoben wurden, die ein statischer Analysator jedoch im Lichte des kürzlich erschienenen Artikels "
Fehler, die bei der statischen Code-Analyse nicht gefunden werden, weil sie nicht verwendet werden " erkennen konnte.
Es gab nicht viele Pull-Anforderungen im Repository und viele davon beziehen sich auf die interne Logik der Engine. Es gab aber auch Fehler, die der Analysator erkennen konnte.
Beispiel 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) { .... } }
Der Kommentar für die Anforderung besagt, dass Sie das Array auf die Tatsache überprüfen mussten, dass es nicht leer war, sondern stattdessen eine bedeutungslose Zeigerprüfung durchgeführt wurde, die immer true zurückgab. Dies ist die Warnung von PVS-Studio bezüglich der ursprünglichen Prüfung:
V600 Überprüfen Sie den Zustand. Der Zeiger 'info.m_deviceExtensions' ist immer ungleich NULL. b3OpenCLUtils.cpp 551
Beispiel 14:Können Sie herausfinden, wo das Problem mit der nächsten Funktion liegt?
inline void Matrix4x4::SetIdentity() { m12 = m13 = m14 = m21 = m23 = m24 = m13 = m23 = m41 = m42 = m43 = 0.0; m11 = m22 = m33 = m44 = 1.0; }
Der Analysator generiert die folgenden Warnungen:
V570 Der Variable 'm23' wird zweimal derselbe Wert zugewiesen. LinearR4.h 627
V570 Der Variable 'm13' wird zweimal derselbe Wert zugewiesen. LinearR4.h 627
Wiederholte Zuordnungen in dieser Form der Aufzeichnung sind mit bloßem Auge schwer zu verfolgen, und infolgedessen haben einige der Matrixelemente nicht den Anfangswert erhalten. Dieser Fehler wurde durch die tabellarische Form der Zuordnungsaufzeichnung korrigiert:
m12 = m13 = m14 = m21 = m23 = m24 = m31 = m32 = m34 = m41 = m42 = m43 = 0.0;
Beispiel 15:Der folgende Fehler in einer der Bedingungen der
Funktion btSoftBody :: addAeroForceToNode () führte zu einem offensichtlichen Fehler. Gemäß dem Kommentar in der Pull-Anfrage wurden die Kräfte von der falschen Seite auf die Objekte ausgeübt.
struct eAeroModel { enum _ { V_Point, V_TwoSided, .... END }; }; void btSoftBody::addAeroForceToNode(....) { .... if (....) { if (btSoftBody::eAeroModel::V_TwoSided) { .... } .... } .... }
PVS-Studio konnte diesen Fehler auch finden und die folgende Warnung generieren:
V768 Die Aufzählungskonstante 'V_TwoSided' wird als Variable eines Booleschen Typs verwendet. btSoftBody.cpp 542
Ein fester Check sieht folgendermaßen aus:
if (m_cfg.aeromodel == btSoftBody::eAeroModel::V_TwoSided) { .... }
Anstatt die Eigenschaft eines Objekts mit einem der Enumeratoren
gleichzusetzen , wurde der
V_TwoSided- Enumerator selbst überprüft.
Es ist klar, dass ich nicht alle Pull-Anfragen angeschaut habe, denn das war nicht der Punkt. Ich wollte Ihnen nur zeigen, dass die regelmäßige Verwendung eines statischen Code-Analysators Fehler sehr früh erkennen kann. Dies ist der richtige Weg, um die statische Code-Analyse zu verwenden. Die statische Analyse muss in den DevOps-Prozess integriert sein und der primäre Fehlerfilter sein. All dies wird im Artikel "
Einführung in die statische Analyse in den Prozess, nicht nur nach Fehlern damit suchen " ausführlich beschrieben.
Fazit
Nach einigen Pull-Anfragen zu urteilen, wird ein Projekt manchmal durch verschiedene Code-Analyse-Tools überprüft, aber Korrekturen werden nicht schrittweise, sondern in Gruppen und mit großen Intervallen vorgenommen. In einigen Anfragen gibt der Kommentar an, dass die Änderungen nur vorgenommen wurden, um Warnungen zu unterdrücken. Dieser Ansatz zur Verwendung der Analyse verringert den Nutzen erheblich, da Sie durch regelmäßige Überprüfungen des Projekts Fehler sofort korrigieren können, anstatt auf explizite Fehler zu warten.
Folgen Sie uns und abonnieren Sie unsere Social-Media-Konten und -Kanäle:
Instagram ,
Twitter ,
Facebook ,
Telegramm . Wir würden gerne bei Ihnen sein, wo immer Sie sind, und Sie auf dem Laufenden halten.