Проверка компилятора GCC 10 с помощью PVS-Studio

Моя цель - предложение широкого ассортимента товаров и услуг на постоянно высоком качестве обслуживания по самым выгодным ценам.

Прежде чем перейти к статье, хочу вам представить, экономическую онлайн игру Brave Knights, в которой вы можете играть и зарабатывать. Регистируйтесь, играйте и зарабатывайте!

PVS-Studio vs GCC 10

Компилятор GCC написан с обильным использованием макросов. Очередная проверка кода GCC с помощью PVS-Studio вновь подтверждает мнение нашей команды, что макросы – это плохо. В таком коде тяжело разбираться не только статическому анализатору, но и программисту. Конечно, разработчики GCC уже привыкли к проекту и хорошо разбираются в нём. Но со стороны очень сложно что-то понять. Собственно, из-за макросов и не удалось полноценно выполнить проверку кода. Тем не менее, анализатор PVS-Studio, как всегда, показал, что может находить ошибки даже в компиляторах.

Время перепроверить код компилятора GCC


Предыдущий раз я проверял компилятор GCC четыре года назад. Время летит быстро и незаметно, и я как-то всё забывал вернуться к этому проекту и перепроверить его. Подтолкнуло вернуться к этой идее публикация "Static analysis in GCC 10".

Собственно, не секрет, что в компиляторах существуют свои собственные встроенные статические анализаторы кода и они тоже развиваются. Поэтому время от времени мы пишем статьи о том, что статический анализатор PVS-Studio может находить ошибки даже внутри компиляторов и что мы не зря едим хлеб :).

На самом деле, сравнивать классические статические анализаторы с компиляторами нельзя. Статические анализаторы – это не только поиск ошибок в коде, но и развитая инфраструктура. Например, это интеграция с такими системами, как SonarQube, PlatformIO, Azure DevOps, Travis CI, CircleCI, GitLab CI/CD, Jenkins, Visual Studio. Это развитые механизмы массового подавления предупреждений, что позволяет быстро начать использовать PVS-Studio даже в большом старом проекте. Это рассылка уведомлений. И так далее, и так далее. Однако, всё равно первый вопрос, который задают: «Может ли PVS-Studio найти такое, что не могут найти компиляторы?». А значит, мы будем вновь и вновь писать статьи о проверке этих самих компиляторов.

Вернёмся к теме проверки проекта GCC. Нет нужды останавливаться на этом проекте и рассказывать, что он из себя представляет. Давайте лучше поговорим, что внутри этого проекта.

А внутри огромное количество макросов, которые мешают проверке. Во-первых, анализатор PVS-Studio выдаёт большое количество ложных срабатываний. В этом нет ничего страшного, но вот так просто взять и начать изучать выданный им отчёт не получается. По-хорошему, нужно проделать работу по подавлению ложных предупреждений в макросах. В противном случае, полезные предупреждения просто тонут в потоке шума. Подобная настройка выходит за границы задачи написания статьи. Собственно, буду совсем честен — мне было просто лень этим заниматься, хотя ничего сложного в этом нет. Из-за шума просмотр отчёта носил достаточно поверхностный характер.

Во-вторых, мне, как человеку, не знакомому с проектом, очень сложно понимать код. Макросы, макросы… Надо смотреть, во что они разворачиваются, чтобы понять, почему анализатор выдаёт предупреждения. Очень тяжело. Не люблю макросы. Кто-то может сказать, что без макросов в C не обойтись. Но GCC давно написан вовсе не на C. Да, файлы по историческим причинам имеют расширение .c, но заглядываешь туда, а там:

// Файл 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;
};
....

Это явно не C, а C++.

В общем, макросы и стиль написания кода очень усложняют изучение отчёта анализатора. Так что в этот раз я не порадую длинным списком разнообразнейших ошибок. Я с трудом и используя несколько чашек кофе, выписал 10 интересных фрагментов, и на этом силы оставили меня :).

10 подозрительных фрагментов кода


Фрагмент N1, Кажется, неудачный Copy-Paste

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 ();
  ....
}

