Der Code der 1980er Jahre, der moderne Zuverlässigkeit unterstützt – und die Fehler, die er immer noch macht

Der Code für einige Erlang/OTP-Module ist älter als die meisten modernen Junior-Entwickler. Diese Dateien sind die wahren digitalen Patriarchen. Seit Jahrzehnten haben sie sichergestellt, dass Bankgeschäfte, Telefonnetze und Messaging-Systeme ordnungsgemäß funktionieren. Wir haben beschlossen, einen Blick unter die Kappe dieser langlebigen Sprache zu werfen, um zu sehen, was genau hinter den Linien liegt, auf die Millionen von Benutzern heute vertrauen. Einführung Bevor wir über Warnungen sprechen, möchte ich ein paar Worte über das Projekt sagen. Erlangs Reise begann 1986 in einem der Laboratorien von Ericsson. Joe Armstrong experimentierte mit Prolog, während er versuchte, ein Telefonprogramm zu bauen. ist eine Programmiersprache, die für fehlertolerante Systeme konzipiert wurde, bei denen ein Versagen inakzeptabel ist. Denken Sie an einen Telefonschalter, bei dem ein gefallener Anruf als Notfall zählt, oder an eine Banktransaktion, die unabhängig davon durchlaufen muss. Dies sind die Aufgaben, für die es verwendet wird. Erlang Erlang geht Hand in Hand mit der , ein Framework, das leistungsstarke Tools, Muster und Verhaltensweisen bietet, um verteilte, fehler-tolerante Anwendungen zu erstellen. Dieses integrierte Toolkit ermöglicht es Systemen, die mit ihm geschrieben wurden, jahrelang kontinuierlich zu laufen, ohne jemals aufhören zu müssen. Offene Telekommunikationsplattform (OTP) Wir haben das Projekt nach diesen Wir haben es bei der Branch verwendet Und die Wir hatten keine Probleme mit dem Aufbau des Projekts, also kudos an die Entwickler. Anweisungen maint-28 Der PVS-Studio Analyzer Visual Studio Code anzeigen Der Artikel enthält nur High- und Medium-Level-Warnungen. Das schließt unsere Einführung ab. Jetzt gehen wir zu den Warnungen. Wir werden sie in drei Kategorien aufteilen: Logische Irrtümer kritische Fehler zu machen; memory errors. Logische Fehler Fragment N1 Das PVS-Studio warnt vor: Es gibt identische Unterausdrücke '(!esock_is_integer((env), (argv[3])))' auf der linken und rechten Seite des Operators 'annoo. Die V501 prim_socket_nif.c 603 static ERL_NIF_TERM nif_sendfile(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) { .... BOOLEAN_T a2ok; if ((! (a2ok = GET_INT64(env, argv[2], &offset64))) || (! GET_UINT64(env, argv[3], &count64u))) { if ((! IS_INTEGER(env, argv[3])) || (! IS_INTEGER(env, argv[3]))) return enif_make_badarg(env); if (! a2ok) return esock_make_error_integer_range(env, argv[2]); else return esock_make_error_integer_range(env, argv[3]); } .... } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Im Code wollten die Entwickler die dritten und vierten Argumente in 64-Bit-Typen (die signierten und nicht signierten) konvertieren.Die Funktion kehrt früh zurück, wenn eines der Argumente keine Zahl ist. Der festgelegte Code: .... if ((! IS_INTEGER(env, argv[2])) || (! IS_INTEGER(env, argv[3]))) return enif_make_badarg(env); .... Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Fragment N2 Das PVS-Studio warnt vor: Es gibt identische Unterausdrücke 'n->type == ycf_node_type_on_save_yield_state_code' auf der linken und rechten Seite des Operators ' Fhane. Die V501 ycf_node.c 218 static void uniqify_local_vars_in_node(ycf_node* n) { if (n->type == ycf_node_type_code_scope) { uniqify_local_vars_in_scope(&n->u.code_scope); } else if(n->type == ycf_node_type_on_restore_yield_state_code) { uniqify_local_vars_in_scope(&n->u.special_code_block .code.if_statement->u.code_scope); } else if (n->type == ycf_node_type_on_save_yield_state_code || n->type == ycf_node_type_on_save_yield_state_code || n->type == ycf_node_type_on_destroy_state_code || n->type == ycf_node_type_on_return_code || n->type == ycf_node_type_on_destroy_state_or_return_code) { uniqify_local_vars_in_scope(&n->u.special_code_block .code.if_statement->u.code_scope); } .... } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Hier ist ein weiterer interessanter Copy-Paste-Fall.Wir haben die Funktion, die wiederkehrend mit Baumknoten umgeht und Sichtbereiche mit lokalen Variablen findet. Es ruft dann die Variablen-Namen-Unifikationsfunktion für jedes dieser Bereiche auf. Branch, der Die Bedingung wird nach dem Der Operator. uniqify_local_vars_in_node else if n->type == ycf_node_type_on_save_yield_state_code || Nach der Vergleichskette werden die Werte der folgenden Liste in der Reihenfolge sortiert, in der sie angegeben sind: typedef enum { .... ycf_node_type_on_save_yield_state_code, ycf_node_type_on_restore_yield_state_code, ycf_node_type_on_destroy_state_code, ycf_node_type_on_return_code, ycf_node_type_on_destroy_state_or_return_code } ycf_node_type; Enter fullscreen mode Exit fullscreen mode Jedoch, die Konstant wird in der vorherigen Also, ich denke, es gibt einen doppelten Check hier, und wir können es löschen. ycf_node_type_on_restore_yield_state_code else if Fragment N3 Das PVS-Studio warnt vor: Mögliche Typo in der Rechtschreibung eines vordefinierten Makro-Namens. „Macro ist ähnlich“ '. V1040 WIN_32 WIN32 inet_drv.c 13506 static int tcp_send_error(tcp_descriptor* desc, int err) { /* EPIPE errors usually occur in one of three ways: * 1. We write to a socket when we've already shutdown() the write side. * On Windows the error returned for this is ESHUTDOWN * rather than EPIPE. * 2. The TCP peer sends us an RST through no fault of our own (perhaps * by aborting the connection using SO_LINGER) and we then attempt * to write to the socket. On Linux and Windows we would actually * receive an ECONNRESET error for this, but on the BSDs, Darwin, * Illumos and presumably Solaris, it's an EPIPE. * 3. We cause the TCP peer to send us an RST by writing to a socket * after we receive a FIN from them. Our first write will be * successful, but if the they have closed the connection (rather * than just shutting down the write side of it) this will cause their * OS to send us an RST. Then, when we attempt to write to the socket * a second time, we will get an EPIPE error. On Windows we get an * ECONNABORTED. * * What we are going to do here is to treat all EPIPE messages that aren't * of type 1 as ECONNRESET errors. This will allow users who have the * show_econnreset socket option enabled to receive {error, econnreset} on * both send and recv operations to indicate that an RST * has been received. */ #ifdef __WIN_32__ if (err == ECONNABORTED) err = ECONNRESET; #endif if (err == EPIPE && !(desc->tcp_add_flags & TCP_ADDF_SHUTDOWN_WR_DONE)) err = ECONNRESET; return tcp_send_or_shutdown_error(desc, err); } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Dies ist ein eher ungewöhnlicher Fall der Überprüfung, ob ein Projekt unter Windows kompiliert. ] , [ ] , [ ], etc.), aber dies ist einzigartig. Dies sieht sehr ähnlich wie ein einfacher Typ (wahrscheinlich sollte es der ) 1 2 3 __WIN32__ Aber warum ist das so? aus Neugier suchte ich nach Mal sehen, was ich gefunden habe, hier sind die Ergebnisse: "__WIN_32__" Wie Sie sehen können, gibt es fast keine Ergebnisse.Es ist lustig, dass einer der Links zu der Datei führt, die wir betrachten . I grepped through the project to find no similar macro definitions. I also thought it might be built into some compiler, like MinGW. Unfortunately, the Nach dem Versuch, den Zweck des Makros zu verstehen, glaube ich, dass dieser Code fehlerhaft ist. Debian Quellen Experimentieren Als Fix schlage ich vor, den Scheck zu verwenden, den ich früher in anderen Teilen des Projekts gefunden habe: static int tcp_send_error(tcp_descriptor* desc, int err) { .... #if defined(__WIN32__) || defined(_WIN32) || defined(_WIN32_) if (err == ECONNABORTED) err = ECONNRESET; #endif .... } Enter fullscreen mode Exit fullscreen mode Fragment N4 Das PVS-Studio warnt vor: Die operative Logik des Codes entspricht nicht seiner Formatierung. Die zweite Aussage wird immer ausgeführt. V640 Dialysator.c 246 int main(int argc, char** argv) { .... if (argc > 2 && strcmp(argv[1], "+P") == 0) { PUSH2("+P", argv[2]); argc--, argv++; argc--, argv++; } else PUSH2("+P", "1000000"); .... } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Hier ist ein interessantes Beispiel für einen gemeinsamen Codierungsfehler, der später schwer zu erkennen sein kann. Die Struktur: PUSH2 #define QUOTE(s) s #define PUSH(s) eargv[eargc++] = QUOTE(s) #define PUSH2(s, t) PUSH(s); PUSH(t) Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Uh-huh, es ist ein Makro, das sich in zwei Konstrukte erweitert. int main(int argc, char** argv) { .... if (argc > 2 && strcmp(argv[1], "+P") == 0) { eargv[eargc++] = "+P"; eargv[eargc++] = argv[2]; argc--, argv++; argc--, argv++; } else eargv[eargc++] = "+P"; eargv[eargc++] = "1000000"; .... } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Infolgedessen, wenn die Prüfung die , nur die erste Aufgabe wird bedingt ausgeführt werden, während die zweite immer nach dem Zweig auftreten wird. false Schreiben und Verwenden von Makros Die perfekte Lösung wäre, sie loszuwerden, aber lassen Sie uns für eine weniger radikale Fix. Konstruktion, wenn sie mehrere Aktionen in einem Makro kombinieren müssen. Bugs anziehen do {...} while (0) #define PUSH2(s, t) do { PUSH(s); PUSH(t); } while (0) Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Fragment N5 The PVS-Studio warning: Die Bedingung 'i >= 0' der Schleife ist immer wahr. V654 wxe_return.cpp 236 ERL_NIF_TERM wxeReturn::make_array_objs(wxAuiPaneInfoArray& arr, WxeApp *app, const char *cname) { ERL_NIF_TERM head, tail; ERL_NIF_TERM class_name = enif_make_atom(env, cname); tail = enif_make_list(env, 0); for (unsigned int i = arr.GetCount() - 1; i >= 0; i--) { head = make_ref(app->getRef((void *) &arr.Item(i),memenv), class_name); tail = enif_make_list_cell(env, head, tail); } return tail; } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Werfen Sie einen Blick auf die Lücke, die besondere Aufmerksamkeit auf ihren Zustand schenkt. Die Induktionsvariable ist von der Typ mit einem Wertbereich von , wird die Schleife unendlich. Nachdem die Schleife mit Die Variable wird gleich als Folge der Abnahme, und der Kreislauf geht weiter. for i unsigned int [0; UINT_MAX] i == 0 UINT_MAX Let's fix the code as follows: for (int64_t i = arr.GetCount() - 1; i >= 0; i--) { .... } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Ähnliche Warnungen: V654 Die Bedingung 'i >= 0' der Schleife ist immer wahr. wxe_return.cpp 248 V654 Die Bedingung 'i >= 0' der Schleife ist immer wahr. wxe_return.cpp 260 Fragment N6 Das PVS-Studio warnt vor: Die Verwendung von "wenn (A) {...} anders wenn (A) {...}" Muster wurde erkannt. Es besteht die Wahrscheinlichkeit eines logischen Fehlers. Checklinien: 2903, 3053. V517 erl_bif_info.c 2903 BIF_RETTYPE system_info_1(BIF_ALIST_1) { .... if (is_tuple(BIF_ARG_1)) { // L2778 .... } else if (BIF_ARG_1 == am_scheduler_id) { // L2782 .... } .... else if (BIF_ARG_1 == am_garbage_collection) { // L2903 .... } else if (BIF_ARG_1 == am_fullsweep_after) { // L2921 .... } else if (BIF_ARG_1 == am_garbage_collection) { // L3053 .... } else if (BIF_ARG_1 == am_instruction_counts) { // L3056 .... } .... else if (ERTS_IS_ATOM_STR("halt_flush_timeout", BIF_ARG_1)) { // L3552 .... } } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Der Analyzer hat mehrere Zweige mit identischen Kontrollen in einer Funktion erkannt, die eine große Anzahl von Aussagen – ungefähr Jeder von ihnen hat jedoch eine andere Logik: die und . Given the number of branches and the 150-line gap between the duplicates, it's hardly surprising that this could happen. Static analysis helps prevent such cases. Unfortunately, I can't think of a fix, so I'll leave that to the developers. if-else if 800 Linien lang Erste Prüfung Zweite Prüfung Fragment N7 Das PVS-Studio warnt vor: Ein Teil des bedingten Ausdrucks ist immer falsch: (Arity == 0). V560 Bischofschaft 3145 BIF_RETTYPE delete_element_2(BIF_ALIST_3) { .... ptr = tuple_val(BIF_ARG_2); arity = arityval(*ptr); ix = signed_val(BIF_ARG_1); if ((ix arity) || (arity == 0)) { BIF_ERROR(BIF_P, BADARG); } .... } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Hier haben wir ein Beispiel, das Mathematik beinhaltet.Der Analysator informiert uns, dass der letzte Untertitel ist immer falsch.Warum? arity == 0 Wenn der Steuerungsfluss den Punkt erreicht, an dem dieser Unterausdruck bewertet wird, werden wir wissen, dass die beiden vorangegangenen Unterausdrücke Wir können auch Folgendes lernen: false ix >= 1 ix = 1 arity Ich werde auch die Fixierung dieses Codes den Entwicklern überlassen, da mein einziger Vorschlag darin besteht, die Bedingung zu entfernen. Critical errors Fragment N8 Die PVS-Studio warnt vor: Der Wert des Index 'prefix_len' könnte 262 erreichen. Die V557 erl_alloc_util.c 5375 Der Wert des Index 'prefix_len' könnte 262 erreichen. Die V557 erl_alloc_util.c 5378 Der Wert des Index 'prefix_len' könnte 262 erreichen. Die V557 erl_alloc_util.c 5381 #define MAX_ATOM_CHARACTERS 255 static void make_name_atoms(Allctr_t *allctr) { char alloc[] = "alloc"; char realloc[] = "realloc"; char free[] = "free"; char buf[MAX_ATOM_CHARACTERS]; size_t prefix_len = sys_strlen(allctr->name_prefix); if (prefix_len > MAX_ATOM_CHARACTERS + sizeof(realloc) - 1) erts_exit(ERTS_ERROR_EXIT, "Too long allocator name: %salloc\n" allctr->name_prefix); sys_memcpy((void *) buf, (void *) allctr->name_prefix, prefix_len); sys_memcpy((void *) &buf[prefix_len], (void *) alloc, sizeof(alloc) - 1); allctr->name.alloc = am_atom_put(buf, prefix_len + sizeof(alloc) - 1); sys_memcpy((void *) &buf[prefix_len], (void *) realloc, sizeof(realloc) - 1); allctr->name.realloc = am_atom_put(buf, prefix_len + sizeof(realloc) - 1); sys_memcpy((void *) &buf[prefix_len], (void *) free, sizeof(free) - 1); allctr->name.free = am_atom_put(buf, prefix_len + sizeof(free) - 1); } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Wir haben eine Funktion, die Namen für Allokations- und Deallokationsfunktionen basierend auf dem an sie übertragenen Präfix erzeugt. Es wird ein 255-Bit-Buffer verwendet, um sie zu erzeugen. Nach den Funktionsinhalten wird es in dem erzeugten Buffer keinen Null-Terminator geben, da Dies entspricht der Gesamtgröße der generierten Zeile. am_atom_put Das erste, was die Entwickler taten, war die Präfixgröße zu berechnen. Und die Länge der String (der längste Postfix) Funktion genannt wird. MAX_ATOM_CHARACTERS realloc noreturn Als nächstes kopierten sie das Präfix zum Puffer, fügten das Postfix hinzu und übergaben die Zeichenfolge an die Die Entwickler führten alle diese Aktionen für alle Postfixes aufeinanderfolgend aus: , der und . am_atom_put alloc realloc free Kannst du den Fang erkennen? Die frühe Rückkehrprüfung wurde falsch durchgeführt. Der Puffer muss genügend Platz haben, um den längsten Postfix, der sieben Zeichen ist, zu schreiben. Wenn die Präfixzeile zwischen 256 und 262 Zeichen lang ist, tritt die frühe Rückkehr nicht auf, so dass die Funktion überfließt den Puffer. Diese Operation hat ein undefiniertes Verhalten. sys_memcpy Unter der Annahme, dass die function works with strings no longer than Lassen Sie uns den Code wie folgt beheben: am_atom_put MAX_ATOM_CHARACTERS if (prefix_len > MAX_ATOM_CHARACTERS - sizeof(realloc) + 1) Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Die maximale Präfixlänge beträgt nun 248 Zeichen. Beim Schreiben der maximalen Postfix tritt kein Überfluss auf. Similar warnings: V557 Array Overrun ist möglich. Der Wert des 'i' Index könnte 40. ycf_lexer.c 493 erreichen V557 Array overrun is possible. The value of 'n' index could reach 16. erl_call.c 1663 Fragment N9 Ich möchte dieses Beispiel auf ungewöhnliche Weise einführen.Zuerst schauen wir uns diese Funktion an: static const char* event_state_flag_to_str(EventStateFlags f, const char *buff, int len) { switch ((int)f) { case ERTS_EV_FLAG_CLEAR: return "CLEAR"; case ERTS_EV_FLAG_USED: return "USED"; /* other cases that return string literals */ default: snprintf((char *)buff, len, "ERROR(%d)", f); return buff; } } Enter fullscreen mode Exit fullscreen mode Die function converts the enumeration to a string. For non- branches, gibt eine Zeichenfolge wörtlich zurück. Andernfalls erzeugt die Funktion eine Zeichenfolge im angegebenen Puffer und gibt ihm einen Zeiger zurück. Beachten Sie, wie genau dies geschieht: Der Puffer entlässt zwingend Konstanz. Gemäß dem C-Standard ist dies nur zulässig, wenn der Quellpuffer nicht als C11 Die C23 der ) event_state_flag_to_str EventStateFlags default switch const 6.7.3.6 6.7 4.1 7 Ist das wirklich der Fall?Lassen Sie uns einen Blick auf den Anrufpunkt werfen: static ERTS_INLINE void print_flags(erts_dsprintf_buf_t *dsbufp, EventStateFlags f) { const char buff[64]; if (f & ERTS_EV_FLAG_WANT_ERROR) { erts_dsprintf(dsbufp, "WANTERR|"); f &= ~ERTS_EV_FLAG_WANT_ERROR; } erts_dsprintf(dsbufp, "%s", event_state_flag_to_str(f, buff, sizeof(buff))); } Enter fullscreen mode Exit fullscreen mode The function in question is called by passing the constant buffer as the second argument. Unfortunately, we get undefined behavior. We found this error using the analyzer, but it issued a confusing warning: Uninitialized buffer 'buff' verwendet. Überlegen Sie, das zweite tatsächliche Argument der 'event_state_flag_to_str'-Funktion zu überprüfen. V614 erl_check_io.c 2833 Let's fix the code by removing the constness from the buffer: static ERTS_INLINE void print_flags(erts_dsprintf_buf_t *dsbufp, EventStateFlags f) { char buff[64]; if (f & ERTS_EV_FLAG_WANT_ERROR) { erts_dsprintf(dsbufp, "WANTERR|"); f &= ~ERTS_EV_FLAG_WANT_ERROR; } erts_dsprintf(dsbufp, "%s", event_state_flag_to_str(f, buff, sizeof(buff))); } Enter fullscreen mode Exit fullscreen mode Ich schlage auch vor, die Funktionsunterschrift, so dass der Parameter eine pointer. This makes it clear that the function can modify the buffer passed to it. We won't be breaking much code in the process since the function exists only in the compiled file and is marked as . event_state_flag_to_str char* static Fragment N10 Das PVS-Studio warnt vor: Das uninitialisierte Klassenmitglied 'a' wird beim Initialisieren der Basisklasse 'BeamAssemblerCommon' verwendet. V1050 Beam_asm.hpp 87 class BeamAssemblerCommon : public ErrorHandler { BaseAssembler &assembler; protected: BeamAssemblerCommon(BaseAssembler &assembler); .... }; struct BeamAssembler : public BeamAssemblerCommon { BeamAssembler() : BeamAssemblerCommon(a) { /* .... */ } protected: a64::Assembler a; .... }; Enter fullscreen mode Exit fullscreen mode Dieses Fragment enthält eine kleine Klassenhierarchie, in der eine Bezugnahme auf eine nicht-statische Mitgliedsfunktion der abgeleiteten Klasse, , is passed to the constructor of the base class, . This operation won't cause any issues as long as the base class constructor doesn't interact with this member function. Why? The lifetime of an object from a derived class starts when the required initializer in the initialization list has been executed or, if there's no initializer, when the control flow enters the constructor body. BeamAssembler BeamAssemblerCommon Unter Berücksichtigung dieser Informationen schauen wir uns den Basisklasse-Konstruktor an. Es verwendet dieses Objekt nicht, richtig? BeamAssemblerCommon::BeamAssemblerCommon(BaseAssembler &assembler_) : assembler(assembler_), .... { .... #ifdef DEBUG assembler.addDiagnosticOptions(DiagnosticOptions::kValidateAssembler); #endif assembler.addEncodingOptions(EncodingOptions::kOptimizeForSize | EncodingOptions::kOptimizedAlign); .... } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Nope :) A non-static member function is called on an object of a derived class via a reference when its lifetime has not yet begun. Again, we get undefined behavior. I also find it difficult to suggest a fix in this case, so I'll leave that to the developers. Gedächtnisfehler Fragment N11 Das PVS-Studio warnt vor: The 'free' function is called twice for deallocation of the same memory space. V586 erl_call.c 668 int main(int argc, char *argv[]) { int fd; char *p = NULL; ei_cnode ec; .... int i = 0; ei_x_buff reply; .... if (ei_rpc(&ec, fd, "c", "c", p, i, &reply) < 0) { free(p); ei_x_free(&reply); fprintf(stderr,"erl_call: can't compile file %s\n", fname); } free(p); /* FIXME complete this code FIXME print out error message as term if (!erl_match(erl_format("{ok,_}"), reply)) { fprintf(stderr,"erl_call: compiler errors\n"); } */ ei_x_free(&reply); .... } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Judging by the name, ist a Eine negative Nummer wird zurückgegeben, wenn der Vorgang fehlschlägt. Wenn der Remote-Anruf fehlschlägt, werden einige Ressourcen gereinigt. pointer is passed to the function and an error is logged to Danach wird, unabhängig vom Funktionsaufrufergebnis, der Zeiger Das Gedächtnis wird zweimal freigegeben. ei_rpc remote procedure call p free stderr free Die Lösung ist ziemlich einfach: Entweder stoppen Sie die Ausführung des Programms oder setzen Sie die freigegebenen Zeiger auf Null. if (ei_rpc(&ec, fd, "c", "c", p, i, &reply) < 0) { free(p); ei_x_free(&reply); fprintf(stderr,"erl_call: can't compile file %s\n", fname); return 1; } free(p); Enter fullscreen mode Exit fullscreen mode Fragment N12 Das PVS-Studio warnt vor: Der „pkey“-Punkt wurde verwendet, bevor er gegen nullptr überprüft wurde. Checklinien: 581, 582. V595 pkey.c 581 static int get_pkey_public_key(ErlNifEnv *env, const ERL_NIF_TERM argv[], int algorithm_arg_num, int key_arg_num, EVP_PKEY **pkey, ERL_NIF_TERM *err_return) { .... if (enif_is_map(env, argv[key_arg_num])) { password = get_key_password(env, argv[key_arg_num]); *pkey = ENGINE_load_public_key(e, id, NULL, password); if (!pkey) assign_goto(*err_return, err, EXCP_BADARG_N(env, key_arg_num, "Couldn't get public key from engine")); } /* other branches */ ret = 1; done: .... return ret; err: .... *pkey = NULL; ret = 0; goto done; } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Wir haben hier ein ziemlich interessantes Code-Fragment. attempts to extract the public key to the fifth out parameter. It returns if successful and if not. In the latter case, the public key is zeroed as well. function 1 0 The analyzer warns that, in one of the function branches, the Der Zeigefinger wird vor der Überprüfung auf Gültigkeit dereferenziert. Allerdings scheinen andere Zweige keine solchen Überprüfungen zu haben. Wir können schließen, dass die Funktion den folgenden Vertrag hat: "Die Parameter dürfen nie gleich A sein. ." Wir können dies bestätigen, indem wir nach Anrufen zu dieser Funktion in der gleichen Datei suchen ([ ] , [ ] , [ ])—the variable address is passed everywhere. pkey pkey nullptr 1 2 3 In fact, the developers intended to handle the incorrect result of the Funktion durch Springen auf die Sie haben jedoch einen Tipp gemacht, indem sie vergessen haben, einen Stern vor den Ein moderner Compiler wird diese Überprüfung auf Freigabe. Als Folge kehrt die Funktion , and a null pointer will end up in the fifth parameter. ENGINE_load_public_key err ! Optimierung abseits 1 Lassen Sie uns den Check durchführen: .... *pkey = ENGINE_load_public_key(e, id, NULL, password); if (!*pkey) assign_goto(*err_return, err, EXCP_BADARG_N(env, key_arg_num, "Couldn't get public key from engine")); .... Enter fullscreen mode Exit fullscreen mode Fragment N13 We often receive messages saying that the analyzer warns about passing a null pointer to Schließlich sagt der Standard, dass nichts Schreckliches passieren wird. about a similar case. free article Yes, that's fine, but such code is probably suspicious. That helped us find an interesting memory leak in the code. Let's take a look at it. The PVS-Studio warning: Der Null-Zeiger wird in die "freie" Funktion übertragen. Überprüfen Sie das erste Argument. V575 erl_misc_utils.c 264 void erts_cpu_info_destroy(erts_cpu_info_t *cpuinfo) { if (cpuinfo) { cpuinfo->configured = 0; cpuinfo->online = 0; cpuinfo->available = 0; #ifdef HAVE_PSET_INFO if (cpuinfo->cpuids) free(cpuinfo->cpuids); #endif cpuinfo->topology_size = 0; if (cpuinfo->topology) { cpuinfo->topology = NULL; free(cpuinfo->topology); } free(cpuinfo); } } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus In diesem Fragment, der Der Pointer ist ausdrücklich auf before calling . As a result, the memory-freeing operation is pointless, since Dies führt zu Speicherlecken, da der ursprüngliche Speicherblock, der pointed to is never freed. cpuinfo->topology NULL free free(NULL) cpuinfo->topology Wir können das Problem beheben, indem wir die Operationen austauschen: if (cpuinfo->topology) { free(cpuinfo->topology); cpuinfo->topology = NULL; } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Fragment N14 Das PVS-Studio warnt vor: The function was exited without releasing the 'entry' pointer. A memory leak is possible. V773 wxe_gl.cpp 88 typedef struct _wxe_glc { wxGLCanvas *canvas; wxGLContext *context; } wxe_glc; void setActiveGL(wxeMemEnv *memenv, ErlNifPid caller, wxGLCanvas *canvas, wxGLContext *context) { ErlNifUInt64 callId = wxe_make_hash(memenv->tmp_env, &caller); wxe_glc * entry = glc[callId]; gl_active_index = callId; gl_active_pid = caller; if (!entry) { entry = (wxe_glc *) malloc(sizeof(wxe_glc)); entry->canvas = NULL; entry->context = NULL; } if (entry->canvas == canvas && entry->context == context) return; entry->canvas = canvas; entry->context = context; glc[gl_active_index] = entry; .... } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus In the function, the pointer is declared and initialized via an array element. If this element is a null pointer, memory is allocated using Anschließend werden die Zeiger verglichen.Wenn es Eine frühzeitige Rückkehr tritt auf. Wenn jedoch eine dynamische Allokation aufgetreten ist, wird das Speicher nicht freigegeben. setActiveGL entry malloc true Yes, such a situation is rather rare and unlikely to happen, because it requires a whole series of events: Wenn die Funktion angerufen wird, werden Nullzeilen als drittes und viertes Argument übertragen. contains a null pointer. glc[callId] Wir können jedoch sicherstellen, dass ein Leck niemals auftritt, nicht einmal theoretisch. ErlNifUInt64 callId = wxe_make_hash(memenv->tmp_env, &caller); wxe_glc * entry = glc[callId]; gl_active_index = callId; gl_active_pid = caller; bool new_alloc = false; if (!entry) { entry = (wxe_glc *) malloc(sizeof(wxe_glc)); entry->canvas = NULL; entry->context = NULL; new_alloc = true; } if (entry->canvas == canvas && entry->context == context) { if (new_alloc) { free(entry); } return; } .... Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Similar warnings: V773 Die Funktion wurde verlassen, ohne die Datei zu schließen, auf die der "f" -Handling verweist.Ein Ressourcenleck ist möglich. erl_poll.c 2423 V773 Der Zeiger 'erl_errno_p' wurde zweimal Werte zugewiesen, ohne den Speicher freizugeben. V773 Die Funktion wurde verlassen, ohne die Datei zu schließen, auf die der "fp" -Handling verweist. V773 Die Ausnahme wurde geworfen, ohne den "Daten"-Punkt freizugeben. ein Speicherleck ist möglich. wxe_wrapper_4.cpp 1300 V773 The exception was thrown without releasing the 'data' pointer. A memory leak is possible. wxe_wrapper_4.cpp 1330 V773 Die Ausnahme wurde geworfen, ohne den "Daten"-Punkt freizugeben. ein Speicherleck ist möglich. wxe_wrapper_4.cpp 1663 V773 Die Ausnahme wurde geworfen, ohne den "Daten"-Punkt freizugeben. ein Speicherleck ist möglich. wxe_wrapper_4.cpp 1690 V773 Die Ausnahme wurde geworfen, ohne den "Daten"-Punkt freizugeben. ein Speicherleck ist möglich. wxe_wrapper_4.cpp 1715 V773 Die Ausnahme wurde geworfen, ohne den „alpha“-Punkt freizugeben. ein Speicherleck ist möglich. wxe_wrapper_4.cpp 1718 V773 Die Ausnahme wurde geworfen, ohne den "Daten"-Punkt freizugeben. ein Speicherleck ist möglich. wxe_wrapper_4.cpp 1747 V773 Die Ausnahme wurde geworfen, ohne den „alpha“-Punkt freizugeben. ein Speicherleck ist möglich. wxe_wrapper_4.cpp 1750 V773 The exception was thrown without releasing the 'alpha' pointer. A memory leak is possible. wxe_wrapper_4.cpp 2680 V773 The exception was thrown without releasing the 'data' pointer. A memory leak is possible. wxe_wrapper_4.cpp 2715 V773 Die Ausnahme wurde geworfen, ohne den "Daten"-Punkt freizugeben. ein Speicherleck ist möglich. wxe_wrapper_4.cpp 2733 Fragment N15 Das PVS-Studio warnt vor: realloc() mögliches Leck: Wenn realloc() beim Zuordnen des Speichers fehlschlägt, wird der ursprüngliche Zeiger 'outbuf_base' verloren. V701 run_erl.c 1324 static void outbuf_append(const char* buf, int n) { .... /* * Allocate a larger buffer if we still cannot fit the data. */ if (outbuf_base+outbuf_total < outbuf_in+n) { int size = outbuf_in - outbuf_out; outbuf_total = size+n; outbuf_base = realloc(outbuf_base, outbuf_total); outbuf_out = outbuf_base; outbuf_in = outbuf_base + size; } .... } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus Es gibt ein trickiges Muster mit der Funktion, bei der der ursprüngliche Zeiger sofort durch den Return-Wert übergeschrieben wird. realloc Die Funktion nimmt den Puffer für die Neuauszuweisung als ihr erstes Argument und die neue Puffergröße als ihr zweites Argument.Wenn die Operation erfolgreich ist, gibt die Funktion einen Zeiger zum neu zugewiesenen Speicher zurück. , auch wenn nur eine einfache Erweiterung des Speicherblocks stattgefunden hat. Wenn der Vorgang fehlschlägt, Die Erinnerung ist vergangen, wie das erste Argument bleibt . invalid null pointer untouched Wenn wir also sofort den ursprünglichen Zeiger mit einem Aufruf Wir werden einen Gedächtnisschlag bekommen. realloc Lassen Sie uns den Code wie folgt beheben: static void outbuf_append(const char* buf, int n) { .... /* * Allocate a larger buffer if we still cannot fit the data. */ if (outbuf_base + outbuf_total < outbuf_in + n) { int size = outbuf_in - outbuf_out; outbuf_total = size+n; char *tmp = realloc(outbuf_base, outbuf_total); if (!tmp) { /* somehow handle this scenario * the `outbuf_base` buffer here is still valid */ } outbuf_base = tmp; outbuf_out = outbuf_base; outbuf_in = outbuf_base + size; } .... } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus In the fixed example, we also need to handle the situation where Dies kann ein Ausnahmewerfen, ein Versuch sein, einen Puffer einer anderen Größe zuzuweisen, und so weiter. realloc Fragment N16 Das PVS-Studio warnt vor: The 'end' pointer in the 'end - mm->sua.top' expression equals nullptr. The resulting value is senseless and it should not be used. von V769 Erl_mmap.c 2411 static void add_free_desc_area(ErtsMemMapper* mm, char *start, char *end) { ERTS_MMAP_ASSERT(end == (void *) 0 || end > start); if (sizeof(ErtsFreeSegDesc) sua.top` always will * compare as larger than all segment pointers * into the super carrier... */ mm->sua.top -= ERTS_PAGEALIGNED_SIZE; mm->size.supercarrier.used.total += ERTS_PAGEALIGNED_SIZE; #ifdef ERTS_HAVE_OS_PHYSICAL_MEMORY_RESERVATION if (!virtual_map || os_reserve_physical(mm->sua.top, ERTS_PAGEALIGNED_SIZE)) #endif add_free_desc_area(mm, mm->sua.top, end); mm->desc.reserved += (end - mm->sua.top) / sizeof(ErtsFreeSegDesc); } .... } Vollbildmodus Eintritt Vollbildmodus Eintritt Vollbildmodus The analyzer has detected suspicious address arithmetic. Let's see what's wrong here. We enter the code branch where Es ist wirklich ein Null-Punkter. Nach dem C-Standard wird das Verhalten von Zeiger, die sich unterscheiden, nur definiert, wenn sie beide auf Elemente des gleichen Arrays (C11). ; C23 ). The null pointer doesn't point to any array element. So, the behavior of this operation is undefined, which isn't good. end mm->sua.top 5.6 von 6.8 6.5.7 Die 10 I was wondering what happens in the Die Funktion. add_free_desc_area static void add_free_desc_area(ErtsMemMapper* mm, char *start, char *end) { ERTS_MMAP_ASSERT(end == (void *) 0 || end > start); if (sizeof(ErtsFreeSegDesc) <= ((UWord) end) - ((UWord) start)) { .... } Enter fullscreen mode Exit fullscreen mode Auch hier wird Pointersubtraktion verwendet, aber mit einem Unterschied: Zuerst werden die Pointer in Zahlen umgewandelt. Die C23 ), was viel besser ist. 6.3.2 und 3.6 6.3.2.3.6 Leider kann ich keine ordnungsgemäße Lösung vorschlagen. Die Entwickler mögen sich dieses Problem ansehen. Schlussfolgerung Nun, das schließt den Artikel. Wie bei jedem großen Projekt, das eine lange und ereignisreiche Geschichte hatte, erwartet, fanden wir verschiedene Fehler. Sie sind auch Teil des Projekts, jedoch. Es ist wichtig zu beachten, dass das Finden dieser Art von Problemen in einer Code-Basis, die über drei Jahrzehnte alt ist, kein Zeichen von Schwäche ist. Vielmehr ist es ein Beweis für die unglaubliche Skala und Langlebigkeit des Codes. Es gibt Millionen von Code-Linien, die von Hunderten von Entwicklern geschrieben wurden, und das System läuft seit Jahren ohne Neustart. Dieses Projekt verdient den tiefsten Respekt, nicht weil es perfekt ist, sondern weil es den Test der Zeit bestanden hat und seine wirkliche Macht bewiesen hat. Thanks to constantly evolving modern technologies, developers have powerful tools at their disposal to improve code reliability. For example, static analyzers help identify hidden errors and potential vulnerabilities. I invite you to see it for yourself :) You can get a trial version of PVS-Studio analyzer . here