Geekly articles weekly

Wie man sich in C und C ++ in den Fuß schießt. Haiku OS Rezeptsammlung

Die Geschichte des Treffens des statischen Analysators PVS-Studio mit dem Haiku-Betriebssystemcode geht auf das Jahr 2015 zurück. Es war ein interessantes Experiment und eine nützliche Erfahrung für die Teams beider Projekte. Warum ein Experiment? Damals gab es keinen Analysator für Linux und es wird keine weiteren anderthalb Jahre geben. Die Arbeit der Enthusiasten unseres Teams wurde jedoch belohnt: Wir haben uns mit den Haiku-Entwicklern getroffen und die Qualität des Codes verbessert, die Datenbank mit seltenen Programmierfehlern aufgefüllt und den Analysator verfeinert. Das Überprüfen des Haiku-Codes auf Fehler ist jetzt schnell und einfach.

Bild 3


Einführung


Die Helden unserer Geschichte sind das Open-Source-Betriebssystem Haiku und der statische Analysator PVS-Studio für C, C ++, C # und Java. Als wir vor 4,5 Jahren mit der Analyse des Projekts begannen, mussten wir nur mit der kompilierten ausführbaren Datei des Analysators arbeiten. Die gesamte Infrastruktur zum Parsen von Kompilierungsparametern, Starten eines Präprozessors, Parallelisieren der Analyse usw. wurde aus dem Dienstprogramm Compiler Monitoring UI in C # übernommen, das teilweise auf die Mono-Plattform portiert wurde, um unter Linux ausgeführt zu werden. Das Haiku-Projekt selbst wird mit einem Cross-Compiler unter verschiedenen Betriebssystemen außer Windows erstellt. Ich möchte noch einmal auf die Bequemlichkeit und Vollständigkeit der Haiku-Baugruppendokumentation hinweisen und mich auch bei den Haiku-Entwicklern für ihre Hilfe beim Aufbau des Projekts bedanken.

Jetzt ist die Analyse viel einfacher. Die Liste aller Befehle zum Erstellen und Analysieren des Projekts sieht folgendermaßen aus:

cd /opt git clone https://review.haiku-os.org/buildtools git clone https://review.haiku-os.org/haiku cd ./haiku mkdir generated.x86_64; cd generated.x86_64 ../configure --distro-compatibility official -j12 \ --build-cross-tools x86_64 ../../buildtools cd ../../buildtools/jam make all cd /opt/haiku/generated.x86_64 pvs-studio-analyzer trace -- /opt/buildtools/jam/bin.linuxx86/jam \ -q -j1 @nightly-anyboot pvs-studio-analyzer analyze -l /mnt/svn/PVS-Studio.lic -r /opt/haiku \ -C x86_64-unknown-haiku-gcc -o /opt/haiku/haiku.log -j12

Die Analyse des Projekts wurde übrigens im Docker-Container durchgeführt. Kürzlich haben wir eine neue Dokumentation zu diesem Thema vorbereitet: Ausführen von PVS-Studio in Docker . Dies kann die Verwendung statischer Projektanalysetechniken für einige Unternehmen erheblich vereinfachen.

Nicht initialisierte Variablen


