Test du compilateur GCC 10 avec PVS-Studio

PVS-Studio vs GCC 10

Le compilateur GCC est écrit avec une utilisation abondante des macros. Une autre vérification du code GCC utilisant PVS-Studio confirme une fois de plus l'opinion de notre équipe que les macros sont mauvaises. Un tel code est difficile à comprendre non seulement pour un analyseur statique, mais aussi pour un programmeur. Bien sûr, les développeurs de GCC sont déjà habitués au projet et le connaissent bien. Mais de côté, il est très difficile de comprendre quelque chose. En fait, en raison des macros, il n'a pas été possible d'effectuer une vérification complète du code. Néanmoins, l'analyseur PVS-Studio, comme toujours, a montré qu'il peut trouver des erreurs même dans les compilateurs.

Il est temps de revérifier le code du compilateur GCC


La dernière fois que j'ai vérifié le compilateur GCC il y a quatre ans. Le temps passe vite et imperceptiblement, et d'une manière ou d'une autre, j'ai oublié de revenir sur ce projet et de le revérifier. La publication " Analyse statique dans GCC 10 " a repoussé cette idée .

En fait, ce n'est un secret pour personne que les compilateurs ont leurs propres analyseurs de code statique intégrés et qu'ils sont également en train de se développer. Par conséquent, de temps en temps, nous écrivons des articles que l'analyseur statique PVS-Studio peut trouver des erreurs même à l'intérieur des compilateurs et que nous ne mangeons pas en vain du pain :).

En fait, vous ne pouvez pas comparer des analyseurs statiques classiques avec des compilateurs. Les analyseurs statiques ne sont pas seulement une recherche d'erreurs dans le code, mais aussi une infrastructure développée. Par exemple, il s'agit d'une intégration avec des systèmes tels que SonarQube, PlatformIO, Azure DevOps, Travis CI, CircleCI, GitLab CI / CD, Jenkins, Visual Studio. Ce sont des mécanismes développés pour la suppression des alertes de masse, qui vous permettent de commencer rapidement à utiliser PVS-Studio même dans un grand projet ancien. Il s'agit d'une liste de diffusion de notifications. Et ainsi de suite. Cependant, de toute façon, la première question posée est: "PVS-Studio peut-il trouver quelque chose que les compilateurs ne peuvent pas trouver?". Ainsi, nous écrirons encore et encore des articles sur la vérification de ces compilateurs eux-mêmes.

Revenons au sujet de la vérification du projet GCC. Il n'est pas nécessaire de s'attarder sur ce projet et de dire de quoi il s'agit. Parlons mieux de ce qui est à l'intérieur de ce projet.

Et à l'intérieur, il existe un grand nombre de macros qui interfèrent avec la vérification. Premièrement, l'analyseur PVS-Studio génère un grand nombre de faux positifs. Il n'y a rien de mal à cela, mais ce n'est pas si facile de prendre et de commencer à étudier le rapport qu'il a publié. Dans le bon sens, vous devez travailler pour supprimer les faux avertissements dans les macros. Sinon, des avertissements utiles se noient dans un flux de bruit. Cette configuration dépasse la portée de cette tâche d'écriture d'article. En fait, je serai complètement honnête - j'étais tout simplement trop paresseux pour le faire, bien qu'il n'y ait rien de compliqué à ce sujet. En raison du bruit, la visualisation du rapport était assez superficielle.

Deuxièmement, il m'est très difficile, en tant que personne qui ne connaît pas le projet, de comprendre le code. Macros, macros ... Il faut regarder ce qu'elles sont déployées pour comprendre pourquoi l'analyseur génère des avertissements. Très dur. Je n'aime pas les macros . Quelqu'un peut dire que sans macros en C ne peut pas faire. Mais GCC n'a pas été écrit en C depuis longtemps. Oui, pour des raisons historiques, les fichiers ont l'extension .c, mais vous regardez là, et là:

//  alias.c
....
struct alias_set_hash : int_hash <int, INT_MIN, INT_MIN + 1> {};
struct GTY(()) alias_set_entry {
  alias_set_type alias_set;
  bool has_zero_child;
  bool is_pointer;
  bool has_pointer;
  hash_map<alias_set_hash, int> *children;
};
....

Ce n'est clairement pas C, mais C ++.

En général, les macros et le style de codage rendent très difficile l'étude du rapport de l'analyseur. Cette fois, je ne vais donc pas plaire à une longue liste d'erreurs diverses. Avec difficulté et en utilisant quelques tasses de café, j'ai écrit 10 fragments intéressants, et cela m'a laissé seul :).

10 extraits de code suspects


Fragment N1, semble avoir échoué copier-coller

