# Perl 5: как в макросах ошибки прятались Для пополнения списка языков программирования с открытым исходным кодом, которые были проверены с помощью статического анализатора кода PVS\-Studio, был выбран Perl 5\. Эта статья о найденных ошибках и трудностях просмотра результатов анализа\. Количество макросов в коде столь велико, что создаётся ощущение, что код написан не на языке Си, а на каком\-то его странном диалекте\. Несмотря на затруднения при просмотре кода, удалось насобирать интересные проблемы, о которых и будет рассказано в этой статье\. ![0583_Perl5_ru/image1.png](https://import.viva64.com/docx/blog/0583_Perl5_ru/image1.png) ## Введение [Perl](https://www.perl.org/) \- высокоуровневый интерпретируемый динамический язык программирования общего назначения \(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](https://pvs-studio.ru/ru/blog/terms/0076/) файлы, в которых уже раскрыты все директивы препроцессора, а выдаёт предупреждения на файлы с исходным кодом\. Это правильное поведение анализатора, ничего менять не нужно, но много предупреждений выдаётся на макросы\! А за макросами скрывается нечитабельный код\. ## Тернарный оператор работает не так, как вы думаете [V502](https://pvs-studio.ru/ru/docs/warnings/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 ```cpp 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)) { .... } .... } ``` Начнём обзор с красивой ошибки\. Каждые несколько обзоров кода приходится повторять, что тернарный оператор имеет чуть ли не самый низкий приоритет в вычислениях\. Рассмотрим фрагмент с ошибкой: ```cpp s <= PL_bufend - (is_utf8) ? UTF8SKIP(s) : 1 ``` Порядок операций, который ожидает программист: 1. ?: 1. \- 1. <\= Что происходит на самом деле: 1. \- 1. <\= 1. ?: Держите табличку с приоритетами операций: "[Приоритет операций в языке Си/Си\+\+](https://pvs-studio.ru/ru/blog/terms/0064/)"\. [V502](https://pvs-studio.ru/ru/docs/warnings/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 ```cpp 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](https://pvs-studio.ru/ru/docs/warnings/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 ```cpp PP(pp_match) { .... MgBYTEPOS_set(mg, TARG, truebase, RXp_OFFS(prog)[0].end); .... } ``` А вот и предупреждение на макрос\.\.\. Чтобы понять, что там происходит, не поможет даже реализация макроса, потому что в нём используется ещё несколько макросов\! Поэтому прикладываю фрагмент препроцессированного файла для этой строки кода: ```cpp (((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](https://pvs-studio.ru/ru/docs/warnings/v523/) The 'then' statement is equivalent to the 'else' statement\. toke\.c 12056 ```cpp 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](https://pvs-studio.ru/ru/docs/warnings/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 ```cpp 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\)" никак не используется\. Здесь явно есть какая\-то опечатка\. Например, возможно, здесь следовало написать: ```cpp : iscv = !(kid->op_private & OPpCONST_ENTERED), iscv // <= ``` [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'RETVAL \=\= 0' is always true\. Typemap\.c 710 ```cpp 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](https://pvs-studio.ru/ru/docs/warnings/v568/) It's odd that the argument of sizeof\(\) operator is the 'len \+ 1' expression\. util\.c 1084 ```cpp char * Perl_savepvn(pTHX_ const char *pv, I32 len) { .... Newx(newaddr,len+1,char); .... } ``` В коде много подобных макросов\. Я выбрал один для примера, нам интересен аргумент "len \+ 1"\. Препроцессором макрос раскрывается в следующий код: ```cpp (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](https://pvs-studio.ru/ru/docs/warnings/v522/) Dereferencing of the null pointer 'sv' might take place\. pp\_ctl\.c 577 ```cpp 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](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'k' pointer was utilized before it was verified against nullptr\. Check lines: 15919, 15920\. op\.c 15919 ```cpp 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](https://pvs-studio.ru/ru/docs/warnings/v595/) находит очень много предупреждений в любом проекте, Perl 5 не исключение\. В статью всё это уместить невозможно, поэтому ограничимся одним примером, а разработчики при желании проверят проект самостоятельно\. ## Разное V779 Unreachable code detected\. It is possible that an error is present\. universal\.c 457 ```cpp 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\-функцию: ```cpp void Perl_croak_xs_usage(const CV *const cv, const char *const params) __attribute__((noreturn)); ``` В подобных местах кода Perl 5 для указания недостижимой ветви используется макрос _NOT\_REACHED_\. [V784](https://pvs-studio.ru/ru/docs/warnings/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 ```cpp 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_\. Это приводит к потере старших бит\. Разработчикам следует обратить внимание на этот код\. ## Заключение ![0583_Perl5_ru/image2.png](https://import.viva64.com/docx/blog/0583_Perl5_ru/image2.png) Находить ошибки сквозь макросы было очень сложно\. Просмотр отчёта отнял много сил и времени\. Тем не менее, в статью вошли очень интересные случаи, похожие на настоящие ошибки\. Отчёт анализатора достаточно большой, там точно есть ещё много интересного\. Но я не в силах его далее просматривать :\)\. Советую разработчиком проверить проект самостоятельно и устранить дефекты, которые им удастся обнаружить\. P\.S\. Нам однозначно хочется поддержать этот интересный проект, и мы готовы предоставить разработчикам лицензию на несколько месяцев\.