Cela fait déjà trois mois que 2018 est terminée. Pour beaucoup, il a volé presque imperceptiblement, mais pour nous, les développeurs de PVS-Studio, il s'est avéré être très saturé. Nous avons travaillé dur, combattu sans peur pour l'avancement de l'analyse statique pour les masses et recherché de nouvelles erreurs dans les projets open source écrits en C, C ++, C # et Java. Les dix plus intéressants d'entre eux que nous avons rassemblés pour vous dans cet article!
Nous avons cherché des endroits intéressants en utilisant l'analyseur de code statique
PVS-Studio . Il peut détecter les erreurs et les vulnérabilités potentielles dans le code écrit dans les langues mentionnées ci-dessus.
Si vous souhaitez rechercher vous-même des erreurs, vous pouvez toujours télécharger et essayer notre analyseur. Nous fournissons une
version gratuite de l' analyseur pour les étudiants et les programmeurs enthousiastes, une
licence gratuite pour les développeurs de projets open-source, ainsi qu'une
version d'essai pour tout-tout-tout. Qui sait, peut-être que l'année prochaine, vous pourrez faire votre top 10? :)
Remarque: je suggère au lecteur de vérifier lui-même et, avant de regarder l'avertissement de l'analyseur, d'essayer d'identifier lui-même les anomalies. Combien d'erreurs pouvez-vous trouver?
Dixième place
Source:
Et encore dans l'espace: comment le licorne Stellarium a visitéCette erreur a été découverte lors de la vérification du planétarium virtuel Stellarium.
L'extrait de code ci-dessus, bien que petit, est lourd d'une erreur assez délicate:
Plane::Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3) : distance(0.0f), sDistance(0.0f) { Plane(v1, v2, v3, SPolygon::CCW); }
L'avez-vous trouvé?
PVS-Studio Warning: V603 L'objet a été créé mais il n'est pas utilisé. Si vous souhaitez appeler le constructeur, 'this-> Plane :: Plane (....)' doit être utilisé. Plane.cpp 29
L'auteur du code a voulu initialiser une partie des champs de l'objet en utilisant un autre constructeur imbriqué dans le principal. Certes, au lieu de cela, il a seulement réussi à créer un objet temporaire qui serait détruit en quittant sa zone de visibilité. Ainsi, plusieurs champs de l'objet resteront non initialisés.
Au lieu d'un appel de constructeur imbriqué, vous devez utiliser le constructeur délégué introduit dans C ++ 11. Par exemple, vous pouvez faire ceci:
Plane::Plane(Vec3f& v1, Vec3f& v2, Vec3f& v3) : Plane(v1, v2, v3, SPolygon::CCW) { distance = 0.0f; sDistance = 0.0f; }
Ensuite, tous les champs requis seraient correctement initialisés. N'est-ce pas merveilleux?
Neuvième place
Source:
Perl 5: comment les erreurs de macro se sont cachéesEn neuvième place, une macro remarquable du code source de Perl 5 est affichée.
Recueillant des erreurs lors de la rédaction de l'article, mon collègue Svyatoslav est tombé sur un avertissement émis par l'analyseur sur l'utilisation d'une macro. Le voici:
PP(pp_match) { .... MgBYTEPOS_set(mg, TARG, truebase, RXp_OFFS(prog)[0].end); .... }
Pour découvrir quel était le problème, Svyatoslav a creusé plus profondément. Il a ouvert la définition de macro et a vu qu'elle contenait plusieurs macros imbriquées, dont certaines avaient également des macros imbriquées. C'était si difficile à comprendre que j'ai dû utiliser un fichier prétraité. Mais, hélas, cela n'a pas aidé. Au lieu de la ligne de code précédente, Sviatoslav a découvert ceci:
(((targ)->sv_flags & 0x00000400) && (!((targ)->sv_flags & 0x00200000) || S_sv_only_taint_gmagic(targ)) ? (mg)->mg_len = ((prog->offs)[0].end), (mg)->mg_flags |= 0x40 : ((mg)->mg_len = (((targ)->sv_flags & 0x20000000) && !__builtin_expect(((((PL_curcop)->cop_hints + 0) & 0x00000008) ? (_Bool)1 :(_Bool)0),(0))) ? (ssize_t)Perl_utf8_length( (U8 *)(truebase), (U8 *)(truebase)+((prog->offs)[0].end)) : (ssize_t)((prog->offs)[0].end), (mg)->mg_flags &= ~0x40));
Avertissement PVS-Studio: V502 L'opérateur '?:'
Fonctionne peut-être d'une manière différente de celle attendue. L'opérateur '?:' A une priorité plus faible que l'opérateur '&&'. pp_hot.c 3036
Je pense qu'il sera difficile de trouver une telle erreur avec mes yeux. Honnêtement, nous avons longuement médité sur ce code et sommes arrivés à la conclusion qu'en fait il n'y a pas d'erreur ici. Mais en tout cas, c'est un exemple assez amusant de code illisible.
On dit que les macros sont mauvaises. Bien sûr, il y a des moments où ils se révèlent indispensables, mais si vous pouvez remplacer la macro par une fonction, vous devez absolument le faire.
Les macros imbriquées sont particulièrement lourdes. Non seulement parce qu'ils sont difficiles à comprendre, mais aussi parce qu'ils peuvent donner des résultats imprévisibles. Si l'auteur d'une macro fait accidentellement une erreur dans une telle macro, il sera beaucoup plus difficile de la trouver que dans une fonction.
Huitième place
Source:
Chrome: autres erreursL'exemple suivant est tiré d'une série d'articles sur l'analyse du projet Chromium. Elle s'est couverte dans la bibliothèque WebRTC.
std::vector<SdpVideoFormat> StereoDecoderFactory::GetSupportedFormats() const { std::vector<SdpVideoFormat> formats = ....; for (const auto& format : formats) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } } return formats; }
Avertissement PVS-Studio: les itérateurs V789 CWE-672 pour le conteneur 'formats', utilisés dans la boucle for basée sur la plage, deviennent invalides lors de l'appel de la fonction 'push_back'. stereocodecfactory.cc 89
L'erreur est que la taille du vecteur de
formats change à l'intérieur de la boucle basée sur la plage. Les boucles basées sur la plage sont basées sur des itérateurs, donc le redimensionnement du conteneur à l'intérieur de ces boucles peut entraîner l'invalidation de ces itérateurs.
Cette erreur persistera si vous réécrivez la boucle à l'aide d'itérateurs explicites. Par conséquent, pour plus de clarté, vous pouvez apporter ce code:
for (auto format = begin(formats), __end = end(formats); format != __end; ++format) { if (cricket::CodecNamesEq(....)) { .... formats.push_back(stereo_format); } }
Par exemple, lorsque vous utilisez la méthode
push_back , un vecteur peut être libéré - puis les itérateurs pointeront vers une zone de mémoire non valide.
Pour éviter de telles erreurs, vous devez respecter la règle: ne redimensionnez jamais le conteneur à l'intérieur de la boucle, dont les conditions sont liées à ce conteneur. Cela s'applique aux boucles basées sur des plages et aux boucles utilisant des itérateurs. Vous pouvez lire quelles opérations peuvent entraîner l'invalidation des itérateurs dans les
discussions sur StackOverflow.
Septième place
Source:
Godot: Utilisation régulière d'analyseurs de code statiqueLe premier exemple de l'industrie du jeu vidéo sera un extrait de code que nous avons découvert dans le moteur de jeu Godot. Vous devrez peut-être transpirer pour découvrir l'erreur avec vos yeux, mais je suis sûr que nos lecteurs sophistiqués peuvent le gérer:
void AnimationNodeBlendSpace1D::add_blend_point( const Ref<AnimationRootNode> &p_node, float p_position, int p_at_index) { ERR_FAIL_COND(blend_points_used >= MAX_BLEND_POINTS); ERR_FAIL_COND(p_node.is_null()); ERR_FAIL_COND(p_at_index < -1 || p_at_index > blend_points_used); if (p_at_index == -1 || p_at_index == blend_points_used) { p_at_index = blend_points_used; } else { for (int i = blend_points_used - 1; i > p_at_index; i++) { blend_points[i] = blend_points[i - 1]; } } .... }
Avertissement PVS-Studio: V621 CWE-835 Envisagez d'inspecter l'opérateur "for". Il est possible que la boucle soit mal exécutée ou ne soit pas exécutée du tout. animation_blend_space_1d.cpp 113
Examinons plus en détail la condition du cycle. La variable de compteur est initialisée avec la valeur
blend_points_used - 1 . En même temps, sur la base des deux vérifications précédentes (dans
ERR_FAIL_COND et dans
if ), il devient clair qu'au moment où la boucle est exécutée,
blend_points_used sera toujours supérieur à
p_at_index . Ainsi, soit la condition de boucle sera toujours vraie, soit la boucle ne sera pas exécutée du tout.
Si
blend_points_used vaut 1 == p_at_index , alors la boucle n'est pas exécutée.
Dans tous les autres cas, la vérification
i> p_at_index sera toujours vraie, puisque le compteur
i augmente à chaque itération de la boucle.
Il peut sembler que le cycle se poursuivra pour toujours, mais ce n'est pas le cas.
Tout d'abord, il y aura un débordement d'entier de la variable
i , qui est un comportement indéfini. Par conséquent, compter sur cela n'en vaut pas la peine.
Si
i était de type
unsigned int , alors une fois que le compteur a atteint la valeur maximale possible, l'opérateur
i ++ le transformerait en
0 . Ce comportement est défini par la norme et est appelé «habillage non signé». Cependant, vous devez être conscient que l'utilisation d'un tel mécanisme n'est
pas non plus une bonne idée .
C'était le premier, mais il y en a encore un deuxième! Le fait est qu'il n'atteindra même pas le débordement d'entier. Où avant le tableau va à l'étranger. Cela signifie qu'une tentative sera effectuée pour accéder à la zone de mémoire en dehors du bloc alloué pour le tableau. Et cela aussi est un comportement vague. Exemple classique :)
Pour éviter plus facilement de telles erreurs, je ne peux donner que quelques recommandations:
- Écrivez du code plus simple et plus intuitif
- Faites une révision de code plus approfondie et écrivez plus de tests pour du code fraîchement écrit
- Utilisez des analyseurs statiques;)
Sixième place
Source:
Amazon Lumberyard: Le cri de l'âmeUn autre exemple de l'industrie gamedev, à savoir le code source du moteur Amazon Lumberyard AAA.
void TranslateVariableNameByOperandType(....) {
PVS-Studio Warning: V523 L'
instruction 'then' est équivalente à l'instruction 'else'. toglsloperand.c 700
Amazon Lumberyard est en cours de développement en tant que moteur multiplateforme. Par conséquent, les développeurs essaient de prendre en charge autant de compilateurs que possible. Le programmeur Igor a rencontré le compilateur Qualcomm, comme nous le disent les commentaires.
On ne sait pas si Igor a pu terminer sa tâche et faire face aux vérifications "paranoïaques" du compilateur, mais il a laissé un code très étrange. Il est étrange que les branches
then - et
else - de l'
instruction if contiennent un code absolument identique. Très probablement, une telle erreur a été commise à la suite d'un copier-coller bâclé.
Je ne sais même pas ce qui peut être conseillé ici. Par conséquent, je souhaite juste aux développeurs d'Amazon Lumberyard de réussir à corriger les bugs et bonne chance au programmeur Igor!
Cinquième place
Source:
Encore une fois, l'analyseur PVS-Studio s'est avéré être plus attentif qu'une personneUne histoire intéressante s'est produite avec l'exemple suivant. Mon collègue Andrei Karpov préparait un article sur le prochain test du framework Qt. Au cours de la rédaction d'erreurs notables, il est tombé sur un avertissement de l'analyseur, qu'il a considéré faux. Voici l'extrait de code et l'avertissement pertinents:
QWindowsCursor::CursorState QWindowsCursor::cursorState() { enum { cursorShowing = 0x1, cursorSuppressed = 0x2 }; CURSORINFO cursorInfo; cursorInfo.cbSize = sizeof(CURSORINFO); if (GetCursorInfo(&cursorInfo)) { if (cursorInfo.flags & CursorShowing)
Avertissement PVS-Studio: V616 CWE-480 La constante nommée 'CursorShowing' avec la valeur 0 est utilisée dans l'opération au niveau du bit. qwindowscursor.cpp 669
Autrement dit, PVS-Studio a juré à un endroit où, évidemment, il n'y avait pas d'erreur! Il ne peut pas être que la constante
CursorShowing soit
0 , car littéralement quelques lignes au-dessus sont initialisées à
1 .
Puisqu'une version instable de l'analyseur a été utilisée pour la vérification, Andrei a douté de l'exactitude de l'avertissement. Il a examiné attentivement cette section de code à plusieurs reprises et n'a toujours pas trouvé d'erreur. En conséquence, il a écrit ce faux positif au bugtracker afin que d'autres collègues puissent corriger la situation.
Et ce n'est qu'avec une analyse détaillée qu'il est devenu clair que PVS-Studio était à nouveau plus attentif qu'une personne. La valeur
0x1 est affectée à la constante nommée
cursorShowing , et l'opération de constante de bit «et» implique la constante nommée
CursorShowing . Ce sont des constantes complètement différentes, car la première commence par une lettre minuscule et la seconde par une lettre majuscule.
Le code se compile avec succès, car la classe
QWindowsCursor contient vraiment une constante avec ce nom. Voici sa définition:
class QWindowsCursor : public QPlatformCursor { public: enum CursorState { CursorShowing, CursorHidden, CursorSuppressed }; .... }
Si vous n'affectez pas explicitement une constante énumérée nommée, elle sera initialisée par défaut. Étant donné que
CursorShowing est le premier élément d'une énumération, il sera défini sur
0 .
Afin d'éviter de telles erreurs, vous ne devez pas donner aux entités des noms trop similaires. Vous devez particulièrement respecter cette règle si ces entités sont du même type ou peuvent être implicitement converties les unes aux autres. En effet, dans de tels cas, il sera presque impossible de détecter une erreur à l'œil nu, et le code incorrect sera compilé avec succès et vivra heureux dans votre projet.
Quatrième place
Source:
Nous tirons dans le pied, en traitant les données d'entréeNous approchons des trois premiers finalistes, et l'erreur du projet FreeSWITCH est à son tour la suivante.
static const char *basic_gets(int *cnt) { .... int c = getchar(); if (c < 0) { if (fgets(command_buf, sizeof(command_buf) - 1, stdin) != command_buf) { break; } command_buf[strlen(command_buf)-1] = '\0'; break; } .... }
Avertissement PVS-Studio: V1010 CWE-20 Les données contaminées non
contrôlées sont utilisées dans l'index: 'strlen (command_buf)'.
L'analyseur avertit que l'expression
strlen (command_buf) - 1 utilise des données non vérifiées. Et vraiment: si
command_buf s'avère vide du point de vue de la chaîne de langage C (contenant un seul caractère - '\ 0') alors
strlen (command_buf) retournera
0 . Dans ce cas,
command_buf [-1] sera
appelé , ce qui représente un comportement non défini. Trouble!
Le jus de cette erreur n'est même pas
pourquoi elle se produit, mais
comment elle se produit. Cette erreur est l'un de ces exemples agréables que vous pouvez "toucher" par vous-même, reproduire. Vous pouvez démarrer FreeSwitch, effectuer certaines actions qui mèneront à l'exécution de la section de code ci-dessus et passer au programme une ligne vide pour la saisie.
En conséquence, d'un simple mouvement du poignet, un programme de travail se transforme (non, pas
un short élégant ) en un non-travail! Des détails sur la façon de reproduire cette erreur peuvent être trouvés dans l'article source sur le lien ci-dessus, mais pour l'instant je vais donner un résultat clair:
N'oubliez pas que l'entrée peut être n'importe quoi, et vous devez toujours la vérifier. Ensuite, l'analyseur ne jurera pas et le programme sera plus fiable.
Il est maintenant temps de traiter avec nos gagnants: nous passons à la finale!
Troisième place
Source:
NCBI Genome Workbench: Endangered ResearchLes trois gagnants sont ouverts par un morceau de code du projet NCBI Genome Workbench - un ensemble d'outils pour étudier et analyser les données génétiques. Bien qu'il ne soit pas nécessaire d'être un surhomme génétiquement modifié pour trouver une erreur ici, beaucoup sont conscients de cette possibilité.
void tds_answer_challenge(....) { .... if (ntlm_v == 1) { .... memset(hash, 0, sizeof(hash)); memset(passwd_buf, 0, sizeof(passwd_buf)); ... } else { .... } }
Avertissements de PVS-Studio:- V597 Le compilateur pourrait supprimer l'appel de fonction 'memset', qui est utilisé pour vider le tampon 'hash'. La fonction memset_s () doit être utilisée pour effacer les données privées. challenge.c 365
- V597 Le compilateur pourrait supprimer l'appel de fonction 'memset', qui est utilisé pour vider le tampon 'passwd_buf'. La fonction memset_s () doit être utilisée pour effacer les données privées. challenge.c 366
Vous avez réussi à trouver une erreur? Si oui, alors vous - bien joué! .. eh bien, ou encore un surhomme génétiquement modifié.
Le fait est que les compilateurs d'optimisation modernes peuvent faire beaucoup pour accélérer le travail du programme assemblé. En particulier, les compilateurs sont capables de suivre que le tampon passé à
memset n'est utilisé nulle part ailleurs.
Dans ce cas, ils peuvent supprimer l'appel
memset «inutile» et ont le droit de le faire. Ensuite, un tampon qui stocke des données importantes peut rester en mémoire pour le plus grand plaisir des attaquants.
Dans ce contexte, le commentaire lettré «avec sécurité, c'est bien d'être pédant» semble encore plus drôle. À en juger par le très petit nombre d'avertissements émis pour ce projet, les développeurs ont fait de leur mieux pour être prudents et écrire du code sûr. Cependant, comme nous pouvons le voir, ignorer cette faille de sécurité est très simple. Selon Common Weakness Enumeration, un défaut est classé
CWE-14 : Compilation Removal of Code to Clear Buffers.
Pour effacer le nettoyage de la mémoire, utilisez la fonction
memset_s () . Il est non seulement plus sûr que
memset () , mais ne peut pas non plus être "ignoré" par le compilateur.
Deuxième place
Source:
Comment PVS-Studio s'est avéré plus attentif que trois programmeurs et demiLe médaillé d'argent de ce haut nous a été envoyé par un de nos clients. Il était sûr que l'analyseur avait généré de faux avertissements.
Eugene a reçu la lettre, l'a scannée brièvement et l'a envoyée à Svyatoslav. Svyatoslav a réfléchi à la section de code envoyée par le client et a pensé: "L'analyseur peut-il être si manifestement faux?" Par conséquent, il est allé consulter Andrei. Il a également vérifié le site et décidé: en effet, l'analyseur donne des faux positifs.
Que pouvez-vous faire, vous devez le réparer. Et ce n'est que lorsque Sviatoslav a commencé à créer des exemples synthétiques pour formaliser la tâche de traqueur de bogues qu'il a réalisé ce qui se passait.
Des erreurs étaient en effet présentes dans le code, mais aucun des programmeurs n'a pu les détecter. Honnêtement, l'auteur de cet article n'a pas réussi non plus.
Et cela malgré le fait que l'analyseur a clairement émis des avertissements pour les mauvais endroits!
Pouvez-vous trouver une erreur aussi astucieuse? Testez-vous pour la vigilance et l'attention.
Avertissement PVS-Studio:- V560 Une partie de l'expression conditionnelle est toujours fausse: (ch> = 0x0FF21). decodew.cpp 525
- V560 Une partie de l'expression conditionnelle est toujours vraie: (ch <= 0x0FF3A). decodew.cpp 525
- V560 Une partie de l'expression conditionnelle est toujours fausse: (ch> = 0x0FF41). decodew.cpp 525
- V560 Une partie de l'expression conditionnelle est toujours vraie: (ch <= 0x0FF5A). decodew.cpp 525
Si vous réussissez - vous ne me respecterez pas!
L'erreur réside dans le fait que l'opérateur de négation logique (!) Ne s'applique pas à la condition entière, mais seulement à sa première sous-expression:
!((ch >= 0x0FF10) && (ch <= 0x0FF19))
Si cette condition est remplie, alors la valeur de la variable
ch se situe dans l'intervalle [0x0FF10 ... 0x0FF19]. Ainsi, les quatre autres comparaisons n'ont plus de sens: elles seront toujours vraies ou fausses.
Pour éviter de telles erreurs, il vaut la peine de suivre quelques règles. Tout d'abord, il est très pratique et clair d'aligner le code sur un tableau. Deuxièmement, ne surchargez pas les expressions entre parenthèses. Par exemple, ce code peut être réécrit comme ceci:
const bool isLetterOrDigit = (ch >= 0x0FF10 && ch <= 0x0FF19)
Ensuite, d'une part, le nombre de parenthèses devient beaucoup plus petit, et d'autre part, la probabilité de «rattraper» une erreur commise par les yeux augmente.
Et maintenant - cerise: nous passons à la première place!
Première place
Source:
System in shock: erreurs intéressantes dans les codes sources du légendaire System ShockAinsi, le finaliste de notre top d'aujourd'hui est une erreur du légendaire System Shock! Ce jeu, sorti en 1994, est devenu l'ancêtre et l'inspirateur de jeux emblématiques tels que Dead Space, BioShock et Deus Ex.
Mais d'abord, je dois admettre quelque chose. Ce que je vais vous montrer maintenant ne contient aucune erreur. En gros, ce n'est même pas un extrait de code, mais je n'ai pas pu résister à ne pas le partager avec vous!
Le fait est qu'au cours de l'analyse du code source du jeu, ma collègue Victoria a trouvé de nombreux commentaires intéressants. Ici et là, tout à coup, il y avait des plaisanteries et des remarques ironiques, et même des versets:
Pour nos lecteurs russophones, j'ai fait une traduction gratuite approximative:
Ces commentaires ont été laissés par les développeurs du jeu au début des années 90 ... Soit dit en passant, Doug Church - le concepteur en chef de System Shock - écrivait également du code. Qui sait, peut-être que certains de ces commentaires ont été écrits par lui personnellement? J'espère que pour les hommes en serviettes - ce n'est pas son affaire :)Conclusion
En conclusion, je tiens à remercier mes collègues d' avoir recherché de nouvelles erreurs et d'avoir écrit des articles à leur sujet. Merci les gars! Sans vous, cet article n'aurait pas été aussi intéressant.Je veux également parler un peu de nos réalisations, car pendant toute une année, nous avons été engagés dans plus que la simple recherche d'erreurs. Nous avons également développé et amélioré l'analyseur, à la suite duquel il a subi des changements importants.Par exemple, nous avons ajouté la prise en charge de plusieurs nouveaux compilateurs et étendu la liste des règles de diagnostic. Nous avons également fourni un support initial pour les normes MISRA C et MISRA C ++ . L'innovation la plus importante et la plus longue a été le support d'une nouvelle langue. Oui, nous pouvons maintenant analyser le code Java ! Et nous avons mis à jour l'icône:)Je tiens également à remercier nos lecteurs. Merci d'avoir lu nos articles et de nous avoir écrit! Vos commentaires sont très agréables et importants pour nous.À ce sujet, nos 10 principales erreurs C ++ pour l'année 2018 ont pris fin. Quels endroits avez-vous le plus appréciés et pourquoi? Avez-vous rencontré des exemples intéressants en 2018? Dites-le nous dans les commentaires!Jusqu'à la prochaine fois!
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: George Gribkov. Top 10 des bugs des projets C ++ trouvés en 2018