V614 Nicht initialisierte Variable 'rval' verwendet. fetch.c 1727

 int auto_fetch(int argc, char *argv[]) { volatile int argpos; int rval; // <= argpos = 0; if (sigsetjmp(toplevel, 1)) { if (connected) disconnect(0, NULL); if (rval > 0) // <= rval = argpos + 1; return (rval); } .... }

Die Variable rval wurde beim Deklarieren nicht initialisiert. Wenn Sie sie also mit einem Nullwert vergleichen, erhalten Sie ein undefiniertes Ergebnis. Wenn die Umstände fehlschlagen, wird der undefinierte Wert der Variablen rval möglicherweise zum Rückgabewert der Funktion auto_fetch .

V614 Nicht initialisierter Zeiger 'res' verwendet. Befehle.c 2873

 struct addrinfo { int ai_flags; int ai_family; int ai_socktype; int ai_protocol; socklen_t ai_addrlen; char *ai_canonname; struct sockaddr *ai_addr; struct addrinfo *ai_next; }; static int sourceroute(struct addrinfo *ai, char *arg, char **cpp, int *lenp, int *protop, int *optp) { static char buf[1024 + ALIGNBYTES]; char *cp, *cp2, *lsrp, *ep; struct sockaddr_in *_sin; #ifdef INET6 struct sockaddr_in6 *sin6; struct ip6_rthdr *rth; #endif struct addrinfo hints, *res; // <= int error; char c; if (cpp == NULL || lenp == NULL) return -1; if (*cpp != NULL) { switch (res->ai_family) { // <= case AF_INET: if (*lenp < 7) return -1; break; .... } } .... }

Ein ähnlicher Fall der Verwendung einer nicht initialisierten Variablen, nur hier ist der nicht initialisierte Zeiger res .

V506 Der Zeiger auf die lokale Variable 'normalisiert' wird außerhalb des Bereichs dieser Variablen gespeichert. Ein solcher Zeiger wird ungültig. TextView.cpp 5596

 void BTextView::_ApplyStyleRange(...., const BFont* font, ....) { if (font != NULL) { BFont normalized = *font; _NormalizeFont(&normalized); font = &normalized; } .... fStyles->SetStyleRange(fromOffset, toOffset, fText->Length(), mode, font, color); }

Wahrscheinlich musste der Programmierer das Objekt durch eine Zwischenvariable normalisieren. Jetzt wurde im Schriftzeiger der Zeiger auf das normalisierte temporäre Objekt gespeichert , der zerstört wird, nachdem der Bereich verlassen wurde, in dem dieses temporäre Objekt erstellt wurde.

V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 27

 int8 BUnicodeChar::Type(uint32 c) { BUnicodeChar(); return u_charType(c); }

Ein sehr häufiger Fehler unter C ++ - Programmierern besteht darin, den Konstruktoraufruf zu verwenden, um angeblich die Klassenfelder zu initialisieren / auf Null zu setzen. In diesem Fall werden die Felder der Klasse nicht geändert, es wird jedoch ein neues unbenanntes Objekt dieser Klasse erstellt, das sofort zerstört wird. Leider gibt es viele solcher Orte:

  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 37
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 49
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 58
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 67
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 77
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 89
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 103
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 115
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 126
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 142
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 152
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 163
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 186
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 196
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 206
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 214
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 222
  • V603 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> BUnicodeChar :: BUnicodeChar (....)' verwendet werden. UnicodeChar.cpp 230

V670 Das nicht initialisierte Klassenmitglied 'fPatternHandler' wird verwendet, um das 'fInternal'-Mitglied zu initialisieren. Denken Sie daran, dass Mitglieder in der Reihenfolge ihrer Deklarationen innerhalb einer Klasse initialisiert werden. Painter.cpp 184

 Painter::Painter() : fInternal(fPatternHandler), .... fPatternHandler(), .... { .... }; class Painter { .... private: mutable PainterAggInterface fInternal; // line 336 bool fSubpixelPrecise : 1; bool fValidClipping : 1; bool fDrawingText : 1; bool fAttached : 1; bool fIdentityTransform : 1; Transformable fTransform; float fPenSize; const BRegion* fClippingRegion; drawing_mode fDrawingMode; source_alpha fAlphaSrcMode; alpha_function fAlphaFncMode; cap_mode fLineCapMode; join_mode fLineJoinMode; float fMiterLimit; PatternHandler fPatternHandler; // line 355 mutable AGGTextRenderer fTextRenderer; };

Ein weiteres Beispiel für eine falsche Initialisierung. Klassenfelder werden in der Reihenfolge initialisiert, in der sie in der Klasse selbst deklariert sind. In diesem Beispiel wird das interne Feld zuerst mit dem nicht initialisierten Wert fPatternHandler initialisiert.

Verdächtig #define


V523 Die Anweisung 'then' entspricht der Anweisung 'else'. subr_gtaskqueue.c 191

 #define TQ_LOCK(tq) \ do { \ if ((tq)->tq_spin) \ mtx_lock_spin(&(tq)->tq_mutex); \ else \ mtx_lock(&(tq)->tq_mutex); \ } while (0) #define TQ_ASSERT_LOCKED(tq) mtx_assert(&(tq)->tq_mutex, MA_OWNED) #define TQ_UNLOCK(tq) \ do { \ if ((tq)->tq_spin) \ mtx_unlock_spin(&(tq)->tq_mutex); \ else \ mtx_unlock(&(tq)->tq_mutex); \ } while (0) void grouptask_block(struct grouptask *grouptask) { .... TQ_LOCK(queue); gtask->ta_flags |= TASK_NOENQUEUE; gtaskqueue_drain_locked(queue, gtask); TQ_UNLOCK(queue); }

Das Code-Snippet sieht erst dann verdächtig aus, wenn Sie sich das Ergebnis des Präprozessors ansehen:

 void grouptask_block(struct grouptask *grouptask) { .... do { if ((queue)->tq_spin) mtx_lock(&(queue)->tq_mutex); else mtx_lock(&(queue)->tq_mutex); } while (0); gtask->ta_flags |= 0x4; gtaskqueue_drain_locked(queue, gtask); do { if ((queue)->tq_spin) mtx_unlock(&(queue)->tq_mutex); else mtx_unlock(&(queue)->tq_mutex); } while (0); }

Der Analysator hat wirklich Recht - die if- und else- Zweige sind identisch. Aber wohin gingen die Funktionen mtx_lock_spin und mtx_unlock_spin ? Die Makros TQ_LOCK , TQ_UNLOCK und die Funktion grouptask_block werden in derselben Datei deklariert und fast nebeneinander, es gab jedoch irgendwo eine Ersetzung.

Eine Suche nach dem Inhalt der Dateien ergab nur mutex.h mit folgendem Inhalt:

 /* on FreeBSD these are different functions */ #define mtx_lock_spin(x) mtx_lock(x) #define mtx_unlock_spin(x) mtx_unlock(x)

Ob eine solche Ersetzung korrekt ist oder nicht, sollte für die Projektentwickler überprüft werden. Ich habe das Projekt unter Linux überprüft, und eine solche Ersetzung schien mir verdächtig.

Fehler mit der freien Funktion


V575 Der Nullzeiger wird an die Funktion 'frei' übergeben. Überprüfen Sie das erste Argument. setmime.cpp 727

 void MimeType::_PurgeProperties() { fShort.Truncate(0); fLong.Truncate(0); fPrefApp.Truncate(0); fPrefAppSig.Truncate(0); fSniffRule.Truncate(0); delete fSmallIcon; fSmallIcon = NULL; delete fBigIcon; fBigIcon = NULL; fVectorIcon = NULL; // <= free(fVectorIcon); // <= fExtensions.clear(); fAttributes.clear(); }

Sie können einen Nullzeiger an die freie Funktion übergeben, aber ein solcher Eintrag ist eindeutig verdächtig. Der Analysator fand also die verwirrten Codezeilen. Zuerst mussten Sie den Speicher mit dem Zeiger fVectorIcon freigeben und erst dann auf NULL setzen .

V575 Der Nullzeiger wird an die Funktion 'frei' übergeben. Überprüfen Sie das erste Argument. driver_settings.cpp 461

 static settings_handle * load_driver_settings_from_file(int file, const char *driverName) { .... handle = new_settings(text, driverName); if (handle != NULL) { // everything went fine! return handle; } free(handle); // <= .... }

Ein weiteres Beispiel für die explizite Übergabe eines Nullzeigers an die freie Funktion. Diese Zeile kann gelöscht werden, weil Nach erfolgreichem Empfang des Zeigers wird die Funktion beendet.

V575 Der Nullzeiger wird an die Funktion 'frei' übergeben. Überprüfen Sie das erste Argument. PackageFileHeapWriter.cpp 166

 void* _GetBuffer() { .... void* buffer = malloc(fBufferSize); if (buffer == NULL && !fBuffers.AddItem(buffer)) { free(buffer); throw std::bad_alloc(); } return buffer; }

Hier ist ein Fehler. Anstelle des Operators && muss der Operator || verwendet werden. Nur in diesem Fall wird eine Ausnahme std :: bad_alloc () ausgelöst, wenn es nicht möglich war, Speicher mit der Funktion malloc zuzuweisen.

Fehler mit Löschoperator


V611 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 [] fMsg;' zu verwenden. Err.cpp 65

 class Err { public: .... private: char *fMsg; ssize_t fPos; }; void Err::Unset() { delete fMsg; // <= fMsg = __null; fPos = -1; } void Err::SetMsg(const char *msg) { if (fMsg) { delete fMsg; // <= fMsg = __null; } if (msg) { fMsg = new(std::nothrow) char[strlen(msg)+1]; // <= if (fMsg) strcpy(fMsg, msg); } }

Der fMsg- Zeiger wird verwendet, um Speicher zum Speichern eines Array von Zeichen zuzuweisen, und der Löschoperator wird verwendet, um Speicher freizugeben , anstatt delete [] .

V611 Der Speicher wurde mit dem Operator 'new' zugewiesen, aber mit der Funktion 'free' freigegeben. Überprüfen Sie die Operationslogik hinter der Variablen 'wrapperPool'. vm_page.cpp 3080

 status_t vm_page_write_modified_page_range(....) { .... PageWriteWrapper* wrapperPool = new(malloc_flags(allocationFlags)) PageWriteWrapper[maxPages + 1]; PageWriteWrapper** wrappers = new(malloc_flags(allocationFlags)) PageWriteWrapper*[maxPages]; if (wrapperPool == NULL || wrappers == NULL) { free(wrapperPool); // <= free(wrappers); // <= wrapperPool = stackWrappersPool; wrappers = stackWrappers; maxPages = 1; } .... }

Hier ist malloc_flags die Funktion, die malloc macht. Und dann konstruiert Placement-New das Objekt hier. Und da die PageWriteWrapper- Klasse wie folgt implementiert ist:

 class PageWriteWrapper { public: PageWriteWrapper(); ~PageWriteWrapper(); void SetTo(vm_page* page); bool Done(status_t result); private: vm_page* fPage; struct VMCache* fCache; bool fIsActive; }; PageWriteWrapper::PageWriteWrapper() : fIsActive(false) { } PageWriteWrapper::~PageWriteWrapper() { if (fIsActive) panic("page write wrapper going out of scope but isn't completed"); }

dann werden Objektdestruktoren dieser Klasse aufgrund der Verwendung der freien Funktion zum Freigeben von Speicher nicht aufgerufen.

V611 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 [] fOutBuffer;' zu verwenden. Überprüfen Sie die Zeilen: 26, 45. PCL6Rasterizer.h 26

 class PCL6Rasterizer : public Rasterizer { public: .... ~PCL6Rasterizer() { delete fOutBuffer; fOutBuffer = NULL; } .... virtual void InitializeBuffer() { fOutBuffer = new uchar[fOutBufferSize]; } private: uchar* fOutBuffer; int fOutBufferSize; };

Die Verwendung des Löschoperators anstelle von delete [] ist ein sehr häufiger Fehler. Der einfachste Weg, beim Schreiben einer Klasse einen Fehler zu machen, ist Der Destruktorcode befindet sich häufig weit entfernt von den Speicherzuordnungsorten. Hier gibt der Programmierer durch den Zeiger fOutBuffer fälschlicherweise Speicher im Destruktor frei .

V772 Das Aufrufen eines Löschoperators für einen ungültigen Zeiger führt zu undefiniertem Verhalten. Hashtable.cpp 207

 void Hashtable::MakeEmpty(int8 keyMode,int8 valueMode) { .... for (entry = fTable[index]; entry; entry = next) { switch (keyMode) { case HASH_EMPTY_DELETE: // TODO: destructors are not called! delete (void*)entry->key; break; case HASH_EMPTY_FREE: free((void*)entry->key); break; } switch (valueMode) { case HASH_EMPTY_DELETE: // TODO: destructors are not called! delete entry->value; break; case HASH_EMPTY_FREE: free(entry->value); break; } next = entry->next; delete entry; } .... }

Zusätzlich zur falschen Wahl zwischen delete / delete [] und free können Sie dem Programm auf andere Weise undefiniertes Verhalten hinzufügen - versuchen Sie, den Speicher durch einen Zeiger auf einen undefinierten Typ (void *) zu löschen.

Funktionen ohne Rückgabewert


V591 Non-void-Funktion sollte einen Wert zurückgeben. Referenceable.h 228

 BReference& operator=(const BReference<const Type>& other) { fReference = other.fReference; }

Der überschriebene Zuweisungsoperator hat nicht genügend Rückgabewert. In diesem Fall gibt der Bediener einen zufälligen Wert zurück, was zu seltsamen Fehlern führen kann.

Ähnliche Probleme in anderen Codefragmenten dieser Klasse:

  • V591 Non-void-Funktion sollte einen Wert zurückgeben. Referenceable.h 233
  • V591 Non-void-Funktion sollte einen Wert zurückgeben. Referenceable.h 239

V591 Non-void-Funktion sollte einen Wert zurückgeben. main.c 1010

 void errx(int, const char *, ...) ; char * getoptionvalue(const char *name) { struct option *c; if (name == NULL) errx(1, "getoptionvalue() invoked with NULL name"); c = getoption(name); if (c != NULL) return (c->value); errx(1, "getoptionvalue() invoked with unknown option '%s'", name); /* NOTREACHED */ }

Ein NOTREACHED-Benutzerkommentar bedeutet hier nichts. Um Code für solche Szenarien korrekt zu schreiben, müssen Sie Funktionen wie noreturn markieren. Hierfür gibt es noreturn-Attribute: standard- und compilerspezifisch. Zuallererst werden solche Attribute von Compilern für die korrekte Codegenerierung oder Benachrichtigung über einige Arten von Fehlern mithilfe von Warings berücksichtigt. Verschiedene statische Analysewerkzeuge berücksichtigen auch Attribute, um die Analysequalität zu verbessern.

Ausnahmebehandlung


V596 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Das Schlüsselwort 'throw' könnte fehlen: throw ParseException (FOO); Response.cpp 659

 size_t Response::ExtractNumber(BDataIO& stream) { BString string = ExtractString(stream); const char* end; size_t number = strtoul(string.String(), (char**)&end, 10); if (end == NULL || end[0] != '\0') ParseException("Invalid number!"); return number; }

Hier wird das Schlüsselwort throw versehentlich vergessen. Daher wird eine ParseException nicht ausgelöst , und ein Objekt dieser Klasse wird einfach zerstört, wenn es den Bereich verlässt. Danach setzt die Funktion ihre Arbeit fort, als wäre nichts passiert, als wäre die richtige Nummer eingegeben worden.

V1022 Eine Ausnahme wurde vom Zeiger ausgelöst. Ziehen Sie es stattdessen nach Wert. gensyscallinfos.cpp 316

 int main(int argc, char** argv) { try { return Main().Run(argc, argv); } catch (Exception& exception) { // <= fprintf(stderr, "%s\n", exception.what()); return 1; } } int Run(int argc, char** argv) { .... _ParseSyscalls(argv[1]); .... } void _ParseSyscalls(const char* filename) { ifstream file(filename, ifstream::in); if (!file.is_open()) throw new IOException(string("Failed to open '") + filename + "'."); // <= .... }

Der Analysator hat eine vom Zeiger ausgelöste IOException erkannt. Wenn Sie einen Zeiger werfen, wird die Ausnahme nicht abgefangen, sodass die Ausnahme schließlich als Referenz abgefangen wird. Die Verwendung eines Zeigers zwingt den Interceptor außerdem, den Löschoperator aufzurufen, um das erstellte Objekt zu zerstören, was ebenfalls nicht erfolgt.

Zwei weitere Problembereiche des Codes:

  • V1022 Eine Ausnahme wurde vom Zeiger ausgelöst. Ziehen Sie es stattdessen nach Wert. gensyscallinfos.cpp 347
  • V1022 Eine Ausnahme wurde vom Zeiger ausgelöst. Ziehen Sie es stattdessen nach Wert. gensyscallinfos.cpp 413

Formale Sicherheit


V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'f_key' geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. dst_api.c 1018

 #ifndef SAFE_FREE #define SAFE_FREE(a) \ do{if(a != NULL){memset(a,0, sizeof(*a)); free(a); a=NULL;}} while (0) .... #endif DST_KEY * dst_free_key(DST_KEY *f_key) { if (f_key == NULL) return (f_key); if (f_key->dk_func && f_key->dk_func->destroy) f_key->dk_KEY_struct = f_key->dk_func->destroy(f_key->dk_KEY_struct); else { EREPORT(("dst_free_key(): Unknown key alg %d\n", f_key->dk_alg)); } if (f_key->dk_KEY_struct) { free(f_key->dk_KEY_struct); f_key->dk_KEY_struct = NULL; } if (f_key->dk_key_name) SAFE_FREE(f_key->dk_key_name); SAFE_FREE(f_key); return (NULL); }

Der Analysator hat verdächtigen Code erkannt, mit dem private Daten sicher bereinigt werden können. Leider macht das SAFE_FREE- Makro, das zu Memset , kostenlosen Aufrufen und NULL- Zuweisung erweitert wurde, den Code nicht sicherer, weil All dies wird vom Compiler während der O2- Optimierung entfernt.

Dies ist übrigens nichts anderes als CWE-14 : Compiler-Entfernung von Code zum Löschen von Puffern.

Die gesamte Liste der Stellen, an denen Puffer nicht gelöscht werden:

  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der Puffer 'encoded_block' geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. dst_api.c 446
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'key_st' geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. dst_api.c 685
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der Puffer 'in_buff' geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. dst_api.c 916
  • V597 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem das Objekt 'ce' gelöscht wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. fs_cache.c 1078

Vorzeichenlose Vergleiche


V547 Der Ausdruck 'verbleibend <0' ist immer falsch. Der vorzeichenlose Typwert ist niemals <0. DwarfFile.cpp 1947

 status_t DwarfFile::_UnwindCallFrame(....) { .... uint64 remaining = lengthOffset + length - dataReader.Offset(); if (remaining < 0) return B_BAD_DATA; .... }

Der Analysator fand einen expliziten Vergleich einer vorzeichenlosen Variablen mit negativen Werten. Vielleicht sollten Sie die verbleibende Variable mit nur Null vergleichen oder eine Überlaufprüfung durchführen.

V547 Der Ausdruck 'sleep ((unsigned) secs) <0' ist immer falsch. Der vorzeichenlose Typwert ist niemals <0. Misc.cpp 56

 status_t snooze(bigtime_t amount) { if (amount <= 0) return B_OK; int64 secs = amount / 1000000LL; int64 usecs = amount % 1000000LL; if (secs > 0) { if (sleep((unsigned)secs) < 0) // <= return errno; } if (usecs > 0) { if (usleep((useconds_t)usecs) < 0) return errno; } return B_OK; }

Um zu verstehen, was der Fehler ist, wenden wir uns den Signaturen der Schlaf- und Schlaffunktionen zu :

 extern unsigned int sleep (unsigned int __seconds); extern int usleep (__useconds_t __useconds);

Wie wir sehen können, gibt die Sleep- Funktion einen Wert eines vorzeichenlosen Typs zurück und seine Verwendung im Code ist falsch.

Gefährliche Hinweise


V774 Der Zeiger 'Gerät' wurde verwendet, nachdem der Speicher freigegeben wurde. xhci.cpp 1572

 void XHCI::FreeDevice(Device *device) { uint8 slot = fPortSlots[device->HubPort()]; TRACE("FreeDevice() port %d slot %d\n", device->HubPort(), slot); // Delete the device first, so it cleans up its pipes and tells us // what we need to destroy before we tear down our internal state. delete device; DisableSlot(slot); fDcba->baseAddress[slot] = 0; fPortSlots[device->HubPort()] = 0; // <= delete_area(fDevices[slot].trb_area); delete_area(fDevices[slot].input_ctx_area); delete_area(fDevices[slot].device_ctx_area); memset(&fDevices[slot], 0, sizeof(xhci_device)); fDevices[slot].state = XHCI_STATE_DISABLED; }

Ein Geräteobjekt wird vom Löschoperator gelöscht . Dies ist eine logische Aktion für eine Funktion namens FreeDevice . Um jedoch aus irgendeinem Grund andere Ressourcen im Code freizugeben, wird erneut ein bereits gelöschtes Objekt angesprochen.

Ein solcher Code ist äußerst gefährlich und kommt an mehreren anderen Stellen vor:

  • V774 Der 'Selbst'-Zeiger wurde verwendet, nachdem der Speicher freigegeben wurde. TranslatorRoster.cpp 884
  • V774 Der 'String'-Zeiger wurde verwendet, nachdem der Speicher freigegeben wurde. RemoteView.cpp 1269
  • V774 Der Zeiger 'bs' wurde verwendet, nachdem der Speicher freigegeben wurde. mkntfs.c 4291
  • V774 Der Zeiger 'bs' wurde verwendet, nachdem der Speicher freigegeben wurde. mkntfs.c 4308
  • V774 Der 'al'-Zeiger wurde verwendet, nachdem der Speicher neu zugewiesen wurde. inode.c 1155

V522 Eine Dereferenzierung des Nullzeigers 'Daten' kann stattfinden. Der Nullzeiger wird an die Funktion 'malo_hal_send_helper' übergeben. Untersuchen Sie das dritte Argument. Überprüfen Sie die Zeilen: 350, 394. if_malohal.c 350

 static int malo_hal_fwload_helper(struct malo_hal *mh, char *helper) { .... /* tell the card we're done and... */ error = malo_hal_send_helper(mh, 0, NULL, 0, MALO_NOWAIT); // <= NULL .... } static int malo_hal_send_helper(struct malo_hal *mh, int bsize, const void *data, size_t dsize, int waitfor) { mh->mh_cmdbuf[0] = htole16(MALO_HOSTCMD_CODE_DNLD); mh->mh_cmdbuf[1] = htole16(bsize); memcpy(&mh->mh_cmdbuf[4], data , dsize); // <= data .... }

Die interprozedurale Analyse ergab eine Situation, in der NULL an die Funktion übergeben wird und der Datenzeiger mit diesem Wert anschließend in der memcpy- Funktion dereferenziert wird.

V773 Die Funktion wurde beendet, ohne den Zeiger 'inputFileFile' freizugeben. Ein Speicherverlust ist möglich. command_recompress.cpp 119

 int command_recompress(int argc, const char* const* argv) { .... BFile* inputFileFile = new BFile; error = inputFileFile->SetTo(inputPackageFileName, O_RDONLY); if (error != B_OK) { fprintf(stderr, "Error: Failed to open input file \"%s\": %s\n", inputPackageFileName, strerror(error)); return 1; } inputFile = inputFileFile; .... }

PVS-Studio kann Speicherlecks erkennen . Hier wird im Falle eines Fehlers kein Speicher für inputFileFile freigegeben. Jemand glaubt, dass Sie sich im Fehlerfall nicht darum kümmern können, Speicher freizugeben - das Programm endet trotzdem. Dies ist jedoch nicht immer der Fall. Behandeln Sie Fehler richtig und arbeiten Sie weiter - eine Voraussetzung für so viele Programme.

V595 Der Zeiger 'fReply' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 49, 52. ReplyBuilder.cpp 49

 RPC::CallbackReply* ReplyBuilder::Reply() { fReply->Stream().InsertUInt(fStatusPosition, _HaikuErrorToNFS4(fStatus)); fReply->Stream().InsertUInt(fOpCountPosition, fOpCount); if (fReply == NULL || fReply->Stream().Error() == B_OK) return fReply; else return NULL; }

Wie oft dereferenzieren Entwickler Zeiger, bevor sie sie überprüfen. Diagnostics V595 ist fast immer führend in der Anzahl der Warnungen in einem Projekt. Die gefährliche Verwendung des fReply- Zeigers in diesem Code.

V595 Der Zeiger 'mq' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 782, 786. oce_queue.c 782

 static void oce_mq_free(struct oce_mq *mq) { POCE_SOFTC sc = (POCE_SOFTC) mq->parent; struct oce_mbx mbx; struct mbx_destroy_common_mq *fwcmd; if (!mq) return; .... }

Ein ähnliches Beispiel. Der mg- Zeiger wird einige Zeilen früher dereferenziert, als er auf einen Nullwert überprüft wird. Es gibt viele ähnliche Stellen im Projekt. An einigen Stellen ist die Verwendung und Überprüfung von Zeigern weit voneinander entfernt, sodass der Artikel zwei Beispiele enthält. Der Rest kann die Entwickler im vollständigen Analysebericht sehen.

Verschiedene Fehler


V645 Der Funktionsaufruf 'strncat' kann zum Pufferüberlauf 'output' führen. Die Grenzen sollten nicht die Größe des Puffers enthalten, sondern eine Anzahl von Zeichen, die er enthalten kann. NamespaceDump.cpp 101

 static void dump_acpi_namespace(acpi_ns_device_info *device, char *root, int indenting) { char output[320]; char tabs[255] = ""; .... strlcat(tabs, "|--- ", sizeof(tabs)); .... while (....) { uint32 type = device->acpi->get_object_type(result); snprintf(output, sizeof(output), "%s%s", tabs, result + depth); switch(type) { case ACPI_TYPE_INTEGER: strncat(output, " INTEGER", sizeof(output)); break; case ACPI_TYPE_STRING: strncat(output, " STRING", sizeof(output)); break; .... } .... } .... }

Der Unterschied zwischen den Funktionen strlcat und strncat ist für eine Person, die mit der Beschreibung dieser Funktionen noch nicht vertraut ist, nicht ganz offensichtlich. Die Funktion strlcat verwendet die Größe des gesamten Puffers als drittes Argument, und die Funktion strncat verwendet die Größe des freien Speicherplatzes im Puffer , für den vor dem Aufruf der Funktion der gewünschte Wert berechnet werden muss. Aber Entwickler vergessen es oft oder wissen es nicht. Das Übergeben der strncat- Funktion an die Größe des gesamten Puffers kann zu einem Pufferüberlauf führen, weil Die Funktion betrachtet diesen Wert als die Anzahl der Zeichen, die zum Kopieren zugelassen sind. Die strlcat- Funktion hat dieses Problem nicht, aber damit es richtig funktioniert, müssen Zeichenfolgen übergeben werden, die mit Terminal Null enden.

Die ganze Liste gefährlicher Orte mit Linien:

  • V645 Der Funktionsaufruf 'strncat' kann zum Pufferüberlauf 'output' führen. Die Grenzen sollten nicht die Größe des Puffers enthalten, sondern eine Anzahl von Zeichen, die er enthalten kann. NamespaceDump.cpp 104
  • V645 Der Funktionsaufruf 'strncat' kann zum Pufferüberlauf 'output' führen. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 107
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 110
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 113
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 118
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 119
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 120
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 123
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 126
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 129
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 132
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 135
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 138
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 141
  • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 144
  • V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 283
  • V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 284
  • V645 Der Funktionsaufruf 'strncat' kann zum Pufferüberlauf 'features_string' führen. Die Grenzen sollten nicht die Größe des Puffers enthalten, sondern eine Anzahl von Zeichen, die er enthalten kann. VirtioDevice.cpp 285

V792 Die Funktion 'SetDecoratorSettings' befindet sich rechts vom Operator '|' wird unabhängig vom Wert des linken Operanden aufgerufen. Vielleicht ist es besser, '||' zu verwenden. DesktopListener.cpp 324

 class DesktopListener : public DoublyLinkedListLinkImpl<DesktopListener> { public: .... virtual bool SetDecoratorSettings(Window* window, const BMessage& settings) = 0; .... }; bool DesktopObservable::SetDecoratorSettings(Window* window, const BMessage& settings) { if (fWeAreInvoking) return false; InvokeGuard invokeGuard(fWeAreInvoking); bool changed = false; for (DesktopListener* listener = fDesktopListenerList.First(); listener != NULL; listener = fDesktopListenerList.GetNext(listener)) changed = changed | listener->SetDecoratorSettings(window, settings); return changed; }

Höchstwahrscheinlich sind die Operatoren '|' und '||'. Ein solcher Fehler führt zu unnötigen Aufrufen der SetDecoratorSettings- Funktion .

V627 Überprüfen Sie den Ausdruck. Das Argument von sizeof () ist das Makro, das zu einer Zahl erweitert wird. device.c 72

 #define PCI_line_size 0x0c /* (1 byte) cache line size in 32 bit words */ static status_t wb840_open(const char* name, uint32 flags, void** cookie) { .... data->wb_cachesize = gPci->read_pci_config(data->pciInfo->bus, data->pciInfo->device, data->pciInfo->function, PCI_line_size, sizeof(PCI_line_size)) & 0xff; .... }

Transfer des Betreibers sizeof Wert 0x0c sieht verdächtig. Vielleicht sollten Sie die Größe eines Objekts berechnen, z. B. Daten .

V562 Es ist seltsam, einen Wert vom Typ Bool mit einem Wert von 18 zu vergleichen: 0x12 == IsProfessionalSpdif (). CEchoGals_mixer.cpp 533

 typedef bool BOOL; virtual BOOL IsProfessionalSpdif() { ... } #define ECHOSTATUS_DSP_DEAD 0x12 ECHOSTATUS CEchoGals::ProcessMixerFunction(....) { .... if ( ECHOSTATUS_DSP_DEAD == IsProfessionalSpdif() ) // <= { Status = ECHOSTATUS_DSP_DEAD; } else { pMixerFunction->Data.bProfSpdif = IsProfessionalSpdif(); } .... }

Die IsProfessionalSpdif- Funktion gibt einen Wert vom Typ bool zurück , während unter der Bedingung das Ergebnis der Funktion mit der Zahl 0x12 verglichen wird .

Fazit


Wir haben die Veröffentlichung der ersten Haiku Beta im letzten Herbst verpasst waren damit beschäftigt, PVS-Studio für die Java-Sprache freizugeben. Die Art der Programmiererfehler ist jedoch so, dass sie nur dann auftreten, wenn Sie nach ihnen suchen und die Qualität des Codes überhaupt nicht beachten. Die Entwickler haben versucht, Coverity Scan zu verwenden , aber der letzte Durchlauf der Analyse war vor fast zwei Jahren. Dies sollte Haiku-Benutzer verärgern. Obwohl die Analyse mit Coverity im Jahr 2014 konfiguriert wurde, hat uns dies nicht davon abgehalten, 2015 zwei große Artikel mit Fehlerprüfungen zu schreiben ( Teil 1 , Teil 2 ).

Diejenigen, die bis zum Ende gelesen haben, warten auf eine weitere Überprüfung des Haiku-Codes von nicht weniger Volumen mit neuen Arten von Fehlern. Vor dem Veröffentlichen von Codeüberprüfungen wird ein vollständiger Analysebericht an die Entwickler gesendet, sodass einige Fehler behoben werden können. Um die Zeit zwischen den Veröffentlichungen zu vertreiben, empfehle ich, PVS-Studio für Ihr Projekt herunterzuladen und auszuprobieren .

Möchten Sie Haiku ausprobieren und haben Sie Fragen? Haiku-Entwickler laden Sie zum Telegrammkanal ein .



Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Svyatoslav Razmyslov. Wie man sich in C und C ++ in den Fuß schießt. Haiku OS Kochbuch

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

More articles:


All Articles