Top 10 Bugs in C ++ Projekten für 2019

Bild 7

Ein weiteres Jahr neigt sich dem Ende zu, also ist es Zeit, Kaffee zu kochen und die Fehlerberichte für das vergangene Jahr erneut zu lesen. Dies wird natürlich viel Zeit in Anspruch nehmen, daher wurde dieser Artikel verfasst. Ich schlage vor, uns die interessantesten dunklen Stellen der Projekte anzuschauen, die wir 2019 in Projekten getroffen haben, die in C und C ++ geschrieben wurden.

Zehnter Platz: "Was ist unser Betriebssystem?"


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 

Hier wurde ein Tippfehler im Makronamen __MINGW32 _ gemacht (MINGW32 deklariert __MINGW32__). An anderen Stellen des Projekts wird die Verifizierung korrekt geschrieben:

Bild 3


Dies war übrigens nicht nur der erste Fehler im Artikel „ CMake: Der Fall, dass das Projekt für die Qualität seines Codes unverzeihlich ist “, sondern im Allgemeinen der erste echte Fehler, der von der V1040-Diagnose in einem echten offenen Projekt (19. August 2019) gefunden wurde.

Neunter Platz: "Wer ist der Erste?"


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 den folgenden Teil dieses Codes:

 if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) { .... } 

Der Operator '==' hat eine höhere Priorität als der ternäre Operator (? :). Aus diesem Grund wird der Bedingungsausdruck nicht korrekt ausgewertet. Der geschriebene Code entspricht dem Folgenden:

 if ((o == !isAssigned) ? OP_intrinsiccall : OP_intrinsiccallassigned) { .... } 

Unter Berücksichtigung der Tatsache, dass die zugewiesenen Konstanten OP_intrinsiccall und OP_intrinsiccall Werte ungleich Null haben, gibt diese Bedingung immer den wahren Wert zurück. Der Body des else- Zweigs ist nicht erreichbarer Code.

Dieser Fehler ist auf den Artikel " Überprüfen des kürzlich von Huawei geöffneten Ark Compiler-Codes " zurückzuführen.

Achter Platz: "Die Gefahr bitweiser 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; } 

Basierend auf dem Code sollte die SetFunctionList- Funktion die Liste der Iteratoren umgehen. Und wenn mindestens einer von ihnen ungültig ist, ist der Rückgabewert falsch , andernfalls wahr .

In der Realität kann die SetFunctionList- Funktion jedoch auch für gültige Iteratoren false zurückgeben . Schauen wir uns die Situation an: Die Funktion AddFunction gibt die Anzahl der gültigen Iteratoren in der Liste fFunctions zurück . Das heißt Wenn Sie Iteratoren ungleich Null hinzufügen, wird die Liste nacheinander größer: 1, 2, 3, 4 usw. Hier beginnt sich der Fehler im Code zu manifestieren:

 ret &= AddFunction(*f); 

Weil Da die Funktion ein Ergebnis vom Typ int und nicht bool zurückgibt, gibt die Operation '& =' mit geraden Zahlen den Wert false zurück . Immerhin wird das niedrigstwertige Bit von geraden Zahlen immer Null sein. Daher wird ein solcher nicht offensichtlicher Fehler das Ergebnis der SetFunctionsList- Funktion auch für gültige Argumente verderben.

Wenn Sie den Code aus dem Beispiel sorgfältig gelesen haben (und genau gelesen haben, oder?), Stellen Sie möglicherweise fest, dass dies Code aus dem ROOT-Projekt ist. Natürlich haben wir es getestet: " ROOT-Code-Analyse - ein Rahmen für die Analyse wissenschaftlicher Forschungsdaten ."

Siebter Platz: „Verwirrung in den Variablen“


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, Funktionsargumenten dieselben Namen wie Klassenmitglieder zu geben. Sehr leicht zu verwechseln. Vor uns liegt so ein Fall. Dieser Ausdruck ergibt keinen Sinn:

 Mode &= Mask; 

Das Argument der Funktion ändert sich. Und alle. Dieses Argument wird nicht mehr verwendet. Am wahrscheinlichsten war es notwendig, dies zu schreiben:

 Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) { this->Mode &= Mask; }; 

Und das ist ein Fehler von LLVM . Wir haben von Zeit zu Zeit die Tradition, dieses Projekt zu analysieren. In diesem Jahr haben wir auch einen Artikel zur Verifikation.

Sechster Platz: „C ++ hat seine eigenen Gesetze“


