Heutzutage, zum Beispiel für die Entwicklung von Spielen, besteht heutzutage keine Notwendigkeit mehr, die Physik von Objekten von Grund auf unabhängig zu implementieren, da es dafür eine große Anzahl von Bibliotheken gibt. Bullet wurde einst in vielen AAA-Spielen, Virtual-Reality-Projekten, verschiedenen Simulationen und maschinellem Lernen aktiv eingesetzt. Ja, und es wird noch heute verwendet, beispielsweise als eine der Red Dead Redemption- und Red Dead Redemption 2-Engines. Warum also nicht Bullet mit PVS-Studio überprüfen, um festzustellen, welche Fehler die statische Analyse in einem so großen Projekt erkennen kann? im Zusammenhang mit der Physiksimulation.
Diese Bibliothek ist
frei verteilt , so dass jeder sie optional in seinen Projekten verwenden kann. Neben Red Dead Redemption wird diese Physik-Engine auch in der Filmindustrie verwendet, um Spezialeffekte zu erzeugen. Zum Beispiel war er an den Dreharbeiten zu Sherlock Holmes durch Guy Ritchie beteiligt, um Kollisionen zu berechnen.
Wenn dies Ihre erste Begegnung mit einem Artikel ist, in dem PVS-Studio Projekte überprüft, werde ich einen kleinen Exkurs machen.
PVS-Studio ist ein statischer Code-Analysator, mit dessen Hilfe Fehler, Mängel und potenzielle Schwachstellen im Quellcode von C-, C ++ -, C # - und Java-Programmen gefunden werden können. Die statische Analyse ist eine Art automatisierter Codeüberprüfungsprozess.
Zum 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 in der Zahl Pi (3.141592653 ...), bei dem die Zahl "6" an der 7. Stelle im Bruchteil fehlt.
Möglicherweise führt der Fehler in der zehnmillionsten Dezimalstelle nicht zu konkreten Konsequenzen, aber Sie sollten die vorhandenen Bibliothekskonstanten ohne Tippfehler verwenden. Für Pi gibt es eine
M_PI- Konstante aus dem
math.h- Header.
Kopieren Einfügen
Beispiel 2:Manchmal können Sie mit dem Analysegerät den Fehler indirekt finden. So werden hier beispielsweise drei verwandte Argumente
halfExtentsX, halfExtentsY, halfExtentsZ an die Funktion übergeben, letzteres wird jedoch nirgendwo in der Funktion verwendet. Möglicherweise stellen Sie fest, dass beim Aufrufen der
addVertex- Methode die Variable
halfExtentsY zweimal verwendet wird. Vielleicht ist dies ein Fehler beim Kopieren und Einfügen, und hier sollte ein vergessenes Argument 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 fand auch das folgende interessante Fragment, ich werde es zuerst in seiner ursprünglichen Form bringen.
Sehen Sie diese letzte Zeile?
Es ist sehr seltsam, dass der Programmierer beschlossen hat, eine so lange Bedingung in eine Zeile zu schreiben. Aber die Tatsache, dass sich höchstwahrscheinlich ein Fehler eingeschlichen hat, ist keineswegs überraschend.
Der Analysator gab die folgenden Warnungen in dieser Zeile aus.
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 visuellen „tabellarischen“ Form schreiben, wird klar, 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 wird der gleiche Vergleich viel deutlicher.
Beispiel 4:Fehler ähnlicher 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 dasselbe Array-Element doppelt geprüft. Höchstwahrscheinlich sollte die Bedingung folgendermaßen aussehen:
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:Ein weiteres Manko wurde entdeckt:
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 eine zurück, wenn sie
erfolgreich ist, und -1, wenn sie fehlschlägt. Höchstwahrscheinlich
hätte der Zweig
else if auf den negativen Wert von
serviceResult reagieren sollen , aber die Überprüfungsbedingung wurde dupliziert. Dies ist höchstwahrscheinlich auch ein Fehler beim Kopieren und Einfügen.
Eine ähnliche Analysatorwarnung, deren Beschreibung im Artikel nicht sinnvoll ist:
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
Über die Grenzen hinaus: Über die Grenzen des Arrays hinaus
Beispiel 6:Eines der unangenehmen Dinge, nach Fehlern zu suchen, ist das Verlassen des Arrays. Oft tritt dieser Fehler aufgrund einer komplexen Indizierung in der Schleife auf.
Hier ist in der Schleifenbedingung die Variable
dofIndex von oben auf 128 und von
dof auf 4 einschließlich begrenzt.
M_desiredState enthält aber auch nur 128 Elemente. Infolgedessen kann der Index
[dofIndex + dof] zum
Abfluss des Arrays führen.
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, aber jetzt führt die Summierung nicht zum Indizieren eines Arrays, sondern in einem Zustand. Wenn der Dateiname so lang wie möglich ist, 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 dies beseitigt den Fehler nicht, sondern macht sein Auftreten 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 einer bestimmten Funktion mehrmals verwenden oder wiederholt eine Variable verwenden müssen, auf die Sie über eine Reihe von Aufrufen zugreifen müssen, sollten Sie temporäre Variablen verwenden, um den Code zu optimieren und besser zu lesen. Der Analysator hat mehr als 100 Stellen im Code gefunden, an denen eine solche Korrektur vorgenommen werden kann.
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(....); } }
Hier wird dieselbe Aufrufkette wiederverwendet, die durch einen einzelnen Zeiger ersetzt werden kann.
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 des folgenden Typs gefunden:
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); }
Hier wurde kein Speicher vom
Importerzeiger freigegeben. Dies kann zu einem Speicherverlust führen. Und für den physischen Motor kann dies ein schlechter Trend sein. Um Leckagen zu vermeiden, reicht es aus, wenn die Variable nicht mehr benötigt wird, um den
Löschimporter hinzuzufügen. Aber natürlich ist es besser, intelligente Zeiger zu verwenden.
Hier sind deine Gesetze
Beispiel 11:Der folgende Fehler tritt im Code auf, da C ++ - Regeln nicht immer mit mathematischen Regeln oder dem „gesunden Menschenverstand“ übereinstimmen. Beachten Sie selbst, wo sich der Fehler in einem kleinen Codeausschnitt befindet?
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 sieht so aus, als hätte dieser Vergleich prüfen sollen, ob sich
f0 und
f1 am Ende des Arrays
m_fractureBodies befinden , da sie die Position des von der
findLinearSearch () -Methode gefundenen Objekts enthalten. Tatsächlich wird dieser Ausdruck jedoch zu einem Test, um
festzustellen, ob
f0 und
f1 gleich sind, und dann zu einer Überprüfung, ob
m_fractureBodies.size () gleich dem Ergebnis von
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 sie nur in zwei offenen Projekten getroffen, und interessanterweise waren alle nur Spiel-Engines.
Beispiel 12:Wenn Sie mit Zeichenfolgen arbeiten, ist es häufig besser, die von der
Zeichenfolgenklasse bereitgestellten Funktionen zu verwenden. In den folgenden beiden Fällen 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, Sie können aufhören. Wie Sie sehen können, kann die statische Code-Analyse eine Vielzahl verschiedener Fehler aufdecken.
Es ist interessant, über einmalige Projektprüfungen zu lesen, aber dies ist nicht der richtige Weg, um statische Code-Analysatoren zu verwenden. Und darüber werden wir weiter unten sprechen.
Vor uns gefundene Fehler
Es war im Geiste des kürzlich erschienenen Artikels „
Fehler, die bei der statischen Code-Analyse nicht gefunden werden, weil sie nicht verwendet wird “ interessant, um zu versuchen, Fehler oder Mängel zu finden, die bereits behoben wurden, die der statische Analysator jedoch erkennen könnte.
Es gab nicht so 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 Bearbeitungskommentar besagt, dass das Array auf die Tatsache überprüft werden musste, dass es nicht leer ist. Stattdessen wurde eine sinnlose Zeigerprüfung durchgeführt, die immer true zurückgab. Dies wird durch die Warnung PVS-Studio bei der ersten Art der Prüfung angezeigt:
V600 Überprüfen Sie den Zustand. Der Zeiger 'info.m_deviceExtensions' ist immer ungleich NULL. b3OpenCLUtils.cpp 551
Beispiel 14:Können Sie in der nächsten Funktion sofort herausfinden, wo das Problem 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 mit dieser Form der Aufzeichnung sind mit bloßem Auge schwer zu verfolgen, und infolgedessen haben einige Elemente der Matrix nicht den Anfangswert erhalten. Dieser Fehler wurde in der tabellarischen Form des Zuordnungsdatensatzes behoben:
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 expliziten Fehler. Gemäß dem Kommentar in der Pull-Anfrage wurden Kräfte von der falschen Seite auf 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 anzeigen:
V768 Die Aufzählungskonstante 'V_TwoSided' wird als Variable eines Booleschen Typs verwendet. btSoftBody.cpp 542
Die korrigierte Prüfung lautet wie folgt:
if (m_cfg.aeromodel == btSoftBody::eAeroModel::V_TwoSided) { .... }
Anstelle der Äquivalenz der Objekteigenschaft zu einem der Enumeratoren wurde der Enumerator
V_TwoSided selbst überprüft.
Es ist klar, dass ich nicht alle Pull-Anfragen angezeigt habe, da ich mir keine solche Aufgabe gestellt habe. Ich wollte 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 sollte in den DevOps-Prozess integriert sein und der Hauptfilter für Fehler sein. All dies wird im Artikel "
Statische Analyse in den Prozess einbetten und keine Fehler damit suchen " ausführlich beschrieben.
Fazit
Nach einigen Pull-Anforderungen zu urteilen, wird ein Projekt manchmal mit verschiedenen Code-Analyse-Tools überprüft. Die Änderungen werden jedoch nicht schrittweise, sondern in Gruppen und in großen Zeitintervallen vorgenommen. In einigen Anfragen gibt ein Kommentar an, dass Änderungen nur vorgenommen wurden, um Warnungen zu unterdrücken. Dieser Ansatz für die Verwendung von Analysen verringert die Nützlichkeit erheblich, da es sich um eine ständige Überprüfung des Projekts handelt, mit der Sie Fehler sofort beheben und nicht warten können, bis offensichtliche Fehler auftreten.
Wenn Sie immer über die Neuigkeiten und Ereignisse unseres Teams informiert sein möchten, abonnieren Sie unsere sozialen Dienste. Netzwerke:
Instagram ,
Twitter ,
Vkontakte ,
Telegramm .
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Übersetzungslink:
PVS-Studio hat einen Blick in die Bullet Engine von Red Dead Redemption geworfen