Fehler, die die statische Code-Analyse nicht findet, weil sie nicht verwendet wird

Von Zeit zu Zeit stellen Leser unserer Artikel zur Überprüfung von Open Source-Projekten fest, dass der statische Code-Analysator PVS-Studio einen großen Prozentsatz von Fehlern erkennt, die unbedeutend sind oder die Anwendung überhaupt nicht beeinträchtigen. Das ist tatsächlich so. Die wichtigsten Fehler wurden bereits dank manueller Tests, Benutzerkritiken und anderer teurer Methoden behoben. Gleichzeitig konnten viele dieser Fehler bereits beim Schreiben des Codes gefunden und mit minimalem Zeit-, Reputations- und Geldverlust korrigiert werden. Dieser Artikel enthält einige Beispiele für echte Fehler, die sofort behoben würden, wenn die Autoren der Projekte eine statische Code-Analyse verwenden würden.

Die Idee ist sehr einfach. Schauen wir uns GitHub an, um Beispiele für Pull-Anfragen zu finden, deren Kommentare darauf hinweisen, dass dies eine Fehlerbehebung ist. Versuchen Sie, diese Fehler mit dem statischen Code-Analysator PVS-Studio zu finden. Wenn der korrigierte Fehler vom Analysegerät gefunden wird, bedeutet dies, dass er bereits beim Schreiben des Codes korrigiert werden kann. Und je früher der Fehler behoben ist, desto billiger ist er.

Leider hat GitHub uns im Stich gelassen und es nicht erlaubt, einen großartigen, intelligenten Artikel zu diesem Thema zu verfassen. GitHub selbst hat auch einen Fehler (oder eine Funktion), mit dem Sie in Pull-Anfragen in Projekten, die nur in bestimmten Programmiersprachen geschrieben wurden, nicht nach Kommentaren suchen können. Nun, oder ich weiß nicht, wie man es kocht. Unabhängig davon, was ich Ihnen sage, um in Projekten in C ++, C # oder Java nach Kommentaren zu suchen, werden die Ergebnisse in allen Sprachen angezeigt, einschließlich PHP, Python, JavaScript usw. Infolgedessen war die Suche nach geeigneten Fällen eine äußerst mühsame Aufgabe, und ich werde mich auf einige Beispiele beschränken. Sie reichen jedoch aus, um die Nützlichkeit statischer Code-Analyse-Tools bei regelmäßiger Verwendung zu demonstrieren.

Was ist, wenn der Fehler frühzeitig erkannt wurde? Die Antwort ist einfach: Programmierer müssten nicht auf ihre Manifestation warten und dann den fehlerhaften Code suchen und beheben.
Schauen wir uns die Fehler an, die PVS-Studio sofort erkennen konnte:

Das erste Beispiel stammt aus dem SatisfactoryModLoader-Projekt. Vor dem Beheben des Fehlers sah der Code folgendermaßen aus:

// gets an API function from the mod handler SML_API PVOID getAPIFunction(std::string name) { bool found = false; for (Registry reg : modHandler.APIRegistry) { if (reg.name == name) { found = true; } } if (!found) { std::string msg = ...; MessageBoxA(NULL, msg.c_str(), "SatisfactoryModLoader Fatal Error", MB_ICONERROR); abort(); } } 

Dieser Code enthielt einen Fehler, dass PVS-Studio sofort eine Warnung ausgab:

V591 Non-void-Funktion sollte einen Wert zurückgeben. ModFunctions.cpp 44

Die obige Funktion hat keine Rückgabe , daher wird ein formal undefinierter Wert zurückgegeben. Der Programmierer benutzte den Code-Analysator nicht, so dass er selbst nach einem Fehler suchen musste. Funktion nach der Bearbeitung:

 // gets an API function from the mod handler SML_API PVOID getAPIFunction(std::string name) { bool found = false; PVOID func = NULL; for (Registry reg : modHandler.APIRegistry) { if (reg.name == name) { func = reg.func; found = true; } } if (!found) { std::string msg = ...; MessageBoxA(NULL, msg.c_str(), "SatisfactoryModLoader Fatal Error", MB_ICONERROR); abort(); } return func; } 