static bool
try_crossjump_to_edge (int mode, edge e1, edge e2,
                       enum replace_direction dir)
{
  ....
  if (FORWARDER_BLOCK_P (s->dest))
    s->dest->count += s->count ();

  if (FORWARDER_BLOCK_P (s2->dest))
    s2->dest->count -= s->count ();
  ....
}

Avertissement PVS-Studio: V778 Deux fragments de code similaires ont été trouvés. Il s'agit peut-être d'une faute de frappe et la variable «s2» devrait être utilisée à la place de «s». cfgcleanup.c 2126

En fait, je ne suis pas sûr que ce soit une erreur. Cependant, j'ai une forte suspicion que ce code a été écrit en utilisant Copy-Paste, et dans le deuxième bloc à un endroit, ils ont oublié de remplacer s par s2 . Autrement dit, il me semble que le deuxième bloc de code devrait être comme ceci:

if (FORWARDER_BLOCK_P (s2->dest))
  s2->dest->count -= s2->count ();

Fragment N2, faute de frappe

tree
vn_reference_lookup_pieces (....)
{
  struct vn_reference_s vr1;
  ....
  vr1.set = set;
  vr1.set = base_set;
  ....
}

PVS-Studio Warning: V519 La variable 'vr1.set' se voit attribuer des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 3448, 3449. tree-ssa-sccvn.c 3449

Il est très étrange que différentes valeurs soient écrites deux fois de suite dans la même variable. C'est une faute de frappe évidente. À côté de ce fichier se trouve ce code:

vr1.set = set;
vr1.base_set = base_set;

Très probablement, dans le code suspect, il aurait dû être écrit exactement de la même manière.

Fragment N3, affectation d'une variable à lui-même

static omp_context *
new_omp_context (gimple *stmt, omp_context *outer_ctx)
{
  omp_context *ctx = XCNEW (omp_context);

  splay_tree_insert (all_contexts, (splay_tree_key) stmt,
         (splay_tree_value) ctx);
  ctx->stmt = stmt;

  if (outer_ctx)
    {
      ctx->outer = outer_ctx;
      ctx->cb = outer_ctx->cb;
      ctx->cb.block = NULL;
      ctx->local_reduction_clauses = NULL;
      ctx->outer_reduction_clauses = ctx->outer_reduction_clauses;  // <=
      ctx->depth = outer_ctx->depth + 1;
    }
  ....
}

Avertissement PVS-Studio: V570 La variable 'ctx-> external_reduction_clauses' est assignée à elle-même. omp-low.c 935

Il est très étrange de s'attribuer une variable à elle-même.

Fragment N4. 0,1,2, Freddy viendra vous chercher.

J'ai récemment publié un article, « Zero, One, Two, Freddy Will Take You ». Il me semble que l'extrait de code suivant continue la collecte des erreurs discutées dans cet article.

#define GET_MODE(RTX)    ((machine_mode) (RTX)->mode)
....
static int
add_equal_note (rtx_insn *insns, rtx target, enum rtx_code code, rtx op0,
                rtx op1, machine_mode op0_mode)
{
  ....
  if (commutative_p
      && GET_MODE (xop0) != xmode0 && GET_MODE (xop1) != xmode1
      && GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode1)
    std::swap (xop0, xop1);
  ....
}

Avertissement PVS-Studio: V560 Une partie de l'expression conditionnelle est toujours fausse:
((machine_mode) (xop1) -> mode) == xmode1. optabs.c 1053

Notez ces deux sous-expressions:

  • GET_MODE (xop1)! = Xmode1
  • GET_MODE (xop1) == xmode1

L'opération ET est effectuée sur les résultats de ces sous-expressions, ce qui n'a évidemment aucune signification pratique. En fait, si la deuxième sous-expression a commencé à être exécutée, alors on sait à l'avance que cela se traduira par faux .

Très probablement, il y a une faute de frappe ici en zéros et en uns, et en fait la condition aurait dû être comme ceci:

&& GET_MODE (xop0) != xmode0 && GET_MODE (xop1) != xmode1
&& GET_MODE (xop0) == xmode1 && GET_MODE (xop1) == xmode0

Bien sûr, je ne sais pas si j'ai changé le code correctement, car je ne suis pas guidé dans le projet.

Fragment N5. Changement suspect dans la valeur de l'argument

bool
ipa_polymorphic_call_context::set_by_invariant (tree cst,
                                                tree otr_type,
                                                HOST_WIDE_INT off)
{
  poly_int64 offset2, size, max_size;
  bool reverse;
  tree base;

  invalid = false;
  off = 0;                // <=
  ....
  if (otr_type && !contains_type_p (TREE_TYPE (base), off, otr_type))
    return false;

  set_by_decl (base, off);
  return true;
}

