Perl 5: как в макросах ошибки прятались

в 6:40, , рубрики: C, c++, open source, perl, Perl5, pvs-studio, static code analysis, Блог компании PVS-Studio, Компиляторы, статический анализ кода
Perl 5: как в макросах ошибки прятались - 1

Для пополнения списка языков программирования с открытым исходным кодом, которые были проверены с помощью статического анализатора кода PVS-Studio, был выбран Perl 5. Эта статья о найденных ошибках и трудностях просмотра результатов анализа. Количество макросов в коде столь велико, что создаётся ощущение, что код написан не на языке Си, а на каком-то его странном диалекте. Несмотря на затруднения при просмотре кода, удалось насобирать интересные проблемы, о которых и будет рассказано в этой статье.

Введение

Perl — высокоуровневый интерпретируемый динамический язык программирования общего назначения (Perl is a family of two high-level, general-purpose, interpreted, dynamic programming languages). Разработка Perl 5 была начата в 1994 году. Спустя пару десятилетий, код на языке Си с многочисленными макросами вызывает нервозность у современных программистов.

Исходный код Perl 5 был взят из официального репозитория (ветка blead). Для проверки проекта использовался статический анализатор кода PVS-Studio. Анализ проводился на операционной системе Linux, но анализатор также доступен для Windows и macOS.

Просмотр результатов анализа был не простой задачей. Дело в том, что анализатор проверяет препроцессированные .i файлы, в которых уже раскрыты все директивы препроцессора, а выдаёт предупреждения на файлы с исходным кодом. Это правильное поведение анализатора, ничего менять не нужно, но много предупреждений выдаётся на макросы! А за макросами скрывается нечитабельный код.

Тернарный оператор работает не так, как вы думаете

V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '-' operator. toke.c 9494

STATIC char *
S_scan_ident(pTHX_ char *s, char *dest, STRLEN destlen, I32 ck_uni)
{
  ....
  if ((s <= PL_bufend - (is_utf8)
                          ? UTF8SKIP(s)
                          : 1)
        && VALID_LEN_ONE_IDENT(s, PL_bufend, is_utf8))
  {
    ....
  }
  ....
}

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

Рассмотрим фрагмент с ошибкой:

s <= PL_bufend - (is_utf8) ? UTF8SKIP(s) : 1

Порядок операций, который ожидает программист:

  1. ?:
  2. -
  3. <=

Что происходит на самом деле:

  1. -
  2. <=
  3. ?:

Держите табличку с приоритетами операций: "Приоритет операций в языке Си/Си++".

V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '==' operator. re_exec.c 9193

STATIC I32
S_regrepeat(pTHX_ regexp *prog, char **startposp, const regnode *p,
            regmatch_info *const reginfo, I32 max _pDEPTH)
{
  ....
  assert(STR_LEN(p) == reginfo->is_utf8_pat ? UTF8SKIP(STRING(p)) : 1);
  ....
}

Простой код с похожей ошибкой. Но если не знать приоритеты операций, то допустить ошибку можно в выражении любого размера.

Ещё одно место с assert'ом:

  • V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '==' operator. re_exec.c 9286

V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '&&' operator. pp_hot.c 3036 

PP(pp_match)
{
  ....
  MgBYTEPOS_set(mg, TARG, truebase, RXp_OFFS(prog)[0].end);
  ....
}

А вот и предупреждение на макрос… Чтобы понять, что там происходит, не поможет даже реализация макроса, потому что в нём используется ещё несколько макросов!

Поэтому прикладываю фрагмент препроцессированного файла для этой строки кода:

(((targ)->sv_flags & 0x00000400) && (!((targ)->sv_flags & 0x00200000) ||
S_sv_only_taint_gmagic(targ)) ? (mg)->mg_len = ((prog->offs)[0].end),
(mg)->mg_flags |= 0x40 : ((mg)->mg_len = (((targ)->sv_flags & 0x20000000)
&& !__builtin_expect(((((PL_curcop)->cop_hints + 0) & 0x00000008) ?
(_Bool)1 :(_Bool)0),(0))) ? (ssize_t)Perl_utf8_length( (U8 *)(truebase),
(U8 *)(truebase)+((prog->offs)[0].end)) : (ssize_t)((prog->offs)[0].end),
(mg)->mg_flags &= ~0x40));

Где-то здесь анализатор усомнился в правильном использовании тернарного оператора (их 3), но я не нашёл в себе сил понять, что делается в этом коде. Мы уже убедились, что разработчики допускают такие ошибки, поэтому тут она тоже может быть с большой вероятностью.

