Amazon Lumberyard: der Schrei der Seele

Spiele sind eines der beliebtesten Softwareprodukte. Dies ist eine riesige Branche, in der eine neue Spiel-Engine - Amazon Lumberyard. Das Projekt befindet sich noch im Beta-Status und es hat Zeit, Fehler zu beheben und die Qualität des Codes zu verbessern. Die Entwickler der Engine haben in naher Zukunft viel zu tun, um Millionen von Spielern und Spieleentwicklern nicht zu enttäuschen.

Einführung

Amazon Lumberyard ist eine kostenlose plattformübergreifende AAA-Spiel-Engine, die von Amazon entwickelt wurde und auf der CryEngine- Engine-Architektur basiert, die 2015 von Crytek lizenziert wurde. Die Analyse von CryEngine wurde von mir im August 2016 und April 2017 bereits zweimal durchgeführt. Gleichzeitig muss ich feststellen, dass sich der Code nach einem Jahr nur noch verschlechterte. Und neulich habe ich mich entschlossen zu sehen, was Amazon basierend auf dieser Spiel-Engine gemacht hat. Sie haben sehr gut mit der Umwelt gearbeitet. Die Dokumentation für Entwickler und Software zur Bereitstellung einer Arbeitsumgebung ist sehr cool und auf hohem Niveau. Aber das Problem ist wieder mit dem Code! Ich hoffe, dass Amazon viel mehr Ressourcen für die Arbeit mit dem Projekt hat und trotzdem auf die Qualität des Codes achtet. Mit diesem Test möchte ich die Entwickler auf die Qualität des Codes aufmerksam machen und auf einen neuen Ansatz bei der Entwicklung dieser Spiel-Engine drängen. Der aktuelle Status des Codes war so bedauerlich, dass ich den Titel des Artikels mehrmals geändert und das Titelbild neu gezeichnet habe, als ich den Bericht mit den Analyseergebnissen durchgesehen habe. Die erste Version des Bildes war weniger emotional:

Wir haben die Quellen von Amazon Lumberyard der neuesten verfügbaren Version 1.14.0.1 analysiert. Der Quellcode wird aus dem Repository auf Github entnommen. Eines der ersten Spiele auf der Lumberyard-Engine sollte Star Citizen sein . Potenzielle Spieler, die auf sie warten, lade ich Sie auch ein, einen Blick auf das zu werfen, was derzeit "unter der Haube" des Spiels ist.

Integration mit PVS-Studio

PVS-Studio wurde als statischer Code-Analysator verwendet. Es ist für Windows, Linux und MacOS verfügbar. Das heißt, Für die Analyse plattformübergreifender Projekte steht sogar eine Auswahl für komfortableres Arbeiten zur Verfügung. Neben C und C ++ wird die Analyse von Projekten in der Sprache C # unterstützt. Java-Pläne . Die überwiegende Mehrheit des Codes auf der Welt ist in den aufgeführten Sprachen geschrieben (natürlich nicht ohne Fehler). Probieren Sie also den PVS-Studio-Analysator für Ihr Projekt aus, Sie werden viele interessante Dinge lernen ;-).

Als Lumberyard-Montagesystem wird WAF verwendet, das auch in CryEngine enthalten war. Der Analysator hat keine spezielle Möglichkeit, sich in dieses Montagesystem zu integrieren. Ich entschied mich für ein Projekt unter Windows und entschied mich für diese Methode zum Starten der Analyse: ein Kompilierungsüberwachungssystem . Die Projektdatei für Visual Studio wird automatisch generiert. Es kann verwendet werden, um das Projekt zu erstellen und den Analysebericht anzuzeigen.

Die Liste der zu analysierenden Befehle sieht ungefähr so ​​aus:

cd /path/to/lumberyard/dev lmbr_waf.bat ... CLMonitor.exe monitor MSBuild.exe ... LumberyardSDK_vs15.sln ... CLMonitor.exe analyze --log /path/to/report.plog 

Der Bericht kann, wie gesagt, in Visual Studio angezeigt werden.

Über Igor und Qualcomm

Amazon Lumberyard positioniert sich als plattformübergreifende Spiel-Engine. Es ist einfach, ein Projekt mit einer solchen Funktion bei den Massen bekannt zu machen, aber es ist sehr schwierig, es zu warten. Eine der Warnungen von PVS-Studio wurde auf einem Codefragment ausgegeben, auf dem der Programmierer Igor mit dem Qualcomm-Compiler kämpfte. Vielleicht hat er sein Problem gelöst, aber einen äußerst verdächtigen Code hinterlassen. Ich beschloss, es mit einem Bild zu zeichnen.

V523 Die Anweisung 'then' entspricht der Anweisung 'else'. toglsloperand.c 700