Seltsamerweise gab der Autor beim Festschreiben den Fehler als kritisch an: " Kritischer Fehler behoben, bei dem API-Funktionen nicht zurückgegeben wurden ".

Beim zweiten Commit aus der mc6809-Projekthistorie wurden Korrekturen am folgenden Code vorgenommen:

 void mc6809dis_direct( mc6809dis__t *const dis, mc6809__t *const cpu, const char *const op, const bool b16 ) { assert(dis != NULL); assert(op != NULL); addr.b[MSB] = cpu->dp; addr.b[LSB] = (*dis->read)(dis, dis->next++); ... if (cpu != NULL) { ... } } 

Der Autor hat nur eine Zeile korrigiert. Er ersetzte den Ausdruck

 addr.b[MSB] = cpu->dp; 

auf Ausdruck

 addr.b[MSB] = cpu != NULL ? cpu->dp : 0; 

In der alten Version des Codes wurde keine CPU- Prüfung auf Gleichheit mit dem Nullzeiger durchgeführt. Wenn sich plötzlich herausstellt, dass ein Nullzeiger als zweites Argument an die Funktion mc6809dis_direct übergeben wird, wird er im Hauptteil der Funktion dereferenziert. Das Ergebnis ist bedauerlich und unvorhersehbar .

Null-Zeiger-Dereferenzierung ist eines der häufigsten Muster, über die wir informiert werden: „Dies ist kein kritischer Fehler. Nun, sie lebt in Code und lebt, und wenn eine Dereferenzierung stattfindet, stürzt das Programm leise ab und das war's. Es ist seltsam und traurig, dies von C ++ - Programmierern zu hören, aber das Leben ist das Leben.

In jedem Fall wurde eine solche Dereferenzierung in diesem Projekt immer noch zu einem Fehler, da der Commit-Header uns sagt: " Bugfix --- NULL Dereferenzierung ".

Wenn der Projektentwickler PVS-Studio verwendet hätte, hätte er vor zweieinhalb Monaten seinen Code überprüfen und eine Warnung erkennen können (so viel ist vergangen, seit der Fehler gemacht wurde):

V595 Der 'cpu'-Zeiger wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 1814, 1821. mc6809dis.c 1814

Somit würde der Fehler bereits zum Zeitpunkt seines Auftretens beseitigt, was Zeit und möglicherweise die Nerven des Entwicklers sparen würde :).

Ein Beispiel für eine weitere interessante Bearbeitung wurde im libmorton-Projekt gefunden.

