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

Leser unserer Artikel stellen gelegentlich fest, dass der statische Code-Analysator PVS-Studio eine große Anzahl von Fehlern erkennt, die unbedeutend sind und die Anwendung nicht beeinträchtigen. Es ist wirklich so. Zum größten Teil wurden wichtige Fehler bereits durch manuelle Tests, Benutzerfeedback und andere teure Methoden behoben. Gleichzeitig konnten viele dieser Fehler beim Schreiben des Codes gefunden und mit minimalem Zeit-, Reputations- und Geldverlust behoben werden. Dieser Artikel enthält einige Beispiele für echte Fehler, die sofort behoben werden könnten, wenn die Projektautoren eine statische Code-Analyse verwendet hätten.

Die Idee ist sehr einfach. Wir werden auf GitHub nach Beispielen für Pull-Anfragen suchen, die angeben, dass ein Problem ein Bugfix ist. Dann werden wir versuchen, diese Fehler mit dem statischen Code-Analysator PVS-Studio zu finden. Wenn der Analysator einen Fehler gefunden hat, handelt es sich um einen Fehler, der möglicherweise beim Schreiben des Codes gefunden wurde. Je früher der Fehler behoben ist, desto günstiger ist er.

Leider hat GitHub uns im Stich gelassen und es ist uns nicht gelungen, einen großen, noblen Artikel zu diesem Thema zu verfassen. GitHub selbst hat einen Fehler (oder eine Funktion), mit der Sie nicht nach Kommentaren von Pull-Anforderungen in Projekten suchen können, die nur in bestimmten Programmiersprachen geschrieben wurden. Oder ich weiß nicht, wie ich es kochen soll. Trotzdem gebe ich an, nach Kommentaren in C-, C ++ - und C # -Projekten zu suchen. Die Ergebnisse werden für alle Sprachen angegeben, einschließlich PHP, Python, JavaScript und andere. Infolgedessen hat sich die Suche nach geeigneten Fällen als äußerst mühsam erwiesen, und ich werde nur einige Beispiele nennen. Sie reichen jedoch aus, um die Nützlichkeit statischer Code-Analyse-Tools bei regelmäßiger Verwendung zu demonstrieren.

Was wäre, wenn der Fehler zum frühesten Zeitpunkt erkannt worden wäre? Die Antwort ist einfach: Programmierer müssten nicht warten, bis es sich zeigt, und dann den fehlerhaften Code suchen und korrigieren.

Schauen wir uns die Fehler an, die PVS-Studio sofort hätte erkennen können:

Das erste Beispiel stammt aus dem SatisfactoryModLoader-Projekt. Vor dem Beheben des Fehlers sah der Code wie folgt 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 ausgeben würde an:

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

Die obige Funktion hat keine return- Anweisung, daher wird ein formal undefinierter Wert zurückgegeben. Der Programmierer benutzte den Code-Analysator nicht, also musste er selbst nach dem Fehler suchen. Die 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 hat der Autor beim Festschreiben den Fehler als kritisch markiert: " Kritischer Fehler behoben, bei dem API-Funktionen nicht zurückgegeben wurden ".

Beim zweiten Commit aus der mc6809-Projekthistorie wurden Änderungen im 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; 

für den folgenden

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

In der alten Codeversion wurde nicht nach einem Nullzeiger gesucht. Wenn ein Nullzeiger als zweites Argument an die Funktion mc6809dis_direct übergeben wird , tritt seine Dereferenzierung im Hauptteil der Funktion auf. Das Ergebnis ist bedauerlich und unvorhersehbar .

Nullzeiger-Dereferenzierung ist eines der häufigsten Muster, über die wir informiert werden: "Es ist kein kritischer Fehler. Wen interessiert es, dass es im Code gedeiht? Wenn eine Dereferenzierung auftritt, stürzt das Programm leise ab und das wars. “ Es ist seltsam und traurig, dies von C ++ - Programmierern zu hören, aber das Leben passiert.

