Geekly articles weekly

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

Die Geschichte, wie sich der statische Analysator PVS-Studio und der Haiku-Betriebssystemcode kennengelernt haben, geht auf das Jahr 2015 zurück. Es war ein aufregendes Experiment und eine nützliche Erfahrung für die Teams beider Projekte. Warum das Experiment? In diesem Moment hatten wir den Analysator für Linux nicht und wir würden ihn noch anderthalb Jahre nicht haben. Wie auch immer, die Bemühungen von Enthusiasten aus unserem Team wurden belohnt: Wir haben uns mit Haiku-Entwicklern getroffen und die Codequalität erhöht, unsere Fehlerbasis mit seltenen Fehlern von Entwicklern erweitert und den Analysator verfeinert. Jetzt können Sie den Haiku-Code einfach und schnell auf Fehler überprüfen.
Bild 1


Einführung


Lernen Sie die Hauptfiguren unserer Geschichte kennen - das Haiku mit Open Source Code und den statischen Analysator PVS-Studio für C, C ++, C # und Java. Als wir uns vor 4,5 Jahren mit der Projektanalyse befassten, mussten wir uns nur mit der kompilierten ausführbaren Analysedatei befassen. Die gesamte Infrastruktur zum Parsen von Compilerparametern, Ausführen eines Präprozessors, Parallelschalten der Analyse usw. wurde aus der in C # geschriebenen Benutzeroberfläche des Dienstprogramms Compiler Monitoring entnommen. Dieses Dienstprogramm wurde teilweise auf die Mono-Plattform portiert, um unter Linux ausgeführt zu werden. Das Haiku-Projekt wird mit dem Cross-Compiler unter verschiedenen Betriebssystemen außer Windows erstellt. Ich möchte noch einmal die Bequemlichkeit und Vollständigkeit der Dokumentation im Zusammenhang mit dem Haiku-Gebäude erwähnen. Außerdem möchte ich den Haiku-Entwicklern für ihre Hilfe beim Aufbau des Projekts danken.

Es ist jetzt viel einfacher, die Analyse durchzuführen. Hier ist die Liste aller Befehle zum Erstellen und Analysieren des Projekts:

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 Projektanalyse wurde übrigens in einem Docker-Container implementiert. Kürzlich haben wir eine neue Dokumentation zu diesem Thema vorbereitet: Ausführen von PVS-Studio in Docker . Dies kann es einigen Unternehmen sehr leicht machen, statische Analysetechniken für ihre Projekte anzuwenden.

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 bei der Deklaration nicht initialisiert, daher führt der Vergleich mit dem Nullwert zu einem undefinierten Ergebnis. Wenn die Umstände fehlschlagen, kann der unsichere Wert der Variablen rval zu einem Rückgabewert der Funktion auto_fetch werden.

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

Hier ist ein ähnlicher Fall der Verwendung der nicht initialisierten Variablen, außer dass res ein nicht initialisierter Zeiger ist, der hier stattfindet.

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

Der Programmierer musste das Objekt wahrscheinlich mithilfe einer Zwischenvariablen normalisieren. Jetzt enthält der Schriftzeiger den Zeiger auf das normalisierte Objekt, der nach dem Verlassen des Bereichs, in dem das temporäre Objekt erstellt wurde, entfernt wird.

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 Aufruf des Konstruktors zu verwenden, um angeblich Klassenfelder zu initialisieren / aufzuheben. In diesem Fall erfolgt keine Änderung der Klassenfelder, aber ein neues unbenanntes Objekt dieser Klasse wird erstellt und sofort zerstört. Leider gibt es viele solcher Stellen im Projekt:

  • 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 ihrer Deklaration in der Klasse selbst initialisiert. In diesem Beispiel wird das Feld " Intern " als erstes 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); }

Dieses Code-Snippet sieht erst dann verdächtig aus, wenn Sie sich das Präprozessor-Ergebnis 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 - wenn und sonst Zweige identisch sind. Aber wo sind die Funktionen mtx_lock_spin und mtx_unlock_spin ? Die Makros TQ_LOCK , TQ_UNLOCK und die Funktion grouptask_block werden in einer Datei fast nebeneinander deklariert, dennoch fand hier irgendwo ein Ersatz statt.

Das Durchsuchen der Dateien führte nur zu 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)

Projektentwickler sollten prüfen, ob ein solcher Ersatz korrekt ist oder nicht. Ich habe dieses Projekt unter Linux überprüft und ein solches Ersetzen 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 den Nullzeiger in der freien Funktion übergeben, aber eine solche Verwendung ist definitiv verdächtig. Somit fand der Analysator gemischte Codezeilen. Zuerst musste der Code-Autor den Speicher durch den fVectorIcon- Zeiger freigeben , nachdem er NULL zugewiesen hatte.

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); // <= .... }

