1980-ųjų kodas, stiprinantis šiuolaikinį patikimumą - ir klaidos, kurias jis vis dar daro

Kai kurių Erlang / OTP modulių kodas yra senesnis nei dauguma šiuolaikinių jaunųjų kūrėjų. Šie failai yra tikrieji skaitmeniniai patriarchai. Dešimtmečius jie užtikrino, kad bankiniai sandoriai, telefono tinklai ir pranešimų sistemos tinkamai veiktų. Mes nusprendėme pažvelgti po šios ilgai gyvenusios kalbos dangteliu, kad pamatytume, kas tiksliai slypi už linijų, kuriomis šiandien pasitiki milijonai vartotojų. Įvadas Prieš pradėdami aptarti įspėjimus, norėčiau pasakyti keletą žodžių apie projektą. Erlang's journey began back in 1986 in one of Ericsson's laboratories. Joe Armstrong was experimenting with Prolog while trying to build a telephony program. Those experiments eventually gave birth to Erlang. yra programavimo kalba, skirta toleruojančioms sistemoms, kuriose gedimas yra nepriimtinas. Pagalvokite apie telefono jungiklį, kuriame prarastas skambutis laikomas avarine situacija, arba banko operacija, kurią reikia atlikti nepriklausomai nuo to. Erlangas Erlang eina rankomis su , sistema, kuri suteikia galingus įrankius, modelius ir elgesį, kad būtų galima kurti paskirstytas, klaidų toleruojančias programas. Atvira telekomunikacijų platforma (OTP) Projekto vykdymas vykdomas laikantis šių Patikrinome jį prie Branchų naudojimas ir The Mes neturėjome problemų kuriant projektą, todėl kudos kūrėjams. instructions maint-28 PVS-Studio analizatorius Visual Studio kodas Straipsnyje pateikiami tik aukšto ir vidutinio lygio įspėjimai. Tai užbaigia mūsų įvadą. Dabar pereikime prie įspėjimų. Mes juos suskirstysime į tris kategorijas: loginės klaidos; kritinės klaidos; Atminties klaidos Loginės klaidos Fragment N1 PVS-Studio įspėjimas apie tai: Kairėje ir dešinėje pusėje yra identiškos sub-išraiškos '(!esock_is_integer((env), (argv[3])))' iš operatoriaus 'annoo. 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]); } .... } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Kode kūrėjai norėjo konvertuoti trečiąjį ir ketvirtąjį argumentus į 64 bitų tipus (pasirašytus ir nepasirašytus).Funkcija grįžta anksti, jei vienas iš argumentų nėra skaičius. Fiksuotas kodas : .... if ((! IS_INTEGER(env, argv[2])) || (! IS_INTEGER(env, argv[3]))) return enif_make_badarg(env); .... Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Fragment N2 PVS-Studio įspėjimas apie tai: Yra identiškos sub-išraiškos 'n->type == ycf_node_type_on_save_yield_state_code' į kairę ir į dešinę nuo operatoriaus 'annoo. 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); } .... } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Štai dar vienas įdomus kopijavimo-pasto atvejis. funkcija, kuri pakartotinai tvarko medžio mazgus ir suranda matomumo sritis su vietiniais kintamaisiais.Tada skambina kintamojo vardo suvienijimo funkcija kiekvienai iš šių sričių. Šakos, The sąlyga yra dubliuojama po to, kai operatorių . uniqify_local_vars_in_node else if n->type == ycf_node_type_on_save_yield_state_code || Atsižvelgiant į palyginimo grandinę, toliau pateikto sąrašo reikšmės rūšiuojamos tokia tvarka, kokia jos deklaruojamos: 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; Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Tačiau The Nuolatinė kontrolė atliekama ankstesniame . So, I guess there's a duplicate check here after all, and we can delete it. ycf_node_type_on_restore_yield_state_code else if Fragment N3 PVS-Studio įspėjimas apie tai: Galimas rašymas iš anksto apibrėžto makro vardo rašybai. „Macro yra panašus į“ „ . 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); } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo This is a rather unusual case of checking whether a project compiles under Windows. Erlang has other checks on Windows ([ ) ir [ ], [ ], ir tt), bet šis yra unikalus.Tai labai panašu į paprastą tipą (tikriausiai tai turėtų būti ) ir 1 2 3 __WIN32__ But why is this the case? Out of curiosity, I searched for Pažiūrėkime, ką mes galime padaryti, čia yra rezultatai: "__WIN_32__" Kaip matote, beveik nėra jokių rezultatų. juokinga, kad viena iš nuorodų veda į failą, kurį žiūrime Aš išnagrinėjau projektą, kad nerasčiau panašių makro apibrėžimų. Aš taip pat maniau, kad jis gali būti įdiegtas į kai kuriuos kompiliatorius, pvz., MinGW. Deja, failed. After trying to understand the macro's purpose, I believe this code is flawed. Debian šaltiniai Eksperimentas Kaip taisyklę siūlau naudoti patikrinimą, kurį radau anksčiau kitose projekto dalyse: static int tcp_send_error(tcp_descriptor* desc, int err) { .... #if defined(__WIN32__) || defined(_WIN32) || defined(_WIN32_) if (err == ECONNABORTED) err = ECONNRESET; #endif .... } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Fragment N4 PVS-Studio įspėjimas apie tai: The code's operational logic does not correspond with its formatting. The second statement will always be executed. It is possible that curly brackets are missing. V640 nuotr. Dializė.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"); .... } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Here's an interesting example of a common coding error that can be difficult to spot later on. Take a look at the struktūrą : PUSH2 #define QUOTE(s) s #define PUSH(s) eargv[eargc++] = QUOTE(s) #define PUSH2(s, t) PUSH(s); PUSH(t) Enter fullscreen mode Exit fullscreen mode Uh-huh, tai makro, kuris plečiasi į dvi konstrukcijas. Išplėskime kodą kaip išankstinis procesorius: 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"; .... } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo As a result, if the check evaluates to , tik pirmoji užduotis bus įvykdyta sąlygiškai, o antrasis visada įvyks po šakos. false Writing and using macros Puikus sprendimas būtų atsikratyti jų, bet leiskite mums susitaikyti su mažiau radikaliu sprendimu. konstruoti, kai reikia sujungti keletą veiksmų į vieną makro. Patraukė bugs do {...} while (0) #define PUSH2(s, t) do { PUSH(s); PUSH(t); } while (0) Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Fragment N5 PVS-Studio įspėjimas apie tai: Sąlyga „i >= 0“ visada yra tiesa. Kėdainiai 654 apyvarta.lt 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; } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Pažvelkite į loop, paying special attention to its condition. Since the Indukcinė kintamoji yra iš type with a value range of , juosta tampa begalinė. Po juostos iteracijos su kintamasis tampa lygus Kaip rezultatas, sumažėjimas ir ciklas tęsiasi. 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--) { .... } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Panašūs įspėjimai: V654 Sąlyga „i >= 0“ visada yra tiesa. wxe_return.cpp 248 V654 Sąlyga „i >= 0“ visada yra tiesa. wxe_return.cpp 260 Fragment N6 The PVS-Studio warning: Naudojant "jei (A) {...} kitaip jei (A) {...}" modelis buvo aptiktas. Yra loginės klaidos buvimo tikimybė. Tikrinimo linijos: 2903, 3053. V717 erl_bif_info.c 293 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 .... } } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Analizatorius aptiko keletą šakų su identiškais patikrinimais funkcijoje, kurioje yra didžiulis skaičius Pranešimai – maždaug Tačiau kiekvienas iš jų turi skirtingą logiką: ir Atsižvelgiant į šakų skaičių ir 150 eilučių atotrūkį tarp dublikatų, vargu ar nenuostabu, kad tai gali atsitikti. if-else if 800 linijų Pirmasis patikrinimas Antrasis patikrinimas Fragment N7 PVS-Studio įspėjimas apie tai: Sąlyginės išraiškos dalis visada yra klaidinga: (arity == 0). V660 Būklė 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); } .... } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Here, we have an example involving math. The analyzer informs us that the last sub-expression is always false. Why? arity == 0 Jei kontrolės srautas pasieks tašką, kuriame ši išraiška yra vertinama, mes žinosime, kad dvi ankstesnės išraiškos yra Taip pat galime išmokti: false ix >= 1 ix = 1 arity I'll also leave fixing this code to the developers, since my only suggestion is to remove the condition. Critical errors Fragment N8 PVS-Studio įspėjimai apie: Indekso „prefix_len“ vertė gali siekti 262. V557 erl_alloc_util.c 5233 Array overrun is possible. The value of 'prefix_len' index could reach 262. Kėdainiai 557 erl_alloc_util.c 5378 Indekso „prefix_len“ vertė gali siekti 262. 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); } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Mes turime funkciją, kuri generuoja paskirstymo ir priskyrimo funkcijų pavadinimus, pagrįstus jai perduotu priedėliu. Jų generavimui naudojamas 255 baitų buferis. Sprendžiant pagal funkcijos turinį, generuojamame buferyje nebus jokių nulinių terminatorių, nes Pasiekia bendrą generuojamos eilutės dydį. am_atom_put Pirmas dalykas, kurį kūrėjai padarė, buvo apskaičiuoti priedėlio dydį.Tada jie apribojo priedėlio dydį: jei jis viršija and the length of the String (ilgiausias pašto nustatymas) Funkcija yra vadinama. MAX_ATOM_CHARACTERS realloc noreturn Next, they copied the prefix to the buffer, added the postfix, and passed the string to the Devs atliko visus šiuos veiksmus nuosekliai visiems postfixes: , ir . am_atom_put alloc realloc free Ar galite pastebėti sugavimą? Ankstyvasis grąžinimo patikrinimas buvo atliktas neteisingai. Puferis turi turėti pakankamai vietos parašyti ilgiausią postfix, kuris yra septyni simboliai. Jei priedėlio eilutė yra tarp 256 ir 262 simbolių ilgio, ankstyvasis grąžinimas neįvyksta, todėl funkcija perteka buferį. Ši operacija turi neapibrėžtą elgesį. sys_memcpy Darant prielaidą, kad funkcija veikia su eilutėmis ne ilgiau kaip Išspręskime kodą taip: am_atom_put MAX_ATOM_CHARACTERS if (prefix_len > MAX_ATOM_CHARACTERS - sizeof(realloc) + 1) Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Now, the maximum prefix length is 248 characters. When writing the maximum postfix, overflow won't occur. Panašūs įspėjimai: V557 Array perpildymas yra įmanomas. „i“ indekso vertė gali siekti 40. ycf_lexer.c 493 V557 Array overrun is possible. The value of 'n' index could reach 16. erl_call.c 1663 Fragment N9 Norėčiau pristatyti šį pavyzdį neįprastai.Pirmiausia pažiūrėkime į šią funkciją: 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; } } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Tų Funkcija konvertuoja enumeration to a string. For non- branches, 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 , C23 ) ir event_state_flag_to_str EventStateFlags default switch const 6.7.3.6 6.7.4.1 ir 7.7 Ar tai iš tikrųjų yra tiesa?Pažvelkime į apeliacinį skundą: 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: Apsvarstykite galimybę patikrinti antrąjį faktinį „event_state_flag_to_str“ funkcijos argumentą. 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 I also suggest changing the function signature, so the parameter can take a 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 The PVS-Studio warning: Neinicializuotos klasės narys „a“ naudojamas inicijuojant bazinę klasę „BeamAssemblerCommon“. V1050 Žymės: 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 Šiame fragmente yra nedidelės klasės hierarchija, kurioje nuoroda į nestatinę išvestinės klasės funkciją, , is passed to the constructor of the base class, Ši operacija nesukels jokių problemų tol, kol bazinės klasės kūrėjas sąveikauja su šia nario funkcija. Kodėl? Išvestinės klasės objekto gyvavimo laikas prasideda, kai inicijuojamojo sąrašo reikalaujamas inicializatorius yra paleistas arba, jei nėra inicializatoriaus, kai valdymo srautas patenka į konstruktoriaus kūną. BeamAssembler BeamAssemblerCommon Atsižvelgdami į šią informaciją, pažvelkime į bazinės klasės konstruktorių. BeamAssemblerCommon::BeamAssemblerCommon(BaseAssembler &assembler_) : assembler(assembler_), .... { .... #ifdef DEBUG assembler.addDiagnosticOptions(DiagnosticOptions::kValidateAssembler); #endif assembler.addEncodingOptions(EncodingOptions::kOptimizeForSize | EncodingOptions::kOptimizedAlign); .... } Enter fullscreen mode Exit fullscreen mode 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. Atminties klaidos Fragment N11 PVS-Studio įspėjimas apie tai: „Laisva“ funkcija skambinama du kartus, kad būtų išdalinta ta pati atminties erdvė. V586 žymės.lt 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); .... } Enter fullscreen mode Exit fullscreen mode 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 is passed to the 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 procedure call p free stderr free Sprendimas yra gana paprastas: arba sustabdyti programos vykdymą arba nustatyti išlaisvintus rodiklius iki nulio. 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 PVS-Studio įspėjimas apie tai: „pkey“ rodyklė buvo panaudota prieš ją patikrinant prieš nullptr. patikrinimo linijos: 581, 582. Žmogus 595 Kėdainiai.lt 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; } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo We have quite an interesting code fragment here. The bando išgauti viešąjį raktą į penktąjį parametrą. grąžina if successful and Jei ne, paskutiniu atveju viešasis raktas taip pat yra nulinis. function 1 0 Analitikas įspėja, kad vienoje iš funkcijų šakų pointer is dereferenced before being checked for validity. However, other branches don't seem to have such checks. We can conclude that the function has the following contract: "The parameter must never be equal to a Mes galime tai patvirtinti ieškodami šios funkcijos skambučių tame pačiame faile ([ ) ir [ ) ir [ ]) – kintamasis adresas perduodamas visur. pkey pkey nullptr 1 2 3 In fact, the developers intended to handle the incorrect result of the Veikla, nukreipta į mark. However, they made a typo by forgetting to put an asterisk before the Operatorius: šiuolaikinis kompiliatorius this check on release. As a result, the function will return , ir nulinis rodyklė baigiasi penktuoju parametru. ENGINE_load_public_key err ! optimize away 1 Let's fix the check: .... *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")); .... Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Fragment N13 Dažnai gauname pranešimus, kad analizatorius įspėja apie nulio rodyklės perdavimą Galų gale, standartas teigia, kad nieko baisaus neįvyks. Apie panašų atvejį. free straipsnis 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. PVS-Studio įspėjimas apie tai: Null rodyklė perkeliama į „laisvą“ funkciją. 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); } } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Šioje dalyje, The Pointer yra aiškiai nustatytas Prieš skambinant . As a result, the memory-freeing operation is pointless, since does nothing. This results in memory leaks because the original memory block that pointed to is never freed. cpuinfo->topology NULL free free(NULL) cpuinfo->topology Mes galime išspręsti problemą pakeičiant operacijas: if (cpuinfo->topology) { free(cpuinfo->topology); cpuinfo->topology = NULL; } Enter fullscreen mode Exit fullscreen mode Fragment N14 PVS-Studio įspėjimas apie tai: The function was exited without releasing the 'entry' pointer. A memory leak is possible. V773 išleidimas 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; .... } Enter fullscreen mode Exit fullscreen mode In the funkcijos, o rodyklė yra deklaruojama ir inicijuojama per matricinį elementą. Jei šis elementas yra null rodyklė, atmintis paskirstoma naudojant Toliau rodikliai lyginami.Jei jis vertinamas pagal Tačiau, jei įvyko dinaminis paskirstymas, atmintis nebus paleista. setActiveGL entry malloc true Taip, tokia situacija yra gana reta ir mažai tikėtina, nes tai reikalauja visos įvykių serijos: kai funkcija skambinama, nuliniai rodikliai perduodami kaip trečiasis ir ketvirtasis argumentai. glc[callId] turi nulinį rodyklę. However, we can ensure that a leak will never occur, not even theoretically. Let's modify the code a little: 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; } .... Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Panašūs įspėjimai: V773 Funkcija buvo išjungta uždarius failą, nurodytą „f“ rankena. V773 „erl_errno_p“ rodyklė buvo priskirta reikšmėms du kartus, neatleidžiant atminties. 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 Išimtis buvo išmesta neišleidžiant „duomenų“ rodyklės. galimas atminties nutekėjimas. wxe_wrapper_4.cpp 1330 V773 Išimtis buvo išmesta neišleidžiant „duomenų“ rodyklės. galimas atminties nutekėjimas. wxe_wrapper_4.cpp 1663 V773 Išimtis buvo išmesta neatleidžiant „duomenų“ rodyklės. galimas atminties nutekėjimas. wxe_wrapper_4.cpp 1690 V773 Išimtis buvo išmesta neišleidžiant „duomenų“ rodyklės. galimas atminties nutekėjimas. wxe_wrapper_4.cpp 1715 V773 Išimtis buvo išmesta neišleidžiant „alfa“ rodyklės. galimas atminties nutekėjimas. wxe_wrapper_4.cpp 1718 V773 The exception was thrown without releasing the 'data' pointer. A memory leak is possible. wxe_wrapper_4.cpp 1747 V773 The exception was thrown without releasing the 'alpha' pointer. A memory leak is possible. 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 The exception was thrown without releasing the 'data' pointer. A memory leak is possible. wxe_wrapper_4.cpp 2733 Fragment N15 PVS-Studio įspėjimas apie tai: realloc() possible leak: when realloc() fails in allocating memory, original pointer 'outbuf_base' is lost. Consider assigning realloc() to a temporary pointer. 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; } .... } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo There's a tricky pattern with the funkcija, kur originalus rodyklė yra nedelsiant užrašyta grąžinimo reikšmė. realloc The function takes the buffer for reallocation as its first argument and the new buffer size as its second argument. If the operation is successful, the function will return a pointer to the newly allocated memory. The memory passed as the first argument becomes , even if only a simple memory block expansion has occurred. If the operation fails, the function returns a Atmintis praėjo kaip pirmasis argumentas lieka . invalid null pointer untouched Taigi, jei mes iš karto perrašome pradinį rodyklę su kvietimu , we will get a memory leak. realloc Išspręskime kodą taip: 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; } .... } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Be to, norint išspręsti šią problemą, reikia atsižvelgti į situaciją, kai 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 The PVS-Studio warning: Išraiškos „pabaiga - mm->sua.top“ rodyklė „pabaiga“ yra lygi nullptr. Gaunama reikšmė yra beprasmiška ir neturėtų būti naudojama. V769 erl_mmap.c 2411 žymės 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); } .... } Įveskite pilno ekrano režimą Išėjimas iš pilno ekrano režimo Analizatorius aptiko įtartiną adreso aritmetiką. pažiūrėkime, kas čia negerai. We enter the code branch where is indeed a null pointer. The Pagal C standartą skirtingų rodiklių elgesys apibrėžiamas tik tada, kai jie abu nurodo į tos pačios masės elementus (C11). • C23 Null rodyklė nenurodo jokio matricinio elemento.Taigi, šios operacijos elgesys nėra apibrėžtas, o tai nėra gerai. end mm->sua.top 6.5.6.8 6.5.7 Iš viso Įdomu, kas vyksta šiame ir funkcijos. 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 Pointer subtraction is also used here, but with a difference: first, the pointers are converted to numbers. This is implementation-defined behavior (C11 • C23 ), kuris yra daug geresnis. 6.3.2.3.6 6.3.2.3.6 Deja, aš negaliu pasiūlyti tinkamą sprendimą. kūrėjai gali norėti pažvelgti į šią problemą. Conclusion Na, tai užbaigia straipsnį. Kaip tikėtasi su bet kokiu dideliu projektu, kuris turėjo ilgą ir įvykių istoriją, mes radome įvairias klaidas. Jie taip pat yra projekto dalis. Svarbu nepamiršti, kad tokių problemų atradimas kodų bazėje, kuri yra daugiau nei tris dešimtmečius sena, nėra silpnumo ženklas. Vietoj to, tai yra kodo neįtikėtino masto ir ilgaamžiškumo įrodymas. Yra milijonai kodų eilučių, parašytų šimtų kūrėjų, ir sistema veikia jau daugelį metų be pakartotinio paleidimo. Šis projektas nusipelno giliausios pagarbos, ne todėl, kad jis yra tobulas, bet todėl, kad jis išlaikė laiko testą ir įrodė savo tikrąją galią. Dėl nuolat besivystančių šiuolaikinių technologijų kūrėjai turi galingų įrankių, kad pagerintų kodo patikimumą. Pavyzdžiui, statiniai analizatoriai padeda nustatyti paslėptas klaidas ir galimus pažeidžiamumus. kviečiu jus tai pamatyti sau :) Galite gauti bandomąją PVS-Studio analizatoriaus versiją . here