Ещё три использования этого макроса:

  • V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '&&' operator. pp_ctl.c 324
  • V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '&&' operator. regexec.c 7335
  • V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '&&' operator. re_exec.c 7335

Примечание коллеги Андрея Карпова. Я 10 минут медитировал над этим кодом и склоняюсь к мнению, что ошибки здесь нет. Но в любом случае, крайне мучительно читать такой код, и лучше так не писать.

Ошибки в условиях

V523 The 'then' statement is equivalent to the 'else' statement. toke.c 12056

static U8 *
S_add_utf16_textfilter(pTHX_ U8 *const s, bool reversed)
{
  ....
  SvCUR_set(PL_linestr, 0);
  if (FILTER_READ(0, PL_linestr, 0)) {
    SvUTF8_on(PL_linestr);
  } else {
    SvUTF8_on(PL_linestr);
  }
  PL_bufend = SvEND(PL_linestr);
  return (U8*)SvPVX(PL_linestr);
}

Я думаю, можно обойтись без изучения содержимого макросов, чтобы убедиться в наличии подозрительно продублированных фрагментов кода.

V564 The '|' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '||' operator. op.c 11494

OP *
Perl_ck_rvconst(pTHX_ OP *o)
{
  ....
  gv = gv_fetchsv(kidsv,
    o->op_type == OP_RV2CV
      && o->op_private & OPpMAY_RETURN_CONSTANT
        ? GV_NOEXPAND
        : iscv | !(kid->op_private & OPpCONST_ENTERED), iscv // <=
        ? SVt_PVCV
        : o->op_type == OP_RV2SV
      ? SVt_PV
      : o->op_type == OP_RV2AV
          ? SVt_PVAV
          : o->op_type == OP_RV2HV
        ? SVt_PVHV
        : SVt_PVGV);
  ....
}

Очень странный код. Выражение «iscv | !(kid->op_private & OPpCONST_ENTERED)» никак не используется. Здесь явно есть какая-то опечатка. Например, возможно, здесь следовало написать:

: iscv = !(kid->op_private & OPpCONST_ENTERED), iscv // <=

V547 Expression 'RETVAL == 0' is always true. Typemap.c 710

XS_EUPXS(XS_XS__Typemap_T_SYSRET_pass);
XS_EUPXS(XS_XS__Typemap_T_SYSRET_pass)
{
  dVAR; dXSARGS;
  if (items != 0)
    croak_xs_usage(cv,  "");
  {
    SysRet  RETVAL;
#line 370 "Typemap.xs"
    RETVAL = 0;
#line 706 "Typemap.c"
    {
      SV * RETVALSV;
      RETVALSV = sv_newmortal();
      if (RETVAL != -1) {          // <=
        if (RETVAL == 0)           // <=
          sv_setpvn(RETVALSV, "0 but true", 10);
        else
          sv_setiv(RETVALSV, (IV)RETVAL);
      }
      ST(0) = RETVALSV;
    }
  }
  XSRETURN(1);
}

Переменная RETVAL проверяется два раза подряд. При этом, из кода видно, что эта переменная всегда равна нулю. Возможно, в одном или обоих условиях хотели проверить указатель RETVALSV, но допустили опечатку.

Вброс предупреждений про оператор sizeof

В анализаторе есть несколько типов диагностических правил, которые ищут ошибки с использованием оператора sizeof. На проекте Perl 5 две таких диагностики суммарно выдали около тысячи предупреждений. В этом случае виноват не анализатор, а макросы.

V568 It's odd that the argument of sizeof() operator is the 'len + 1' expression. util.c 1084

char *
Perl_savepvn(pTHX_ const char *pv, I32 len)
{
  ....
  Newx(newaddr,len+1,char);
  ....
}

В коде много подобных макросов. Я выбрал один для примера, нам интересен аргумент «len + 1».

Препроцессором макрос раскрывается в следующий код:

(newaddr = ((void)(__builtin_expect(((((( sizeof(size_t) < sizeof(len+1) ||
sizeof(char) > ((size_t)1 << 8*(sizeof(size_t) - sizeof(len+1)))) ?
(size_t)(len+1) : ((size_t)-1)/sizeof(char)) > ((size_t)-1)/sizeof(char))) ?
(_Bool)1 : (_Bool)0),(0)) && (S_croak_memory_wrap(),0)),
(char*)(Perl_safesysmalloc((size_t)((len+1)*sizeof(char))))));