Wie auch immer, in diesem Projekt hat sich eine solche Dereferenzierung in einen Fehler verwandelt, wie das Thema des Commits uns sagt: " Bugfix --- NULL Dereferenzierung ".

Wenn der Projektentwickler PVS-Studio verwendet hätte, hätte er die Warnung vor zweieinhalb Monaten überprüfen und finden können. Zu diesem Zeitpunkt wurde der Fehler eingeführt. Hier ist die Warnung:

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

Somit wäre der Fehler zum Zeitpunkt seines Auftretens behoben worden, was dem Entwickler Zeit und Nerven gespart hätte :).

Ein Beispiel für einen weiteren interessanten Fix wurde im libmorton-Projekt gefunden.

Zu reparierender 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 } 

In seiner Bearbeitung ersetzt ein Programmierer den Ausdruck " firstbit_location + = 32 " durch " * firstbit_location + = 32 ". Der Programmierer erwartete, dass 32 zum Wert der Variablen hinzugefügt wird, auf die der Zeiger firstbit_location verweist , aber 32 wurde dem Zeiger selbst hinzugefügt. Der geänderte Wert des Zeigers wurde nirgendwo mehr verwendet und der erwartete Variablenwert blieb unverändert.

PVS-Studio würde eine Warnung zu diesem Code ausgeben:

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

Was ist so schlimm an dem modifizierten, aber weiter unbenutzten Ausdruck? Die V1001-Diagnose scheint nicht dazu gedacht zu sein, besonders gefährliche Fehler zu erkennen. Trotzdem wurde ein wichtiger Fehler gefunden, der die Programmlogik beeinflusste.

Außerdem stellte sich heraus, dass dieser Fehler nicht so leicht zu finden war! Es war nicht nur im Programm, seit die Datei erstellt wurde, sondern es hat auch viele Änderungen in benachbarten Zeilen erfahren und war im Projekt bis zu 3 (!) Jahre lang vorhanden! Während dieser ganzen Zeit war die Logik des Programms gebrochen und es funktionierte nicht so, wie es die Entwickler erwartet hatten. Wenn sie PVS-Studio verwendet hätten, wäre der Fehler viel früher erkannt worden.

Schauen wir uns zum Schluss noch ein schönes Beispiel an. Während ich Fehlerbehebungen auf GitHub sammelte, stieß ich mehrmals auf eine Korrektur mit dem folgenden Inhalt . Der behobene Fehler war hier:

 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 für dieses Code-Snippet 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 Deklarationen von Variablen ausgecheckt , die im Ausdruck " phys_addr_t pa = (vma-> vm_pgoff << PAGE_SHIFT) + vm_start - vma-> vm_start; " verwendet wurden, und festgestellt, dass der oben angegebene Code dem folgenden synthetischen Beispiel entspricht:

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

Wenn der Wert einer 32-Bit-Variablen größer als 0xFFFFF ist , haben 12 höchste Bits mindestens einen Nicht-Null-Wert. Nach dem Verschieben dieser Variablen nach links gehen diese signifikanten Bits verloren, was zu falschen Informationen in x führt.

Um den Verlust hoher Bits zu vermeiden, müssen wir zuerst a in den vorzeichenlosen langen langen Typ umwandeln und erst nach dieser Verschiebung die Variable:

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

Auf diese Weise wird immer ein korrekter Wert in pa geschrieben.

Das wäre in Ordnung, aber dieser Fehler, genau wie das erste Beispiel aus dem Artikel, erwies sich ebenfalls als kritisch. Der Autor hat darüber im Kommentar geschrieben. Darüber hinaus fand dieser Fehler seinen Weg zu einer enormen Anzahl von Projekten. 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 Vorteile einer regelmäßigen Verwendung des 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 überprüfen. Zum Zeitpunkt des Schreibens sind ungefähr 700 Diagnoseregeln implementiert, um eine Vielzahl von Fehlermustern zu erkennen. Unterstützt C, C ++, C # und Java.