Предупреждение PVS-Studio: V778 Two similar code fragments were found. Perhaps, this is a typo and 's2' variable should be used instead of 's'. cfgcleanup.c 2126

На самом деле, я не уверен, что это именно ошибка. Однако, у меня есть сильное подозрение, что этот код написан с помощью Copy-Paste, и во втором блоке в одном месте забыли заменить s на s2. То есть, мне кажется второй блок кода должен быть таким:

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

Фрагмент N2, Опечатка

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

Предупреждение PVS-Studio: V519 The 'vr1.set' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 3448, 3449. tree-ssa-sccvn.c 3449

Очень странно, что в одну и туже переменную дважды подряд записываются разные значения. Это явная опечатка. Рядом в этом файле есть вот такой код:

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

Скорее всего, и в подозрительном коде должно было быть написано точно так же.

Фрагмент N3, Присваивание переменной самой себе

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;
    }
  ....
}

Предупреждение PVS-Studio: V570 The 'ctx->outer_reduction_clauses' variable is assigned to itself. omp-low.c 935

Очень странно присваивать переменную самой себе.

Фрагмент N4. 0,1,2, Фредди заберёт тебя.

Недавно я опубликовал статью "Ноль, один, два, Фредди заберёт тебя". Мне кажется, следующий фрагмент кода продолжает коллекцию ошибок, рассмотренных в этой статье.

#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);
  ....
}

Предупреждение PVS-Studio: V560 A part of conditional expression is always false:
((machine_mode)(xop1)->mode) == xmode1. optabs.c 1053

Обратите внимание на эти два подвыражения:

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

Над результатами этих подвыражений выполняется операция AND, что явно не имеет практического смысла. Собственно, если начало выполняться второе подвыражение, то заранее известно, что она даст в результате false.

Скорее всего, здесь опечатка в нулях и единицах, и на самом деле условие должно был быть таким:

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

Конечно, я не уверен, что правильно изменил код, так как не ориентируюсь в проекте.

Фрагмент N5. Подозрительное изменение значения аргумента

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: V763 Parameter 'off' is always rewritten in function body before being used. ipa-polymorphic-call.c 766

Значение аргумента off сразу заменяется на 0. Более того, нет поясняющего комментария. Всё это очень подозрительно. Иногда такой код появляется в процессе отладки. Программисту было нужно посмотреть, как поведёт себя функция в определённом режиме, поэтому он временно изменил значение аргумента, а удалить эту строчку потом забыли. В результате в коде появляется ошибка. Конечно, возможно здесь всё правильно, но этот код явно нуждается в проверке и поясняющем комментарии, чтобы в будущем подобные вопросы не возникали.

Фрагмент N6. Мелочь

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;
  ....
}

Предупреждение PVS-Studio: V519 The 'new_node->merged_comdat' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 406, 409. cgraphclones.c 409

Случайно продублировано присваивание. Скорее всего ничего страшного. Однако, всегда есть риск, что в действительности здесь забыли выполнить какое-то другое присваивание.

Фрагмент N7. Код, который выглядит опасно

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: V595 The 'm->component' pointer was utilized before it was verified against nullptr. Check lines: 407, 415. genmodes.c 407

В начале указатель m->component разыменовывается в одной из веток оператора if. Я имею в виду это выражение: m->component->bytesize.

Далее оказывается, что этот указатель может быть нулевым. Это следует из проверки: if (m->component).

Этот код необязательно ошибочен. Вполне возможно, ветка с разыменованием выполняется только в том случае, если указатель не нулевой. То есть существует косвенная взаимосвязь между значением переменной m->cl и m->component. Но выглядит такой код в любом случае очень опасным. И нет никаких поясняющих комментариев.

Фрагмент N8. Двойная проверка

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);
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions 'wi_zero_p(type, lh_lb, lh_ub)' to the left and to the right of the '||' operator. range-op.cc 2657

Какая-то странная проверка. Функция wi_zero_p вызывается дважды с одним и тем же набором фактических аргументов. Можно заподозрить, что на самом деле при втором вызове должны быть использованы принятые извне аргументы: rh_lb, rh_ub. Но нет, эти аргументы помечены как неиспользуемые (ATTRIBUTE_UNUSED).