Hier wird unabhängig von der berechneten Bedingung derselbe Code ausgeführt. Vor dem Hintergrund der Kommentare sieht eine solche Entscheidung verdächtig aus.

Im Allgemeinen ist das Projekt nicht der einzige Ort, an dem die Bedingungen aus Gründen der Klarheit oder zur Korrektur eines echten Fehlers vereinfacht werden müssen. Hier ist eine Liste solcher Orte:

• V523 Die Anweisung 'then' entspricht der Anweisung 'else'. livingentity.cpp 1385
• V523 Die Anweisung 'then' entspricht der Anweisung 'else'. tometalinstruction.c 4201
• V523 Die Anweisung 'then' entspricht der Anweisung 'else'. scripttable.cpp 905
• V523 Die Anweisung 'then' entspricht der Anweisung 'else'. budgetingsystem.cpp 701
• V523 Die Anweisung 'then' entspricht der Anweisung 'else'. editorframeworkapplication.cpp 562
• V523 Die Anweisung 'then' entspricht der Anweisung 'else'. partikular.cpp 130
• V523 Die Anweisung 'then' entspricht der Anweisung 'else'. trackviewnodes.cpp 1223
• V523 Die Anweisung 'then' entspricht der Anweisung 'else'. propertyoarchive.cpp 447

Python ++

Der Analysator hat einen so lustigen Code gefunden:

