Le code des années 1980 renforçant la fiabilité moderne - et les erreurs qu'il fait encore

Le code de certains modules Erlang/OTP est plus ancien que la plupart des développeurs junior modernes. Ces fichiers sont les véritables patriarches numériques. Pendant des décennies, ils ont fait en sorte que les transactions bancaires, les réseaux téléphoniques et les systèmes de messagerie fonctionnent correctement. Nous avons décidé de jeter un coup d'oeil sous le capot de ce langage de longue date pour voir ce qui se cache exactement derrière les lignes sur lesquelles des millions d'utilisateurs comptent aujourd'hui. Introduction Avant de passer à discuter des avertissements, je voudrais dire quelques mots sur le projet. Le voyage d'Erlang a commencé en 1986 dans l'un des laboratoires d'Ericsson.Joe Armstrong expérimentait avec Prolog tout en essayant de construire un programme téléphonique.Ces expériences ont finalement donné naissance à Erlang. est un langage de programmation conçu pour les systèmes tolérants aux défauts où l'échec est inacceptable. Pensez à un commutateur de téléphone où un appel abandonné compte comme une urgence, ou une transaction bancaire qui doit passer quoi que ce soit. Ce sont les tâches pour lesquelles il est utilisé. Il permet à un système de se diviser en millions de processus isolés et indépendants, de sorte que lorsque l'un échoue, le reste continue à fonctionner sans manquer de battement. Erlang Il va de pair avec le , un cadre qui fournit des outils puissants, des modèles et des comportements pour la construction d'applications distribuées tolérantes aux défauts. Ce kit d'outils intégré permet aux systèmes écrits avec elle d'exécuter en continu pendant des années sans jamais avoir à s'arrêter. Plateforme de télécommunications ouverte (OTP) Nous avons construit le projet en suivant ces Nous l’avons vérifié à la Branch utilisé et le Nous n'avons rencontré aucun problème avec la construction du projet, donc kudos aux développeurs. instructions maint-28 PVS-Studio analyzer Création de Visual Studio Code The article includes only High- and Medium-level warnings. Cela conclut notre introduction. Maintenant, passons aux avertissements. Nous les diviserons en trois catégories: erreur de logique ; critical errors; Les erreurs de mémoire. erreur logique Fragment N1 Le PVS-Studio met en garde : Il y a des sous-expressions identiques '(!esock_is_integer((env), (argv[3])))' à la gauche et à la droite de l'opérateur 'annoo. 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]); } .... } Entrer mode plein écran Sortir mode plein écran Dans le code, les développeurs voulaient convertir les troisième et quatrième arguments en types 64 bits (les signés et les non signés, respectivement). La fonction retourne tôt si l'un des arguments n'est pas un nombre. Le code fixe : .... if ((! IS_INTEGER(env, argv[2])) || (! IS_INTEGER(env, argv[3]))) return enif_make_badarg(env); .... Enter fullscreen mode Exit fullscreen mode Fragment N2 The PVS-Studio warning: Il y a des sous-expressions identiques 'n->type == ycf_node_type_on_save_yield_state_code' à la gauche et à la droite de l'opérateur 'annoo. Le 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); } .... } Entrer mode plein écran Sortir mode plein écran Voici un autre article très intéressant, nous avons fonction, qui traite récurremment les nœuds d'arbre et trouve des zones de visibilité avec des variables locales. Il appelle ensuite la fonction d'unification des noms des variables pour chacune de ces zones. branch, the La condition est dupliquée après la l’opérateur . uniqify_local_vars_in_node else if n->type == ycf_node_type_on_save_yield_state_code || À en juger par la chaîne de comparaison, les valeurs de la liste suivante sont triées dans l'ordre dans lequel elles sont déclarées: 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; Entrer mode plein écran Sortir mode plein écran Cependant, le La constante est vérifiée dans le précédent Donc, je suppose qu'il y a un double contrôle ici après tout, et nous pouvons le supprimer. ycf_node_type_on_restore_yield_state_code else if Fragment N3 The PVS-Studio warning: Possible typo in the spelling of a pre-defined macro name. The ' « Macro est similaire » « . V1040 à WIN_32 WIN32 Téléchargez la page 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); } Entrer mode plein écran Sortir mode plein écran C'est un cas assez inhabituel de vérifier si un projet compile sous Windows. Erlang a d'autres vérifications sur Windows ([ ] , [ ], [ ], etc.), mais ceci est unique. Cela ressemble beaucoup à un simple typographe (probablement, il devrait être ) de 1 2 3 __WIN32__ Pourquoi ce n'est pas le cas ? j'ai cherché Pour voir ce que j'ai trouvé, voici les résultats : "__WIN_32__" Comme vous pouvez le voir, il n'y a presque pas de résultats.C'est drôle que l'un des liens mène au fichier que nous regardons J'ai saisi le projet pour ne pas trouver de définitions de macro similaires. j'ai également pensé qu'il pourrait être intégré dans un compilateur, comme MinGW. failed. After trying to understand the macro's purpose, I believe this code is flawed. Sources de Debian Expérimentation En guise de correction, je suggère d'utiliser le contrôle que j'ai trouvé plus tôt dans d'autres parties du projet: static int tcp_send_error(tcp_descriptor* desc, int err) { .... #if defined(__WIN32__) || defined(_WIN32) || defined(_WIN32_) if (err == ECONNABORTED) err = ECONNRESET; #endif .... } Entrer mode plein écran Sortir mode plein écran Fragment N4 Le PVS-Studio met en garde : La logique opérationnelle du code ne correspond pas à sa mise en forme. La deuxième déclaration sera toujours exécutée. V640 à Dialysateur.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"); .... } Entrer mode plein écran Sortir mode plein écran Here's an interesting example of a common coding error that can be difficult to spot later on. Take a look at the La structure : PUSH2 #define QUOTE(s) s #define PUSH(s) eargv[eargc++] = QUOTE(s) #define PUSH2(s, t) PUSH(s); PUSH(t) Entrer mode plein écran Sortir mode plein écran Uh-huh, c’est une macro qui s’étend en deux constructions. 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"; .... } Entrer mode plein écran Sortir mode plein écran Par conséquent, si l’évaluation de , seule la première assignation sera exécutée conditionnellement, tandis que la seconde se produira toujours après la branche. false Écrire et utiliser des macros La solution idéale serait de se débarrasser d'eux, mais fixons-nous pour une correction moins radicale. construct when they need to combine several actions into one within a macro. Attirer les bugs do {...} while (0) #define PUSH2(s, t) do { PUSH(s); PUSH(t); } while (0) Enter fullscreen mode Exit fullscreen mode Fragment N5 The PVS-Studio warning: The condition 'i >= 0' of loop is always true. Le V654 Télécharger wxe_return.cpp 236 ERL_NIF_TERM wxeReturn::make_array_objs(wxAuiPaneInfoArray& arr, WxeApp *app, const char *cname) { ERL_NIF_TERM head, tail; ERL_NIF_TERM class_name = enif_make_atom(env, cname); tail = enif_make_list(env, 0); for (unsigned int i = arr.GetCount() - 1; i >= 0; i--) { head = make_ref(app->getRef((void *) &arr.Item(i),memenv), class_name); tail = enif_make_list_cell(env, head, tail); } return tail; } Entrer mode plein écran Sortir mode plein écran Jetez un coup d'oeil à la loop, paying special attention to its condition. Since the La variable d’induction est la type avec une gamme de valeurs de , le cercle devient infini. Après que le cercle itère avec La variable devient égale à En conséquence du déclin, et le cycle se poursuit. for i unsigned int [0; UINT_MAX] i == 0 UINT_MAX Fixons le code comme suit : for (int64_t i = arr.GetCount() - 1; i >= 0; i--) { .... } Entrer mode plein écran Sortir mode plein écran Similar warnings: V654 La condition 'i >= 0' de la boucle est toujours vraie. wxe_return.cpp 248 V654 La condition 'i >= 0' de la boucle est toujours vraie. wxe_return.cpp 260 Fragment N6 Le PVS-Studio met en garde : Utilisation du motif « si (A) {...} autre si (A) {...} » a été détecté. Il y a une probabilité de présence d'erreur logique. Lignes de contrôle: 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 .... } } Entrer mode plein écran Sortir mode plein écran L'analyseur a détecté plusieurs branches avec des contrôles identiques dans une fonction contenant un grand nombre de statements—roughly Chacun d’eux a une logique différente : le and Compte tenu du nombre de branches et de l'écart de 150 lignes entre les duplicats, il n'est pas surprenant que cela puisse se produire.L'analyse statique aide à prévenir de tels cas.Malheureusement, je ne peux pas penser à une correction, alors je laisserai cela aux développeurs. if-else if 800 lignes longues Première vérification Deuxième vérification Fragment N7 Le PVS-Studio met en garde : Une partie de l'expression conditionnelle est toujours fausse : (arité == 0). V560 bif.c 3145 BIF_RETTYPE delete_element_2(BIF_ALIST_3) { .... ptr = tuple_val(BIF_ARG_2); arity = arityval(*ptr); ix = signed_val(BIF_ARG_1); if ((ix arity) || (arity == 0)) { BIF_ERROR(BIF_P, BADARG); } .... } Entrer mode plein écran Sortir mode plein écran Here, we have an example involving math. The analyzer informs us that the last sub-expression is always false. Why? arity == 0 Let's dig into it. If the control flow reaches the point where this subexpression is evaluated, we will know that two previous subexpressions are . We can also learn the following: false ix >= 1 ix = 1 arity Je laisserai également la correction de ce code aux développeurs, car ma seule suggestion est de supprimer la condition. Les erreurs critiques Fragment N8 Le PVS-Studio met en garde : Array overrun is possible. The value of 'prefix_len' index could reach 262. Le V557 erl_alloc_util.c 5375 Array overrun is possible. The value of 'prefix_len' index could reach 262. Le V557 erl_alloc_util.c 5378 La valeur de l'index 'prefix_len' pourrait atteindre 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); } Entrer mode plein écran Sortir mode plein écran Nous avons une fonction qui génère des noms pour les fonctions d'allocation et de désallocation en fonction du préfixe qui lui est transmis. Un tampon de 255 octets est utilisé pour les générer. À en juger par le contenu de la fonction, il n'y aura pas de terminateur nul dans le tampon généré, puisque dépasse la taille de la chaîne générée. am_atom_put La première chose que les développeurs ont fait était de calculer la taille du préfixe. Ensuite, ils ont limité la taille du préfixe: si elle dépasse la somme de La longueur de la string (the longest postfix), the function is called. MAX_ATOM_CHARACTERS realloc noreturn Ensuite, ils ont copié le préfixe au tampon, ajouté le postfix et transmis la chaîne au Les développeurs ont effectué toutes ces actions de manière séquentielle pour tous les postfixes : , à , and . am_atom_put alloc realloc free La vérification du retour précoce a été effectuée incorrectement. Le tampon doit avoir suffisamment d'espace pour écrire le postfix le plus long, qui est de sept caractères. Si la chaîne de préfixes est longue entre 256 et 262 caractères, le retour précoce ne se produit pas, de sorte que le fonction dépasse le tampon. Cette opération a un comportement indéfini. sys_memcpy En supposant que le fonction fonctionne avec des cordes pas plus longtemps que , let's fix the code as follows: am_atom_put MAX_ATOM_CHARACTERS if (prefix_len > MAX_ATOM_CHARACTERS - sizeof(realloc) + 1) Entrer mode plein écran Sortir mode plein écran Now, the maximum prefix length is 248 characters. When writing the maximum postfix, overflow won't occur. Similar warnings: V557 Array Overrun est possible. La valeur de l'index 'i' pourrait atteindre 40. ycf_lexer.c 493 V557 Array Overrun est possible. La valeur de l'index 'n' pourrait atteindre 16. erl_call.c 1663 Fragment N9 Je voudrais présenter cet exemple d'une manière inhabituelle.D'abord, regardons cette fonction: 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; } } Entrer mode plein écran Sortir mode plein écran Le La fonction convertit le enumeration to a string. For non- Les 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 ). event_state_flag_to_str EventStateFlags default switch const 6.7.3.6 6.7.4.1.7 Est-ce que c'est vraiment le cas ? regardons le point d'invocation : 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))); } Entrer mode plein écran Sortir mode plein écran The function in question is called by passing the constant buffer as the second argument. Unfortunately, we get undefined behavior. We found this error using the analyzer, but it issued a confusing warning: Uninitialized buffer 'buff' used. Consider checking the second actual argument of the 'event_state_flag_to_str' function. 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))); } Entrer mode plein écran Sortir mode plein écran Je suggère également de modifier le fonction signature, de sorte que le paramètre peut prendre une 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 Le PVS-Studio met en garde : Le membre de classe non initialisé 'a' est utilisé lors de l'initialisation de la classe de base 'BeamAssemblerCommon'. Le V1050 beam_asm.hpp 87 class BeamAssemblerCommon : public ErrorHandler { BaseAssembler &assembler; protected: BeamAssemblerCommon(BaseAssembler &assembler); .... }; struct BeamAssembler : public BeamAssemblerCommon { BeamAssembler() : BeamAssemblerCommon(a) { /* .... */ } protected: a64::Assembler a; .... }; Entrer mode plein écran Sortir mode plein écran This fragment contains a small class hierarchy where a reference to a non-static member function of the derived class, , est transmis au constructeur de la classe de base, Cette opération ne causera aucun problème tant que le constructeur de classe de base n'interagira pas avec cette fonction membre. Pourquoi? La durée de vie d'un objet d'une classe dérivée commence lorsque l'initialisateur requis dans la liste d'initialisation a été exécuté ou, s'il n'y a pas d'initialisateur, lorsque le flux de contrôle entre dans le corps du constructeur. BeamAssembler BeamAssemblerCommon Avec ces informations à l'esprit, regardons le constructeur de classe de base. Il n'utilise pas cet objet, n'est-ce pas? BeamAssemblerCommon::BeamAssemblerCommon(BaseAssembler &assembler_) : assembler(assembler_), .... { .... #ifdef DEBUG assembler.addDiagnosticOptions(DiagnosticOptions::kValidateAssembler); #endif assembler.addEncodingOptions(EncodingOptions::kOptimizeForSize | EncodingOptions::kOptimizedAlign); .... } Entrer mode plein écran Sortir mode plein écran Nope :) Une fonction membre non statique est appelée sur un objet d'une classe dérivée via une référence lorsque sa durée de vie n'a pas encore commencé. I also find it difficult to suggest a fix in this case, so I'll leave that to the developers. Memory errors Fragment N11 Le PVS-Studio met en garde : The 'free' function is called twice for deallocation of the same memory space. Voir 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); .... } Entrer mode plein écran Sortir mode plein écran Judging by the name, est a Un numéro négatif est retourné si l'opération échoue. Si l'appel à distance échoue, certaines ressources sont nettoyées. Le pointeur est passé au fonction et une erreur est enregistrée à . Next, regardless of the function call result, the pointer is passed to La mémoire est libérée deux fois. 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); Entrer mode plein écran Sortir mode plein écran Fragment N12 Le PVS-Studio met en garde : Le pointeur « clé » a été utilisé avant qu’il ne soit vérifié contre nullptr. lignes de contrôle: 581, 582. V595 Chapitre 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; } Entrer mode plein écran Sortir mode plein écran We have quite an interesting code fragment here. The attempts to extract the public key to the fifth out parameter. It returns if successful and Dans ce dernier cas, la clé publique est également zéro. function 1 0 L'analyseur avertit que, dans l'une des branches de la fonction, le Le pointeur est déferencé avant d'être vérifié pour la validité. Cependant, d'autres branches ne semblent pas avoir de tels vérifications. Nous pouvons conclure que la fonction a le contrat suivant: "Le parameter must never be equal to a ." We can confirm this by searching for calls to this function in the same file ([ ] , [ ], [ ]) — l’adresse variable est transmise partout. pkey pkey nullptr 1 2 3 In fact, the developers intended to handle the incorrect result of the fonction en sautant vers le Cependant, ils ont fait un type en oubliant de mettre un astérisque avant le operator. A modern compiler will 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 ! Optimisez loin 1 Faisons le 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")); .... Entrer mode plein écran Sortir mode plein écran Fragment N13 Nous recevons souvent des messages indiquant que l'analyseur avertit de passer un pointeur null à . After all, the standard states that nothing terrible will happen. We even wrote an about a similar case. free Article de Yes, that's fine, but such code is probably suspicious. That helped us find an interesting memory leak in the code. Let's take a look at it. The PVS-Studio warning: The null pointer is passed into 'free' function. Inspect the first argument. Le 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 Le pointeur est explicitement défini Avant d’appeler En conséquence, l'opération de libération de la mémoire est inutile, puisque 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 We can fix the issue by swapping the operations: if (cpuinfo->topology) { free(cpuinfo->topology); cpuinfo->topology = NULL; } Entrer mode plein écran Sortir mode plein écran Fragment N14 The PVS-Studio warning: La fonction est sortie sans libérer le pointeur « entrée ». une fuite de mémoire est possible. V773 à Résumé : 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 fonctionnel, le pointeur est déclaré et initialisé via un élément d'array. Si cet élément est un pointeur null, la mémoire est Ensuite, les indicateurs sont comparés. Si elle évalue à , an early return will occur. However, if dynamic allocation occurred, the memory won't be released. setActiveGL entry malloc true Yes, such a situation is rather rare and unlikely to happen, because it requires a whole series of events: when the function is called, zero pointers are passed as the third and fourth arguments. contains a null pointer. glc[callId] Cependant, nous pouvons nous assurer qu'une fuite ne se produira jamais, pas même théoriquement. ErlNifUInt64 callId = wxe_make_hash(memenv->tmp_env, &caller); wxe_glc * entry = glc[callId]; gl_active_index = callId; gl_active_pid = caller; bool new_alloc = false; if (!entry) { entry = (wxe_glc *) malloc(sizeof(wxe_glc)); entry->canvas = NULL; entry->context = NULL; new_alloc = true; } if (entry->canvas == canvas && entry->context == context) { if (new_alloc) { free(entry); } return; } .... Enter fullscreen mode Exit fullscreen mode Similar warnings: V773 The function was exited without closing the file referenced by the 'f' handle. A resource leak is possible. erl_poll.c 2423 V773 The 'erl_errno_p' pointer was assigned values twice without releasing the memory. A memory leak is possible. ei_pthreads.c 195 V773 La fonction est sortie sans fermer le fichier référencé par le manuel fp. Une fuite de ressource est possible. cpu_sup.c 433 V773 L'exception a été jetée sans libérer le pointeur « données ». une fuite de mémoire est possible. wxe_wrapper_4.cpp 1300 V773 L'exception a été jetée sans libérer le pointeur « données ». une fuite de mémoire est possible. wxe_wrapper_4.cpp 1330 V773 The exception was thrown without releasing the 'data' pointer. A memory leak is possible. wxe_wrapper_4.cpp 1663 V773 L'exception a été jetée sans libérer le pointeur « données ». une fuite de mémoire est possible. wxe_wrapper_4.cpp 1690 V773 L'exception a été jetée sans libérer le pointeur « données ». une fuite de mémoire est possible. wxe_wrapper_4.cpp 1715 V773 L'exception a été jetée sans libérer le pointeur "alpha". une fuite de mémoire est possible. wxe_wrapper_4.cpp 1718 V773 L'exception a été jetée sans libérer le pointeur « données ». une fuite de mémoire est possible. wxe_wrapper_4.cpp 1747 V773 L'exception a été jetée sans libérer le pointeur "alpha". une fuite de mémoire est possible. wxe_wrapper_4.cpp 1750 V773 L'exception a été jetée sans libérer le pointeur "alpha". une fuite de mémoire est possible. wxe_wrapper_4.cpp 2680 V773 L'exception a été jetée sans libérer le pointeur « données ». une fuite de mémoire est possible. wxe_wrapper_4.cpp 2715 V773 L'exception a été jetée sans libérer le pointeur « données ». une fuite de mémoire est possible. wxe_wrapper_4.cpp 2733 Fragment N15 The PVS-Studio warning: Realloc() possible leak: lorsque realloc() échoue dans l'allocation de la mémoire, le pointeur d'origine 'outbuf_base' est perdu. 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; } .... } Entrer mode plein écran Sortir mode plein écran There's a tricky pattern with the fonction, où le pointeur d'origine est immédiatement surécrit par la valeur de retour. realloc La fonction prend le tampon pour la réallocation comme premier argument et la nouvelle taille de tampon comme deuxième argument. Si l'opération est réussie, la fonction renvoie un pointeur à la mémoire nouvellement allouée. , even if only a simple memory block expansion has occurred. If the operation fails, the function returns a . The memory passed as the first argument remains . invalid null pointer untouched Donc, si nous surécrivons immédiatement le pointeur original avec un appel à , we will get a memory leak. realloc Let's fix the code as follows: 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 Dans l'exemple fixe, nous devons également gérer la situation où 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: Le pointeur « fin » dans l'expression « fin - mm->sua.top » est égal à nullptr. La valeur résultante n'a pas de sens et ne doit pas être utilisée. V769 mmmap.fr 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); } .... } Entrer mode plein écran Sortir mode plein écran L'analyseur a détecté une arithmétique d'adresse suspecte. Voyons ce qui ne va pas ici. Nous entrons dans la branche de code où 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 Le C23 Le pointeur null ne montre pas un élément d'array, donc le comportement de cette opération n'est pas défini, ce qui n'est pas bon. end mm->sua.top 6.5 à 6.8 6.5.7.10 I was wondering what happens in the 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)) { .... } 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 ), which is much better. 6.3.2.3.6 6.3.2.3.6 Malheureusement, je ne peux pas suggérer une correction appropriée.Les développeurs pourraient vouloir jeter un coup d'œil à ce problème. Conclusion Eh bien, c'est la conclusion de l'article. Comme on pouvait s'y attendre avec tout grand projet qui a eu une longue histoire, nous avons trouvé diverses erreurs. Ils font également partie du projet, cependant. Il est important de garder à l'esprit que trouver ce genre de problèmes dans une base de code qui a plus de trois décennies n'est pas un signe de faiblesse. Au lieu de cela, c'est la preuve de l'incroyable échelle et de la longévité du code. Il y a des millions de lignes de code écrites par des centaines de développeurs, et le système fonctionne depuis des années sans redémarrer. Ce projet mérite le plus profond respect, non pas parce qu'il est parfait, mais parce qu'il a résisté à l'épreuve du temps et prouvé sa 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 . ici