Предупреждение анализатора выдаётся на конструкцию sizeof(len +1). Дело в том, что никакие вычисления в аргументах оператора sizeof не выполняются. В подобный код раскрываются многие макросы. Наверное, это старый legacy-код, в котором уже никто не хочет что-то трогать, но текущие разработчики продолжают использовать старые макросы, предполагая об ином их поведении.

Разыменование нулевых указателей

V522 Dereferencing of the null pointer 'sv' might take place. pp_ctl.c 577

OP * Perl_pp_formline(void)
{
  ....
  SV *sv = ((void *)0);
  ....
  switch (*fpc++) {
  ....
  case 4:
    arg = *fpc++;
    f += arg;
    fieldsize = arg;
    if (mark < sp)
      sv = *++mark;
    else {
      sv = &(PL_sv_immortals[2]);
      Perl_ck_warner( (28 ), "....");
    }
    ....
    break;
  case 5:
  {
    const char *s = item = ((((sv)->sv_flags & (....)) == 0x00000400) ? ....
    ....
  }
  ....
}

Этот фрагмент кода полностью взят из препроцессированного файла, потому что по исходному файлу убедиться в наличии проблемы невозможно, опять же из-за макросов.

Указатель sv при объявлении инициализируется нулём. Анализатор обнаружил, что при переходе по значению 5 в операторе switch происходит разыменование этого указателя, который так и не был перед этим инициализирован. Изменение указателя sv присутствует в ветви со значением 4, но в конце этого блока кода находится оператор break. Скорее всего, в этом месте требуется написание дополнительного кода.

V595 The 'k' pointer was utilized before it was verified against nullptr. Check lines: 15919, 15920. op.c 15919

void
Perl_rpeep(pTHX_ OP *o)
{
  ....
  OP *k = o->op_next;
  U8 want = (k->op_flags & OPf_WANT);   // <=
  if (   k                              // <=
      && k->op_type == OP_KEYS
      && (   want == OPf_WANT_VOID
          || want == OPf_WANT_SCALAR)
      && !(k->op_private & OPpMAYBE_LVSUB)
      && !(k->op_flags & OPf_MOD)
  ) {
  ....
}

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

Диагностика V595 находит очень много предупреждений в любом проекте, Perl 5 не исключение. В статью всё это уместить невозможно, поэтому ограничимся одним примером, а разработчики при желании проверят проект самостоятельно.

Разное

V779 Unreachable code detected. It is possible that an error is present. universal.c 457

XS(XS_utf8_valid);
XS(XS_utf8_valid)
{
  dXSARGS;
  if (items != 1)
    croak_xs_usage(cv, "sv");
  else {
    SV * const sv = ST(0);
    STRLEN len;
    const char * const s = SvPV_const(sv,len);
    if (!SvUTF8(sv) || is_utf8_string((const U8*)s,len))
      XSRETURN_YES;
    else
      XSRETURN_NO;
  }
  XSRETURN_EMPTY;
}

В строке с XSRETURN_EMPTY анализатор обнаружил недостижимый код. В этой функции есть два оператора return и croak_xs_usage – макрос, который раскрывается в noreturn-функцию:

void Perl_croak_xs_usage(const CV *const cv, const char *const params)
  __attribute__((noreturn));

В подобных местах кода Perl 5 для указания недостижимой ветви используется макрос NOT_REACHED.

V784 The size of the bit mask is less than the size of the first operand. This will cause the loss of higher bits. inffast.c 296

void ZLIB_INTERNAL inflate_fast(z_streamp strm, unsigned start)
{
  ....
  unsigned long hold;         /* local strm->hold */
  unsigned bits;              /* local strm->bits */
  ....
  hold &= (1U << bits) - 1;
  ....
}

Анализатор обнаружил подозрительную операцию при работе с битовыми масками. В качестве битовой маски используется переменная меньшей разрядности, чем переменная hold. Это приводит к потере старших бит. Разработчикам следует обратить внимание на этот код.

Заключение

Picture 6

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

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

Perl 5: как в макросах ошибки прятались - 3

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Svyatoslav Razmyslov. Perl 5: How to Hide Errors in Macros

Автор: SvyatoslavMC

Источник

* - обязательные к заполнению поля

Главная   |  Архив новостей  |   Android  |   Google  |   Apple  |   Microsoft  |   Информационная безопасность  |   Веб – разработка
Публикации RSS  |  Комментарии RSS
https://ajax.googleapis.com/ajax/libs/jquery/3.4.1/jquery.min.js