Ein weiteres Jahr neigt sich dem Ende zu und es ist eine perfekte Zeit, sich eine Tasse Kaffee zu machen und die Kritiken der Fehler, die in diesem Jahr in Open-Source-Projekten gesammelt wurden, noch einmal durchzulesen. Dies würde natürlich eine Weile dauern, daher haben wir diesen Artikel vorbereitet, um es Ihnen zu erleichtern. Heute erinnern wir uns an die interessantesten dunklen Flecken, die wir 2019 in Open-Source-C / C ++ -Projekten entdeckt haben.
Nr 10. Auf welchem Betriebssystem laufen wir?
V1040 Mögliche Tippfehler in der Schreibweise eines vordefinierten
Makronamens . Das Makro '__MINGW32_' ähnelt dem Makro '__MINGW32__'. winapi.h 4112
#if !defined(__UNICODE_STRING_DEFINED) && defined(__MINGW32_) #define __UNICODE_STRING_DEFINED #endif
Der Name des
Makros __MINGW32 _ enthält einen Tippfehler (MINGW32 wird tatsächlich von __MINGW32__ deklariert). An anderer Stelle im Projekt wird der Scheck korrekt geschrieben:
Übrigens war dieser Fehler nicht nur der erste, der im Artikel "
CMake: Der Fall, in dem die Qualität des Projekts unverzeihlich ist " beschrieben wurde, sondern auch der allererste echte Fehler, den die V1040-Diagnose in einem echten Open-Source-Projekt gefunden hat (19. August) 2019).
Nr 9. Wer ist zuerst da?
V502 Vielleicht arbeitet der Operator '?:' Anders als erwartet. Der Operator '?:' Hat eine niedrigere Priorität als der Operator '=='. mir_parser.cpp 884
enum Opcode : uint8 { kOpUndef, .... OP_intrinsiccall, OP_intrinsiccallassigned, .... kOpLast, }; bool MIRParser::ParseStmtIntrinsiccall(StmtNodePtr &stmt, bool isAssigned) { Opcode o = !isAssigned ? (....) : (....); auto *intrnCallNode = mod.CurFuncCodeMemPool()->New<IntrinsiccallNode>(....); lexer.NextToken(); if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) { intrnCallNode->SetIntrinsic(GetIntrinsicID(lexer.GetTokenKind())); } else { intrnCallNode->SetIntrinsic(static_cast<MIRIntrinsicID>(....)); } .... }
Wir interessieren uns für folgenden Teil:
if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) { .... }
Die Priorität des Operators '==' ist höher als die des ternären Operators (? :). Daher wird der Bedingungsausdruck in der falschen Reihenfolge ausgewertet und entspricht dem folgenden Code:
if ((o == !isAssigned) ? OP_intrinsiccall : OP_intrinsiccallassigned) { .... }
Da die Konstanten
OP_intrinsiccall und
OP_intrinsiccallassigned nicht null sind, wird die Bedingung die ganze Zeit
wahr zurückgegeben , was bedeutet, dass der Body des
else- Zweigs nicht erreichbarer Code ist.
Dieser Fehler wurde im Artikel "
Überprüfen des kürzlich von Huawei als Open Source erstellten Arche-Compilers " beschrieben.
Nr 8. Gefährliche bitweise Operationen
V1046 Unsichere Verwendung der Typen bool 'und' int 'zusammen in der Operation' & = '. GSLMultiRootFinder.h 175
int AddFunction(const ROOT::Math::IMultiGenFunction & func) { ROOT::Math::IMultiGenFunction * f = func.Clone(); if (!f) return 0; fFunctions.push_back(f); return fFunctions.size(); } template<class FuncIterator> bool SetFunctionList( FuncIterator begin, FuncIterator end) { bool ret = true; for (FuncIterator itr = begin; itr != end; ++itr) { const ROOT::Math::IMultiGenFunction * f = *itr; ret &= AddFunction(*f); } return ret; }
Der Code schlägt vor, dass die
SetFunctionList- Funktion eine Iteratorliste durchläuft. Wenn mindestens ein Iterator ungültig ist, gibt die Funktion
false oder andernfalls
true zurück .
Die
SetFunctionList- Funktion kann jedoch auch für gültige Iteratoren
false zurückgeben . Lassen Sie uns herausfinden, warum.
Die Funktion
AddFunction gibt die Anzahl der gültigen Iteratoren in der Liste
fFunctions zurück . Das heißt, das Hinzufügen von Nicht-Null-Iteratoren führt dazu, dass die Liste inkrementell größer wird: 1, 2, 3, 4 usw. Hier kommt der Fehler ins Spiel:
ret &= AddFunction(*f);
Da die Funktion einen Wert vom Typ
int anstelle von
bool zurückgibt, gibt die Operation '& =' für gerade Werte
false zurück , da das niedrigstwertige Bit einer geraden Zahl immer auf Null gesetzt ist. Auf diese Weise kann ein
kleiner Fehler den Rückgabewert von
SetFunctionsList unterbrechen, selbst wenn seine Argumente gültig sind.
Wenn Sie das Snippet sorgfältig gelesen haben (und Sie waren es auch?), Könnten Sie bemerkt haben, dass es aus dem Projekt ROOT stammt. Ja, wir haben es auch überprüft: "
Analyse des Code of ROOT, Scientific Data Analysis Framework ".
Nr 7. Variablen vertauscht
V1001 [CWE-563] Die Variable 'Mode' wird zugewiesen, aber am Ende der Funktion nicht verwendet. SIModeRegister.cpp 48
struct Status { unsigned Mask; unsigned Mode; Status() : Mask(0), Mode(0){}; Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) { Mode &= Mask; }; .... };
Es ist sehr gefährlich, für Funktionsargumente die gleichen Namen zu verwenden wie für Klassenmitglieder, da Sie die Gefahr haben, sie zu verwechseln. Und genau das ist hier passiert. Der folgende Ausdruck macht keinen Sinn:
Mode &= Mask;
Das Argument der Funktion ändert sich und das war's. Dieses Argument wird danach in keiner Weise mehr verwendet. Was der Programmierer eigentlich schreiben wollte, war wahrscheinlich folgendes:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) { this->Mode &= Mask; };
Dieser Fehler wurde in
LLVM gefunden . Wir haben die Tradition, dieses Projekt von Zeit zu Zeit zu überprüfen. In diesem Jahr haben wir es noch einmal
überprüft .
Nr 6. C ++ hat seine eigenen Gesetze
Dieser Fehler beruht auf der Tatsache, dass C ++ - Regeln nicht immer mathematischen Regeln oder "gesundem Menschenverstand" folgen. Schauen Sie sich das kleine Snippet unten an und versuchen Sie, den Fehler selbst zu finden.
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
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; .... } .... }
Die Bedingung scheint zu prüfen,
ob f0 gleich
f1 und gleich der Anzahl der Elemente in
m_fractureBodies ist . Es sollte wahrscheinlich überprüft werden, ob sich
f0 und
f1 am Ende des Arrays
m_fractureBodies befinden, da sie eine Objektposition enthalten, die mit der Methode
findLinearSearch () gefunden wurde . In Wirklichkeit prüft dieser Bedingungsausdruck jedoch, ob
f0 gleich
f1 ist und ob
m_fractureBodies.size () gleich dem Ergebnis des Ausdrucks
f0 == f1 ist . Das heißt, der dritte Operand hier wird gegen 0 oder 1 geprüft.
Das ist ein schöner Bug! Und zum Glück eine ziemlich seltene. Bisher haben wir es nur in drei Open-Source-Projekten gesehen, und interessanterweise waren alle drei Spiele-Engines. Dies ist nicht der einzige Fehler in Bullet. Die interessantesten wurden im Artikel "
PVS-Studio blickte in die Bullet Engine von Red Dead Redemption " beschrieben.
Nr 5. Was steht am Ende der Zeile?
Dieser ist einfach, wenn Sie ein heikles Detail kennen.
V739 EOF sollte nicht mit einem Wert vom Typ 'char' verglichen werden. Das 'ch' sollte vom Typ 'int' sein. json.cpp 762
void JsonIn::skip_separator() { signed char ch; .... if (ch == ',') { if( ate_separator ) { .... } .... } else if (ch == EOF) { .... }
Dies ist einer der Fehler, die Sie nicht leicht erkennen können, wenn Sie nicht wissen, dass
EOF als -1 definiert ist. Wenn Sie also versuchen, es mit einer Variablen mit Vorzeichen zu vergleichen, ist die Bedingung fast immer
falsch . Die einzige Ausnahme ist das als 0xFF (255) codierte Zeichen. Im Vergleich zu
EOF wird dieses Zeichen zu -1, wodurch die Bedingung erfüllt wird.
Viele Fehler in den diesjährigen Top 10 wurden in Computerspielsoftware gefunden: Engines oder Open-Source-Spiele. Wie Sie bereits vermutet haben, kam auch dieser aus dieser Gegend. Weitere Fehler sind im Artikel "
Cataclysm Dark Days Ahead: Static Analysis und Roguelike Games " beschrieben.
Nr 4. Die magische Konstante Pi
V624 Die Konstante '3.141592538' enthält wahrscheinlich einen Druckfehler. 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); .... }
Die Pi-Nummer enthält einen winzigen Tippfehler (3,141592653 ...): Bei der 7. Dezimalstelle fehlt die Zahl „6“.
Eine falsche 1-millionste Dezimalstelle würde kaum nennenswerten Schaden anrichten, es ist jedoch besser, vorhandene Konstanten aus Bibliotheken zu verwenden, deren Korrektheit garantiert ist. Die Pi-Nummer wird zum Beispiel durch die Konstante M_PI aus dem Header math.h dargestellt.
Sie haben diesen Fehler bereits in dem Artikel "
PVS-Studio hat sich mit der Bullet Engine von Red Dead Redemption befasst " gelesen, in dem es den sechsten Platz belegte. Wenn Sie es noch nicht gelesen haben, ist dies Ihre letzte Chance.
Eine kleine Ablenkung
Wir nähern uns den Top 3 der interessantesten Bugs. Wie Sie wahrscheinlich bemerkt haben, sortiere ich die Fehler nicht nach ihrer Auswirkung, sondern nach dem Aufwand, den ein menschlicher Prüfer benötigt, um sie zu finden. Schließlich ist der Vorteil der statischen Analyse gegenüber Codeüberprüfungen im Grunde die Unfähigkeit von Softwaretools, müde zu werden oder Dinge zu vergessen. :)
Nun wollen wir sehen, was wir in unseren Top 3 haben.
Nr 3. Eine schwer fassbare Ausnahme
V702- Klassen sollten immer von std :: exception (und ähnlichem) als 'public' abgeleitet werden (es wurde kein Schlüsselwort angegeben, daher verwendet der Compiler standardmäßig 'private'). CalcManager CalcException.h 4
class CalcException : std::exception { public: CalcException(HRESULT hr) { m_hr = hr; } HRESULT GetException() { return m_hr; } private: HRESULT m_hr; };
Der Analysator hat eine Klasse erkannt, die von der Klasse
std :: exception mit dem Modifikator
private abgeleitet wurde (der standardmäßig verwendet wird, wenn nichts anderes angegeben ist). Das Problem mit diesem Code besteht darin, dass das Programm beim Versuch, eine generische
std :: Ausnahmebedingung abzufangen , eine Ausnahmebedingung vom Typ
CalcException übersieht . Dieses Verhalten beruht auf der Tatsache, dass die private Vererbung die implizite Typkonvertierung verbietet.
Sie möchten definitiv nicht, dass Ihr Programm aufgrund eines fehlenden
öffentlichen Modifikators abstürzt. Übrigens, ich wette, Sie haben diese Anwendung mindestens einmal in Ihrem Leben verwendet, da es sich um den guten alten
Windows-Rechner handelt , den wir auch Anfang dieses Jahres
überprüft haben.
Nr 2. Nicht geschlossene HTML-Tags
V735 Möglicherweise falsches HTML. Das schließende Tag "</ body>" wurde gefunden, während das Tag "</ div>" erwartet wurde. book.cpp 127
static QString makeAlgebraLogBaseConversionPage() { return BEGIN INDEX_LINK TITLE(Book::tr("Logarithmic Base Conversion")) FORMULA(y = log(x) / log(a), log<sub>a</sub>x = log(x) / log(a)) END; }
Wie so oft sagt der C / C ++ - Quellcode nicht viel aus. Schauen wir uns also den vorverarbeiteten Code an, der aus dem obigen Snippet generiert wurde:
Der Analysator hat ein nicht geschlossenes
<div> -Tag gefunden. Es gibt hier viele HTML-Code-Fragmente, daher müssen die Autoren diese überarbeiten.
Überrascht, dass wir diese Art von Fehlern diagnostizieren können? Ich war auch beeindruckt, als ich das zum ersten Mal sah. Also, ja, wir wissen etwas über das Analysieren von HTML-Code. Nun, nur wenn es sich um C ++ - Code handelt. :)
Dieser Fehler steht nicht nur an zweiter Stelle, sondern ist auch ein zweiter Rechner in unserer Top-10-Liste. Weitere Informationen zu den in diesem Projekt gefundenen Fehlern finden Sie im Artikel "
Auf den Spuren der Taschenrechner: SpeedCrunch ".
Nr 1. schwer fassbare Standardfunktionen
Hier ist der Fehler zuerst platziert. Dies ist ein beeindruckend seltsamer Fehler, der es durch die Codeüberprüfung geschafft hat.
Versuchen Sie es selbst zu finden:
static int EatWhitespace (FILE * InFile) { int c; for (c = getc (InFile); isspace (c) && ('\n' != c); c = getc (InFile)) ; return (c); }
Nun wollen wir sehen, was der Analysator zu sagen hat:
V560 Ein Teil eines bedingten Ausdrucks ist immer wahr: ('\ n'! = C). params.c 136.
Seltsam, nicht wahr? Schauen wir uns einen anderen merkwürdigen Punkt an, aber in einer anderen Datei (charset.h):
#ifdef isspace #undef isspace #endif .... #define isspace(c) ((c)==' ' || (c) == '\t')
Hm, das ist in der Tat seltsam ... Wenn also die Variable
c gleich
'\ n' ist, wird die scheinbar harmlose Funktion
isspace (c) auf
false zurückgesetzt , wodurch verhindert wird, dass der zweite Teil der Prüfung aufgrund einer Kurzschlussbewertung ausgeführt wird. Und wenn
isspace (c) ausgeführt wird, ist die Variable
c entweder gleich
'' oder
'\ t', was offensichtlich nicht gleich
'\ n' ist .
Sie könnten argumentieren, dass dieses Makro ähnlich wie
#define true false ist und Code wie dieser niemals eine Codeüberprüfung durchlaufen würde. Aber dieser spezielle Ausschnitt tat es - und saß im Archiv und wartete darauf, entdeckt zu werden.
Ausführlichere Kommentare zu diesem Fehler finden Sie im Artikel "
Willst du einen Detektiv spielen? Finde den Fehler
in einer Funktion von Midnight Commander ".
Fazit
Wir haben in diesem Jahr Unmengen von Fehlern gefunden. Dies waren häufige Fehler beim Kopieren und Einfügen, ungenaue Konstanten, nicht geschlossene Tags und viele andere Fehler. Unser Analyzer entwickelt sich jedoch weiter und
lernt , immer mehr Arten von Problemen zu diagnostizieren. Daher werden wir mit Sicherheit nicht nachlassen und neue Artikel über Fehler veröffentlichen, die in Projekten genauso regelmäßig wie zuvor gefunden wurden.
Für den Fall, dass Sie unsere Artikel noch nicht gelesen haben, wurden all diese Fehler mit unserem statischen Analysator PVS-Studio gefunden. Sie können ihn gerne
herunterladen und Ihre eigenen Projekte ausprobieren. Es erkennt Fehler in Programmen, die in C, C ++, C # und Java geschrieben wurden.
Sie haben endlich die Ziellinie! Wenn Sie die ersten beiden Ebenen verpasst haben, schlage ich vor, dass Sie die Gelegenheit nutzen und diese Ebenen mit uns abschließen:
C # und
Java .