PVS-Studio Avertissement: le paramètre V763 «off» est toujours réécrit dans le corps de la fonction avant d'être utilisé. ipa-polymorphic-call.c 766

La valeur de l'argument off est immédiatement remplacée par 0. De plus, il n'y a pas de commentaire explicatif. Tout cela est très suspect. Parfois, ce code apparaît lors du débogage. Le programmeur avait besoin de voir comment la fonction se comportait dans un certain mode, alors il a temporairement changé la valeur de l'argument, puis ils ont oublié de supprimer cette ligne. Par conséquent, une erreur apparaît dans le code. Bien sûr, tout peut être ici, mais ce code doit clairement être vérifié et clarifié pour garantir que des questions similaires ne se poseront pas à l'avenir.

Fragment N6. Petite chose

cgraph_node *
cgraph_node::create_clone (....)
{
  ....
  new_node->icf_merged = icf_merged;
  new_node->merged_comdat = merged_comdat;   // <=
  new_node->thunk = thunk;
  new_node->unit_id = unit_id;
  new_node->merged_comdat = merged_comdat;   // <=
  new_node->merged_extern_inline = merged_extern_inline;
  ....
}

Avertissement PVS-Studio: V519 La variable 'new_node-> merged_comdat' reçoit des valeurs successives deux fois. C'est peut-être une erreur. Vérifiez les lignes: 406, 409. cgraphclones.c 409 L'affectation est

dupliquée de manière aléatoire. Probablement rien de mal. Cependant, il y a toujours un risque qu'en réalité ils oublient d'effectuer une autre tâche.

Fragment N7. Un code qui a l'air dangereux

static void
complete_mode (struct mode_data *m)
{
  ....
  if (m->cl == MODE_COMPLEX_INT || m->cl == MODE_COMPLEX_FLOAT)
    alignment = m->component->bytesize;
  else
    alignment = m->bytesize;

  m->alignment = alignment & (~alignment + 1);

  if (m->component)
  ....
}

PVS-Studio Warning: V595 Le pointeur 'm-> component' a été utilisé avant d'être vérifié par rapport à nullptr. Vérifiez les lignes: 407, 415. genmodes.c 407

Au début, le pointeur du composant m-> est déréférencé dans l'une des branches de l' instruction if . Je veux dire cette expression: m-> component-> bytesize .

Il s'avère en outre que ce pointeur peut être nul. Cela découle de la vérification: si (m-> composant) .

Ce code n'est pas nécessairement faux. Il est tout à fait possible qu'une branche déréférencée ne soit exécutée que si le pointeur n'est pas nul. Autrement dit, il existe une relation indirecte entre la valeur de la variable m-> cl et m-> composante. Mais ce code semble en tout cas très dangereux. Et il n'y a pas de commentaires explicatifs.

Fragment N8. Revérifier

void
pointer_and_operator::wi_fold (value_range &r, tree type,
                               const wide_int &lh_lb,
                               const wide_int &lh_ub,
                               const wide_int &rh_lb ATTRIBUTE_UNUSED,
                               const wide_int &rh_ub ATTRIBUTE_UNUSED) const
{
  // For pointer types, we are really only interested in asserting
  // whether the expression evaluates to non-NULL.
  if (wi_zero_p (type, lh_lb, lh_ub) || wi_zero_p (type, lh_lb, lh_ub))
    r = range_zero (type);
  else 
    r = value_range (type);
}

Avertissement PVS-Studio: V501 Il existe des sous-expressions identiques 'wi_zero_p (type, lh_lb, lh_ub)' à gauche et à droite de '||' opérateur. range-op.cc 2657

Une sorte de vérification bizarre. La fonction wi_zero_p est appelée deux fois avec le même ensemble d'arguments réels. On peut soupçonner qu'en fait, le deuxième appel devrait utiliser les arguments acceptés de l'extérieur: rh_lb , rh_ub. Mais non, ces arguments sont marqués comme inutilisés ( ATTRIBUTE_UNUSED ).

Par conséquent, il n'est pas clair pour moi pourquoi ne pas rédiger un chèque est plus simple:

if (wi_zero_p (type, lh_lb, lh_ub))
  r = range_zero (type);
else 
  r = value_range (type);

Ou y a-t-il une faute de frappe ici? Ou une erreur logique? Je ne sais pas, mais le code est très étrange.

Fragment N9. Accès dangereux aux baies

struct algorithm
{
  struct mult_cost cost;
  short ops;
  enum alg_code op[MAX_BITS_PER_WORD];
  char log[MAX_BITS_PER_WORD];
};

