Continuation: insultant pour les opinions sur les analyseurs de code statique

image1.png


Il était prévu qu'après avoir écrit un article «C'est dommage pour les opinions sur les analyseurs de code statique», nous parlerions et lâcherions calmement le sujet. Mais de manière inattendue, cet article a provoqué une réponse orageuse. Malheureusement, la discussion a mal tourné et nous allons maintenant faire une deuxième tentative pour expliquer notre vision de la situation.



Anecdote-analogie



Donc, tout a commencé avec l'article " C'est dommage pour les avis sur les analyseurs de code statique ". Elle a commencé à être activement discutée sur certaines ressources et cette discussion est très similaire à la vieille anecdote suivante.
Nous avons acheté une tronçonneuse japonaise pour certains bûcherons sibériens durs.

Les bûcherons se sont rassemblés en cercle et ont décidé de le tester.

Ils l'ont amenée, lui ont donné un arbre.

"Zipper", dit la scie japonaise.

"Oh, putain ..." - ont dit les bûcherons.

Ils lui ont fait glisser un arbre plus épais. "Vzh-zh-zhik!" - dit la scie.

"Wow, merde!" - ont dit les bûcherons.

Ils lui ont glissé un épais cèdre. "VZH-ZH-ZH-ZH-ZH-ZH-ZHIK !!!" - dit la scie.

"Wow, putain !!" - ont dit les bûcherons.

Ils lui ont glissé un morceau de fer. "FISSURE!" - dit la scie.

"Ouais, putain !!!" - Les bûcherons sibériens sévères ont dit avec reproche! Et ils sont partis couper la forêt avec des haches ...
Une à une histoire. Les gens ont regardé le code:



if (A[0] == 0)
{
  X = Y;
  if (A[0] == 0)
    ....
}


Et ils ont commencé à inventer des situations où cela pouvait être justifié, ce qui signifie que l'avertissement de l'analyseur PVS-Studio est faux-positif. Le raisonnement est entré dans le cours sur le changement de mémoire entre deux contrôles, résultant de:



  • travail de flux parallèles;
  • gestionnaires de signaux / d'interruptions;
  • la variable X est une référence à l'élément A [0] ;
  • le matériel, comme l'exécution d'opérations DMA;
  • etc.


Et après avoir discuté du fait que l'analyseur ne peut pas comprendre toutes les situations, ils sont partis couper la forêt avec des haches. Autrement dit, nous avons trouvé une excuse pour laquelle il est possible de continuer à ne pas utiliser l'analyseur de code statique dans notre travail.



Notre vision de la situation



Cette approche est contre-productive. Un outil imparfait peut bien être utile, mais son utilisation est économiquement viable.



Oui, tout analyseur statique génère des faux positifs. Et rien ne peut être fait à ce sujet. Cependant, ce malheur est grandement exagéré. En pratique, les analyseurs statiques peuvent être configurés et utilisés de différentes manières pour supprimer et travailler avec les faux positifs (voir 1 , 2 , 3 , 4 ). De plus, il convient ici de rappeler l'article "Les faux positifs sont nos ennemis, mais peuvent encore être vos amis ".



Cependant, même ce n'est pas l'essentiel. Cela n'a aucun sens de considérer des cas particuliers de code exotique!Pouvez-vous confondre l'analyseur avec un code complexe? Oui, vous pouvez. Cependant, pour un tel cas, il y aura des centaines de déclencheurs d'analyseurs utiles. De nombreuses erreurs peuvent être trouvées et corrigées très tôt. Et un ou deux faux positifs peuvent être supprimés en toute sécurité et ne plus y prêter attention.



Et encore une fois PVS-Studio a raison



Ici, l'article pourrait être terminé. Cependant, certains peuvent considérer que la section précédente n'est pas des considérations rationnelles, mais tente de cacher les faiblesses et les lacunes de l'outil PVS-Studio. Il faut donc continuer.



Considérez un code compilé concret qui inclut des déclarations de variables:



