Une autre année touche à sa fin, et c'est le moment idéal pour vous préparer une tasse de café et relire les critiques de bugs collectés dans le cadre de projets open source au cours de cette année. Cela prendrait un certain temps, bien sûr, nous avons donc préparé cet article pour vous faciliter la tâche. Aujourd'hui, nous nous souviendrons des points noirs les plus intéressants que nous ayons rencontrés dans des projets C / C ++ open source en 2019.
Non. 10. Sur quel système d'exploitation fonctionnons-nous?
V1040 Typo possible dans l'orthographe d'un nom de macro prédéfini. La macro '__MINGW32_' est similaire à '__MINGW32__'. winapi.h 4112
#if !defined(__UNICODE_STRING_DEFINED) && defined(__MINGW32_) #define __UNICODE_STRING_DEFINED #endif
Il existe une faute de frappe dans le nom de la macro
__MINGW32 _ (MINGW32 est en fait déclaré par __MINGW32__). Ailleurs dans le projet, le chèque est écrit correctement:
Soit dit en passant, ce bogue n'était pas seulement le premier à être décrit dans l'article «
CMake: le cas où la qualité du projet est impardonnable », mais le tout premier bogue authentique trouvé par le diagnostic V1040 dans un véritable projet open source (19 août , 2019).
Non. 9. Qui est le premier?
V502 L'opérateur '?:'
Fonctionne peut-être d'une manière différente de celle attendue. L'opérateur '?:' A une priorité inférieure à l'opérateur '=='. mir_parser.cpp 884
enum Opcode : uint8 { kOpUndef, .... OP_intrinsiccall, OP_intrinsiccallassigned, .... kOpLast, }; bool MIRParser::ParseStmtIntrinsiccall(StmtNodePtr &stmt, bool isAssigned) { Opcode o = !isAssigned ? (....) : (....); auto *intrnCallNode = mod.CurFuncCodeMemPool()->New<IntrinsiccallNode>(....); lexer.NextToken(); if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) { intrnCallNode->SetIntrinsic(GetIntrinsicID(lexer.GetTokenKind())); } else { intrnCallNode->SetIntrinsic(static_cast<MIRIntrinsicID>(....)); } .... }
Nous sommes intéressés par la partie suivante:
if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) { .... }
La priorité de l'opérateur '==' est supérieure à celle de l'opérateur ternaire (? :). Par conséquent, l'expression conditionnelle est évaluée dans le mauvais ordre et équivaut au code suivant:
if ((o == !isAssigned) ? OP_intrinsiccall : OP_intrinsiccallassigned) { .... }
Étant donné que les constantes
OP_intrinsiccall et
OP_intrinsiccallassigned ne sont pas nulles, la condition retournera
true tout le temps, ce qui signifie que le corps de la branche
else est du code inaccessible.
Ce bogue a été décrit dans l'article "
Vérification du compilateur Ark récemment créé en open-source par Huawei ".
Non. 8. Opérations dangereuses au niveau du bit
V1046 Utilisation non
sûre des types bool 'et' int 'ensemble dans l'opération' & = '. GSLMultiRootFinder.h 175
int AddFunction(const ROOT::Math::IMultiGenFunction & func) { ROOT::Math::IMultiGenFunction * f = func.Clone(); if (!f) return 0; fFunctions.push_back(f); return fFunctions.size(); } template<class FuncIterator> bool SetFunctionList( FuncIterator begin, FuncIterator end) { bool ret = true; for (FuncIterator itr = begin; itr != end; ++itr) { const ROOT::Math::IMultiGenFunction * f = *itr; ret &= AddFunction(*f); } return ret; }
Le code suggère que la fonction
SetFunctionList traverse une liste d'itérateurs. Si au moins un itérateur n'est pas valide, la fonction renvoie
false , ou
true sinon.
Cependant, la fonction
SetFunctionList peut retourner
false même pour les itérateurs valides. Voyons pourquoi
. La fonction
AddFunction renvoie le nombre d'itérateurs valides dans la liste
fFunctions . Autrement dit, l'ajout d'itérateurs non nuls entraînera une augmentation incrémentielle de la liste: 1, 2, 3, 4, etc. C'est là que le bug entre en jeu:
ret &= AddFunction(*f);
Étant donné que la fonction renvoie une valeur de type
int plutôt que
bool , l'opération '& =' renvoie
false pour les valeurs paires car le bit le moins significatif d'un nombre pair est toujours défini sur zéro. C'est ainsi qu'un bug subtil peut casser la valeur de retour de
SetFunctionsList même lorsque ses arguments sont valides.
Si vous lisiez attentivement l'extrait (et vous l'étiez, n'est-ce pas?), Vous auriez pu remarquer qu'il provenait du projet ROOT. Oui, nous l'avons vérifié aussi: "
Analyse du code de ROOT, cadre d'analyse des données scientifiques ."
Non. 7. Variables mélangées
V1001 [CWE-563] La variable 'Mode' est affectée mais n'est pas utilisée à la fin de la fonction. SIModeRegister.cpp 48
struct Status { unsigned Mask; unsigned Mode; Status() : Mask(0), Mode(0){}; Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) { Mode &= Mask; }; .... };
Il est très dangereux d'utiliser les mêmes noms pour les arguments de fonction que pour les membres de la classe car vous risquez de les mélanger. Et c'est exactement ce qui s'est passé ici. L'expression suivante n'a pas de sens:
Mode &= Mask;
L'argument de la fonction change, et c'est tout. Cet argument n'est en aucun cas utilisé après cela. Ce que le programmeur voulait vraiment écrire était probablement le suivant:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) { this->Mode &= Mask; };
Ce bogue a été trouvé dans
LLVM . Nous avons une tradition de vérifier ce projet de temps en temps. Cette année, nous l'avons
vérifié une fois de plus.
Non. 6. C ++ a ses propres lois
Ce bogue provient du fait que les règles C ++ ne suivent pas toujours les règles mathématiques ou le "bon sens". Regardez le petit extrait ci-dessous et essayez de trouver le bogue vous-même.
V709 Comparaison suspecte trouvée: 'f0 == f1 == m_fractureBodies.size ()'. N'oubliez pas que «a == b == c» n'est pas égal à «a == b && b == c». btFractureDynamicsWorld.cpp 483
btAlignedObjectArray<btFractureBody*> m_fractureBodies; void btFractureDynamicsWorld::fractureCallback() { for (int i = 0; i < numManifolds; i++) { .... int f0 = m_fractureBodies.findLinearSearch(....); int f1 = m_fractureBodies.findLinearSearch(....); if (f0 == f1 == m_fractureBodies.size()) continue; .... } .... }
La condition semble vérifier que
f0 est égal à
f1 et est égal au nombre d'éléments dans
m_fractureBodies . Il était probablement destiné à vérifier si
f0 et
f1 sont situés à la fin du tableau
m_fractureBodies car ils contiennent une position d'objet trouvée par la méthode
findLinearSearch () . Mais en réalité, cette expression conditionnelle vérifie si
f0 est égal à
f1 et ensuite si
m_fractureBodies.size () est égal au résultat de l'expression
f0 == f1 . Autrement dit, le troisième opérande ici est vérifié par rapport à 0 ou 1.
C'est un joli bug! Et, heureusement, assez rare. Jusqu'à présent, nous ne l'avons
vu que dans trois projets open-source, et, fait intéressant, tous les trois étaient des moteurs de jeu. Ce n'est pas le seul bug trouvé dans Bullet; les plus intéressants ont été décrits dans l'article "
PVS-Studio a examiné le moteur de balle de Red Dead Redemption ".
Non. 5. Qu'est-ce qui se trouve à la fin de la ligne?
Celui-ci est facile si vous connaissez un détail délicat.
V739 EOF ne doit pas être comparé à une valeur de type 'char'. Le «ch» doit être de type «int». json.cpp 762
void JsonIn::skip_separator() { signed char ch; .... if (ch == ',') { if( ate_separator ) { .... } .... } else if (ch == EOF) { .... }
C'est l'un de ces bogues que vous ne pouvez pas facilement repérer si vous ne savez pas que
EOF est défini sur -1. Donc, si vous essayez de le comparer avec une variable de type
signé char , la condition sera presque toujours
fausse . La seule exception est le caractère codé en 0xFF (255). Par rapport à
EOF , ce caractère se transformera en -1, rendant ainsi la condition vraie.
Beaucoup de bugs dans le Top 10 de cette année ont été trouvés dans les logiciels de jeux informatiques: moteurs ou jeux open-source. Comme vous l'avez déjà deviné, celui-ci provenait également de cette région. D'autres erreurs sont décrites dans l'article "
Cataclysm Dark Days Ahead: Static Analysis and Roguelike Games ".
Non. 4. La constante magique Pi
V624 Il y a probablement une
faute d' impression dans la constante '3.141592538'. Pensez à utiliser la constante M_PI de <math.h>. PhysicsClientC_API.cpp 4109
B3_SHARED_API void b3ComputeProjectionMatrixFOV(float fov, ....) { float yScale = 1.0 / tan((3.141592538 / 180.0) * fov / 2); .... }
Il y a une petite faute de frappe dans le nombre Pi (3,141592653 ...): le nombre "6" est manquant à la 7ème décimale.
Un millionième chiffre décimal incorrect ne causerait guère de dommages notables, mais il est toujours préférable d'utiliser les constantes existantes des bibliothèques, dont l'exactitude est garantie. Le nombre Pi, par exemple, est représenté par la constante M_PI de l'en-tête math.h.
Vous avez déjà lu ce bug dans l'article "
PVS-Studio a regardé le moteur de puces de Red Dead Redemption ", où il était placé sixième. Si vous ne l'avez pas encore lu, c'est votre dernière chance.
Une petite diversion
Nous approchons du Top 3 des bugs les plus intéressants. Comme vous l'avez probablement remarqué, je trie les bugs non pas par leur impact mais par l'effort qu'il faut à un critique humain pour les trouver. Après tout, l'avantage de l'analyse statique sur les revues de code est essentiellement l'incapacité des outils logiciels à se fatiguer ou à oublier des choses. :)
Voyons maintenant ce que nous avons dans notre Top 3.
Non. 3. Une exception insaisissable
Les classes
V702 doivent toujours être dérivées de std :: exception (et similaires) en tant que «public» (aucun mot-clé n'a été spécifié, donc le compilateur le définit par défaut sur «privé»). CalcManager CalcException.h 4
class CalcException : std::exception { public: CalcException(HRESULT hr) { m_hr = hr; } HRESULT GetException() { return m_hr; } private: HRESULT m_hr; };
L'analyseur a détecté une classe dérivée de la classe
std :: exception à l'aide du modificateur
private (qui est utilisé par défaut s'il n'est pas spécifié autrement). Le problème avec ce code est qu'une tentative d'attraper une
exception std :: générique entraînera le programme à manquer une exception de type
CalcException . Ce comportement provient du fait que l'héritage privé interdit la conversion de type implicite.
Vous n'aimeriez certainement pas voir votre programme planter à cause d'un modificateur
public manquant. Soit dit en passant, je parie que vous avez utilisé cette application au moins une fois dans votre vie car c'est la bonne vieille
calculatrice Windows , que nous avons également
vérifiée plus tôt cette année.
Non. 2. Balises HTML non fermées
V735 Peut-être un HTML incorrect. La balise de fermeture "</body>" a été rencontrée, tandis que la balise "</div>" était attendue. book.cpp 127
static QString makeAlgebraLogBaseConversionPage() { return BEGIN INDEX_LINK TITLE(Book::tr("Logarithmic Base Conversion")) FORMULA(y = log(x) / log(a), log<sub>a</sub>x = log(x) / log(a)) END; }
Comme cela arrive souvent, le code source C / C ++ ne dit pas grand-chose en soi, alors jetons un coup d'œil au code prétraité généré à partir de l'extrait ci-dessus:
L'analyseur a trouvé une
balise <div> non fermée. Il existe de nombreux fragments de code html ici, donc les auteurs doivent le réviser.
Surpris de pouvoir diagnostiquer ce genre de bugs? J'ai aussi été impressionné lorsque j'ai vu cela pour la première fois. Donc, oui, nous savons quelque chose sur l'analyse du code html. Eh bien, seulement si c'est dans le code C ++. :)
Non seulement ce bug est placé en deuxième position, mais c'est une deuxième calculatrice dans notre liste des 10 meilleurs. Pour savoir quels autres bugs nous avons trouvé dans ce projet, voir l'article "
Suivre les traces des calculatrices: SpeedCrunch ".
Non. 1. Fonctions standard insaisissables
Voici le bug placé en premier. Celui-ci est un bug incroyablement étrange, qui a réussi à passer à travers la révision du code.
Essayez de le trouver vous-même:
static int EatWhitespace (FILE * InFile) { int c; for (c = getc (InFile); isspace (c) && ('\n' != c); c = getc (InFile)) ; return (c); }
Voyons maintenant ce que l'analyseur a à dire:
V560 Une partie de l'expression conditionnelle est toujours vraie: ('\ n'! = C). params.c 136.
Bizarre, non? Jetons un coup d'œil à un autre endroit curieux mais dans un fichier différent (charset.h):
#ifdef isspace #undef isspace #endif .... #define isspace(c) ((c)==' ' || (c) == '\t')
Hm, c'est vraiment étrange ... Donc, si la variable
c est égale à
'\ n', alors la fonction apparemment inoffensive
isspace (c) retournera
fausse , empêchant ainsi la deuxième partie du contrôle de s'exécuter en raison d'une évaluation en court-circuit. Et si
isspace (c) s'exécute, la variable
c sera égale à
'' ou
'\ t', ce qui n'est évidemment pas égal à
'\ n' .
Vous pourriez faire valoir que cette macro est similaire à
#define true false et qu'un code comme celui-ci ne passerait jamais par un examen du code. Mais cet extrait particulier l'a fait - et était assis dans le référentiel en attente d'être découvert.
Pour des commentaires plus détaillés sur ce bogue, consultez l'article "Vous
voulez jouer un détective? Trouvez le bogue dans une fonction de Midnight Commander ".
Conclusion
Nous avons trouvé des tonnes de bugs cette année. Il s'agissait d'erreurs de copier-coller courantes, de constantes inexactes, de balises non fermées et de nombreux autres défauts. Mais notre analyseur évolue et
apprend à diagnostiquer de plus en plus de types de problèmes, donc nous n'allons certainement pas ralentir et publierons de nouveaux articles sur les bugs trouvés dans les projets aussi régulièrement qu'auparavant.
Juste au cas où vous n'avez pas lu nos articles auparavant, tous ces bogues ont été trouvés en utilisant notre analyseur statique PVS-Studio, que vous pouvez
télécharger et essayer sur vos propres projets. Il détecte les bogues dans les programmes écrits en C, C ++, C # et Java.
Vous avez enfin atteint la ligne d'arrivée! Si vous avez manqué les deux premiers niveaux, je vous suggère de saisir l'opportunité et de compléter ces niveaux avec nous:
C # et
Java .