1980'ernes kode, der styrer moderne pålidelighed - og de fejl, det stadig gør

Koden til nogle Erlang/OTP-moduler er ældre end de fleste moderne junior-udviklere. Disse filer er de sande digitale patriarker. I årtier har de sørget for, at banktransaktioner, telefonnettet og messaging-systemer alle fungerer korrekt. Vi besluttede at tage et kig under kappen af dette langvarige sprog for at se, hvad der præcis ligger bag de linjer, som millioner af brugere stoler på i dag. Introduktion Før vi går videre til at diskutere advarsler, vil jeg gerne sige et par ord om projektet. Erlangs rejse begyndte tilbage i 1986 i et af Ericssons laboratorier. Joe Armstrong eksperimenterede med Prolog, mens han forsøgte at opbygge et telefonprogram. er et programmeringssprog designet til fejl-tolerante systemer, hvor fejl er uacceptabelt. Tænk på en telefon switch, hvor et faldet opkald tæller som en nødsituation, eller en banktransaktion, der skal gå igennem uanset hvad. Erlang Erlang går hånd i hånd med , en ramme, der giver kraftfulde værktøjer, mønstre og adfærd til opbygning af distribuerede, fejl-tolerante applikationer. Denne indbyggede værktøjskasse gør det muligt for systemer skrevet med det at køre kontinuerligt i årevis uden nogensinde at skulle stoppe. Den åbne telekommunikationsplatform (OTP) Vi har udviklet projektet efter disse Vi har tjekket det på Branchen bruger Og den Vi havde ingen problemer med at opbygge projektet, så kudos til udviklerne. Instruktioner maint-28 PVS-Studio analyser Visual Studio Kode Artiklen indeholder kun høj- og mellemniveau advarsler. Det afslutter vores introduktion. Nu, lad os gå videre til advarslerne. Vi vil opdele dem i tre kategorier: Logiske fejl critical errors; Fejl i hukommelsen. Logiske fejl Fragment N1 Den PVS-Studio advarsel: Der er identiske underudtryk '((!esock_is_integer((env), (argv[3])))' til venstre og til højre for operatoren 'annoo. af V501 prim_socket_nif.c 6035 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]); } .... } Indtast fuldskærmstilstand Udtast fuldskærmstilstand I koden ønskede udviklerne at konvertere de tredje og fjerde argumenter til 64-bit-typer (de signerede og ikke-signerede, henholdsvis). Funktionen returnerer tidligt, hvis et af argumenterne ikke er et tal. Den faste kode: .... if ((! IS_INTEGER(env, argv[2])) || (! IS_INTEGER(env, argv[3]))) return enif_make_badarg(env); .... Indtast fuldskærmstilstand Udtast fuldskærmstilstand Fragment N2 Den PVS-Studio advarsel: Der er identiske underekspresser 'n->type == ycf_node_type_on_save_yield_state_code' til venstre og til højre for operatoren 'annoo. af V501 ycf_node.dk 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); } .... } Indtast fuldskærmstilstand Udtast fuldskærmstilstand Her er en anden interessant kopi-paste sag. Vi har den funktionen, som recursivt håndterer trænoder og finder synlighedsområder med lokale variabler. Det kalder derefter den variable navneunifikationsfunktion for hvert af disse områder. Branch, den Forholdet er fordoblet efter af operatøren. uniqify_local_vars_in_node else if n->type == ycf_node_type_on_save_yield_state_code || Efter sammenligningskæden er værdierne i følgende liste sorteret i den rækkefølge, de er erklæret: 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; Indtast fuldskærmstilstand Udtast fuldskærmstilstand Men den konstant kontrolleres i den foregående Så jeg tror, at der er en duplikatcheck her trods alt, og vi kan slette det. ycf_node_type_on_restore_yield_state_code else if Fragment N3 Den PVS-Studio advarsel: Mulig typo i stavningen af et foruddefineret makronavn. ‘Macro er ligner’ » . V1040 WIN_32 WIN32 iværksætter.dk 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); } Indtast fuldskærmstilstand Udtast fuldskærmstilstand Dette er et temmelig usædvanligt tilfælde af at kontrollere, om et projekt kompilerer under Windows. Erlang har andre kontroller på Windows ([ ], [ ] , [ ], etc.), but this one is unique. This looks very much like a simple typo (most likely, it should be ) af 1 2 3 __WIN32__ Men hvorfor er det sådan? af nysgerrighed, jeg søgte efter Se, hvad jeg har fundet, her er resultaterne: "__WIN_32__" Som du kan se, er der næsten ingen resultater. Det er sjovt, at en af linkene fører til den fil, vi kigger på Jeg greb igennem projektet for ikke at finde lignende makrodefinitioner. jeg troede også, at det kunne være indbygget i nogle compiler, som MinGW. Desværre, den failed. After trying to understand the macro's purpose, I believe this code is flawed. Debian kilder Eksperimentet Som en løsning foreslår jeg at bruge tjekket, som jeg fandt tidligere i andre dele af projektet: static int tcp_send_error(tcp_descriptor* desc, int err) { .... #if defined(__WIN32__) || defined(_WIN32) || defined(_WIN32_) if (err == ECONNABORTED) err = ECONNRESET; #endif .... } Enter fullscreen mode Exit fullscreen mode Fragment N4 Den PVS-Studio advarsel: Kodens operationelle logik svarer ikke til dens formatering. Den anden sætning vil altid blive udført. V640 Håndværker.dk 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"); .... } Indtast fuldskærmstilstand Udtast fuldskærmstilstand Her er et interessant eksempel på en almindelig kodefejl, der kan være svært at opdage senere. structure: PUSH2 #define QUOTE(s) s #define PUSH(s) eargv[eargc++] = QUOTE(s) #define PUSH2(s, t) PUSH(s); PUSH(t) Indtast fuldskærmstilstand Udtast fuldskærmstilstand Uh-huh, it's a macro that expands into two constructs. Let's expand the code as a preprocessor would: 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"; .... } Indtast fuldskærmstilstand Udtast fuldskærmstilstand Det vil sige, at hvis undersøgelsen vurderer, at , kun den første opgave vil blive betinget udført, mens den anden altid vil forekomme efter grenen. false Writing and using macros Den perfekte løsning ville være at slippe af med dem, men lad os stille op for en mindre radikal løsning. Konstruktion, når de har brug for at kombinere flere handlinger til en i en makro. Tiltrækker bugs do {...} while (0) #define PUSH2(s, t) do { PUSH(s); PUSH(t); } while (0) Indtast fuldskærmstilstand Udtast fuldskærmstilstand Fragment N5 Den PVS-Studio advarsel: Betingelsen 'i >= 0' af loop er altid sandt. V645 iværksætter.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; } Indtast fuldskærmstilstand Udtast fuldskærmstilstand Tag et kig på den for at lægge særlig vægt på deres tilstand, da de induction variable is of the Type med et værdiområde på , loop bliver uendelig. Efter loop itererer med , 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--) { .... } Indtast fuldskærmstilstand Udtast fuldskærmstilstand Lignende advarsler: V654 The condition 'i >= 0' of loop is always true. wxe_return.cpp 248 V654 Betingelsen 'i >= 0' af loop er altid sandt. wxe_return.cpp 260 Fragment N6 The PVS-Studio warning: Brug af 'hvis (A) {...} ellers hvis (A) {...}' mønster blev opdaget. Der er en sandsynlighed for logisk fejl tilstedeværelse. Tjeklinjer: 2903, 3053. V517 erl_bif_info.c 2903 BIF_RETTYPE system_info_1(BIF_ALIST_1) { .... if (is_tuple(BIF_ARG_1)) { // L2778 .... } else if (BIF_ARG_1 == am_scheduler_id) { // L2782 .... } .... else if (BIF_ARG_1 == am_garbage_collection) { // L2903 .... } else if (BIF_ARG_1 == am_fullsweep_after) { // L2921 .... } else if (BIF_ARG_1 == am_garbage_collection) { // L3053 .... } else if (BIF_ARG_1 == am_instruction_counts) { // L3056 .... } .... else if (ERTS_IS_ATOM_STR("halt_flush_timeout", BIF_ARG_1)) { // L3552 .... } } Enter fullscreen mode Exit fullscreen mode Analysatoren har opdaget flere grene med identiske kontroller i en funktion, der indeholder et stort antal Udtalelser – ca. Hver af dem har dog en anden logik: and I betragtning af antallet af grene og 150-linjeforskellen mellem duplikaterne er det næppe overraskende, at dette kan ske. Statisk analyse hjælper med at forhindre sådanne tilfælde. if-else if 800 linjer i længden Første tjek Anden kontrol Fragment N7 The PVS-Studio warning: En del af det betingede udtryk er altid falsk: (arity == 0). K560 Bjælke 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 Her har vi et eksempel, der involverer matematik. Analytikeren oplyser os, at den sidste Udtrykket er altid falsk, hvorfor? arity == 0 Hvis kontrolstrømmen når det punkt, hvor dette underudtryk evalueres, vil vi vide, at de to foregående underudtryk er Vi kan også lære følgende: 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 advarer om: Værdien af 'prefix_len' indeks kunne nå 262. af 557 erl_alloc_util.c 5375 Værdien af 'prefix_len' indeks kunne nå 262. V557 erl_alloc_util.c 5237 Array overrun is possible. The value of 'prefix_len' index could reach 262. af 557 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); } Enter fullscreen mode Exit fullscreen mode Vi har en funktion, der genererer navne til tildeling og deallocation-funktioner baseret på præfikset, der sendes til den. En 255-byte buffer bruges til at generere dem. Dømmende efter funktionens indhold, vil der ikke være nogen null terminator i den genererede buffer, da Den samlede størrelse af den genererede streng. am_atom_put Det allerførste, udviklerne gjorde, var at beregne præfiksstørrelsen. Og længden af den string (the longest postfix), the function is called. MAX_ATOM_CHARACTERS realloc noreturn Derefter kopierede de præfikset til bufferen, tilføjede postfixet og passede strengen til Udviklerne udførte alle disse handlinger sekventielt for alle postfixes: , og . am_atom_put alloc realloc free Kan du spotte fangsten? Den tidlige returkontrol blev udført forkert. Bufferen skal have nok plads til at skrive den længste postfix, som er syv tegn. Hvis præfixstrengen er mellem 256 og 262 tegn lang, sker den tidlige retur ikke, så funktionen overskrider bufferen. Denne operation har en ubestemt adfærd. sys_memcpy Forudsat at den Funktionen fungerer med strenge ikke længere end Lad os lave koden som følger: am_atom_put MAX_ATOM_CHARACTERS if (prefix_len > MAX_ATOM_CHARACTERS - sizeof(realloc) + 1) Enter fullscreen mode Exit fullscreen mode Den maksimale præfixlængde er nu 248 tegn. Når du skriver den maksimale postfix, vil der ikke forekomme overflow. Lignende advarsler: V557 Array overrun er mulig. Værdien af 'i' indeks kunne nå 40. ycf_lexer.c 493 V557 Array overrun er mulig. Værdien af 'n' indeks kan nå 16. erl_call.c 1663 Fragment N9 Jeg vil gerne præsentere dette eksempel på en usædvanlig måde. Lad os først se på denne funktion: 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; } } Indtast fuldskærmstilstand Udtast fuldskærmstilstand Den function converts the For at opstille en linje. for ikke- 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 af C11 , C23 ). event_state_flag_to_str EventStateFlags default switch const 6.7.3.6 6.7 af 4.1 Er det virkelig tilfældet?Lad os tage et kig på indkaldelsespunktet: 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))); } Indtast fuldskærmstilstand Udtast fuldskærmstilstand Den pågældende funktion kaldes ved at passere den konstante buffer som det andet argument. Desværre får vi ubestemt adfærd. Vi fandt denne fejl ved hjælp af analysatoren, men den udstedte en forvirrende advarsel: Uninitialiseret buffer 'buff' bruges. Overvej at tjekke det andet faktiske argument for funktionen 'event_state_flag_to_str'. H614 erl_check_io.c 2833 Lad os løse koden ved at fjerne konstansen fra bufferen: 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 funktionssignatur, så parameteren kan tage en Dette gør det klart, at funktionen kan ændre den buffer, der sendes til den. Vi vil ikke bryde meget kode i processen, da funktionen kun eksisterer i den kompilerede fil og er markeret som . event_state_flag_to_str char* static Fragment N10 Den PVS-Studio advarsel: Det ikke-initialiserede klassemedlem 'a' anvendes, når du initialiserer basisklassen 'BeamAssemblerCommon'. af V1050 Køge.dk 87 class BeamAssemblerCommon : public ErrorHandler { BaseAssembler &assembler; protected: BeamAssemblerCommon(BaseAssembler &assembler); .... }; struct BeamAssembler : public BeamAssemblerCommon { BeamAssembler() : BeamAssemblerCommon(a) { /* .... */ } protected: a64::Assembler a; .... }; Indtast fuldskærmstilstand Udtast fuldskærmstilstand This fragment contains a small class hierarchy where a reference to a non-static member function of the derived class, , is passed to the constructor of the base class, Denne operation vil ikke forårsage problemer, så længe basisklassebyggeren ikke interagerer med denne medlemsfunktion.Hvorfor?Livetiden for et objekt fra en afledt klasse starter, når den krævede initializer i initialiseringslisten er blevet udført, eller, hvis der ikke er nogen initializer, når kontrolstrømmen kommer ind i konstruktorens krop. BeamAssembler BeamAssemblerCommon Med disse oplysninger i tankerne, lad os tage et kig på basisklasse konstruktøren. Det bruger ikke dette objekt, ikke? BeamAssemblerCommon::BeamAssemblerCommon(BaseAssembler &assembler_) : assembler(assembler_), .... { .... #ifdef DEBUG assembler.addDiagnosticOptions(DiagnosticOptions::kValidateAssembler); #endif assembler.addEncodingOptions(EncodingOptions::kOptimizeForSize | EncodingOptions::kOptimizedAlign); .... } Indtast fuldskærmstilstand Udtast fuldskærmstilstand En ikke-statisk medlemsfunktion kaldes på et objekt i en afledt klasse via en reference, når dets levetid endnu ikke er begyndt. I also find it difficult to suggest a fix in this case, so I'll leave that to the developers. hukommelsesfejl Fragment N11 Den PVS-Studio advarsel: The 'free' function is called twice for deallocation of the same memory space. Køge 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); .... } Indtast fuldskærmstilstand Udtast fuldskærmstilstand Judging by the name, er a Et negativt nummer returneres, hvis operationen mislykkes. Hvis fjernopkaldet mislykkes, renses nogle ressourcer. pointer is passed to the funktion og en fejl er logget til Dernæst, uanset funktionsopkaldsresultatet, sendes pointeren til hukommelse frigives to gange. ei_rpc Fjernprocedure opkald p free stderr free Løsningen er ganske enkel: enten stoppe programkørslen eller indstille de frigjorte pointers til 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); Indtast fuldskærmstilstand Udtast fuldskærmstilstand Fragment N12 Den PVS-Studio advarsel: P-nøglen blev brugt, før den blev verificeret mod nullptr. af 595 Køge.dk 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 We have quite an interesting code fragment here. The forsøger at udtrække den offentlige nøgle til den femte ud parameter. if successful and I sidstnævnte tilfælde er den offentlige nøgle også nulstillet. Funktioner 1 0 Analysatoren advarer om, at i en af funktionerne, den pointer er dereferenced, før det kontrolleres for gyldighed. Men andre grene synes ikke at have sådanne kontroller. Vi kan konkludere, at funktionen har følgende kontrakt: "Den Parameteren må aldrig være lig med en ." Vi kan bekræfte dette ved at søge efter opkald til denne funktion i samme fil ([ ], [ ] , [ ])—the variable address is passed everywhere. pkey pkey nullptr 1 2 3 Faktisk havde udviklerne til hensigt at håndtere det forkerte resultat af function by jumping to the Men de lavede en typografi ved at glemme at sætte en asterisk foran En moderne kompilator vil denne tjek på frigivelse. Som følge heraf returnerer funktionen , and a null pointer will end up in the fifth parameter. ENGINE_load_public_key err ! optimize away 1 Lad os få styr på checken: .... *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")); .... Indtast fuldskærmstilstand Udtast fuldskærmstilstand Fragment N13 Vi modtager ofte beskeder, der siger, at analysatoren advarer om at passere en null pointer til . After all, the standard states that nothing terrible will happen. We even wrote an Om en lignende sag. free Artikler Ja, det er fint, men en sådan kode er sandsynligvis mistænkelig. Det hjalp os med at finde en interessant hukommelseslek i koden. Den PVS-Studio advarsel: Pointeren null overføres til funktionen "gratis". af 575 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); } } Indtast fuldskærmstilstand Udtast fuldskærmstilstand I dette afsnit, den Pointeren er udtrykkeligt indstillet til before calling Som følge heraf er hukommelsesfrigivelsen meningsløs, da Dette resulterer i hukommelseslekkasjer, fordi den oprindelige hukommelsesblok, der Det siges, at man aldrig bliver løsladt. cpuinfo->topology NULL free free(NULL) cpuinfo->topology We can fix the issue by swapping the operations: if (cpuinfo->topology) { free(cpuinfo->topology); cpuinfo->topology = NULL; } Enter fullscreen mode Exit fullscreen mode Fragment N14 The PVS-Studio warning: The function was exited without releasing the 'entry' pointer. A memory leak is possible. af V773 wxe_gl.cpp 88 typedef struct _wxe_glc { wxGLCanvas *canvas; wxGLContext *context; } wxe_glc; void setActiveGL(wxeMemEnv *memenv, ErlNifPid caller, wxGLCanvas *canvas, wxGLContext *context) { ErlNifUInt64 callId = wxe_make_hash(memenv->tmp_env, &caller); wxe_glc * entry = glc[callId]; gl_active_index = callId; gl_active_pid = caller; if (!entry) { entry = (wxe_glc *) malloc(sizeof(wxe_glc)); entry->canvas = NULL; entry->context = NULL; } if (entry->canvas == canvas && entry->context == context) return; entry->canvas = canvas; entry->context = context; glc[gl_active_index] = entry; .... } Enter fullscreen mode Exit fullscreen mode I den Funktioner, den pointer is declared and initialized via an array element. If this element is a null pointer, memory is allocated using Efterfølgende sammenlignes pointerne. Hvis det vurderes til , an early return will occur. However, if dynamic allocation occurred, the memory won't be released. setActiveGL entry malloc true Ja, en sådan situation er ret sjælden og usandsynlig at ske, fordi det kræver en hel række begivenheder: Når funktionen kaldes, passeres nulpointere som det tredje og fjerde argument. Glc[callId] indeholder en null pointer. Men vi kan sikre, at et læk aldrig vil forekomme, ikke engang teoretisk. lad os ændre koden lidt: 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; } .... Indtast fuldskærmstilstand Udtast fuldskærmstilstand Lignende advarsler: V773 Funktionen blev udgået uden at lukke den fil, der er henvist til af håndtaget 'f'. V773 Pointeren 'erl_errno_p' blev tildelt værdier to gange uden at frigive hukommelsen. 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 Undtagelsen blev kastet uden at frigive pointeren 'data'. en hukommelseslek er mulig. wxe_wrapper_4.cpp 1330 V773 Undtagelsen blev kastet uden at frigive pointeren 'data'. en hukommelseslek er mulig. 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 Undtagelsen blev kastet uden at frigive pointeren 'data'. en hukommelseslek er mulig. wxe_wrapper_4.cpp 1715 V773 Undtagelsen blev kastet uden at frigive 'alfa' pointeren. V773 The exception was thrown without releasing the 'data' pointer. A memory leak is possible. wxe_wrapper_4.cpp 1747 V773 Undtagelsen blev kastet uden at frigive 'alfa' pointeren. V773 Undtagelsen blev kastet uden at frigive 'alfa' pointeren. V773 Undtagelsen blev kastet uden at frigive pointeren 'data'. en hukommelseslek er mulig. 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 The PVS-Studio warning: 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; } .... } Enter fullscreen mode Exit fullscreen mode There's a tricky pattern with the funktionen, hvor den oprindelige pointer umiddelbart overskrives af returværdien. realloc Funktionen tager bufferen til omfordeling som sit første argument og den nye bufferstørrelse som sit andet argument. Hvis operationen er vellykket, returnerer funktionen en pointer til den nyligt tildelte hukommelse. , selv om der kun er sket en simpel udvidelse af hukommelsesblokken. Hvis operationen mislykkes, returnerer funktionen en Hukommelsen gik som det første argument forbliver . invalid null pointer untouched Så hvis vi straks overskriver den oprindelige pointer med et opkald til Vi får et hukommelsestab. realloc Lad os lave koden som følger: 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; } .... } Indtast fuldskærmstilstand Udtast fuldskærmstilstand 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 Den PVS-Studio advarsel: Pointeren 'end' i udtrykket 'end - mm->sua.top' er lig med nullptr. Den resulterende værdi er meningsløs og bør ikke anvendes. af 769 erl_mmap.dk 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 Analysatoren har opdaget mistænkelig adresse aritmetik. lad os se, hvad der er galt her. Vi indtaster kodekassen, hvor Det er faktisk en null pointer. 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 Null pointeren peger ikke på noget array element. Så adfærden af denne operation er undefineret, hvilket ikke er godt. end mm->sua.top 6.5 af 6.8 5.7 af 10 Jeg undrede mig over, hvad der skete i 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)) { .... } Indtast fuldskærmstilstand Udtast fuldskærmstilstand Pointertrækning anvendes også her, men med en forskel: Først konverteres pointererne til tal. af C23 ), which is much better. 6.3.2.3.6 6.3.2 af 3.6 Desværre kan jeg ikke foreslå en ordentlig løsning. Udviklerne vil måske tage et kig på dette problem. 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. Takket være konstant udviklende moderne teknologier har udviklere kraftfulde værktøjer til deres rådighed for at forbedre kodens pålidelighed. For eksempel hjælper statiske analysatorer med at identificere skjulte fejl og potentielle sårbarheder. jeg inviterer dig til at se det selv :) Du kan få en prøveversion af PVS-Studio analyzer . here