void SetSynchronizeVar(int *);

int foo()
{
    int flag = 0;
    SetSynchronizeVar(&flag);

    int X, Y = 1;

    if (flag == 0)
    {
        X = Y;
        if (flag == 0)
            return 1;
    }
    return 2;
}


L'analyseur PVS-Studio émet raisonnablement un avertissement: V547 Expression 'flag == 0' est toujours vrai.



Et l'analyseur a tout à fait raison. Si quelqu'un commence à dire qu'une variable peut changer dans un autre thread, dans un gestionnaire de signaux, etc., alors il ne comprend tout simplement pas les langages C et C ++. Vous ne pouvez pas écrire de cette façon.



À des fins d'optimisation, le compilateur a le droit de rejeter la deuxième vérification et aura tout à fait raison. D'un point de vue linguistique, une variable ne peut pas changer. Son changement d'arrière-plan n'est rien de plus qu'un comportement indéfini.



Pour que le contrôle reste en place, la variable doit être déclarée volatile :



void SetSynchronizeVar(volatile int *);

int foo()
{
    volatile int flag = 0;
    SetSynchronizeVar(&flag);
    ....
}


L'analyseur PVS-Studio le sait et n'émet plus d'avertissement pour un tel code .



Nous revenons ici à ce qui a été discuté dans le premier article . Il n'y a pas de problème. Mais il y a des critiques ou un malentendu sur les raisons pour lesquelles l'analyseur a le droit d'émettre un avertissement.



Note aux lecteurs les plus méticuleux



Certains lecteurs peuvent revenir à l'exemple synthétique du premier article:



char get();
int foo(char *p, bool arg)
{
    if (p[1] == 1)
    {
        if (arg)
            p[0] = get();
        if (p[1] == 1)          // Warning
            return 1;
    }
    // ....
    return 3;
}


Et ajoutez volatile :



char get();
int foo(volatile char *p, bool arg)
{
    if (p[1] == 1)
    {
        if (arg)
            p[0] = get();
        if (p[1] == 1)          // Warning :-(
            return 1;
    }
    // ....
    return 3;
}


Après cela, il est juste de dire que l'analyseur émet toujours l'avertissement V547 Expression 'p [1] == 1' est toujours vrai.



Hourra, il a finalement été montré que l'analyseur est toujours faux :). C'est un faux positif!



Comme vous pouvez le voir, nous ne cachons aucun défaut. Lors de l'analyse du flux de données pour les éléments du tableau, ce volatile malheureux s'est perdu. La faille a déjà été trouvée et corrigée. Le correctif sera disponible dans la prochaine version de l'analyseur. Il n'y aura pas de faux positifs.



Pourquoi ce bogue n'a-t-il pas été identifié plus tôt? Car en fait, c'est encore un code irréel qui ne se trouve pas dans de vrais projets. En fait, nous n’avons pas rencontré un tel code pour le moment, même si nous avons vérifié de nombreux projets open source .



Pourquoi le code est-il irréaliste? Premièrement, dans la pratique, il y aura une sorte de fonction de synchronisation ou de retard entre les deux contrôles. Deuxièmement, aucune personne sensée, à moins que cela ne soit absolument nécessaire, ne crée des tableaux composés d'éléments volatils. Travailler avec un tel tableau est une baisse colossale des performances.



Résumons. Il est facile de créer des exemples où l'analyseur échoue. Mais d'un point de vue pratique, les failles identifiées n'affectent pratiquement pas la qualité de l'analyse du code et le nombre d'erreurs réelles détectées. Après tout, le code des applications réelles n'est qu'un code compréhensible à la fois pour l'analyseur et la personne, et non un rébus ou un puzzle. Si le code est un puzzle, alors il n'y a pas de temps pour les analyseurs :).



Merci de votre attention.





Liens supplémentaires









Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien de traduction: Andrey Karpov. Partie 2: Opinions bouleversantes sur les analyseurs statiques .



All Articles