V709 CWE-682 Verdächtiger Vergleich gefunden: 'a == b == c'. Denken Sie daran, dass 'a == b == c' nicht gleich 'a == b && b == c' ist. toglslinstruction.c 564

 void CallBinaryOp(....) { .... uint32_t src1SwizCount = GetNumSwizzleElements(....); uint32_t src0SwizCount = GetNumSwizzleElements(....); uint32_t dstSwizCount = GetNumSwizzleElements(....); .... if (src1SwizCount == src0SwizCount == dstSwizCount) // <= { .... } .... } 

In C ++ wird ein solcher Code leider erfolgreich kompiliert, aber er wird überhaupt nicht ausgeführt, wie es scheint. Ausdrücke in C ++ werden in der Reihenfolge ihrer Priorität ausgewertet und führen bei Bedarf implizite Castes aus.

Eine solche Überprüfung wäre beispielsweise in der Python-Sprache korrekt. Aber in dieser Situation hat sich der Entwickler "in den Fuß geschossen".

3 weitere Kontrollaufnahmen:

• V709 CWE-682 Verdächtiger Vergleich gefunden: 'a == b == c'. Denken Sie daran, dass 'a == b == c' nicht gleich 'a == b && b == c' ist. toglslinstruction.c 654
• V709 CWE-682 Verdächtiger Vergleich gefunden: 'a == b == c'. Denken Sie daran, dass 'a == b == c' nicht gleich 'a == b && b == c' ist. toglslinstruction.c 469
• V709 CWE-682 Verdächtiger Vergleich gefunden: 'a == b == c'. Denken Sie daran, dass 'a == b == c' nicht gleich 'a == b && b == c' ist. tometalinstruction.c 539

Die erste und eine der besten Diagnosen

Es geht um die Warnung V501 - die erste Allzweckdiagnose in PVS-Studio. Fehler, die allein durch diese Diagnose gefunden werden, können ausreichen, um einen Artikel zu schreiben. Und das Amazon Lumberyard-Projekt zeigt dies sehr gut.

Leider ist es langweilig, Fehler desselben Typs lange Zeit zu betrachten. Daher werde ich in diesem Abschnitt nur einige Codefragmente kommentieren und die restlichen Warnungen werden aufgelistet.

V501 Links und rechts vom '||' befinden sich identische Unterausdrücke. Operator: hotX <0 || hotX <0 editorutils.cpp 166

 QCursor CMFCUtils::LoadCursor(....) { .... if (!pm.isNull() && (hotX < 0 || hotX < 0)) { QFile f(path); f.open(QFile::ReadOnly); QDataStream stream(&f); stream.setByteOrder(QDataStream::LittleEndian); f.read(10); quint16 x; stream >> x; hotX = x; stream >> x; hotY = x; } .... } 

Der Bedingung fehlt die Variable hot Y. Klassischer Tippfehler.

V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke 'sp.m_pTexture == m_pTexture'. shadercomponents.h 487

V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke 'sp.m_eCGTextureType == m_eCGTextureType'. shadercomponents.h 487

 bool operator != (const SCGTexture& sp) const { if (sp.m_RegisterOffset == m_RegisterOffset && sp.m_Name == m_Name && sp.m_pTexture == m_pTexture && // <= 1 sp.m_RegisterCount == m_RegisterCount && sp.m_eCGTextureType == m_eCGTextureType && // <= 2 sp.m_BindingSlot == m_BindingSlot && sp.m_Flags == m_Flags && sp.m_pAnimInfo == m_pAnimInfo && sp.m_pTexture == m_pTexture && // <= 1 sp.m_eCGTextureType == m_eCGTextureType && // <= 2 sp.m_bSRGBLookup == m_bSRGBLookup && sp.m_bGlobal == m_bGlobal) { return false; } return true; } 

In diesem Fragment wurden zwei Kopierpasten gleichzeitig gefunden. Aus Gründen der Klarheit habe ich Pfeile gemalt.

V501 Links und rechts vom Operator '==' befinden sich identische Unterausdrücke: pSrc.GetLen () == pSrc.GetLen () fbxpropertytypes.h 978

 inline bool FbxTypeCopy(FbxBlob& pDst, const FbxString& pSrc) { bool lCastable = pSrc.GetLen() == pSrc.GetLen(); FBX_ASSERT( lCastable ); if( lCastable ) pDst.Assign(pSrc.Buffer(), (int)pSrc.GetLen()); return lCastable; } 

Hier möchte ich die Entwickler von AUTODESK begrüßen . Dieser Fehler stammt aus der FBX SDK- Bibliothek. Verwirrte Variablen pSrc und pDst . Ich denke, neben Lumberyard gibt es auch viele andere Benutzer, deren Projekte von Code mit diesem Fehler abhängen.

V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke: pTS-> pRT_ALD_1 && pTS-> pRT_ALD_1 d3d_svo.cpp 857

 void CSvoRenderer::ConeTracePass(SSvoTargetsSet* pTS) { .... if (pTS->pRT_ALD_1 && pTS->pRT_ALD_1) { static int nPrevWidth = 0; if (....) { .... } else { pTS->pRT_ALD_1->Apply(10, m_nTexStateLinear); pTS->pRT_RGB_1->Apply(11, m_nTexStateLinear); } } .... } 

Zurück zum Lumberyard-Code. In der Bedingung wird der gleiche Zeiger pTS-> pRT_ALD_1 überprüft . Einer von ihnen sollte pTS-> pRT_RGB_1 gewesen sein . Vielleicht ist es auch nach der Erklärung nicht sofort möglich, den Unterschied zu erkennen, aber es gibt einen: Der Unterschied liegt in den kurzen Teilzeichenfolgen ALD und RGB . Wenn Ihnen mitgeteilt wird, dass die manuelle Codeüberprüfung ausreicht, zeigen Sie dieses Beispiel.

Und wenn dieses Beispiel nicht ausreicht, gibt es 5 weitere ähnliche.

• V501 Links und rechts vom '||' befinden sich identische Unterausdrücke. Operator:! pTS-> pRT_ALD_0 ||! pTS-> pRT_ALD_0 d3d_svo.cpp 1041
• V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke: m_pRT_AIR_MIN && m_pRT_AIR_MIN d3d_svo.cpp 1808
• V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke: m_pRT_AIR_MAX && m_pRT_AIR_MAX d3d_svo.cpp 1819
• V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke: m_pRT_AIR_SHAD && m_pRT_AIR_SHAD d3d_svo.cpp 1830
• V501 Links und rechts vom Operator '&&' befinden sich identische Unterausdrücke: s_pPropertiesPanel && s_pPropertiesPanel entityobject.cpp 1700

Und wie versprochen, hier eine Liste der verbleibenden V501- Warnungen ohne Codebeispiele:

Über die Kameraposition in Spielen

Es gibt die zweitschnellste Diagnose in PVS-Studio - V502 . Diese Diagnose ist älter als einige neue Programmiersprachen, in denen ein solcher Fehler nicht mehr gemacht werden kann. Und für C ++ ist dieser Fehler möglicherweise immer relevant.

Beginnen wir mit einem kleinen einfachen Beispiel.

V502 Vielleicht arbeitet der Operator '?:' Anders als erwartet. Der Operator '?:' Hat eine niedrigere Priorität als der Operator '+'. zipencryptor.cpp 217

 bool ZipEncryptor::ParseKey(....) { .... size_t pos = i * 2 + (v1 == 0xff) ? 1 : 2; RCLogError("....", pos); return false; .... } 

Die Additionsoperation hat eine höhere Priorität als der ternäre Operator. Daher hat dieser Ausdruck eine völlig andere Berechnungslogik als der Autor angenommen hat.

Sie können den Fehler folgendermaßen beheben:

 size_t pos = i * 2 + (v1 == 0xff ? 1 : 2); 

V502 Vielleicht arbeitet der Operator '?:' Anders als erwartet. Der Operator '?:' Hat eine niedrigere Priorität als der Operator '-'. 3dengine.cpp 1898

 float C3DEngine::GetDistanceToSectorWithWater() { .... return (bCameraInTerrainBounds && (m_pTerrain && m_pTerrain->GetDistanceToSectorWithWater() > 0.1f)) ? m_pTerrain->GetDistanceToSectorWithWater() : max(camPostion.z - OceanToggle::IsActive() ? OceanRequest::GetOceanLevel() : GetWaterLevel(), 0.1f); } 

Und hier ist ein Beispielcode, in dem sie mit der Kameraposition arbeiten. Ein Beispiel ist mit den Augen schwer wahrzunehmen und es liegt ein Fehler vor. Zur Veröffentlichung wurde die Formatierung des Codes geändert, aber in der Quelldatei ist dieser Code noch unlesbarer.

In diesem Teilstring liegt ein Fehler vor:

 camPostion.z - OceanToggle::IsActive() ? .... : .... 

Wenn sich die Kamera in Ihrem Spiel plötzlich unangemessen verhält, sollten Sie wissen, dass die Entwickler bei der statischen Code-Analyse Folgendes gespeichert haben: D.

Andere Beispiele mit ähnlichen Warnungen:

• V502 Vielleicht arbeitet der Operator '?:' Anders als erwartet. Der Operator '?:' Hat eine niedrigere Priorität als der Operator '-'. scriptbind_ai.cpp 5203
• V502 Vielleicht arbeitet der Operator '?:' Anders als erwartet. Der Operator '?:' Hat eine niedrigere Priorität als der Operator '+'. qcolumnwidget.cpp 136
• V502 Vielleicht arbeitet der Operator '?:' Anders als erwartet. Der Operator '?:' Hat eine niedrigere Priorität als der Operator '&&'. Shapetool.h 98

CryEngine Legacy

Amazon Lumberyard basiert auf CryEngine- Code. Und nicht auf seiner besten Version. Ich habe eine solche Schlussfolgerung gezogen, indem ich mir den Analysatorbericht angesehen habe. Einige gefundene Fehler wurden bereits in der neuesten Version von CryEngine für meine beiden Codeüberprüfungen behoben, sind jedoch im Lumberyard-Code noch vorhanden. Außerdem wurde der Analysator im vergangenen Jahr erheblich verbessert, und es konnten zusätzliche Fehler gefunden werden, die jetzt in zwei Spiel-Engines vorhanden sind. Aber mit Lumberyard ist die Situation schlimmer. Amazon hat im Wesentlichen alle technischen Schulden von CryEngine geerbt. Aber seine eigene technische Pflicht erscheint natürlich in jeder Firma für sich :).

In diesem Abschnitt werde ich Ihnen einige Fehler nennen, die in der neuesten Version von CryEngine behoben wurden und jetzt nur noch ein Problem des Lumberyard-Projekts sind.

V519 Der Variablen 'BlendFactor [2]' werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 1283, 1284. ccrydxgldevicecontext.cpp 1284

Ungefähr solche Emotionen werden von Lumberyard-Entwicklern erfahren, wenn sie herausfinden, dass dieser Fehler nur bei ihnen geblieben ist.

Übrigens gibt es noch zwei:

• V519 Der Variablen 'm_auBlendFactor [2]' werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 919, 920. ccrydxgldevicecontext.cpp 920
• V519 Der Variablen 'm_auBlendFactor [2]' werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 926, 927. ccrydxgldevicecontext.cpp 927

Es liegt ein solcher Fehler vor:

V546 Das Mitglied einer Klasse wird von selbst initialisiert: 'eConfigMax (eConfigMax.VeryHigh)'. Partikelparameterh 1837

 ParticleParams() : .... fSphericalApproximation(1.f), fVolumeThickness(1.0f), fSoundFXParam(1.f), eConfigMax(eConfigMax.VeryHigh), // <= fFadeAtViewCosAngle(0.f) {} 

In CryEngine wurde diese Klasse im Allgemeinen neu geschrieben, aber hier blieb der Initialisierungsfehler bestehen.

V521 Solche Ausdrücke mit dem Operator ',' sind gefährlich. Stellen Sie sicher, dass der Ausdruck '! SWords [iWord] .empty (), iWord ++' korrekt ist. takticalpointsystem.cpp 3376

 bool CTacticalPointSystem::Parse(....) const { string sInput(sSpec); const int MAXWORDS = 8; string sWords[MAXWORDS]; int iC = 0, iWord = 0; for (; iWord < MAXWORDS; !sWords[iWord].empty(), iWord++) { sWords[iWord] = sInput.Tokenize("_", iC); } .... } 

Der verdächtige Zyklus, den CryEngine bereits neu geschrieben hat.

Fehler leben länger als Sie denken

Für Benutzer, die PVS-Studio zum ersten Mal verwenden, ist die Situation ungefähr dieselbe: Sie finden einen Fehler, stellen fest, dass er vor einigen Monaten hinzugefügt wurde, und freuen sich zu erkennen, dass sie die Manifestation des Problems auf wundersame Weise bei ihren Benutzern vermieden haben. Viele unserer Kunden haben PVS-Studio genau nach solchen Geschichten regelmäßig genutzt.

Manchmal muss ein Unternehmen solche Situationen mehrmals besuchen, um Prozesse zur Kontrolle der Codequalität zu starten. Hier ist ein Beispiel für CryEngine und Lumberyard:

V557 CWE-119 Array-Überlauf ist möglich. Der 'id'-Index zeigt über die Array-Grenze hinaus. gameobjectsystem.cpp 113

 uint32 CGameObjectSystem::GetExtensionSerializationPriority(....) { if (id > m_extensionInfo.size()) { return 0xffffffff; // minimum possible priority } else { return m_extensionInfo[id].serializationPriority; } } 

Wie Sie wissen, basiert Amazon Lumberyard nicht auf der neuesten Version von CryEngine. Trotzdem konnte mit Hilfe des PVS-Studio-Analysators ein Fehler gefunden werden, der jetzt in zwei Game-Engines vorhanden ist. Der Index musste mit dem Operator '> =' überprüft werden ...

Der Indizierungsfehler ist schwerwiegend. Darüber hinaus gibt es sechs solcher Orte ! Hier ist ein weiteres Beispiel:

V557 CWE-119 Array-Überlauf ist möglich. Der 'Index'-Index zeigt über die Array-Grenze hinaus. Fahrzeugsitzgruppe.cpp 73

 CVehicleSeat* CVehicleSeatGroup::GetSeatByIndex(unsigned index) { if (index >= 0 && index <= m_seats.size()) { return m_seats[index]; } return NULL; } 

Jemand hat die Fehler des gleichen Typs gemacht und sie wurden nicht behoben, nur weil sie nicht einmal in den CryEngine-Bug-Reviews enthalten waren.

Verbleibende Warnungen:

• V557 CWE-119 Array-Überlauf ist möglich. Der 'id'-Index zeigt über die Array-Grenze hinaus. gameobjectsystem.cpp 195
• V557 CWE-119 Array-Überlauf ist möglich. Der 'id'-Index zeigt über die Array-Grenze hinaus. gameobjectsystem.cpp 290
• V557 CWE-119 Array-Überlauf ist möglich. Der 'stateId'-Index zeigt über die Array-Grenze hinaus. vehicleanimation.cpp 311
• V557 CWE-119 Array-Überlauf ist möglich. Der 'stateId'-Index zeigt über die Array-Grenze hinaus. vehicleanimation.cpp 354

Das lange Vorhandensein von Fehlern im Code kann nur durch die entsprechende Ebene der Projekttests erklärt werden. Es wird angenommen, dass die statische Analyse Fehler nur in nicht verwendetem Code findet. Das ist also nicht so. Entwickler vergessen, dass die meisten Benutzer stillschweigend unter nicht offensichtlichen Fehlern in Programmen leiden, und die Manifestation dieser sehr seltenen Fehler wirkt sich häufig nachteilig auf die Arbeit des gesamten Unternehmens, den Ruf und gegebenenfalls seine Verkäufe aus.

Verschiedene Schattierungen der Copy-Paste-Programmierung

Als Sie diesen Abschnitt des Artikels erreichten, haben Sie wahrscheinlich bemerkt, dass das Kopieren und Einfügen die Ursache vieler Probleme ist. In PVS-Studio wird die Suche nach solchen Fehlern in verschiedenen Diagnosen implementiert. In diesem Abschnitt finden Sie Beispiele für das Kopieren und Einfügen des V561 .

Hier ist ein Beispiel für verdächtigen Code beim Deklarieren gleichnamiger Variablen in überlappenden Bereichen.

V561 CWE-563 Es ist wahrscheinlich besser, der Variablen 'pLibrary' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Deklaration: entityobject.cpp, Zeile 4703. entityobject.cpp 4706

 void CEntityObject::OnMenuConvertToPrefab() { .... IDataBaseLibrary* pLibrary = GetIEditor()->Get....; if (pLibrary == NULL) { IDataBaseLibrary* pLibrary = GetIEditor()->Get....; } if (pLibrary == NULL) { QString sError = tr(....); CryMessageBox(....); return; } .... } 

Der pLibrary-Zeiger überschreibt nicht wie erwartet. Die Initialisierung dieses Zeigers wurde unter der Bedingung zusammen mit der Typdeklaration vollständig kopiert.

Ich werde alle ähnlichen Orte auflisten:

• V561 CWE-563 Es ist wahrscheinlich besser, der Variablen 'eType' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Erklärung: toglsloperand.c, Zeile 838. toglsloperand.c 1224
• V561 CWE-563 Es ist wahrscheinlich besser, der Variablen 'eType' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Erklärung: toglsloperand.c, Zeile 838. toglsloperand.c 1305
• V561 CWE-563 Es ist wahrscheinlich besser, der Variablen 'rSkelPose' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Deklaration: Anhangmanager.cpp, Zeile 409. Anhangmanager.cpp 458
• V561 CWE-563 Es ist wahrscheinlich besser, der Variablen 'nThreadID' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Deklaration: d3dmeshbaker.cpp, Zeile 797. d3dmeshbaker.cpp 867
• V561 CWE-563 Es ist wahrscheinlich besser, der Variablen 'directoryNameList' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Deklaration: Assetimportermanager.cpp, Zeile 720. Assetimportermanager.cpp 728
• V561 CWE-563 Es ist wahrscheinlich besser, der Variablen 'pNode' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Deklaration: breakpointsctrl.cpp, Zeile 340. breakpointsctrl.cpp 349
• V561 CWE-563 Es ist wahrscheinlich besser, der Variablen 'pLibrary' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Deklaration: prefabobject.cpp, Zeile 1443. prefabobject.cpp 1446
• V561 CWE-563 Es ist wahrscheinlich besser, der Variablen 'pLibrary' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Deklaration: prefabobject.cpp, Zeile 1470. prefabobject.cpp 1473
• V561 CWE-563 Es ist wahrscheinlich besser, der Variablen 'cmdLine' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Deklaration: fileutil.cpp, Zeile 110. fileutil.cpp 130
• V561 CWE-563 Es ist wahrscheinlich besser, der Variablen 'sfunctionArgs' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Erklärung: attributeitemlogiccallbacks.cpp, Zeile 291. attributeitemlogiccallbacks.cpp 303
• V561 CWE-563 Es ist wahrscheinlich besser, der Variablen 'edgeName' einen Wert zuzuweisen, als ihn erneut zu deklarieren. Vorherige Deklaration: qgradientselectorwidget.cpp, Zeile 475. qgradientselectorwidget.cpp 488

Eine lange Liste ... einige der aufgelisteten Orte sind sogar vollständige Kopien des beschriebenen Beispiels.

Eigenwertinitialisierung

Im Code der Spiel-Engine wurden viele Stellen gefunden, an denen die Variable sich selbst zugewiesen ist. Irgendwo wurde dieser Code zum Debuggen gelassen, irgendwo ist der Code einfach wunderschön gestaltet (er ist auch oft eine Fehlerquelle), also werde ich Ihnen einen Code geben, der für mich am verdächtigsten ist.

V570 Die Variable 'behaviourParams.ignoreOnVehicleDestroyed' wird sich selbst zugewiesen. Fahrzeugkomponente.cpp 168

 bool CVehicleComponent::Init(....) { .... if (!damageBehaviorTable.getAttr(....) { behaviorParams.ignoreOnVehicleDestroyed = false; } else { behaviorParams.ignoreOnVehicleDestroyed = // <= behaviorParams.ignoreOnVehicleDestroyed; // <= } .... } 

In der aktuellen Ansicht wird der Zweig else überhaupt nicht benötigt. Aber vielleicht enthält dieser Code einen Fehler: Sie wollten der Variablen den entgegengesetzten Wert zuweisen:

 bValue = !bValue 

Für Entwickler ist es jedoch besser, sich mit den Ergebnissen dieser Diagnose vertraut zu machen.

Problembehandlung

In diesem Abschnitt finden Sie viele Beispiele, wenn bei der Fehlerbehandlung ein Fehler aufgetreten ist.

Beispiel 1

V606 Besitzerloses Token 'nullptr'. dx12rootsignature.cpp 599

 RootSignature* RootSignatureCache::AcquireRootSignature(....) { .... RootSignature* result = new RootSignature(m_pDevice); if (!result->Init(params)) { DX12_ERROR("Could not create root signature!"); nullptr; } m_RootSignatureMap[hash] = result; return result; } } 

Vergessen zu schreiben return nullptr; . Jetzt wird der ungültige Wert der Ergebnisvariablen an anderer Stelle im Code verwendet.

Der exakt gleiche Code wurde an eine weitere Stelle kopiert:

• V606 Besitzerloses Token 'nullptr'. dx12rootsignature.cpp 621

Beispiel 2

V606 Besitzerloses Token 'falsch'. fillspacetool.cpp 191

 bool FillSpaceTool::FillHoleBasedOnSelectedElements() { .... if (validEdgeList.size() == 2) { .... } if (validEdgeList.empty()) { .... for (int i = 0, iVertexSize(....); i < iVertexSize; ++i) { validEdgeList.push_back(....); } } if (validEdgeList.empty()) // <= { false; // <= fail } std::vector<BrushEdge3D> linkedEdgeList; std::set<int> usedEdgeSet; linkedEdgeList.push_back(validEdgeList[0]); // <= fail .... } 

Ein sehr interessantes Beispiel für einen Fehler mit einer fehlenden return-Anweisung . Jetzt ist eine Situation des Zugriffs auf einen leeren Container auf den Index möglich.

Beispiel 3

V564 CWE-480 Der Operator '&' wird auf den Wert des Bool -Typs angewendet. Sie haben wahrscheinlich vergessen, Klammern einzufügen, oder wollten den Operator '&&' verwenden. toglslinstruction.c 2914

 void SetDataTypes(....) { .... // Check assumption that both the values which MOVC might pick // have the same basic data type. if(!psContext->flags & HLSLCC_FLAG_AVOID_TEMP_REGISTER_ALIASING) { ASSERT(GetOperandDataType(psContext, &psInst->asOperands[2]) == GetOperandDataType(psContext, &psInst->asOperands[3])); } .... } 

Falsch auf das Vorhandensein von Bits im Flag überprüft. Der Negationsoperator gilt für den Flag-Wert, nicht für den gesamten Ausdruck. Schreiben Sie richtig so:

 if(!(psContext->flags & ....)) 

Weitere ähnliche Warnungen:

• V564 CWE-480 Das '|' Der Operator wird auf den Wert des Bool-Typs angewendet. Sie haben wahrscheinlich vergessen, Klammern einzufügen, oder wollten das '||' Betreiber. d3dhwshader.cpp 1832
• V564 CWE-480 Der Operator '&' wird auf den Wert des Bool-Typs angewendet. Sie haben wahrscheinlich vergessen, Klammern einzufügen, oder wollten den Operator '&&' verwenden. trackviewdialog.cpp 2112
• V564 CWE-480 Das '|' Der Operator wird auf den Wert des Bool-Typs angewendet. Sie haben wahrscheinlich vergessen, Klammern einzufügen, oder wollten das '||' Betreiber. imagecompiler.cpp 1039

Beispiel 4

V596 CWE-390 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Das Schlüsselwort 'throw' könnte fehlen: throw runtime_error (FOO); prefabobject.cpp 1491

 static std::vector<std::string> PyGetPrefabLibrarys() { CPrefabManager* pPrefabManager = GetIEditor()->GetPrefabMa....; if (!pPrefabManager) { std::runtime_error("Invalid Prefab Manager."); } .... } 

Fehler beim Auslösen einer Ausnahme. Es war notwendig, so zu schreiben:

 throw std::runtime_error("Invalid Prefab Manager."); 

Die ganze Liste solcher Fehler:

• V596 CWE-390 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Das Schlüsselwort 'throw' könnte fehlen: throw runtime_error (FOO); prefabobject.cpp 1515
• V596 CWE-390 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Das Schlüsselwort 'throw' könnte fehlen: throw runtime_error (FOO); prefabobject.cpp 1521
• V596 CWE-390 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Das Schlüsselwort 'throw' könnte fehlen: throw runtime_error (FOO); prefabobject.cpp 1543
• V596 CWE-390 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Das Schlüsselwort 'throw' könnte fehlen: throw runtime_error (FOO); prefabobject.cpp 1549
• V596 CWE-390 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Das Schlüsselwort 'throw' könnte fehlen: throw runtime_error (FOO); prefabobject.cpp 1603
• V596 CWE-390 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Das Schlüsselwort 'throw' könnte fehlen: throw runtime_error (FOO); prefabobject.cpp 1619
• V596 CWE-390 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Das Schlüsselwort 'throw' könnte fehlen: throw runtime_error (FOO); prefabobject.cpp 1644

Ein paar Probleme beim Arbeiten mit dem Speicher

V549 CWE-688 Das erste Argument der Funktion 'memcmp' entspricht dem zweiten Argument. meshutils.h 894

 struct VertexLess { .... bool operator()(int a, int b) const { .... if (m.m_links[a].links.size() != m.m_links[b].links.size()) { res = (m.m_links[a].links.size() < m.m_links[b].links.size()) ? -1 : +1; } else { res = memcmp(&m.m_links[a].links[0], &m.m_links[a].links[0], sizeof(m.m_links[a].links[0]) * m.m_links[a].links.size()); } .... } .... }; 

Die Bedingung vergleicht die Größen von zwei Vektoren.Wenn sie gleich sind, werden im else- Zweig die Werte der ersten Elemente der Vektoren mit der Funktion memcmp () verglichen . Das erste und das zweite Argument für diese Funktion sind jedoch dieselben! Der Zugriff auf Array-Elemente ist ziemlich umständlich. Es gibt Indizes a und b . Höchstwahrscheinlich ist ein Tippfehler in ihnen.

V611 CWE-762 Der Speicher wurde mit dem Operator 'new T []' zugewiesen, aber mit dem Operator 'delete' freigegeben. Überprüfen Sie diesen Code. Es ist wahrscheinlich besser, 'delete [] data;' zu verwenden. vectorn.h 102

 ~vectorn_tpl() { if (!(flags & mtx_foreign_data)) { delete[] data; } } vectorn_tpl& operator=(const vectorn_tpl<ftype>& src) { if (src.len != len && !(flags & mtx_foreign_data)) { delete data; // <= data = new ftype[src.len]; } .... } 

Der Datenzeigerspeicher wird mit einer ungültigen Anweisung freigegeben. Der Operator delete [] muss überall verwendet werden .

Nicht erreichbarer Code

V779 CWE-561 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. fbxskinimporter.cpp 67

 Events::ProcessingResult FbxSkinImporter::ImportSkin(....) { .... if (BuildSceneMeshFromFbxMesh(....) { context.m_createdData.push_back(std::move(createdData)); return Events::ProcessingResult::Success; // <= } else { return Events::ProcessingResult::Failure; // <= } context.m_createdData.push_back(); // <= fail return Events::ProcessingResult::Success; } 

Alle Zweige der bedingten Anweisung enden mit dem Verlassen der Funktion. Einige Codes werden jedoch nicht ausgeführt.

V779 CWE-561 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. Dockablelibrarytreeview.cpp 153

 bool DockableLibraryTreeView::Init(IDataBaseLibrary* lib) { .... if (m_treeView && m_titleBar && m_defaultView) { if (m_treeView->topLevelItemCount() > 0) { ShowTreeView(); } else { ShowDefaultView(); } return true; // <= } else { return false; // <= } emit SignalFocused(this); // <= fail } 

Es ist leicht, einen Fehler in diesem Code zu erkennen. Wenn Sie den Code jedoch längere Zeit schreiben, sinkt die Aufmerksamkeit stark und solche Fehler fallen leicht in das Projekt.

V622 CWE-478 Überprüfen Sie die Anweisung 'switch'. Möglicherweise fehlt der erste 'case'-Operator. datum.cpp 872

 AZ_INLINE bool IsDataGreaterEqual(....) { switch (type.GetType()) { AZ_Error("ScriptCanvas", false, "...."); return false; case Data::eType::Number: return IsDataGreaterEqual<Data::NumberType>(lhs, rhs); .... case Data::eType::AABB: AZ_Error("ScriptCanvas", false, "....", Data::Traits<Data::AABBType>::GetName()); return false; case Data::eType::OBB: AZ_Error("ScriptCanvas", false, "....", Data::Traits<Data::OBBType>::GetName()); return false; .... } 

Wenn der Switch Code außerhalb der case / default-Anweisung enthält , wird er niemals ausgeführt.

Fazit

Der Artikel enthält 95 Warnungen des Analysators, 25 davon mit Codebeispielen. Wie viel Material stammt aus dem gesamten Analysatorbericht? Ich habe schnell nur ein Drittel der High-Level-Warnungen gescrollt . Es gibt auch Medium und Low, eine Gruppe von Diagnosen für die Suche nach Optimierungen und andere ungenutzte Möglichkeiten des Analysators - dies sind Hunderte offensichtlicherer Fehler und Tausende unerforschter Warnungen.

Und dann muss sich der Leser die Frage stellen: "Ist es mit dieser Herangehensweise an das Projekt möglich, eine gute Spiel-Engine zu veröffentlichen?". Es gibt keine Codequalitätskontrolle. CryEngine-Code mit alten Fehlern wurde zugrunde gelegt, neue Fehler wurden hinzugefügt. CryEngine selbst wird erst nach der nächsten Überprüfung des Codes fertiggestellt. Amazon hat mit seinen Ressourcen jede Chance, in Richtung Codequalität zu arbeiten und die coolste Spiel-Engine herauszubringen!

Sei nicht sehr verärgert. Unter den PVS-Studio-Kunden gibt es mehr als 30 andere Unternehmen, die an Spielen beteiligt sind. Sie können sie und ihre Produkte auf der Seite unserer Website " Clients " kennenlernen, indem Sie den Filter "Spieleentwicklung" auswählen. Wir verbessern also allmählich die Welt. Vielleicht können wir Amazon Lumberyard verbessern :).

Ein Kollege hat kürzlich einen Artikel zum Thema Qualität von Spielesoftware geschrieben. Ich schlage vor, dass Sie interessiert sind: " Statische Analyse in der Videospielbranche: Top 10 Softwarefehler ."

Link zum Herunterladen des PVS-Studio- Analysators , wie kann man darauf verzichten ;-)

Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Svyatoslav Razmyslov. Amazon Lumberyard: Ein Schrei der Angst