static void
synth_mult (struct algorithm *alg_out, unsigned HOST_WIDE_INT t,
            const struct mult_cost *cost_limit, machine_mode mode)
{
  int m;
  struct algorithm *alg_in, *best_alg;
  ....
  /* Cache the result.  */
  if (!cache_hit)
  {
    entry_ptr->t = t;
    entry_ptr->mode = mode;
    entry_ptr->speed = speed;
    entry_ptr->alg = best_alg->op[best_alg->ops];
    entry_ptr->cost.cost = best_cost.cost;
    entry_ptr->cost.latency = best_cost.latency;
  }

  /* If we are getting a too long sequence for `struct algorithm'
     to record, make this search fail.  */
  if (best_alg->ops == MAX_BITS_PER_WORD)
    return;
  ....
}

PVS-Studio Warning: V781 La valeur de la variable 'best_alg-> ops' est vérifiée après son utilisation. Il y a peut-être une erreur dans la logique du programme. Vérifiez les lignes: 3157, 3164. expmed.c 3157 Raccourcissons

le code pour indiquer clairement ce que l'analyseur n'aime pas:

if (!cache_hit)
{
  entry_ptr->alg = best_alg->op[best_alg->ops];
}
if (best_alg->ops == MAX_BITS_PER_WORD)

Au début, la variable best_alg-> ops est utilisée pour indexer le tableau. Et alors seulement, cette variable vérifie une valeur limite. Théoriquement, un débordement d'un tableau peut se produire (un type classique d'erreur CWE-193: erreur Off-by-one ).

Est-ce une vraie erreur? Et comme cela se produit constamment dans cet article, je ne suis pas sûr :). Il existe peut-être une relation entre la valeur de cet index et la variable cache_hit . Peut-être que rien n'est mis en cache si l'index est au maximum ( MAX_BITS_PER_WORD ). Le code de fonction est grand et je ne l'ai pas compris.

Dans tous les cas, ce code est mieux vérifié. Et même si cela s'avère correct, je recommanderais d'accompagner la section considérée du programme d'un commentaire. Cela peut confondre non seulement moi ou PVS-Studio, mais aussi quelqu'un d'autre.

Fragment N10. Code qui n'a pas été corrigé

depuis 4 ans. Dans le dernier article, j'ai attiré l'attention sur ce code:

static bool
dw_val_equal_p (dw_val_node *a, dw_val_node *b)
{
  ....
  case dw_val_class_vms_delta:
    return (!strcmp (a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1)
            && !strcmp (a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1));
  ....
}

Avertissement PVS-Studio: V501 Il existe des sous-expressions identiques '! Strcmp (a-> v.val_vms_delta.lbl1, b-> v.val_vms_delta.lbl1)' à gauche et à droite de l'opérateur '&&'. dwarf2out.c 1481

Deux fonctions strcmp comparent les mêmes pointeurs. Autrement dit, une vérification clairement redondante est effectuée. Dans un article précédent, j'ai suggéré qu'il s'agissait d'une faute de frappe, et devrait en fait être écrit:

return (   !strcmp (a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1)
        && !strcmp (a->v.val_vms_delta.lbl2, b->v.val_vms_delta.lbl2));

Cependant, depuis 4 ans ce code n'a pas été corrigé. Dans le même temps, nous avons informé les auteurs des sections de code suspectes que nous avons décrites dans l'article. Maintenant, je ne suis pas sûr que ce soit une erreur. Il s'agit peut-être simplement d'un code redondant. Dans ce cas, l'expression peut être simplifiée:

return (!strcmp (a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1));

Voyons si les développeurs de GCC changeront ce morceau de code après un nouvel article.

Conclusion


Je vous rappelle que vous pouvez utiliser cette option de licence gratuite pour vérifier les projets ouverts . Soit dit en passant, il existe d'autres options pour la licence gratuite de PVS-Studio, y compris même pour les projets fermés. Ils sont répertoriés ici: " Options de licence gratuite de PVS-Studio ".

Merci pour l'attention. Et venez lire notre blog . Il y a beaucoup de choses intéressantes.

Nos autres articles sur la vérification des compilateurs


  1. Chèque LLVM (Clang) (août 2011), deuxième chèque (août 2012), troisième chèque (octobre 2016), quatrième chèque (avril 2019)
  2. Examen du CCG (août 2016)
  3. Vérifiez Huawei Ark Compiler (décembre 2019)
  4. Vérification de la plate-forme du compilateur .NET («Roslyn») (décembre 2015), deuxième inspection (avril 2019)
  5. Revue des analyseurs de Roslyn (août 2019)
  6. Vérification de PascalABC.NET (mars 2017)



Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Andrey Karpov. Vérification du compilateur GCC 10 avec PVS-Studio .

All Articles