Для пополнения списка языков программирования с открытым исходным кодом, которые были проверены с помощью статического анализатора кода 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Порядок операций, который ожидает программист:
- ?:
- —
- <=
Что происходит на самом деле:
- —
- <=
- ?:
Держите табличку с приоритетами операций: «Приоритет операций в языке Си/Си++«.
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. Это приводит к потере старших бит. Разработчикам следует обратить внимание на этот код.
Заключение
Находить ошибки сквозь макросы было очень сложно. Просмотр отчёта отнял много сил и времени. Тем не менее, в статью вошли очень интересны случаи, похожие на настоящие ошибки. Отчёт анализатора достаточно большой, там точно есть ещё много интересного. Но я не в силах его далее просматривать :). Советую разработчиком проверить проект самостоятельно и устранить дефекты, которые им удастся обнаружить.
P.S. Нам однозначно хочется поддержать этот интересный проект, и мы готовы предоставить разработчикам лицензию на несколько месяцев.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Svyatoslav Razmyslov. Perl 5: How to Hide Errors in Macros
ссылка на оригинал статьи https://habr.com/post/425415/
Добавить комментарий