Der folgende Fehler wird im Code angezeigt, da C ++ - Regeln nicht immer mit mathematischen Regeln oder dem "gesunden Menschenverstand" übereinstimmen. Merken Sie sich, wo der Fehler in einem kleinen Codeausschnitt liegt?

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; .... } .... } 

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 müssen, ob sich f0 und f1 am Ende des Arrays m_fractureBodies befinden , da sie die Position des Objekts enthalten, das von der Methode findLinearSearch () gefunden wurde . Tatsächlich wird dieser Ausdruck jedoch zu einem Test, um festzustellen, ob f0 und f1 gleich sind, und um dann zu überprüfen, ob m_fractureBodies.size () dem Ergebnis von f0 == f1 entspricht . 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 drei offenen Projekten getroffen, und interessanterweise waren alle nur Game-Engines. Dies ist übrigens nicht der einzige Fehler, den wir in Bullet gefunden haben. Die interessantesten sind in unserem Artikel " PVS-Studio hat sich mit der Red Dead Redemption - Bullet Engine befasst ".

Fünfter Platz: "Was ist das Ende der Linie?"


Der folgende Fehler ist leicht zu erkennen, wenn Sie über eine Subtilität Bescheid wissen.

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 möglicherweise schwer zu bemerken sind, wenn Sie nicht wissen, dass EOF als -1 definiert ist. Dementsprechend ist die Bedingung fast immer falsch , wenn Sie versuchen, sie mit einer Variablen mit Vorzeichen zu vergleichen. Die einzige Ausnahme ist, wenn der Zeichencode 0xFF (255) ist. Beim Vergleich wird ein solches Symbol zu -1 und die Bedingung ist wahr.

Es gibt eine Menge Fehler in Bezug auf Spiele in diesem Top: von Engines bis zu offenen Spielen. Wie Sie vielleicht erraten haben, kam dieser Ort auch aus dieser Gegend zu uns. Weitere Fehler finden Sie im Artikel " Cataclysm Dark Days Ahead, Static Analysis and Bagels ".

Vierter Platz: "Die Magie der Zahl 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); .... } 


Ein kleiner Tippfehler in der Zahl Pi (3.141592653 ...), bei dem die Zahl "6" an der 7. Stelle im Bruchteil fehlt.

Bild 4
Vielleicht führt der Fehler in der 10-millionsten Dezimalstelle nicht zu greifbaren Konsequenzen, aber Sie sollten trotzdem die vorhandenen Bibliothekskonstanten ohne Tippfehler verwenden. Für Pi gibt es beispielsweise eine M_PI-Konstante aus dem math.h-Header.

Dieser Fehler stammt aus dem Artikel „ PVS-Studio hat sich mit der Red Dead Redemption - Bullet Engine befasst “, der uns bereits auf dem sechsten Platz bekannt war. Wenn Sie es nicht für später verschoben haben, ist dies die letzte Chance.

Ein kleiner Exkurs


Wir befinden uns also in der Nähe der drei interessantesten Fehler. Wie Sie vielleicht bemerkt haben, werden sie nicht nach den katastrophalen Folgen ihres Vorhandenseins sortiert, sondern nach der Komplexität der Erkennung. Schließlich ist der wichtigste Vorteil der statischen Analyse gegenüber der Codeüberprüfung, dass die Maschine nicht müde wird und nichts vergisst. :)

Und jetzt mache ich Sie auf die ersten drei aufmerksam.

Bild 8


Dritter Platz: "The Elusive Exception"


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 über den Modifizierer private von der Klasse std :: exception geerbt wurde (der Standardmodifizierer, wenn nichts angegeben ist). Das Problem mit diesem Code ist, dass beim Versuch, die allgemeine Ausnahme std :: exception abzufangen , eine Ausnahme vom Typ CalcException übersprungen wird. Liegt daran, dass private Vererbung implizite Typkonvertierung ausschließt.

Ja, ich möchte nicht, dass das Programm aufgrund des fehlenden öffentlichen Modifikators abstürzt . Ich bin mir übrigens sicher, dass Sie die Anwendung in Ihrem Leben definitiv mindestens einmal verwendet haben, deren Quellcode wir uns gerade angesehen haben. Dies ist der gute alte Windows- Standardrechner, den wir auch getestet haben .

Zweiter Platz: 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 bei C / C ++ - Code ist aus der Quelle nichts ersichtlich. Wenden wir uns daher dem vorverarbeiteten Code für dieses Fragment zu:

