De code van de jaren tachtig die moderne betrouwbaarheid ondersteunt - en de fouten die het nog steeds maakt

De code voor sommige Erlang/OTP-modules is ouder dan de meeste moderne junior-ontwikkelaars. Deze bestanden zijn de echte digitale patriarchen. Al tientallen jaren hebben ze ervoor gezorgd dat banktransacties, telefoonnetwerken en messaging-systemen goed werken. We hebben besloten onder de hoed van deze lang bestaande taal te kijken om te zien wat er precies achter de lijnen ligt waarop miljoenen gebruikers vandaag de dag vertrouwen. Introductie Voordat we verder gaan met het bespreken van waarschuwingen, wil ik een paar woorden over het project zeggen. Erlang's reis begon in 1986 in een van de laboratoria van Ericsson. Joe Armstrong experimenteerde met Prolog terwijl hij probeerde een telefoonprogramma te bouwen. is een programmeertaal ontworpen voor fouttolerante systemen waar falen onaanvaardbaar is. Denk aan een telefoonschakelaar waar een gehaalde oproep wordt geteld als een noodsituatie, of een banktransactie die moet gaan door wat er ook gebeurt. Erlang Erlang gaat hand in hand met de , een framework dat krachtige hulpmiddelen, patronen en gedragingen biedt voor het bouwen van gedistribueerde, fouttolerante applicaties. Open Telecom Platform (OTP) We bouwden het project op basis van deze We hebben het in de Branch Gebruik En de We hadden geen problemen met het bouwen van het project, dus kudos aan de ontwikkelaars. Instructies maint-28 PvS-Studio analyser De Visual Studio Code Het artikel bevat alleen waarschuwingen op hoog en medium niveau. Dat sluit onze inleiding af. Laten we nu verder gaan met de waarschuwingen. We zullen ze in drie categorieën verdelen: logische fouten maken; de kritische fouten; De geheugenfouten. Logische fouten Fragment N1 De PVS-Studio waarschuwt: Er zijn identieke sub-uitdrukkingen '(!esock_is_integer((env), (argv[3])))' aan de linkerkant en aan de rechterkant van de operator 'annoo. De 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]); } .... } Enter fullscreen mode Exit fullscreen mode In de code wilden de ontwikkelaars de derde en vierde argumenten omzetten naar 64-bits typen (de ondertekende en de niet-ondertekende, respectievelijk). The fixed code: .... if ((! IS_INTEGER(env, argv[2])) || (! IS_INTEGER(env, argv[3]))) return enif_make_badarg(env); .... Enter fullscreen mode Exit fullscreen mode Fragment N2 De PVS-Studio waarschuwt: Er zijn identieke sub-uitdrukkingen 'n->type == ycf_node_type_on_save_yield_state_code' aan de linkerkant en aan de rechterkant van de operator van de 'annoo. De 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); } .... } Fullscreen mode invoeren Fullscreen mode invoeren Hier is een andere interessante copy-paste case. we hebben de functie, die recursief met boomknopen omgaat en zichtbare gebieden met lokale variabelen vindt. Het roept vervolgens de variabele naam-unificatiefunctie voor elk van deze gebieden op. Branch, de De voorwaarde wordt verdubbeld na de De operator. uniqify_local_vars_in_node else if n->type == ycf_node_type_on_save_yield_state_code || Door de vergelijkingsketen te oordelen, worden de waarden van de volgende lijst gerangschikt in de volgorde waarin ze zijn verklaard: 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; Fullscreen mode invoeren Fullscreen mode invoeren Echter, de constant wordt gecontroleerd in de vorige Dus, ik denk dat er tenslotte een dubbele cheque is en we kunnen het verwijderen. ycf_node_type_on_restore_yield_state_code else if Fragment N3 De PVS-Studio waarschuwt: Possible typo in the spelling of a pre-defined macro name. The ' ‘Macro is vergelijkbaar met’ ‘ . . De 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); } Fullscreen mode invoeren Fullscreen mode invoeren Dit is een vrij ongebruikelijk geval van het controleren of een project compileert onder Windows. Erlang heeft andere controles op Windows ([ De [ De [ ], etc.), maar dit is uniek. Dit ziet er heel erg uit als een eenvoudige typo (meest waarschijnlijk zou het ) van 1 2 3 __WIN32__ Maar waarom is dit zo? uit nieuwsgierigheid heb ik gezocht naar Om te zien wat ik kon vinden, hier zijn de resultaten: "__WIN_32__" As you can see, there are almost no results. It's funny that one of the links leads to the file we're looking at on Ik greep het project door om geen vergelijkbare macro-definities te vinden. ik dacht ook dat het misschien in een compiler zou kunnen worden ingebouwd, zoals MinGW. Na het proberen om het doel van de macro te begrijpen, geloof ik dat deze code defect is. Debian bronnen Experimenteren As a fix, I suggest using the check that I found earlier in other parts of the project: static int tcp_send_error(tcp_descriptor* desc, int err) { .... #if defined(__WIN32__) || defined(_WIN32) || defined(_WIN32_) if (err == ECONNABORTED) err = ECONNRESET; #endif .... } Fullscreen mode invoeren Fullscreen mode invoeren Fragment N4 De PVS-Studio waarschuwt: De operationele logica van de code komt niet overeen met de opmaak ervan. De tweede verklaring wordt altijd uitgevoerd. V640 dialyzer.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"); .... } Enter fullscreen mode Exit fullscreen mode Here's an interesting example of a common coding error that can be difficult to spot later on. Take a look at the De structuur: PUSH2 #define QUOTE(s) s #define PUSH(s) eargv[eargc++] = QUOTE(s) #define PUSH2(s, t) PUSH(s); PUSH(t) Fullscreen mode invoeren Fullscreen mode invoeren Uh-huh, het is een macro die zich uitbreidt tot twee constructen.Laten we de code uitbreiden zoals een preprocessor zou: 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"; .... } Enter fullscreen mode Exit fullscreen mode Bijgevolg, als de controle beoordeelt , alleen de eerste opdracht wordt voorwaardelijk uitgevoerd, terwijl de tweede altijd zal plaatsvinden na de tak. false Makro's schrijven en gebruiken De perfecte oplossing zou zijn om ze kwijt te raken, maar laten we ons verzoenen voor een minder radicale oplossing. Dit gebeurt wanneer meerdere acties moeten worden gecombineerd in één macro. attracts bugs do {...} while (0) #define PUSH2(s, t) do { PUSH(s); PUSH(t); } while (0) Fullscreen mode invoeren Fullscreen mode invoeren Fragment N5 The PVS-Studio warning: The condition 'i >= 0' of loop is always true. De 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; } Fullscreen mode invoeren Fullscreen mode invoeren Neem een kijkje op de bijzondere aandacht te besteden aan zijn conditie. sinds de De variabele inductie is van de type met een waardebereik van , de loop wordt oneindig. Na de loop itereert met De variabele wordt gelijk aan Als gevolg van de afname, en de loop gaat verder. for i unsigned int [0; UINT_MAX] i == 0 UINT_MAX Laten we de code als volgt corrigeren: for (int64_t i = arr.GetCount() - 1; i >= 0; i--) { .... } Fullscreen mode invoeren Fullscreen mode invoeren Vergelijkbare waarschuwingen : V654 De voorwaarde 'i >= 0' van de loop is altijd waar. wxe_return.cpp 248 V654 De voorwaarde 'i >= 0' van de loop is altijd waar. wxe_return.cpp 260 Fragment N6 De PVS-Studio waarschuwt: The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 2903, 3053. De 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 .... } } Enter fullscreen mode Exit fullscreen mode De analyzer heeft verschillende takken met identieke controles gedetecteerd in een functie die een groot aantal Verklaringen - grofweg Elk van hen heeft echter een andere logica: de and Gezien het aantal takken en de 150-lijn kloof tussen de duplicaten, is het nauwelijks verrassend dat dit zou kunnen gebeuren. if-else if 800 lines long Eerste check second check Fragment N7 De PVS-Studio waarschuwt: Een deel van de voorwaardelijke uitdrukking is altijd vals: (arity == 0). V560 Bijv. c 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); } .... } Fullscreen mode invoeren Fullscreen mode invoeren Here, we have an example involving math. The analyzer informs us that the last De uitdrukking is altijd onjuist, waarom? arity == 0 Als de besturingsstroom het punt bereikt waar deze subexpressie wordt geëvalueerd, zullen we weten dat de twee vorige subexpressie We kunnen ook het volgende leren: false ix >= 1 ix = 1 arity Ik zal ook de correctie van deze code aan de ontwikkelaars overlaten, omdat mijn enige suggestie is om de voorwaarde te verwijderen. Kritische fouten Fragment N8 De PVS-Studio waarschuwt voor: De waarde van de index 'prefix_len' kan 262 bereiken. De V557 erl_alloc_util.c 5375 Array overrun is possible. The value of 'prefix_len' index could reach 262. De V557 erl_alloc_util.c 5378 De waarde van de index 'prefix_len' kan 262 bereiken. De 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); } Fullscreen mode invoeren Fullscreen mode invoeren We hebben een functie die namen genereert voor toewijzings- en hertoewijzingsfuncties op basis van het voorvoegsel dat aan het wordt doorgegeven. Een buffer van 255-byte wordt gebruikt om ze te genereren. passes the total size of the generated string. am_atom_put Het eerste wat de ontwikkelaars deden was de prefixgrootte berekenen. En de lengte van de string (de langste postfix), de De functie wordt genoemd. MAX_ATOM_CHARACTERS realloc noreturn Vervolgens kopieerden ze het voorvoegsel naar de buffer, voegden ze de postfix toe en gaven ze de string door naar de buffer. De ontwikkelaars voerden al deze acties opeenvolgend uit voor alle postfixes: , en . am_atom_put alloc realloc free Can you spot the catch? The early return check was performed incorrectly. The buffer must have enough space to write the longest postfix, which is seven characters. If the prefix string is between 256 and 262 characters long, the early return doesn't happen, so the functie overstroomt de buffer. Deze operatie heeft onbepaald gedrag. sys_memcpy Aannemen dat de functie werkt met strings niet langer dan , let's fix the code as follows: am_atom_put MAX_ATOM_CHARACTERS if (prefix_len > MAX_ATOM_CHARACTERS - sizeof(realloc) + 1) Fullscreen mode invoeren Fullscreen mode invoeren Nu is de maximale prefixlengte 248 tekens. Bij het schrijven van de maximale postfix, overflow zal niet optreden. Similar warnings: V557 Array overrun is mogelijk. De waarde van 'i' index zou kunnen bereiken 40. ycf_lexer.c 493 V557 Array overrun is mogelijk. De waarde van 'n' index zou kunnen bereiken 16. erl_call.c 1663 Fragment N9 Ik wil dit voorbeeld op een ongewone manier presenteren.Laten we eerst eens kijken naar deze functie: 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; } } Fullscreen mode invoeren Fullscreen mode invoeren De function converts the een lijstje op een lijstje. voor niet- De takken, returns a string literal. Otherwise, the function creates a string in the provided buffer and returns a pointer to it. Note how exactly it does this: the buffer forcibly discards constness. According to the C standard, this is allowed only if the source buffer hasn't been declared as (C11 De C23 ). event_state_flag_to_str EventStateFlags default switch const 6.7 van 3.6 6.7.4.1.7 Is dit echt het geval?Laten we eens kijken naar het invocatiepunt: 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 De functie in kwestie wordt genoemd door de constante buffer door te geven als het tweede argument. Helaas krijgen we ongedefinieerd gedrag. We vonden deze fout met behulp van de analyser, maar het gaf een verwarrende waarschuwing af: Uninitialized buffer 'buff' used. Consider checking the second actual argument of the 'event_state_flag_to_str' function. De V614 erl_check_io.c 2833 Laten we de code repareren door de constness uit de buffer te verwijderen: 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))); } Fullscreen mode invoeren Fullscreen mode invoeren I also suggest changing the functionele handtekening, zodat de parameter een We zullen niet veel code breken in het proces omdat de functie alleen bestaat in het gecompileerde bestand en is gemarkeerd als . event_state_flag_to_str char* static Fragment N10 De PVS-Studio waarschuwt: Het niet-geïnitieerde klasse lid 'a' wordt gebruikt bij het initialiseren van de basisklasse 'BeamAssemblerCommon'. 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; .... }; Fullscreen mode invoeren Fullscreen mode invoeren Dit fragment bevat een kleine klassenhiërarchie waarbij een verwijzing naar een niet-statische lidfunctie van de afgeleide klasse, , wordt doorgegeven aan de constructeur van de basisklasse, . 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 With this information in mind, let's take a look at the base class constructor. It doesn't use this object, right? Right? BeamAssemblerCommon::BeamAssemblerCommon(BaseAssembler &assembler_) : assembler(assembler_), .... { .... #ifdef DEBUG assembler.addDiagnosticOptions(DiagnosticOptions::kValidateAssembler); #endif assembler.addEncodingOptions(EncodingOptions::kOptimizeForSize | EncodingOptions::kOptimizedAlign); .... } Fullscreen mode invoeren Fullscreen mode invoeren Nope :) Een niet-statische lidfunctie wordt opgeroepen op een object van een afgeleide klasse via een referentie wanneer de levensduur ervan nog niet is begonnen. Ik vind het ook moeilijk om een oplossing voor te stellen in dit geval, dus ik laat dat over aan de ontwikkelaars. Memory errors Fragment N11 The PVS-Studio warning: The 'free' function is called twice for deallocation of the same memory space. De 586 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); .... } Fullscreen mode invoeren Fullscreen mode invoeren Judging by the name, is a . A negative number is returned if the operation fails. If the remote call fails, some resources are cleaned up. The Pointer wordt doorgegeven aan de function and an error is logged to . Next, regardless of the function call result, the pointer is passed to again. Memory is released twice. ei_rpc Remote Call Procedure p free stderr free De oplossing is vrij eenvoudig: stop de uitvoering van het programma of stel de vrijgegeven pointers op nul. 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 The PVS-Studio warning: De 'pkey' pointer werd gebruikt voordat het werd gecontroleerd tegen nullptr. 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; } Fullscreen mode invoeren Fullscreen mode invoeren We hebben een heel interessant code fragment hier. de attempts to extract the public key to the fifth out parameter. It returns Als succesvol en Als dat niet het geval is, is de openbare sleutel ook nul. Functie 1 0 The analyzer warns that, in one of the function branches, the de pointer wordt herverwezen voordat deze wordt gecontroleerd op geldigheid. echter lijken andere takken dergelijke controles niet te hebben. We kunnen concluderen dat de functie het volgende contract heeft: "De De parameter mag nooit gelijk zijn aan een ." We kunnen dit bevestigen door te zoeken naar oproepen naar deze functie in hetzelfde bestand ([ ], [ ], [ ]) - het variabele adres wordt overal doorgegeven. pkey pkey nullptr 1 2 3 In fact, the developers intended to handle the incorrect result of the functioneren door te springen naar de Echter, ze maakten een typo door te vergeten om een asterisk voor de Een moderne compilator zal this check on release. As a result, the function will return , and a null pointer will end up in the fifth parameter. ENGINE_load_public_key err ! optimize away 1 Laten we de cheque regelen: .... *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 ontvangen vaak berichten waarin staat dat de analyzer waarschuwt voor het doorgeven van een nullpointer naar Immers, de standaard stelt dat er niets verschrikkelijks zal gebeuren. over een soortgelijk geval. free artikel 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: The null pointer is passed into 'free' function. Inspect the first 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); } } Fullscreen mode invoeren Fullscreen mode invoeren In this fragment, the Pointer is uitdrukkelijk ingesteld op before calling Als gevolg hiervan is de operatie geheugenvrijmaken zinloos, omdat Dit resulteert in geheugenlekken omdat het oorspronkelijke geheugenblok dat Hij wijst erop dat hij nooit bevrijd wordt. cpuinfo->topology NULL free free(NULL) cpuinfo->topology We kunnen het probleem oplossen door de operaties te wisselen: if (cpuinfo->topology) { free(cpuinfo->topology); cpuinfo->topology = NULL; } Enter fullscreen mode Exit fullscreen mode Fragment N14 The PVS-Studio warning: De functie is geëxecuteerd zonder de 'entry'-pointer vrij te geven. De V773 wxe_nl.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; .... } Fullscreen mode invoeren Fullscreen mode invoeren In de De functie, de Pointer wordt aangegeven en geïnitieerd via een matrixelement. Als dit element een nullpointer is, wordt geheugen toegewezen met behulp van . Next, the pointers are compared. If it evaluates to Echter, als dynamische toewijzing heeft plaatsgevonden, zal het geheugen niet worden vrijgegeven. setActiveGL entry malloc true Yes, such a situation is rather rare and unlikely to happen, because it requires a whole series of events: Wanneer de functie wordt opgeroepen, worden nulpunters doorgegeven als de derde en vierde argumenten. glc[callId] bevat een null-pointer. Echter, we kunnen ervoor zorgen dat een lek nooit zal optreden, zelfs niet 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; } .... Enter fullscreen mode Exit fullscreen mode Similar warnings: V773 De functie is verlaten zonder het bestand te sluiten waarnaar wordt verwezen door de 'f' handleiding. Een resource lek is mogelijk. erl_poll.c 2423 V773 The 'erl_errno_p' pointer was assigned values twice without releasing the memory. A memory leak is possible. ei_pthreads.c 195 V773 The function was exited without closing the file referenced by the 'fp' handle. A resource leak is possible. cpu_sup.c 433 V773 The exception was thrown without releasing the 'data' pointer. A memory leak is possible. 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 The exception was thrown without releasing the 'data' pointer. A memory leak is possible. wxe_wrapper_4.cpp 1663 V773 De uitzondering werd weggegooid zonder de 'gegevens'-pointer vrij te geven. een geheugenlek is mogelijk. wxe_wrapper_4.cpp 1690 V773 De uitzondering werd weggegooid zonder de 'gegevens'-pointer vrij te geven. een geheugenlek is mogelijk. wxe_wrapper_4.cpp 1715 V773 De uitzondering werd weggegooid zonder de 'alpha'-pointer vrij te geven. een geheugenlek is mogelijk. wxe_wrapper_4.cpp 1718 V773 De uitzondering werd weggegooid zonder de 'gegevens'-pointer vrij te geven. een geheugenlek is mogelijk. wxe_wrapper_4.cpp 1747 V773 De uitzondering werd weggegooid zonder de 'alpha'-pointer vrij te geven. een geheugenlek is mogelijk. wxe_wrapper_4.cpp 1750 V773 De uitzondering werd weggegooid zonder de 'alpha'-pointer vrij te geven. een geheugenlek is mogelijk. wxe_wrapper_4.cpp 2680 V773 De uitzondering werd weggegooid zonder de 'gegevens'-pointer vrij te geven. een geheugenlek is mogelijk. wxe_wrapper_4.cpp 2715 V773 De uitzondering werd weggegooid zonder de 'gegevens'-pointer vrij te geven. een geheugenlek is mogelijk. wxe_wrapper_4.cpp 2733 Fragment N15 The PVS-Studio warning: realloc() mogelijk lekken: wanneer realloc() mislukt in het toewijzen van geheugen, wordt de oorspronkelijke pointer '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; } .... } Fullscreen mode invoeren Fullscreen mode invoeren There's a tricky pattern with the function, where the original pointer is immediately overwritten by the return value. Let's get to the bottom of this. realloc De functie neemt de buffer voor herallocatie als eerste argument en de nieuwe buffergrootte als tweede argument. Als de operatie succesvol is, geeft de functie een pointer terug naar het nieuw toegewezen geheugen. , zelfs als er slechts een eenvoudige uitbreiding van het geheugenblok is plaatsgevonden. Als de operatie mislukt, geeft de functie een . The memory passed as the first argument remains . invalid null pointer untouched So, if we immediately overwrite the original pointer with a call to , we will get a memory leak. realloc Laten we de code als volgt corrigeren: 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; } .... } Enter fullscreen mode Exit fullscreen mode In the fixed example, we also need to handle the situation where returns the null pointer. This may be an exception throwing, an attempt to allocate a buffer of a different size, and so on. realloc Fragment N16 De PVS-Studio waarschuwt: De 'einde' pointer in de 'einde - mm->sua.top' expressie is gelijk aan nullptr. De resulterende waarde is zinloos en mag niet worden gebruikt. De 769 Erl_mmap.nl 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); } .... } Enter fullscreen mode Exit fullscreen mode De analyzer heeft verdachte adresarithmetica gedetecteerd.Laten we eens kijken wat hier mis is. We voeren de code branche in waar is indeed a null pointer. The data member is also a pointer, but we don't know its value yet. According to the C standard, the behavior of pointers that differ is defined only when they both point to elements of the same array (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 6.5.6.8 6.5.7.10 I was wondering what happens in the Het functioneren. 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)) { .... } Fullscreen mode invoeren Fullscreen mode invoeren Pointer subtraction is also used here, but with a difference: first, the pointers are converted to numbers. This is implementation-defined behavior (C11 ; C23 ), which is much better. 6.3.2 van 3.6 6.3.2 van 3.6 Unfortunately, I can't suggest a proper fix. The developers might want to take a look into this issue. Conclusion Well, that concludes the article. As expected with any large project that has had a long and eventful history, we found various errors. They're also part of the project, though. It's important to keep in mind that finding these kinds of issues in a code base that's over three decades old isn't a sign of weakness. Rather, it's proof of the code's incredible scale and longevity. There are millions of code lines written by hundreds of developers, and the system has been running for years without rebooting. This project deserves the deepest respect, not because it's perfect, but because it has stood the test of time and proven its real power. 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 . hier