Dies ist ein weiteres Beispiel für die explizite Übergabe eines Nullzeigers an die freie Funktion. Diese Zeile kann gelöscht werden, da die Funktion nach erfolgreichem Abrufen des Zeigers beendet wird.

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

Jemand hat hier einen Fehler gemacht. Der Operator || muss anstelle von && verwendet werden. Nur in diesem Fall wird die Ausnahme std :: bad_alloc () ausgelöst, wenn die Speicherzuweisung (mit der Funktion malloc ) fehlgeschlagen ist.

Fehler mit dem 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 für ein Array von Zeichen zuzuweisen. Der Löschoperator wird verwendet, um den Speicher freizugeben, anstatt [] zu löschen .

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 eine Funktion, die malloc aufruft. Dann erstellt platzierungsneu das Objekt hier. Da die PageWriteWrapper- Klasse folgendermaßen implementiert wird:

 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"); }

Die Objektdestruktoren dieser Klasse werden 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; };

Es ist ein häufiger Fehler, den Löschoperator anstelle von delete [] zu verwenden. Es ist am einfachsten, beim Schreiben einer Klasse einen Fehler zu machen, da der Code des Destruktors häufig weit von den Speicherorten entfernt ist. Hier gibt der Programmierer den vom fOutBuffer- Zeiger im Destruktor gespeicherten Speicher fälschlicherweise 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 zu einer falschen Auswahl zwischen Löschen / Löschen [] und Frei können Sie auch auf undefiniertes Verhalten stoßen, wenn Sie versuchen, den Speicher durch einen Zeiger auf den Void-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; }

Einem überladenen Zuweisungsoperator fehlt ein Rückgabewert. In diesem Fall gibt der Bediener einen zufälligen Wert zurück, was zu seltsamen Fehlern führen kann.

Hier sind ä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 */ }

Der Kommentar eines Benutzers NOTREACHED bedeutet hier nichts. Sie müssen Funktionen als noreturn mit Anmerkungen versehen, um Code für solche Szenarien ordnungsgemäß schreiben zu können. Zu diesem Zweck gibt es noreturn-Attribute: standard und compilerspezifisch. Zuallererst werden diese Attribute von Compilern für die ordnungsgemäße Codegenerierung oder Benachrichtigung über bestimmte Arten von Fehlern mithilfe von Warnungen berücksichtigt. Verschiedene statische Analysewerkzeuge berücksichtigen auch Attribute, um die Analysequalität zu verbessern.

Ausnahmen behandeln


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

Das Schlüsselwort throw wurde hier versehentlich vergessen. Daher wird die ParseException- Ausnahme nicht generiert, während das Objekt dieser Klasse beim Verlassen des Bereichs einfach zerstört wird. Danach arbeitet die Funktion weiter, 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 die vom Zeiger ausgelöste IOException- Ausnahme erkannt. Das Werfen eines Zeigers führt dazu, dass die Ausnahme nicht abgefangen wird. Die Ausnahme wird also schließlich durch Bezugnahme erfasst. Darüber hinaus zwingt die Verwendung eines Zeigers die Fangseite, den Löschoperator aufzurufen, um das erstellte Objekt zu zerstören, was noch nicht geschehen war.

Einige andere Codefragmente mit Problemen:

  • 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, der für die sichere Löschung privater Daten vorgesehen ist. Leider macht das SAFE_FREE- Makro, das sich in das Memset , kostenlose Aufrufe und die NULL- Zuweisung erweitert, den Code nicht sicherer, da alles vom Compiler direkt bei der Optimierung mit O2 entfernt wird .

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

Hier ist die Liste der Stellen, an denen das Löschen von Puffern tatsächlich nicht durchgeführt wird:

  • 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

Vergleiche mit vorzeichenlosen Variablen


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 der vorzeichenlosen Variablen mit negativen Werten. Vielleicht sollte man die verbleibende Variable nur mit null vergleichen oder eine Überprüfung auf Überlauf 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 den Hauptpunkt des Fehlers zu ermitteln, gehen wir auf die Signaturen der Schlaf- und Schlaffunktionen ein :

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

Wie wir sehen können, gibt die Sleep- Funktion den vorzeichenlosen Wert 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 freigegeben. Für die FreeDevice- Funktion ist dies ziemlich logisch. Aus irgendeinem Grund wird jedoch das bereits entfernte Objekt adressiert, um andere Ressourcen freizugeben.