Der korrigierte Code:

 template<typename morton> inline bool findFirstSetBitZeroIdx(const morton x, unsigned long* firstbit_location) { #if _MSC_VER && !_WIN64 // 32 BIT on 32 BIT if (sizeof(morton) <= 4) { return _BitScanReverse(firstbit_location, x) != 0; } // 64 BIT on 32 BIT else { *firstbit_location = 0; if (_BitScanReverse(firstbit_location, (x >> 32))) { // check first part firstbit_location += 32; return true; } return _BitScanReverse(firstbit_location, (x & 0xFFFFFFFF)) != 0; } #elif _MSC_VER && _WIN64 .... #elif __GNUC__ .... #endif } 

Bei seiner Bearbeitung ersetzt der Programmierer den Ausdruck " firstbit_location + = 32 " durch " * firstbit_location + = 32 ". Der Programmierer erwartete, dass die Zahl 32 zum Wert der Variablen hinzugefügt wird, an die der Zeiger firstbit_location gebunden ist, sie wurde jedoch direkt zum Zeiger hinzugefügt. Der geänderte Wert des Zeigers wurde nirgendwo anders verwendet, und der erwartete Wert der Variablen blieb unverändert.

PVS-Studio würde eine Warnung für diesen Code ausgeben:

V1001 Die Variable 'firstbit_location' wird zugewiesen, aber am Ende der Funktion nicht verwendet. morton_common.h 22

Es scheint, dass was in einem modifizierten, aber weiter unbenutzten Ausdruck schrecklich sein könnte? Die Diagnose von V1001 sieht offensichtlich nicht so aus, als ob besonders gefährliche Fehler identifiziert werden sollen. Trotzdem entdeckte sie einen wichtigen Fehler, der die Programmlogik beeinflusste.

Außerdem stellte sich heraus, dass dieser Fehler nicht so leicht zu finden war! Sie war nicht nur von dem Moment an im Programm, als die Datei erstellt wurde, sie überlebte auch viele Änderungen in benachbarten Zeilen und existierte 3 ( ! ) Jahre im Projekt! Während dieser ganzen Zeit war die Programmlogik kaputt und funktionierte nicht genau so, wie es die Entwickler erwartet hatten. Wenn sie PVS-Studio verwendet hätten, wäre der Fehler viel früher erkannt worden.

Betrachten Sie am Ende ein weiteres schönes Beispiel. Während ich Fehlerbehebungen auf GitHub sammelte, stieß ich mehrmals auf eine Korrektur mit diesem Inhalt . Der hier behobene Fehler:

 int kvm_arch_prepare_memory_region(...) { ... do { struct vm_area_struct *vma = find_vma(current->mm, hva); hva_t vm_start, vm_end; ... if (vma->vm_flags & VM_PFNMAP) { ... phys_addr_t pa = (vma->vm_pgoff << PAGE_SHIFT) + vm_start - vma->vm_start; ... } ... } while (hva < reg_end); ... } 

PVS-Studio hat eine Warnung zu diesem Codeabschnitt ausgegeben:

V629 Überprüfen Sie den Ausdruck 'vma-> vm_pgoff << 12'. Bitverschiebung des 32-Bit-Werts mit anschließender Erweiterung auf den 64-Bit-Typ. mmu.c 1795

Ich habe mir die Variablendeklarationen angesehen, die im Ausdruck " phys_addr_t pa = (vma-> vm_pgoff << PAGE_SHIFT) + vm_start - vma-> vm_start; " verwendet werden, und festgestellt, dass der obige Code dem folgenden synthetischen Beispiel ähnlich ist:

 void foo(unsigned long a, unsigned long b) { unsigned long long x = (a << 12) + b; } 

Wenn der Wert der 32-Bit-Variablen a größer als 0xFFFFF ist , haben die 12 höchstwertigen Bits mindestens einen Wert ungleich Null. Nach dem Anwenden der Linksverschiebungsoperation auf diese Variable gehen diese signifikanten Bits verloren, wodurch falsche Informationen in x geschrieben werden .

Um den Verlust hoher Bits zu vermeiden, müssen Sie zuerst a in den Typ unsigned long long umwandeln und erst danach die Verschiebungsoperation ausführen:

 pa = (phys_addr_t)vma->vm_pgoff << PAGE_SHIFT; pa += vm_start - vma->vm_start; 

Dann schreibt pa immer den richtigen Wert.

Alles wäre in Ordnung, aber dieser Fehler, wie das erste Beispiel aus dem Artikel, erwies sich auch als kritisch, wie der Autor des Commits in seinem Kommentar separat schrieb. Darüber hinaus ist dieser Fehler gerade in einer Vielzahl von Projekten aufgetreten. Um das Ausmaß der Tragödie vollständig einzuschätzen, empfehle ich , die Anzahl der Ergebnisse bei der Suche nach diesem Bugfix auf GitHub zu überprüfen. Beängstigend, nicht wahr?

Daher habe ich einen neuen Ansatz gewählt, um die Nützlichkeit der regelmäßigen Verwendung eines statischen Code-Analysators zu demonstrieren. Ich hoffe es hat euch gefallen. Laden Sie den statischen Code-Analysator PVS-Studio herunter und testen Sie ihn, um Ihre eigenen Projekte zu testen. Zum Zeitpunkt des Schreibens wurden etwa 700 Diagnoseregeln zum Erkennen einer Vielzahl von Fehlermustern implementiert. C, C ++, C # und Java werden unterstützt.

Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: George Gribkov. Fehler, die die statische Code-Analyse nicht findet, weil sie nicht verwendet wird