De temps en temps, les lecteurs de nos articles sur la vérification des projets open source remarquent que l'analyseur de code statique PVS-Studio détecte un grand pourcentage d'erreurs qui sont insignifiantes ou n'affectent pas du tout l'application. Ça l'est vraiment. La plupart des bogues importants ont déjà été corrigés grâce à des tests manuels, des critiques d'utilisateurs et d'autres méthodes coûteuses. Dans le même temps, bon nombre de ces erreurs peuvent être trouvées même au stade de l'écriture du code et corrigées avec une perte minimale de temps, de réputation et d'argent. Cet article fournira quelques exemples d'erreurs réelles qui seraient corrigées immédiatement si les auteurs des projets utilisaient l'analyse de code statique.
L'idée est très simple. Regardons GitHub pour des exemples de demandes de tirage, dont les commentaires indiquent qu'il s'agit d'un correctif de bogue. Et essayez de trouver ces erreurs en utilisant l'analyseur de code statique PVS-Studio. Si l'erreur corrigée est trouvée par l'analyseur, cela signifie qu'elle pourrait être corrigée même au stade de l'écriture du code. Et plus tôt l'erreur est corrigée, moins elle coûte cher.
Malheureusement, GitHub nous a laissé tomber et n'a pas permis de faire un excellent article intelligent sur ce sujet. GitHub lui-même a également un problème (ou une fonctionnalité) qui ne vous permet pas de rechercher des commentaires dans les demandes d'extraction dans des projets écrits uniquement dans certains langages de programmation. Eh bien, ou je ne sais pas "comment faire cuire". Indépendamment de ce que je vous dis de rechercher des commentaires dans les projets en C ++, C # ou Java, les résultats sont affichés dans toutes les langues, y compris PHP, Python, JavaScript et ainsi de suite. Par conséquent, la recherche de cas appropriés a été une tâche extrêmement fastidieuse et je me limiterai à quelques exemples. Cependant, ils suffisent à démontrer l'utilité des outils d'analyse de code statique lorsqu'ils sont utilisés régulièrement.
Et si l'erreur était détectée tôt? La réponse est simple: les programmeurs n'auraient pas à attendre sa manifestation, puis chercher et corriger le code défectueux.
Regardons les erreurs que PVS-Studio a pu détecter immédiatement:
Le premier exemple est tiré du projet SatisfactoryModLoader. Avant de corriger l'erreur, le code ressemblait à ceci:
Ce code contenait une erreur que PVS-Studio donnerait immédiatement un avertissement:
V591 La fonction non vide doit renvoyer une valeur. ModFunctions.cpp 44
La fonction ci-dessus n'a pas de
retour , elle renverra donc une valeur formellement non définie. Le programmeur n'a pas utilisé l'analyseur de code, il a donc dû rechercher lui-même une erreur. Fonction après édition:
Curieusement, dans le commit, l'auteur a indiqué le bogue comme critique: "
correction d' un bogue critique où les fonctions API n'étaient pas retournées ".
Dans le deuxième
commit de l'historique du projet mc6809, des corrections ont été apportées au code suivant:
void mc6809dis_direct( mc6809dis__t *const dis, mc6809__t *const cpu, const char *const op, const bool b16 ) { assert(dis != NULL); assert(op != NULL); addr.b[MSB] = cpu->dp; addr.b[LSB] = (*dis->read)(dis, dis->next++); ... if (cpu != NULL) { ... } }
L'auteur n'a corrigé qu'une seule ligne. Il a remplacé l'expression
addr.b[MSB] = cpu->dp;
sur l'expression
addr.b[MSB] = cpu != NULL ? cpu->dp : 0;
Dans l'ancienne version du code, aucune vérification du
processeur n'était effectuée pour l'égalité avec le pointeur nul. S'il s'avère soudainement qu'un pointeur nul est passé à la fonction
mc6809dis_direct comme deuxième argument, il sera déréférencé dans le corps de la fonction. Le résultat est
déplorable et imprévisible .
Le déréférencement de pointeurs nuls est l'un des modèles les plus courants dont on nous parle: «Ce n'est pas une erreur critique. Eh bien, elle vit dans le code et vit, et si le déréférencement se produit, le programme se bloquera tranquillement et c'est tout. " C'est étrange et triste d'entendre cela des programmeurs C ++, mais la vie c'est la vie.
Dans tous les cas, dans ce projet, un tel déréférencement s'est toujours transformé en bogue, comme l'en-tête de commit nous le dit: "
Bug fix --- NULL dereference ".
Si le développeur du projet avait utilisé PVS-Studio, il aurait pu vérifier son code et détecter un avertissement il y a deux mois et demi (c'est ce qui s'est passé depuis que l'erreur a été commise):
V595 Le pointeur 'cpu' a été utilisé avant d'être vérifié par rapport à nullptr. Lignes de contrôle: 1814, 1821. mc6809dis.c 1814
Ainsi, l'erreur serait éliminée même au moment de son apparition, ce qui ferait gagner du temps et, éventuellement, les nerfs du développeur :).
Un exemple d'un autre
montage intéressant a été trouvé dans le projet libmorton.
Le code corrigé:
template<typename morton> inline bool findFirstSetBitZeroIdx(const morton x, unsigned long* firstbit_location) { #if _MSC_VER && !_WIN64
Dans son montage, le programmeur remplace l'expression "
firstbit_location + = 32 " par "
* firstbit_location + = 32 ". Le programmeur s'attendait à ce que le nombre
32 soit ajouté à la valeur de la variable à laquelle le pointeur
firstbit_location est
lié , mais il a été ajouté directement au pointeur. La valeur modifiée du pointeur n'a été utilisée nulle part ailleurs et la valeur attendue de la variable est restée inchangée.
PVS-Studio émettrait un avertissement pour ce code:
V1001 La variable 'firstbit_location' est affectée mais n'est pas utilisée à la fin de la fonction. morton_common.h 22
Il semblerait que ce qui pourrait être terrible dans une expression modifiée, mais encore inutilisée? Les diagnostics du V1001 ne semblent évidemment pas destinés à identifier des bogues particulièrement dangereux. Malgré cela, elle a découvert une erreur importante qui a affecté la logique du programme.
De plus, il s'est avéré que cette erreur n'était pas si facile à trouver! Non seulement elle était dans le programme dès le moment où le fichier a été créé, elle a également survécu à de nombreuses modifications dans les lignes adjacentes et a existé dans le projet aussi longtemps que 3 (
! ) Ans! Pendant tout ce temps, la logique du programme a été brisée et cela n'a pas fonctionné exactement comme les développeurs s'y attendaient. S'ils utilisaient PVS-Studio, l'erreur aurait été détectée beaucoup plus tôt.
À la fin, considérons un autre bel exemple. Pendant que je collectais des correctifs de bogues sur GitHub, je suis tombé plusieurs fois sur un correctif avec
ce contenu . Le bug corrigé ici:
int kvm_arch_prepare_memory_region(...) { ... do { struct vm_area_struct *vma = find_vma(current->mm, hva); hva_t vm_start, vm_end; ... if (vma->vm_flags & VM_PFNMAP) { ... phys_addr_t pa = (vma->vm_pgoff << PAGE_SHIFT) + vm_start - vma->vm_start; ... } ... } while (hva < reg_end); ... }
PVS-Studio a émis un avertissement à cette section de code:
V629 Envisagez d'inspecter l'expression 'vma-> vm_pgoff << 12'. Décalage binaire de la valeur 32 bits avec une extension ultérieure au type 64 bits. mmu.c 1795
J'ai regardé les déclarations de variables utilisées dans l'expression "
phys_addr_t pa = (vma-> vm_pgoff << PAGE_SHIFT) + vm_start - vma-> vm_start; ", et
j'ai constaté que le code ci-dessus est similaire à l'exemple synthétique suivant:
void foo(unsigned long a, unsigned long b) { unsigned long long x = (a << 12) + b; }
Si la valeur de la variable 32 bits
a est supérieure à
0xFFFFF , les 12 bits les plus significatifs auront au moins une valeur non nulle. Après avoir appliqué l'opération de décalage à gauche à cette variable, ces bits significatifs seront perdus, à la suite de quoi des informations incorrectes seront écrites dans
x .
Pour éliminer la perte de bits élevés, vous devez d'abord convertir
un en type
unsigned long long , et seulement après cela effectuer l'opération de décalage:
pa = (phys_addr_t)vma->vm_pgoff << PAGE_SHIFT; pa += vm_start - vma->vm_start;
Alors
pa écrira toujours la valeur correcte.
Tout irait bien, mais ce bogue, comme le premier exemple de l'article, s'est également avéré critique, comme l'auteur du commit l'a écrit séparément dans son commentaire. De plus, cette erreur vient de pénétrer dans un grand nombre de projets. Pour apprécier pleinement l'ampleur de la tragédie, je
suggère de regarder le nombre de résultats lors de la recherche de ce correctif sur GitHub. Effrayant, non?
J'ai donc adopté une nouvelle approche pour démontrer l'utilité d'utiliser régulièrement un analyseur de code statique. J'espère que ça vous a plu.
Téléchargez et essayez l'analyseur de code statique PVS-Studio pour tester vos propres projets. Au moment de la rédaction de cet article, il avait implémenté environ 700 règles de diagnostic pour détecter une variété de modèles d'erreur. C, C ++, C # et Java sont pris en charge.
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: George Gribkov.
Erreurs que l'analyse de code statique ne trouve pas car il n'est pas utilisé