Ein solcher Code ist äußerst gefährlich und kann an mehreren anderen Stellen angewendet werden:

  • 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 den Fall, dass NULL an die Funktion übergeben wird und der Datenzeiger mit einem solchen Wert schließlich 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 . In diesem Beispiel wird im Falle eines Fehlers der Speicher nicht freigegeben. Jemand könnte denken, dass Sie sich im Fehlerfall nicht um die Speicherfreigabe kümmern sollten, da das Programm immer noch endet. Das ist aber nicht immer so. Viele Programme müssen Fehler korrekt behandeln und weiterarbeiten.

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

Es ist ein sehr häufiger Fehler, Zeiger zu dereferenzieren, bevor sie überprüft werden. Die V595- Diagnose hat fast immer Vorrang vor der Anzahl der Warnungen in einem Projekt. Dieses Codefragment enthält die gefährliche Verwendung des fReply- Zeigers.

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 mehrere Zeilen früher dereferenziert, als er auf null geprüft wurde. Es gibt viele ähnliche Stellen im Projekt. In einigen Snippets sind die Verwendung und Überprüfung von Zeigern ziemlich weit voneinander entfernt. In diesem Artikel finden Sie daher nur einige Beispiele. Entwickler können gerne weitere Beispiele im vollständigen Analysebericht lesen.

Verschiedenes


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 strlcat- und strncat- Funktionen ist für jemanden, der mit der Beschreibung dieser Funktionen nicht vertraut ist, nicht sehr offensichtlich. Die Funktion strlcat erwartet die Größe des gesamten Puffers als drittes Argument, während die Funktion strncat - die Größe des freien Speicherplatzes in einem Puffer, für den vor dem Aufrufen der Funktion ein erforderlicher Wert ausgewertet werden muss. Aber Entwickler vergessen es oft oder wissen es nicht. Das Übergeben der gesamten Puffergröße an die strncat- Funktion kann zu einem Pufferüberlauf führen, da die Funktion diesen Wert als akzeptable Anzahl zu kopierender Zeichen betrachtet. Die strlcat- Funktion hat kein solches Problem. Sie müssen jedoch Zeichenfolgen übergeben, die mit dem Terminal null enden, damit es ordnungsgemäß funktioniert.

Hier ist die gesamte Liste gefährlicher Orte mit Zeichenfolgen:

  • 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. Die Grenzen sollten nicht die Größe des Puffers enthalten, sondern eine Anzahl von Zeichen, die er enthalten kann. NamespaceDump.cpp 107
  • 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 110
  • 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 113
  • 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 118
  • 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 119
  • 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 120
  • 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 123
  • 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 126
  • 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 129
  • 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 132
  • 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 135
  • 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 138
  • 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 141
  • 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 144
  • 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 283
  • 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 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 '|' und '||' Die Betreiber waren durcheinander. Dieser 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; .... }

Das Übergeben des Werts 0x0c an den Operator sizeof sieht verdächtig aus. Vielleicht hätte der Autor die Größe eines Objekts bewerten sollen, zum Beispiel 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 Funktion IsProfessionalSpdif gibt den Wert des Bool- Typs zurück. Dabei wird das Ergebnis der Funktion mit der Zahl 0x12 in der Bedingung verglichen.

Fazit


Wir haben die Veröffentlichung der ersten Haiku-Beta im letzten Herbst verpasst, da wir gerade mit der Veröffentlichung von PVS-Studio für Java beschäftigt waren. Programmierfehler sind jedoch so beschaffen, dass sie nicht verschwinden, wenn Sie nicht nach ihnen suchen und nicht auf die Codequalität achten. Die Projektentwickler verwendeten Coverity Scan , aber der letzte Lauf war vor fast zwei Jahren. Dies muss für Haiku-Benutzer ärgerlich sein. Obwohl die Analyse 2014 mit Coverity konfiguriert wurde, hat sie uns nicht davon abgehalten, 2015 zwei lange Artikel zur Fehlerüberprüfung zu schreiben ( Teil 1 , Teil 2 ).

Eine weitere Haiku-Überprüfung der Fehler erscheint in Kürze für diejenigen, die diesen Beitrag bis zum Ende lesen. Der vollständige Analysatorbericht wird an die Entwickler gesendet, bevor diese Fehlerüberprüfung veröffentlicht wird. Daher können einige Fehler behoben sein, wenn Sie diese lesen. Um die Zeit zwischen den Artikeln zu vertreiben, empfehle ich, PVS-Studio für Ihr Projekt herunterzuladen und auszuprobieren .

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

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

More articles:


All Articles