Kód 80-tych rokov posilňujúci modernú spoľahlivosť - a chyby, ktoré stále robí

Kód pre niektoré moduly Erlang/OTP je starší ako väčšina moderných juniorových vývojárov. Tieto súbory sú skutočnými digitálnymi patriarchami. Po desaťročia sa uistili, že bankové transakcie, telefónne siete a systémy zasielania správne fungujú. Rozhodli sme sa pozrieť pod kapotu tohto dlho žijúceho jazyka, aby sme zistili, čo presne leží za líniami, na ktoré sa dnes spoliehajú milióny používateľov. Úvodná Predtým, než začneme diskutovať o varovaniach, chcel by som povedať pár slov o projekte. Erlangova cesta začala v roku 1986 v jednom z Ericssonových laboratórií. Joe Armstrong experimentoval s Prologom, zatiaľ čo sa snažil vybudovať telefónny program. je programovací jazyk určený pre systémy s toleranciou na chyby, kde je zlyhanie neprijateľné.Premýšľajte o telefónnom prepínači, kde sa zlyhanie počítalo ako núdzová situácia, alebo banková transakcia, ktorá musí prejsť bez ohľadu na to, čo.Toto sú úlohy, na ktoré sa používa.Umožňuje systému rozdeliť sa na milióny izolovaných, nezávislých procesov, takže keď jeden zlyhá, zvyšok beží bez toho, aby chýbal úder. Erlang Erlang ide ruka v ruke s , rámec, ktorý poskytuje výkonné nástroje, vzory a správanie pre budovanie distribuovaných aplikácií s toleranciou na chyby. Tento vstavaný súbor nástrojov umožňuje systémom napísaným s ním bežať nepretržite po celé roky bez toho, aby ste sa museli zastaviť. Otvorená telekomunikačná platforma (OTP) Projekt sme realizovali podľa týchto Skontrolovali sme to na Branch používa a tie Nemali sme žiadne problémy s budovaním projektu, takže kudos k vývojárom. Inštrukcie maint-28 PVS-Studio analyzátor Visual Studio kódovanie Článok obsahuje iba varovania na vysokej a strednej úrovni. To končí náš úvod. Teraz prejdime na varovania. Rozdelíme ich do troch kategórií: logické chyby kritické chyby ; Pamäťové chyby Logic errors Fragment N1 Výstraha zo štúdia PVS: Existujú identické podvýrazy '(!esock_is_integer((env), (argv[3])))' na ľavej a pravej strane operátora '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]); } .... } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky V kóde chceli vývojári previesť tretie a štvrté argumenty na 64-bitové typy (podpísané a nepodpísané). Funkcia sa vráti skôr, ak jeden z argumentov nie je číslo. Zadaný pevný kód: .... if ((! IS_INTEGER(env, argv[2])) || (! IS_INTEGER(env, argv[3]))) return enif_make_badarg(env); .... vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Fragment N2 Výstraha zo štúdia PVS: There are identical sub-expressions 'n->type == ycf_node_type_on_save_yield_state_code' to the left and to the right of the '||' operator. V501 ycf_node.c 218 Zľavy 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); } .... } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Tu je ďalší zaujímavý prípad kopírovania pasty. funkcia, ktorá rekurzívne spracováva stromové uzly a nájde oblasti viditeľnosti s lokálnymi premennými. Potom volá funkciu zjednotenia názvu premenných pre každú z týchto oblastí. Brankáreň, The condition is duplicated after the operator. uniqify_local_vars_in_node else if n->type == ycf_node_type_on_save_yield_state_code || Unfortunately, I don't have a proper fix for this fragment. Judging by the comparison chain, the values of the following list are sorted in the order they're declared: 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; vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Avšak, The constant is checked in the previous Takže predpokladám, že tu je duplicitná kontrola, a my ju môžeme odstrániť. ycf_node_type_on_restore_yield_state_code else if Fragment N3 Výstraha zo štúdia PVS: Možné typovanie v pravopise vopred definovaného názvu makra. „Macro je podobné“ „... 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); } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky This is a rather unusual case of checking whether a project compiles under Windows. Erlang has other checks on Windows ([ ] , , [ ] , , [ ], atď.), ale tento je jedinečný. To vyzerá veľmi ako jednoduchý typ (s najväčšou pravdepodobnosťou by to malo byť ) sa 1 2 3 __WIN32__ Prečo je to tak?Zaujala ma zvedavosť. Pozrite sa, čo sa podarilo dosiahnuť, tu sú výsledky: "__WIN_32__" Ako vidíte, neexistujú takmer žiadne výsledky.Je zábavné, že jeden z odkazov vedie k súboru, ktorý hľadáme Prešiel som projektom, aby som nenašiel žiadne podobné definície makier. Taktiež som si myslel, že by to mohlo byť zabudované do nejakého kompilátora, ako je MinGW. Po pokuse pochopiť účel makra, verím, že tento kód je chybný. Debian Zdroje Experimentovať Ako opravu navrhujem použiť kontrolu, ktorú som našiel skôr v iných častiach projektu: static int tcp_send_error(tcp_descriptor* desc, int err) { .... #if defined(__WIN32__) || defined(_WIN32) || defined(_WIN32_) if (err == ECONNABORTED) err = ECONNRESET; #endif .... } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Fragment N4 Výstraha zo štúdia PVS: Operačná logika kódu nezodpovedá jeho formátovaniu. Druhá veta bude vždy vykonaná. V640 Zľavy dialyzér.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"); .... } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Tu je zaujímavý príklad bežnej kódovacej chyby, ktorú môže byť ťažké zistiť neskôr. structure: PUSH2 #define QUOTE(s) s #define PUSH(s) eargv[eargc++] = QUOTE(s) #define PUSH2(s, t) PUSH(s); PUSH(t) vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Uh-huh, je to makro, ktoré sa rozširuje na dve konštrukcie. Rozšírime kód ako preprocesor: 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"; .... } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky V dôsledku toho, ak kontrolný orgán posúdi , iba prvá úloha bude podmienene vykonaná, zatiaľ čo druhá sa vždy vyskytne po pobočke. false Písanie a používanie makier Ideálnym riešením by bolo zbaviť sa ich, ale poďme sa uspokojiť s menej radikálnym riešením. konštrukcie, keď potrebujú skombinovať niekoľko akcií do jedného v rámci makra. attracts bugs do {...} while (0) #define PUSH2(s, t) do { PUSH(s); PUSH(t); } while (0) vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Fragment N5 Výstraha zo štúdia PVS: Podmienka "i >= 0" v kruhu je vždy pravdivá. V645 zvrátenie.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; } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Take a look at the loop, paying special attention to its condition. Since the induction variable is of the typ s rozsahom hodnôt , the loop becomes infinite. After the loop iterates with , the variable becomes equal to as a result of the decrement, and the loop continues. 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--) { .... } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Similar warnings: V654 The condition 'i >= 0' of loop is always true. wxe_return.cpp 248 V654 Podmienka „i >= 0“ v kruhu je vždy pravdivá. wxe_return.cpp 260 Fragment N6 Výstraha zo štúdia PVS: The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 2903, 3053. V517 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 .... } } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Analyzátor zistil niekoľko pobočiek s identickými kontrolami vo funkcii obsahujúcej obrovské množstvo Výsledky - zhruba Každý z nich má však inú logiku: a Vzhľadom na počet pobočiek a rozdiel 150 riadkov medzi duplikátmi nie je prekvapujúce, že by sa to mohlo stať. Statická analýza pomáha predchádzať takýmto prípadom. if-else if 800 riadkov Prvá kontrola second check Fragment N7 Výstraha zo štúdia PVS: Časť podmieneného výrazu je vždy nepravdivá: (arity == 0). V560 Zľavy Býk 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); } .... } Enter fullscreen mode Exit fullscreen mode Tu máme príklad zahŕňajúci matematiku. analyzátor nás informuje, že posledný Príspevok v téme je vždy falošný, prečo? arity == 0 Ak tok riadenia dosiahne bod, kde je tento podvýraz vyhodnotený, budeme vedieť, že dve predchádzajúce podvýrazy sú Môžeme sa tiež naučiť nasledovné: false ix >= 1 ix = 1 arity Opravu tohto kódu tiež ponechám vývojárom, pretože mojím jediným návrhom je odstrániť podmienku. Kritické chyby Fragment N8 The PVS-Studio warnings: Array overrun is possible. The value of 'prefix_len' index could reach 262. V557 erl_alloc_util.c 5233 Hodnota indexu 'prefix_len' by mohla dosiahnuť 262. V 557 erl_alloc_util.c 5237 Hodnota indexu 'prefix_len' by mohla dosiahnuť 262. V 557 erl_alloc_util.c 5381 Zľavy #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); } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Máme funkciu, ktorá generuje mená pre alokácie a dealokácie funkcií na základe predpony odovzdané k nemu. Používa sa 255-byte buffer na ich generovanie. Súdiac podľa obsahu funkcie, nebude tam žiadny null terminator v generovanej buffer, pretože presahuje celkovú veľkosť generovaného reťazca. am_atom_put Prvá vec, ktorú vývojári urobili, bolo vypočítať veľkosť predpony. Potom obmedzili veľkosť predpony: ak prevyšuje súčet a dĺžku trvania String (najdlhší poštový fix) Funkcia sa volá. MAX_ATOM_CHARACTERS realloc noreturn Ďalej skopírovali predponu do vyrovnávača, pridali postfix a odovzdali reťazec do function. The devs performed all these actions sequentially for all postfixes: , , , a . am_atom_put alloc realloc free Môžete nájsť úlovok? Skúška skorého vrátenia bola vykonaná nesprávne. Puffer musí mať dostatok priestoru na napísanie najdlhšieho postfix, čo je sedem znakov. Ak je predponový reťazec medzi 256 a 262 znakmi dlhý, skorý návrat sa nevyskytuje, takže funkcia prekrýva buffer. Táto operácia má nedefinované správanie. sys_memcpy Za predpokladu, že funkcia pracuje s reťazcami nie dlhšie ako , let's fix the code as follows: am_atom_put MAX_ATOM_CHARACTERS if (prefix_len > MAX_ATOM_CHARACTERS - sizeof(realloc) + 1) vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Teraz je maximálna dĺžka predpony 248 znakov. Keď napíšete maximálnu postfix, nedôjde k pretečeniu. Podobné varovania: V557 Array overrun je možný. Hodnota indexu 'i' by mohla dosiahnuť 40. ycf_lexer.c 493 V557 Array overrun je možný. Hodnota indexu 'n' môže dosiahnuť 16. erl_call.c 1663 Fragment N9 Chcel by som tento príklad predstaviť nezvyčajným spôsobom. Najprv sa pozrime na túto funkciu: 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; } } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky The Funkcia premení na Vybrať položku String. pre non- odvetviach , Vráti reťazec doslovný. V opačnom prípade funkcia vytvorí reťazec v poskytnutom vyrovnávači a vráti naň ukazovateľ. Všimnite si, ako presne to robí: vyrovnávač núteným spôsobom vylučuje konštantnosť. Podľa štandardu C je to povolené len vtedy, ak zdrojový vyrovnávač nebol vyhlásený ako - C11 , C23 ) sa event_state_flag_to_str EventStateFlags default switch const 6.7 3 6 6.7.4.1.7 Is this really the case? Let's take a look at the invocation point: 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 Funkcia sa volá tak, že prechádza konštantným vyrovnávačom ako druhým argumentom. Bohužiaľ, máme nedefinované správanie. Túto chybu sme zistili pomocou analyzátora, ale vydalo to mätúce upozornenie: Používa sa nezačiatočný vyrovnávač buff. Zvážte kontrolu druhého skutočného argumentu funkcie event_state_flag_to_str. V614 erl_check_io.c 283 Opravme kód odstránením konštantnosti z vyrovnávača: 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 Navrhujem tiež zmeniť funkcie, takže parameter môže mať To jasne ukazuje, že funkcia môže zmeniť vyrovnávaciu pamäť, ktorá sa na ňu prenáša. Nebudeme v procese rozbíjať veľa kódu, pretože funkcia existuje iba v kompilovanom súbore a je označená ako . event_state_flag_to_str char* static Fragment N10 Výstraha zo štúdia PVS: The uninitialized class member 'a' is used when initializing the base class 'BeamAssemblerCommon'. V1050 beam_asm.hpp 87 Zľavy 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 Tento fragment obsahuje malú hierarchiu tried, kde odkaz na neštatickú členskú funkciu odvodenej triedy, , is passed to the constructor of the base class, Táto operácia nespôsobí žiadne problémy, pokiaľ konštruktér základnej triedy neinteraguje s touto členskou funkciou. Prečo? Životnosť objektu z odvodenej triedy začína, keď bol spustený požadovaný inicializátor v zozname inicializácie alebo, ak neexistuje žiadny inicializátor, keď tok riadenia vstúpi do tela konštruktéra. BeamAssembler BeamAssemblerCommon S týmito informáciami na mysli, poďme sa pozrieť na konštruktor základnej triedy. Nepoužíva tento objekt, nie? 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 :) Neštatická členská funkcia sa volá na objekt odvodenej triedy prostredníctvom odkazu, keď jeho životnosť ešte nezačala. Tiež sa mi zdá, že je ťažké navrhnúť opravu v tomto prípade, takže to nechám vývojárom. Memory errors Fragment N11 The PVS-Studio warning: Funkcia "voľný" sa volá dvakrát pre prerozdelenie rovnakého pamäťového priestoru. 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); .... } Enter fullscreen mode Exit fullscreen mode súdiť podľa mena, is a . A negative number is returned if the operation fails. If the remote call fails, some resources are cleaned up. The Pointer prešiel na function and an error is logged to Ďalej, bez ohľadu na výsledok funkčného volania, sa ukazovateľ prenesie na again. Memory is released twice. ei_rpc remote procedure call p free stderr free The fix is pretty simple: either stop the program execution or set the freed pointers to zero. To fix the issue, I would interrupt execution here: 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); vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Fragment N12 Výstraha zo štúdia PVS: The 'pkey' pointer was utilized before it was verified against nullptr. Check lines: 581, 582. V 595 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; } Enter fullscreen mode Exit fullscreen mode Máme tu veľmi zaujímavý fragment kódu. 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 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 Môžeme to potvrdiť vyhľadávaním hovorov na túto funkciu v tom istom súbore ([ ], [ ], [ ]) – variabilná adresa sa prenáša všade. pkey pkey nullptr 1 2 3 In fact, the developers intended to handle the incorrect result of the function by jumping to the Napriek tomu napísali, že zabudli dať hviezdičku pred moderný kompilátor bude 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 Poďme si urobiť kontrolu: .... *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 . After all, the standard states that nothing terrible will happen. We even wrote an o podobnom prípade. free article Áno, je to v poriadku, ale takýto kód je pravdepodobne podozrivý.To nám pomohlo nájsť zaujímavý únik pamäte v kóde. Poďme sa na to pozrieť. The PVS-Studio warning: Ukazovateľ null sa prevedie na funkciu "voľný". skontrolujte prvý 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); } } Enter fullscreen mode Exit fullscreen mode In this fragment, the pointer is explicitly set to Pred volaním V dôsledku toho je operácia uvoľňovania pamäti zbytočná, pretože 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 Problém môžeme vyriešiť výmenou operácií: if (cpuinfo->topology) { free(cpuinfo->topology); cpuinfo->topology = NULL; } Enter fullscreen mode Exit fullscreen mode Fragment N14 Výstraha zo štúdia PVS: The function was exited without releasing the 'entry' pointer. A memory leak is possible. V773 Zľavy.sk 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; .... } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky V tejto funkcie, na ukazovateľ je deklarovaný a inicializovaný prostredníctvom prvku mriežky. Ak je tento prvok ukazovateľom null, pamäť je priradená pomocou Ďalej sa ukazovatele porovnávajú.Ak hodnotí , dôjde k predčasnému návratu. Avšak, ak dôjde k dynamickému prideľovaniu, pamäť sa uvoľní. setActiveGL entry malloc true Áno, takáto situácia je pomerne zriedkavá a nepravdepodobná, pretože si vyžaduje celú sériu udalostí: keď je funkcia volaná, nulové ukazovatele sa prejdú ako tretí a štvrtý argument. glc[callId] obsahuje ukazovateľ null. Môžeme však zabezpečiť, že únik sa nikdy nevyskytne, ani teoreticky. 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; } .... vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Similar warnings: V773 Funkcia bola ukončená bez uzavretia súboru, na ktorý odkazuje ovládač "f". únik zdroja je možný. erl_poll.c 2423 V773 Pointeru 'erl_errno_p' boli pridelené hodnoty dvakrát bez uvoľnenia pamäte. V773 The function was exited without closing the file referenced by the 'fp' handle. A resource leak is possible. cpu_sup.c 433 V773 Výnimka bola vyhodená bez uvoľnenia ukazovateľa "údaje". únik pamäte je možný. wxe_wrapper_4.cpp 1300 V773 Výnimka bola vyhodená bez uvoľnenia ukazovateľa "údaje". únik pamäte je možný. 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 The exception was thrown without releasing the 'data' pointer. A memory leak is possible. wxe_wrapper_4.cpp 1690 V773 Výnimka bola vyhodená bez uvoľnenia ukazovateľa "údaje". únik pamäte je možný. wxe_wrapper_4.cpp 1715 V773 The exception was thrown without releasing the 'alpha' pointer. A memory leak is possible. 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 Výnimka bola uvoľnená bez uvoľnenia ukazovateľa „alpha“. únik pamäte je možný. 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 Výnimka bola vyhodená bez uvoľnenia ukazovateľa "údaje". únik pamäte je možný. 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 Výstraha zo štúdia PVS: realloc() možný únik: keď realloc() zlyhá v alokácii pamäte, pôvodný ukazovateľ 'outbuf_base' sa stratí. 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; } .... } Enter fullscreen mode Exit fullscreen mode 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 Funkcia berie buffer pre prerozdelenie ako svoj prvý argument a novú veľkosť bufetu ako svoj druhý argument. Ak je operácia úspešná, funkcia vráti ukazovateľ do novo pridelenej pamäte. , aj keď sa vyskytlo iba jednoduché rozšírenie bloku pamäte. Ak operácia zlyhá, funkcia vráti . The memory passed as the first argument remains . invalid null pointer untouched Takže, ak okamžite prepíšeme pôvodný ukazovateľ s výzvou na Budeme mať pamäťový únik. realloc Opravme kód nasledovne: 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 Výstraha zo štúdia PVS: Ukazovateľ "koniec" vo výraze "koniec - mm->sua.top" sa rovná nullptr. Výsledná hodnota nemá zmysel a nemala by sa používať. V769 Zľavy erl_mmap.c 2411 Zľavy 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); } .... } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky The analyzer has detected suspicious address arithmetic. Let's see what's wrong here. Vstúpime do kódu, kde 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.6 6 8 6.5.7.10 Zaujímalo ma, čo sa deje v function. 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)) { .... } vstup do režimu plnej obrazovky výstup z režimu plnej obrazovky Pointer subtraction is also used here, but with a difference: first, the pointers are converted to numbers. This is implementation-defined behavior (C11 • C23 ), čo je oveľa lepšie. 6.3.2 a 3.6 6.3.2 a 3.6 Unfortunately, I can't suggest a proper fix. The developers might want to take a look into this issue. Conclusion No, to končí článok. Ako sa očakávalo s akýmkoľvek veľkým projektom, ktorý mal dlhú a udalostnú históriu, našli sme rôzne chyby. Sú tiež súčasťou projektu. Je dôležité mať na pamäti, že nájdenie týchto problémov v databáze kódu, ktorá je stará viac ako tri desaťročia, nie je známkou slabosti. Skôr je to dôkazom neuveriteľného rozsahu a dlhovekosti kódu. Existujú milióny riadkov kódu napísaných stovkami vývojárov a systém beží už roky bez reštartu. Tento projekt si zaslúži najhlbšiu úctu, nie preto, že je dokonalý, ale preto, že vydržal skúšku času a dokázal svoju skutočnú silu. 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 . tu