Bild 6


Der Analysator hat ein nicht geschlossenes <div> -Tag erkannt. Es gibt viele Fragmente von HTML-Code in dieser Datei und jetzt sollte es zusätzlich von den Entwicklern überprüft werden.

Überrascht, dass wir das überprüfen können und so? Als ich das zum ersten Mal sah, war ich beeindruckt. Also analysieren wir einen Teil des HTML-Codes. Richtig, nur in C ++ Code. :)

Dies ist nicht nur der zweite Platz, sondern auch der zweite Rechner in unserer Top. Eine Liste aller Fehler finden Sie im Artikel " Auf den Spuren der Taschenrechner: SpeedCrunch ".

Erster Platz: „Schwer fassbare Standardfunktionen“


Also kamen wir zum ersten Platz. Ein beeindruckend seltsames Problem, das eine Codeüberprüfung durchlief.

Versuchen Sie es selbst zu entdecken:

 static int EatWhitespace (FILE * InFile) /* ----------------------------------------------------------------------- ** * Scan past whitespace (see ctype(3C)) and return the first non-whitespace * character, or newline, or EOF. * * Input: InFile - Input source. * * Output: The next non-whitespace character in the input stream. * * Notes: Because the config files use a line-oriented grammar, we * explicitly exclude the newline character from the list of * whitespace characters. * - Note that both EOF (-1) and the nul character ('\0') are * considered end-of-file markers. * * ----------------------------------------------------------------------- ** */ { int c; for (c = getc (InFile); isspace (c) && ('\n' != c); c = getc (InFile)) ; return (c); } /* EatWhitespace */ 

Nun wollen wir sehen, was der Analysator schwört:

V560 Ein Teil eines bedingten Ausdrucks ist immer wahr: ('\ n'! = C). params.c 136.

Merkwürdig, nicht wahr? Schauen wir uns etwas Interessantes im selben Projekt an, aber in einer anderen Datei (charset.h):

 #ifdef isspace #undef isspace #endif .... #define isspace(c) ((c)==' ' || (c) == '\t') 

Also, und das ist schon merkwürdig ... Es stellt sich heraus, dass, wenn die Variable c gleich '\ n' ist, die auf den ersten Blick absolut harmlose Funktion isspace (c) false zurückgibt und der zweite Teil dieses Tests aufgrund einer Kurzschlussbewertung nicht durchgeführt wird. Wenn isspace (c) ausgeführt wird, ist die Variable c entweder '' oder '\ t', und dies ist eindeutig nicht gleich '\ n' .

Natürlich kann man sagen, dass dieses Makro wie #define true false ist und ein solcher Code niemals die Codeüberprüfung besteht. Dieser Code bestand jedoch die Überprüfung und wartete sicher im Projekt-Repository auf uns.

Wenn Sie eine genauere Analyse des Fehlers benötigen, lesen Sie den Artikel " Für diejenigen, die den Detektiv spielen wollen: Finden Sie den Fehler in der Funktion von Midnight Commander ".

Fazit


Bild 9


Im vergangenen Jahr haben wir viele Fehler gefunden. Dies waren die üblichen Copy-Paste-Fehler, Fehler in Konstanten, nicht geschlossene Tags und viele andere Probleme. Da sich unser Analyser jedoch verbessert und lernt , nach immer mehr Fehlern zu suchen, ist dies noch lange nicht alles und neue Artikel zur Überprüfung von Projekten werden so oft wie bisher veröffentlicht.

Wenn jemand unsere Artikel zum ersten Mal liest, werde ich für alle Fälle klarstellen, dass all diese Fehler mit dem statischen Code-Analysator von PVS-Studio gefunden wurden. Wir empfehlen, ihn herunterzuladen und zu testen. Der Analyzer kann Fehler im Code von Programmen erkennen, die in den Sprachen C, C ++, C # und Java geschrieben sind.

Wir sind also am Ende angelangt! Wenn Sie die ersten beiden Ebenen verpasst haben, dann schlage ich vor, die Gelegenheit nicht zu verpassen und sie mit uns durchzugehen: C # und Java .



Wenn Sie diesen Artikel mit einem englischsprachigen Publikum teilen möchten, verwenden Sie bitte den Link zur Übersetzung: Maxim Zvyagintsev. Die 10 häufigsten Fehler in C ++ - Projekten im Jahr 2019 .

Source: https://habr.com/ru/post/de481190/


All Articles