Поэтому мне непонятно, почему бы не написать проверку проще:

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

Или здесь какая-то опечатка? Или логическая ошибка? Не знаю, но код очень странный.

Фрагмент N9. Опасный доступ к массиву

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: V781 The value of the 'best_alg->ops' variable is checked after it was used. Perhaps there is a mistake in program logic. Check lines: 3157, 3164. expmed.c 3157

Давайте сократим код, чтобы стало понятнее, что не нравится анализатору:

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

В начале переменная best_alg->ops используется для индексирования массива. И только потом происходит проверка этой переменной на граничное значение. Теоретически может произойти выход за границу массива (классическая разновидность ошибки CWE-193: Off-by-one Error).

Действительно ли это настоящая ошибка? И как это постоянно происходит в этой статье, я не уверен :). Возможно, есть взаимосвязь между значением этого индекса и переменной cache_hit. Возможно, ничего не кэшируется, если индекс равен максимуму (MAX_BITS_PER_WORD). Код функции большой, и я не разобрался.

В любом случае, этот код лучше всего проверить. И даже если он окажется правильным, я бы рекомендовал сопроводить рассмотренный участок программы комментарием. Он может сбить с толку не только меня или PVS-Studio, но и ещё кого-то.

Фрагмент N10. Код, который за 4 года не исправили

Ещё в прошлой статье я обратил внимание вот на этот код:

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));
  ....
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions '!strcmp(a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1)' to the left and to the right of the '&&' operator. dwarf2out.c 1481

Две функции strcmp сравнивают одни и те же указатели. То есть выполняется явно избыточная проверка. В предыдущей статье я предположил, что это опечатка, и на самом деле должно быть написано:

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));

Однако за 4 года этот код так и не был исправлен. При этом мы сообщали авторам о подозрительных участках кода, которые мы описали в статье. Теперь я уже не так уверен, что это ошибка. Возможно, это просто избыточный код. В этом случае, выражение можно упростить:

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

Посмотрим, изменят ли разработчики GCC этот фрагмент кода после новой статьи.

Заключение


Напоминаю, что для проверки открытых проектов вы можете использовать вот этот бесплатный вариант лицензии. Кстати, есть и другие варианты бесплатного лицензирования PVS-Studio, в том числе даже для закрытых проектов. Они перечислены здесь: "Бесплатные варианты лицензирования PVS-Studio".

Спасибо за внимание. И приходите читать наш блог. Там много интересного.

Другие наши статьи про проверку компиляторов


  1. Проверка LLVM (Clang) (август 2011), вторая проверка (август 2012), третья проверка (октябрь 2016), четвёртая проверка (апрель 2019)
  2. Проверка GCC (август 2016)
  3. Проверка Huawei Ark Compiler (декабрь 2019)
  4. Проверка .NET Compiler Platform («Roslyn») (декабрь 2015), вторая проверка (апрель 2019)
  5. Проверка Roslyn Analyzers (август 2019)
  6. Проверка PascalABC.NET (март 2017)



Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Checking the GCC 10 Compiler with PVS-Studio.
Источник: https://habr.com/ru/company/pvs-studio/blog/497642/


Интересные статьи

Интересные статьи

Для хранения тестовых данных обычно требуется такой тип данных, который:• допускает объявление нескольких свойств;• имеет минимальное поведение или вообще его не имеет;• позволяет легко с...
FT4232H – это интегральная микросхема преобразователя USB – UART, поддерживающая режим High Speed на стороне USB и имеющая четыре порта на стороне UART. Подключив к этой микросхеме EE...
Подслушивание личных разговоров — одна из самых распространённых и неприятных угроз приватности. Не зря многие заклеивают микрофоны и камеры на ноутбуках, чтобы сохранить в секрете са...
Привет, Хабр. Недавно мне выпала возможность потестировать платежный терминал с возможностью бесконтактной оплаты. Ну а раз оплата бесконтактная, значит сигнал передается по радио,...
В анализаторе PVS-Studio для языков С и C++ на Linux и macOS, начиная с версии 7.04, появилась тестовая возможность проверить список указанных файлов. С помощью нового режима можно настроить ан...