# Наследие кода: разбор С и С\+\+ модулей Erlang, которые работают десятилетиями Код некоторых модулей Erlang/OTP старше, чем большинство современных junior\-разработчиков\. Эти файлы — настоящие цифровые патриархи, десятилетиями обеспечивающие работу банковских транзакций, телефонных сетей и систем обмена сообщениями\. Мы решили заглянуть под "капот" этого языка\-долгожителя, чтобы проверить, что именно скрывается в строках, на которые сегодня полагаются миллионы пользователей\. А вот что мы нашли, узнаем в этой статье\. ![1305_Erlang_ru/image1.png](https://import.viva64.com/docx/blog/1305_Erlang_ru/image1.png) ## Введение Перед тем как перейти к рассматриванию срабатываний, стоит сказать пару слов о нашем проекте\. История языка Erlang началась в далёком 1986 году в одной из лабораторий компании Ericsson\. Джо Армстронг тестировал Prolog, чтобы написать программу для телефонии\. В ходе таких экспериментов и появился Erlang\. [Erlang](https://www.erlang.org/) — это язык программирования, созданный для высоконадёжных систем, в которых падение недопустимо\. Представьте телефонную станцию, где обрыв вызова — это ЧП, или банковскую транзакцию, которая должна пройти в любом случае\. Именно для таких задач он и используется\. Благодаря Erlang система может легко разделяться на миллионы изолированных, независимых процессов, где сбой одного не приводит к падению всей системы\. Erlang неразрывно связан с [OTP \(Open Telecom Platform\)](https://en.wikipedia.org/wiki/Open_Telecom_Platform) — фреймворком, который представляет набор мощных инструментов, паттернов и поведений для построения распределённых, отказоустойчивых приложений\. Весь этот "коробочный набор" позволяет системам, написанным на нём, работать без остановки годами\. Проект был собран по этой [инструкции](https://github.com/erlang/otp?tab=readme-ov-file#installation)\. Проверка производилась на ветке [maint\-28](https://github.com/erlang/otp/tree/maint-28) с помощью статического анализатора [PVS\-Studio](https://pvs-studio.ru/ru/docs/manual/0055/) и плагина для [Visual Studio Code](https://pvs-studio.ru/ru/docs/manual/6646/)\. Со сборкой проекта проблем не возникло, за что отдельная благодарность разработчикам\. В статью вошли предупреждения только уровней High и Medium\. На этом наша вводная часть окончена, давайте перейдём к срабатываниям\. Разделим их на три части: * логические ошибки; * критические ошибки; * ошибки при работе с памятью\. ## Логические ошибки **Фрагмент N1** Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions '\(\!esock\_is\_integer\(\(env\), \(argv\[3\]\)\)\)' to the left and to the right of the '\|\|' operator\. [prim\_socket\_nif\.c 6035](https://github.com/erlang/otp/blob/maint-28/erts/emulator/nifs/common/prim_socket_nif.c#L6035) ```cpp static ERL_NIF_TERM nif_sendfile(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) { .... BOOLEAN_T a2ok; if ((! (a2ok = GET_INT64(env, argv[2], &offset64))) || (! GET_UINT64(env, argv[3], &count64u))) { if ((! IS_INTEGER(env, argv[3])) || (! IS_INTEGER(env, argv[3]))) return enif_make_badarg(env); if (! a2ok) return esock_make_error_integer_range(env, argv[2]); else return esock_make_error_integer_range(env, argv[3]); } .... } ``` В коде разработчик пытался конвертировать 3\-й и 4\-й аргументы в 64\-битные типы \(знаковый и беззнаковый соответственно\)\. Ранний выход из функции происходит в том случае, если один из аргументов не является числом\. Однако здесь проверяется один и тот же аргумент\. Исправленный код: ```cpp .... if ((! IS_INTEGER(env, argv[2])) || (! IS_INTEGER(env, argv[3]))) return enif_make_badarg(env); .... ``` **Фрагмент N2** Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions 'n\-\>type \=\= ycf\_node\_type\_on\_save\_yield\_state\_code' to the left and to the right of the '\|\|' operator\. [ycf\_node\.c 218](https://github.com/erlang/otp/blob/maint-28/erts/lib_src/yielding_c_fun/ycf_node.c#L218) ```cpp static void uniqify_local_vars_in_node(ycf_node* n) { if (n->type == ycf_node_type_code_scope) { uniqify_local_vars_in_scope(&n->u.code_scope); } else if(n->type == ycf_node_type_on_restore_yield_state_code) { uniqify_local_vars_in_scope(&n->u.special_code_block .code.if_statement->u.code_scope); } else if (n->type == ycf_node_type_on_save_yield_state_code || n->type == ycf_node_type_on_save_yield_state_code || n->type == ycf_node_type_on_destroy_state_code || n->type == ycf_node_type_on_return_code || n->type == ycf_node_type_on_destroy_state_or_return_code) { uniqify_local_vars_in_scope(&n->u.special_code_block .code.if_statement->u.code_scope); } .... } ``` Ещё один интересный пример копипасты\. Перед нами функция `uniqify_local_vars_in_node`, которая рекурсивно обрабатывает узлы дерева, находя области видимости с локальными переменными и вызывая для них функцию унификации имён переменных\. Однако в последней продемонстрированной ветке `else if` условие `n->type == ycf_node_type_on_save_yield_state_code` дублируется после оператора `||`\. К сожалению, я не могу предложить корректное исправление для этого фрагмента\. Судя по цепочке сравнений, перебираются значения следующего перечисления в порядке их объявления: ```cpp typedef enum { .... ycf_node_type_on_save_yield_state_code, ycf_node_type_on_restore_yield_state_code, ycf_node_type_on_destroy_state_code, ycf_node_type_on_return_code, ycf_node_type_on_destroy_state_or_return_code } ycf_node_type; ``` Однако константа `ycf_node_type_on_restore_yield_state_code` проверяется в предыдущем `else if`\. Поэтому предполагаю, что здесь всё же есть дублирующая проверка, и её можно убрать\. **Фрагмент N3** Предупреждение PVS\-Studio: [V1040](https://pvs-studio.ru/ru/docs/warnings/v1040/) Possible typo in the spelling of a pre\-defined macro name\. The '\_\_WIN\_32\_\_' macro is similar to '\_\_WIN32\_\_'\. [inet\_drv\.c 13506](https://github.com/erlang/otp/blob/maint-28/erts/emulator/drivers/common/inet_drv.c#L13506) ```cpp static int tcp_send_error(tcp_descriptor* desc, int err) { /* EPIPE errors usually occur in one of three ways: * 1. We write to a socket when we've already shutdown() the write side. * On Windows the error returned for this is ESHUTDOWN * rather than EPIPE. * 2. The TCP peer sends us an RST through no fault of our own (perhaps * by aborting the connection using SO_LINGER) and we then attempt * to write to the socket. On Linux and Windows we would actually * receive an ECONNRESET error for this, but on the BSDs, Darwin, * Illumos and presumably Solaris, it's an EPIPE. * 3. We cause the TCP peer to send us an RST by writing to a socket * after we receive a FIN from them. Our first write will be * successful, but if the they have closed the connection (rather * than just shutting down the write side of it) this will cause their * OS to send us an RST. Then, when we attempt to write to the socket * a second time, we will get an EPIPE error. On Windows we get an * ECONNABORTED. * * What we are going to do here is to treat all EPIPE messages that aren't * of type 1 as ECONNRESET errors. This will allow users who have the * show_econnreset socket option enabled to receive {error, econnreset} on * both send and recv operations to indicate that an RST * has been received. */ #ifdef __WIN_32__ if (err == ECONNABORTED) err = ECONNRESET; #endif if (err == EPIPE && !(desc->tcp_add_flags & TCP_ADDF_SHUTDOWN_WR_DONE)) err = ECONNRESET; return tcp_send_or_shutdown_error(desc, err); } ``` Здесь достаточно нестандартный случай проверки того, компилируется ли проект под Windows\. В Erlang присутствуют и другие проверки на Windows \(\[[1](https://github.com/erlang/otp/blob/maint-28/erts/emulator/beam/erl_driver.h#L46)\], \[[2](https://github.com/erlang/otp/blob/maint-28/erts/emulator/beam/erl_nif.h#L379)\], \[[3](https://github.com/erlang/otp/blob/maint-28/erts/emulator/beam/erl_nif_api_funcs.h#L434)\] и пр\.\), однако только в этом месте она отличается от всех остальных\. Это вызывает сильное подозрение на простую опечатку \(скорее всего, должно быть `__WIN32__`\)\. Всё же возникает вопрос: почему именно так? Стало интересно проверить, что можно найти по поисковому запросу [`"__WIN_32__"`](https://www.google.com/search?q=%22__WIN_32__%22)\. Результаты прилагаю: ![1305_Erlang_ru/image2.png](https://import.viva64.com/docx/blog/1305_Erlang_ru/image2.png) Как можете заметить, результатов почти нет\. Забавно, что одна из ссылок — это рассматриваемый нами файл на [Debian Sources](https://sources.debian.org/src/erlang/1:21.2.6+dfsg-1/erts/emulator/drivers/common/inet_drv.c/)\. Поиск grep'ом не нашёл подобного макроопределения по всему проекту\. Было также предположение, что он может быть вшит в какой\-то компилятор, например, MinGW\. К сожалению, [эксперимент](https://godbolt.org/z/v63aKaojG) окончился неудачей\. Думаю, после всех попыток понять суть этого макроса, могу сказать, что этот код — ошибка\. В качестве исправления предлагаю воспользоваться проверкой, которую я находила ранее в других местах проекта: ```cpp static int tcp_send_error(tcp_descriptor* desc, int err) { .... #if defined(__WIN32__) || defined(_WIN32) || defined(_WIN32_) if (err == ECONNABORTED) err = ECONNRESET; #endif .... } ``` **Фрагмент N4** Предупреждение PVS\-Studio: [V640](https://pvs-studio.ru/ru/docs/warnings/v640/) The code's operational logic does not correspond with its formatting\. The second statement will always be executed\. It is possible that curly brackets are missing\. [dialyzer\.c 246](https://github.com/erlang/otp/blob/maint-28/erts/etc/common/dialyzer.c#L246) ```cpp int main(int argc, char** argv) { .... if (argc > 2 && strcmp(argv[1], "+P") == 0) { PUSH2("+P", argv[2]); argc--, argv++; argc--, argv++; } else PUSH2("+P", "1000000"); .... } ``` Интересный пример ошибки, которую легко допустить при кодировании и не увидеть после\. Давайте посмотрим, что из себя представляет конструкция `PUSH2`: ```cpp #define QUOTE(s) s #define PUSH(s) eargv[eargc++] = QUOTE(s) #define PUSH2(s, t) PUSH(s); PUSH(t) ``` Ага, это макрос, который раскроется в две конструкции\. Раскроем код так, как бы это сделал препроцессор: ```cpp int main(int argc, char** argv) { .... if (argc > 2 && strcmp(argv[1], "+P") == 0) { eargv[eargc++] = "+P"; eargv[eargc++] = argv[2]; argc--, argv++; argc--, argv++; } else eargv[eargc++] = "+P"; eargv[eargc++] = "1000000"; .... } ``` В итоге, если проверка вычислится как `false`, будет условно исполнено лишь первое присвоение, а второе будет всегда происходить после ветвления\. Вряд ли это то, что хотел получить разработчик\. Написание и использование макросов [притягивает](https://habr.com/ru/companies/pvs-studio/articles/444612/) к себе ошибки\. В идеале было бы избавиться от них, но давайте придём к менее радикальному исправлению\. Когда требуется объединить несколько действий в одно в пределах макроса, это обычно делают с помощью конструкции `do { .... } while (0)`: ```cpp #define PUSH2(s, t) do { PUSH(s); PUSH(t); } while (0) ``` **Фрагмент N5** Предупреждение PVS\-Studio: [V654](https://pvs-studio.ru/ru/docs/warnings/v654/) The condition 'i \>\= 0' of loop is always true\. [wxe\_return\.cpp 236](https://github.com/erlang/otp/blob/maint-28/lib/wx/c_src/wxe_return.cpp#L236) ```cpp ERL_NIF_TERM wxeReturn::make_array_objs(wxAuiPaneInfoArray& arr, WxeApp *app, const char *cname) { ERL_NIF_TERM head, tail; ERL_NIF_TERM class_name = enif_make_atom(env, cname); tail = enif_make_list(env, 0); for (unsigned int i = arr.GetCount() - 1; i >= 0; i--) { head = make_ref(app->getRef((void *) &arr.Item(i),memenv), class_name); tail = enif_make_list_cell(env, head, tail); } return tail; } ``` Обратите внимание на цикл `for`, а именно на его условие\. Из\-за того, что тип индуктивной переменной `i` — `unsigned int`, диапазон значений которого `[0; UINT_MAX]`, цикл становится бесконечным\. После итерации цикла c `i == 0` в результате декремента переменная станет равна `UINT_MAX`, и цикл продолжится\. Исправим код следующим образом: ```cpp for (int64_t i = arr.GetCount() - 1; i >= 0; i--) { .... } ``` Аналогичные срабатывания: * V654 The condition 'i \>\= 0' of loop is always true\. wxe\_return\.cpp 248 * V654 The condition 'i \>\= 0' of loop is always true\. wxe\_return\.cpp 260 **Фрагмент N6** Предупреждение PVS\-Studio: [V517](https://pvs-studio.ru/ru/docs/warnings/v517/) The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 2903, 3053\. [erl\_bif\_info\.c 2903](https://github.com/erlang/otp/blob/maint-28/erts/emulator/beam/erl_bif_info.c#L2903) ```cpp BIF_RETTYPE system_info_1(BIF_ALIST_1) { .... if (is_tuple(BIF_ARG_1)) { // L2778 .... } else if (BIF_ARG_1 == am_scheduler_id) { // L2782 .... } .... else if (BIF_ARG_1 == am_garbage_collection) { // L2903 .... } else if (BIF_ARG_1 == am_fullsweep_after) { // L2921 .... } else if (BIF_ARG_1 == am_garbage_collection) { // L3053 .... } else if (BIF_ARG_1 == am_instruction_counts) { // L3056 .... } .... else if (ERTS_IS_ATOM_STR("halt_flush_timeout", BIF_ARG_1)) { // L3552 .... } } ``` Анализатор обнаружил в функции, содержащей огромное количество `if-else if` \([\~800 строк кода](https://github.com/erlang/otp/blob/maint-28/erts/emulator/beam/erl_bif_info.c#L2771-L3564)\), несколько веток с одинаковыми проверками\. При этом логика в них отличается: [первая проверка](https://github.com/erlang/otp/blob/maint-28/erts/emulator/beam/erl_bif_info.c#L2903-L2920), [вторая проверка](https://github.com/erlang/otp/blob/maint-28/erts/emulator/beam/erl_bif_info.c#L3053-L3054)\. С учётом числа веток в этой функции и дистанции между одинаковыми проверками \(150 строк\) неудивительно, что такое могло произойти\. Предотвратить такие случаи как раз и помогает статический анализ\. К сожалению, мне сложно предложить вариант исправления, поэтому просто оставлю это разработчикам\. **Фрагмент N7** Предупреждение PVS\-Studio: [V560](https://pvs-studio.ru/ru/docs/warnings/v560/) A part of conditional expression is always false: \(arity \=\= 0\)\. [bif\.c 3145](https://github.com/erlang/otp/blob/maint-28/erts/emulator/beam/bif.c#L3145) ```cpp BIF_RETTYPE delete_element_2(BIF_ALIST_3) { .... ptr = tuple_val(BIF_ARG_2); arity = arityval(*ptr); ix = signed_val(BIF_ARG_1); if ((ix < 1) || (ix > arity) || (arity == 0)) { BIF_ERROR(BIF_P, BADARG); } .... } ``` Здесь у нас пример с математикой\. Анализатор говорит о том, что последнее подвыражение `arity == 0` всегда ложное\. Почему же? Давайте копать глубже\. Если поток управления дойдёт до вычисления этого подвыражения, то мы будем знать, что два предыдущих подвыражения уже имеют результат `false`\. При этом мы можем понять следующие вещи: ```cpp ix >= 1 ix <= arity ``` Перепишем эти выражения в математическом стиле: ```cpp 1 <= ix <= arity ``` Далее вспоминаем про [правило транзитивности](https://ru.wikipedia.org/wiki/%D0%A2%D1%80%D0%B0%D0%BD%D0%B7%D0%B8%D1%82%D0%B8%D0%B2%D0%BD%D0%BE%D1%81%D1%82%D1%8C), по которому получается, что `arity >= 1`\. Следовательно, `arity` никогда не может быть равен 0 в этом контексте\. Оставлю это исправление также разработчикам, так как я могу лишь предложить удалить это условие\. ## Критические ошибки **Фрагмент N8** Предупреждения PVS\-Studio: [V557](https://pvs-studio.ru/ru/docs/warnings/v557/) Array overrun is possible\. The value of 'prefix\_len' index could reach 262\. [erl\_alloc\_util\.c 5375](https://github.com/erlang/otp/blob/maint-28/erts/emulator/beam/erl_alloc_util.c#L5375) [V557](https://pvs-studio.ru/ru/docs/warnings/v557/) Array overrun is possible\. The value of 'prefix\_len' index could reach 262\. [erl\_alloc\_util\.c 5378](https://github.com/erlang/otp/blob/maint-28/erts/emulator/beam/erl_alloc_util.c#L5378) [V557](https://pvs-studio.ru/ru/docs/warnings/v557/) Array overrun is possible\. The value of 'prefix\_len' index could reach 262\. [erl\_alloc\_util\.c 5381](https://github.com/erlang/otp/blob/maint-28/erts/emulator/beam/erl_alloc_util.c#L5381) ```cpp #define MAX_ATOM_CHARACTERS 255 static void make_name_atoms(Allctr_t *allctr) { char alloc[] = "alloc"; char realloc[] = "realloc"; char free[] = "free"; char buf[MAX_ATOM_CHARACTERS]; size_t prefix_len = sys_strlen(allctr->name_prefix); if (prefix_len > MAX_ATOM_CHARACTERS + sizeof(realloc) - 1) erts_exit(ERTS_ERROR_EXIT, "Too long allocator name: %salloc\n" allctr->name_prefix); sys_memcpy((void *) buf, (void *) allctr->name_prefix, prefix_len); sys_memcpy((void *) &buf[prefix_len], (void *) alloc, sizeof(alloc) - 1); allctr->name.alloc = am_atom_put(buf, prefix_len + sizeof(alloc) - 1); sys_memcpy((void *) &buf[prefix_len], (void *) realloc, sizeof(realloc) - 1); allctr->name.realloc = am_atom_put(buf, prefix_len + sizeof(realloc) - 1); sys_memcpy((void *) &buf[prefix_len], (void *) free, sizeof(free) - 1); allctr->name.free = am_atom_put(buf, prefix_len + sizeof(free) - 1); } ``` Итак, перед нами функция, которая генерирует имена аллоцирующих/деаллоцирующих функций на основе переданного префикса\. Для того, чтобы их сгенерировать, используется буфер на 255 байт\. Судя по содержимому функции, в формируемом буфере не будет нуль\-терминала, так как в `am_atom_put` передаётся итоговый размер сформированной строки\. Самое первое, что делает разработчик, — это считает размер префикса\. Далее он ограничивает размер префикса: если он больше суммы `MAX_ATOM_CHARACTERS` и длины строки `realloc` \(самый длинный постфикс\), то произойдёт вызов функции `noreturn`\. После этого в буфер копируется префикс, дописывается постфикс, и затем полученная строка уходит в функцию `am_atom_put`\. Все эти действия выполняются последовательно для всех постфиксов: `alloc`, `realloc` и `free`\. Чувствуете, где подвох? Проверка с ранним выходом выполнена некорректно\. В буфере должно оставаться достаточно места, чтобы записать самый длинный постфикс \(7 символов\)\. Если префиксная строка будет размером от 256 до 262 символов, то раннего возврата не произойдёт, следовательно, функция `sys_memcpy` переполнит буфер\. Поведение такой операции не определено\. Предполагая, что функция `am_atom_put` работает со строками длиной не более `MAX_ATOM_CHARACTERS`, поправим код следующим образом: ```cpp if (prefix_len > MAX_ATOM_CHARACTERS - sizeof(realloc) + 1) ``` Теперь максимальная длина префикса будет составлять 248 символов\. При записи максимального постфикса переполнения не произойдёт\. Аналогичные срабатывания: * V557 Array overrun is possible\. The value of 'i' index could reach 40\. ycf\_lexer\.c 493 * V557 Array overrun is possible\. The value of 'n' index could reach 16\. erl\_call\.c 1663 **Фрагмент N9** Этот пример начну нестандартно\. Для начала рассмотрим одну функцию: ```cpp static const char* event_state_flag_to_str(EventStateFlags f, const char *buff, int len) { switch ((int)f) { case ERTS_EV_FLAG_CLEAR: return "CLEAR"; case ERTS_EV_FLAG_USED: return "USED"; /* other cases that return string literals */ default: snprintf((char *)buff, len, "ERROR(%d)", f); return buff; } } ``` Функция `event_state_flag_to_str` преобразует перечисление `EventStateFlags` в строку\. Для не\-`default` веток `switch` возвращает строковый литерал, в ином случае функция формирует строку в переданном буфере и возвращает указатель на него\. Заметьте, как именно она это делает: у буфера принудительно отбрасывают константность\. Согласно стандарту C, делать такое можно только если исходный буфер не был объявлен как `const` \(C11 [6\.7\.3\.6](https://open-std.org/JTC1/SC22/WG14/www/docs/n1570.pdf#page=140), C23 [6\.7\.4\.1](https://open-std.org/JTC1/SC22/WG14/www/docs/n3220.pdf#page=134)\.7\)\. А так ли это на самом деле? Давайте взглянем на точку вызова: ```cpp static ERTS_INLINE void print_flags(erts_dsprintf_buf_t *dsbufp, EventStateFlags f) { const char buff[64]; if (f & ERTS_EV_FLAG_WANT_ERROR) { erts_dsprintf(dsbufp, "WANTERR|"); f &= ~ERTS_EV_FLAG_WANT_ERROR; } erts_dsprintf(dsbufp, "%s", event_state_flag_to_str(f, buff, sizeof(buff))); } ``` Исследуемую функцию как раз и вызывают, передавая вторым аргументом константный буфер\. К сожалению, получаем неопределённое поведение\. Собственно, нашли мы это место с помощью анализатора, правда при этом было выдано не совсем логичное предупреждение: [V614](https://pvs-studio.ru/ru/docs/warnings/v614/) Uninitialized buffer 'buff' used\. Consider checking the second actual argument of the 'event\_state\_flag\_to\_str' function\. [erl\_check\_io\.c 2833](https://github.com/erlang/otp/blob/maint-28/erts/emulator/sys/common/erl_check_io.c#L2833) Исправим код, убрав константность у буфера: ```cpp static ERTS_INLINE void print_flags(erts_dsprintf_buf_t *dsbufp, EventStateFlags f) { char buff[64]; if (f & ERTS_EV_FLAG_WANT_ERROR) { erts_dsprintf(dsbufp, "WANTERR|"); f &= ~ERTS_EV_FLAG_WANT_ERROR; } erts_dsprintf(dsbufp, "%s", event_state_flag_to_str(f, buff, sizeof(buff))); } ``` Также рекомендую поменять сигнатуру функции `event_state_flag_to_str`, чтобы второй параметр принимал указатель типа `char *`\. Так будет понятно, что функция может модифицировать переданный буфер\. Много кода мы при этом не поломаем, т\.к\. функция находится только в компилируемом файле и помечена как `static`\. **Фрагмент N10** Предупреждение PVS\-Studio: [V1050](https://pvs-studio.ru/ru/docs/warnings/v1050/) The uninitialized class member 'a' is used when initializing the base class 'BeamAssemblerCommon'\. [beam\_asm\.hpp 87](https://github.com/erlang/otp/blob/maint-28/erts/emulator/beam/jit/arm/beam_asm.hpp#L68) ```cpp class BeamAssemblerCommon : public ErrorHandler { BaseAssembler &assembler; protected: BeamAssemblerCommon(BaseAssembler &assembler); .... }; struct BeamAssembler : public BeamAssemblerCommon { BeamAssembler() : BeamAssemblerCommon(a) { /* .... */ } protected: a64::Assembler a; .... }; ``` В этом фрагменте у нас небольшая иерархия классов: в конструктор базового класса \(`BeamAssemblerCommon`\) передаётся ссылка на нестатическое поле производного класса \(`BeamAssembler`\)\. Такая операция не приводит к проблемам до тех пор, пока в конструкторе базового класса не работают с этим полем\. Почему так? Время жизни объекта из производного класса начинается, когда отработает нужный инициализатор в списке инициализации или поток управления попадёт в тело конструктора, если инициализатор отсутствует\. Вооружившись этой информацией, смотрим на конструктор базового класса\. Он же не использует этот объект? Правда ведь?\.\. ```cpp BeamAssemblerCommon::BeamAssemblerCommon(BaseAssembler &assembler_) : assembler(assembler_), .... { .... #ifdef DEBUG assembler.addDiagnosticOptions(DiagnosticOptions::kValidateAssembler); #endif assembler.addEncodingOptions(EncodingOptions::kOptimizeForSize | EncodingOptions::kOptimizedAlign); .... } ``` Неправда :\) У объекта производного класса по ссылке вызывается нестатическая функция\-член, а его время жизни ещё не началось\. И снова поведение не определено\. Предложить исправление в этой ситуации я также затрудняюсь, оставлю срабатывание разработчикам\. ## Ошибки при работе с памятью **Фрагмент N11** Предупреждение PVS\-Studio: [V586](https://pvs-studio.ru/ru/docs/warnings/v586/) The 'free' function is called twice for deallocation of the same memory space\. [erl\_call\.c 668](https://github.com/erlang/otp/blob/maint-28/lib/erl_interface/src/prog/erl_call.c#L668) ```cpp int main(int argc, char *argv[]) { int fd; char *p = NULL; ei_cnode ec; .... int i = 0; ei_x_buff reply; .... if (ei_rpc(&ec, fd, "c", "c", p, i, &reply) < 0) { free(p); ei_x_free(&reply); fprintf(stderr,"erl_call: can't compile file %s\n", fname); } free(p); /* FIXME complete this code FIXME print out error message as term if (!erl_match(erl_format("{ok,_}"), reply)) { fprintf(stderr,"erl_call: compiler errors\n"); } */ ei_x_free(&reply); .... } ``` Судя по названию, `ei_rpc` — это удалённый вызов процедуры \([Remote Procedure Call](https://en.wikipedia.org/wiki/Remote_procedure_call)\)\. Отрицательное число возвращается в случае неуспеха операции\. Если удалённый вызов не отработал, начинается очистка некоторых ресурсов\. Указатель `p` передаётся в функцию `free` и происходит логирование ошибки в `stderr`\. Дальше, вне зависимости от результата вызова функции, указатель снова передаётся во `free`\. Получаем двойное освобождение памяти\. Решение проблемы довольно простое: нужно или прервать выполнение программы, или занулить освобождённые указатели\. Я бы исправила проблему, прервав исполнение в этой ветке: ```cpp if (ei_rpc(&ec, fd, "c", "c", p, i, &reply) < 0) { free(p); ei_x_free(&reply); fprintf(stderr,"erl_call: can't compile file %s\n", fname); return 1; } free(p); ``` **Фрагмент N12** Предупреждение PVS\-Studio: [V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'pkey' pointer was utilized before it was verified against nullptr\. Check lines: 581, 582\. [pkey\.c 581](https://github.com/erlang/otp/blob/maint-28/lib/crypto/c_src/pkey.c#L581) ```cpp static int get_pkey_public_key(ErlNifEnv *env, const ERL_NIF_TERM argv[], int algorithm_arg_num, int key_arg_num, EVP_PKEY **pkey, ERL_NIF_TERM *err_return) { .... if (enif_is_map(env, argv[key_arg_num])) { password = get_key_password(env, argv[key_arg_num]); *pkey = ENGINE_load_public_key(e, id, NULL, password); if (!pkey) assign_goto(*err_return, err, EXCP_BADARG_N(env, key_arg_num, "Couldn't get public key from engine")); } /* other branches */ ret = 1; done: .... return ret; err: .... *pkey = NULL; ret = 0; goto done; } ``` Перед нами достаточно занятный фрагмент\. [Функция](https://github.com/erlang/otp/blob/maint-28/lib/crypto/c_src/pkey.c#L562-L646) пытается извлечь публичный ключ в 5\-й выходной параметр и возвращает в случае успеха `1`, а в случае неудачи — `0`\. В последнем случае публичный ключ также обнуляется\. Анализатор предупреждает, что в одной из веток функции указатель `pkey` сначала разыменовывается, а затем проверяется на валидность\. При этом в других ветках такой проверки не наблюдается\. Делаем вывод, что у функции есть контракт "параметр `pkey` никогда не должен быть равен нулевому указателю"\. Убедиться в этом можно, если поискать вызовы этой функции в этом же файле \(\[[1](https://github.com/erlang/otp/blob/maint-28/lib/crypto/c_src/pkey.c#L941)\], \[[2](https://github.com/erlang/otp/blob/maint-28/lib/crypto/c_src/pkey.c#L1035)\], \[[3](https://github.com/erlang/otp/blob/maint-28/lib/crypto/c_src/pkey.c#L1260)\]\), — везде передаётся адрес переменной\. На самом деле, разработчик хотел обработать некорректный результат функции `ENGINE_load_public_key`, прыгнув на метку `err`, но в итоге опечатался, забыв написать звёздочку перед оператором `!`\. Современный компилятор [удалит](https://godbolt.org/z/WxGW9nnP9) такую проверку на релизе, что также приведёт к тому, что функция может вернуть `1` и нулевой указатель в пятом параметре\. Поправим проверку: ```cpp .... *pkey = ENGINE_load_public_key(e, id, NULL, password); if (!*pkey) assign_goto(*err_return, err, EXCP_BADARG_N(env, key_arg_num, "Couldn't get public key from engine")); .... ``` **Фрагмент N13** Довольно часто нам в поддержку пишут, что анализатор ругается на то, что во `free` толкается нулевой указатель\. Ведь по стандарту ничего страшного не произойдёт\. Мы даже писали про похожий случай [статью](https://pvs-studio.ru/ru/blog/posts/cpp/1068/)\. Да, ничего страшного нет, но этот код, скорее всего, подозрителен\. И это помогло найти интересную утечку памяти в коде\. Давайте посмотрим на неё\. Предупреждение PVS\-Studio: [V575](https://pvs-studio.ru/ru/docs/warnings/v575/) The null pointer is passed into 'free' function\. Inspect the first argument\. [erl\_misc\_utils\.c 264](https://github.com/erlang/otp/blob/maint-28/erts/lib_src/common/erl_misc_utils.c#L264) ```cpp void erts_cpu_info_destroy(erts_cpu_info_t *cpuinfo) { if (cpuinfo) { cpuinfo->configured = 0; cpuinfo->online = 0; cpuinfo->available = 0; #ifdef HAVE_PSET_INFO if (cpuinfo->cpuids) free(cpuinfo->cpuids); #endif cpuinfo->topology_size = 0; if (cpuinfo->topology) { cpuinfo->topology = NULL; free(cpuinfo->topology); } free(cpuinfo); } } ``` В этом фрагменте указатель `cpuinfo->topology` явно устанавливается в `NULL` перед вызовом `free`\. По итогу операция освобождения памяти бессмысленна, так как `free(NULL)` не выполняет никаких действий\. Это приводит к утечке памяти, потому что оригинальный блок памяти, на который указывал `cpuinfo->topology`, никогда не освобождается\. Исправим проблему, поменяв операции местами: ```cpp if (cpuinfo->topology) { free(cpuinfo->topology); cpuinfo->topology = NULL; } ``` **Фрагмент N14** Предупреждение PVS\-Studio: [V773](https://pvs-studio.ru/ru/docs/warnings/v773/) The function was exited without releasing the 'entry' pointer\. A memory leak is possible\. [wxe\_gl\.cpp 88](https://github.com/erlang/otp/blob/maint-28/lib/wx/c_src/wxe_gl.cpp#L88) ```cpp typedef struct _wxe_glc { wxGLCanvas *canvas; wxGLContext *context; } wxe_glc; void setActiveGL(wxeMemEnv *memenv, ErlNifPid caller, wxGLCanvas *canvas, wxGLContext *context) { ErlNifUInt64 callId = wxe_make_hash(memenv->tmp_env, &caller); wxe_glc * entry = glc[callId]; gl_active_index = callId; gl_active_pid = caller; if (!entry) { entry = (wxe_glc *) malloc(sizeof(wxe_glc)); entry->canvas = NULL; entry->context = NULL; } if (entry->canvas == canvas && entry->context == context) return; entry->canvas = canvas; entry->context = context; glc[gl_active_index] = entry; .... } ``` В функции `setActiveGL` объявлен указатель `entry`, который инициализируют элементом из массива\. Если этот элемент оказывается нулевым указателем, то происходит выделение памяти через `malloc`\. Далее происходит сравнение указателей\. Если оно вычислится как `true`, то произойдёт ранний возврат\. При этом если динамическая аллокация произошла, то память не освободили\. Да, такая ситуация, скорее, редкость и никогда не произойдёт, ведь для этого нужен целый комплекс событий: * при вызове функции 3\-м и 4\-м аргументами передали нулевые указатели; * в `glc[callId]` находится нулевой указатель\. Но мы можем сделать так, чтобы даже теоретически утечка никогда не происходила\. Немного модифицируем код: ```cpp ErlNifUInt64 callId = wxe_make_hash(memenv->tmp_env, &caller); wxe_glc * entry = glc[callId]; gl_active_index = callId; gl_active_pid = caller; bool new_alloc = false; if (!entry) { entry = (wxe_glc *) malloc(sizeof(wxe_glc)); entry->canvas = NULL; entry->context = NULL; new_alloc = true; } if (entry->canvas == canvas && entry->context == context) { if (new_alloc) { free(entry); } return; } .... ```
Аналогичные срабатывания * V773 The function was exited without closing the file referenced by the 'f' handle\. A resource leak is possible\. erl\_poll\.c 2423 * V773 The 'erl\_errno\_p' pointer was assigned values twice without releasing the memory\. A memory leak is possible\. ei\_pthreads\.c 195 * V773 The function was exited without closing the file referenced by the 'fp' handle\. A resource leak is possible\. cpu\_sup\.c 433 * V773 The exception was thrown without releasing the 'data' pointer\. A memory leak is possible\. wxe\_wrapper\_4\.cpp 1300 * V773 The exception was thrown without releasing the 'data' pointer\. A memory leak is possible\. wxe\_wrapper\_4\.cpp 1330 * V773 The exception was thrown without releasing the 'data' pointer\. A memory leak is possible\. wxe\_wrapper\_4\.cpp 1663 * V773 The exception was thrown without releasing the 'data' pointer\. A memory leak is possible\. wxe\_wrapper\_4\.cpp 1690 * V773 The exception was thrown without releasing the 'data' pointer\. A memory leak is possible\. wxe\_wrapper\_4\.cpp 1715 * V773 The exception was thrown without releasing the 'alpha' pointer\. A memory leak is possible\. wxe\_wrapper\_4\.cpp 1718 * V773 The exception was thrown without releasing the 'data' pointer\. A memory leak is possible\. wxe\_wrapper\_4\.cpp 1747 * V773 The exception was thrown without releasing the 'alpha' pointer\. A memory leak is possible\. wxe\_wrapper\_4\.cpp 1750 * V773 The exception was thrown without releasing the 'alpha' pointer\. A memory leak is possible\. wxe\_wrapper\_4\.cpp 2680 * V773 The exception was thrown without releasing the 'data' pointer\. A memory leak is possible\. wxe\_wrapper\_4\.cpp 2715 * V773 The exception was thrown without releasing the 'data' pointer\. A memory leak is possible\. wxe\_wrapper\_4\.cpp 2733
**Фрагмент N15** Предупреждение PVS\-Studio: [V701](https://pvs-studio.ru/ru/docs/warnings/v701/) realloc\(\) possible leak: when realloc\(\) fails in allocating memory, original pointer 'outbuf\_base' is lost\. Consider assigning realloc\(\) to a temporary pointer\. [run\_erl\.c 1324](https://github.com/erlang/otp/blob/maint-28/erts/etc/unix/run_erl.c#L1324) ```cpp static void outbuf_append(const char* buf, int n) { .... /* * Allocate a larger buffer if we still cannot fit the data. */ if (outbuf_base+outbuf_total < outbuf_in+n) { int size = outbuf_in - outbuf_out; outbuf_total = size+n; outbuf_base = realloc(outbuf_base, outbuf_total); outbuf_out = outbuf_base; outbuf_in = outbuf_base + size; } .... } ``` Перед нами коварный паттерн с функцией [`realloc`](https://en.cppreference.com/w/c/memory/realloc), когда исходный указатель сразу перебивается возвращаемым значением\. Разберёмся, в чём же проблема\. Функция принимает первым аргументом буфер для реаллокации, а вторым — новый размер буфера\. Если операция завершилась успешно, то функция вернёт указатель на новую память\. Память, переданная первым аргументом, становится **невалидной**, даже если по факту произошло простое расширение блока памяти\. Если операция завершилась неудачно, то функция вернёт **нулевой указатель**\. Память, переданная первым аргументом, остаётся **нетронутой**\. Получается, если сразу же перезаписывать исходный указатель вызовом `realloc`, то получим утечку памяти\. Исправим код следующим образом: ```cpp static void outbuf_append(const char* buf, int n) { .... /* * Allocate a larger buffer if we still cannot fit the data. */ if (outbuf_base + outbuf_total < outbuf_in + n) { int size = outbuf_in - outbuf_out; outbuf_total = size+n; char *tmp = realloc(outbuf_base, outbuf_total); if (!tmp) { /* somehow handle this scenario * the `outbuf_base` buffer here is still valid */ } outbuf_base = tmp; outbuf_out = outbuf_base; outbuf_in = outbuf_base + size; } .... } ``` В исправленном примере тоже нужно как\-то обработать ситуацию, когда `realloc` возвращает нулевой указатель\. Это может быть аварийное завершение работы, попытка выделить буфер другого размера и др\. **Фрагмент N16** Предупреждение PVS\-Studio: [V769](https://pvs-studio.ru/ru/docs/warnings/v769/) The 'end' pointer in the 'end \- mm\-\>sua\.top' expression equals nullptr\. The resulting value is senseless and it should not be used\. [erl\_mmap\.c 2411](https://github.com/erlang/otp/blob/maint-28/erts/emulator/sys/common/erl_mmap.c#L2411) ```cpp static void add_free_desc_area(ErtsMemMapper* mm, char *start, char *end) { ERTS_MMAP_ASSERT(end == (void *) 0 || end > start); if (sizeof(ErtsFreeSegDesc) <= ((UWord) end) - ((UWord) start)) { .... } .... } void erts_mmap_init(ErtsMemMapper* mm, ErtsMMapInit *init) { .... if (end == (void *) 0) { /* * Very unlikely, but we need a guarantee * that `mm->sua.top` always will * compare as larger than all segment pointers * into the super carrier... */ mm->sua.top -= ERTS_PAGEALIGNED_SIZE; mm->size.supercarrier.used.total += ERTS_PAGEALIGNED_SIZE; #ifdef ERTS_HAVE_OS_PHYSICAL_MEMORY_RESERVATION if (!virtual_map || os_reserve_physical(mm->sua.top, ERTS_PAGEALIGNED_SIZE)) #endif add_free_desc_area(mm, mm->sua.top, end); mm->desc.reserved += (end - mm->sua.top) / sizeof(ErtsFreeSegDesc); } .... } ``` Анализатор обнаружил подозрительную адресную арифметику\. Давайте посмотрим, что тут не так\. Мы попадаем в ветку кода, когда `end` — это действительно нулевой указатель\. Поле `mm->sua.top` также указатель, но о его значении мы пока ничего сказать не можем\. Стандарт C определяет поведение при разнице указателей только в случае, когда они оба указывают на элементы одного и того же массива \(C11 [6\.5\.6\.8](https://open-std.org/JTC1/SC22/WG14/www/docs/n1570.pdf#page=110); C23 [6\.5\.7\.10](https://open-std.org/JTC1/SC22/WG14/www/docs/n3220.pdf#page=98)\)\. Нулевой указатель не указывает на какой\-либо элемент массива\. Следовательно, поведение такой операции не определено, что не есть хорошо\. Мне стало интересно, а что же происходит одной строкой выше, в функции [`add_free_desc_area`](https://github.com/erlang/otp/blob/maint-28/erts/emulator/sys/common/erl_mmap.c#L457-L482)? ```cpp static void add_free_desc_area(ErtsMemMapper* mm, char *start, char *end) { ERTS_MMAP_ASSERT(end == (void *) 0 || end > start); if (sizeof(ErtsFreeSegDesc) <= ((UWord) end) - ((UWord) start)) { .... } ``` И тут тоже вычитание указателей, с небольшим "но"\.\.\. перед этим указатели приводят к числам\. А это уже поведение, определяемое реализацией \(C11 [6\.3\.2\.3\.6](https://open-std.org/JTC1/SC22/WG14/www/docs/n1570.pdf#page=73); C23 [6\.3\.2\.3\.6](https://open-std.org/JTC1/SC22/WG14/www/docs/n3220.pdf#page=64)\), поэтому тут ситуация уже много лучше\. К сожалению, предложить корректный вариант исправления я не могу\. Думаю, разработчикам стоит обратить внимание на этот код\. ## Заключение Ну что ж, на этом наши срабатывания закончились\. Мы нашли разные ошибки, как и в любом огромном проекте, которому довелось прожить долгую и насыщенную жизнь, но и они — часть проекта\. Думаю, важно понимать, что наличие таких находок в кодовой базе возрастом в три десятилетия — это не признак слабости, а свидетельство невероятного масштаба и долголетия\. Миллионы строк кода, написанных сотнями разработчиков, и при этом система работает годами без перезагрузки\. Это проект, который заслуживает глубочайшего уважения не за идеальность, а за свою реальную, проверенную временем мощь\. Современные технологии не стоят на месте, и сегодня у разработчиков есть мощные инструменты для повышения надёжности кода\. Например, статические анализаторы помогают выявлять скрытые ошибки и потенциальные уязвимости\. Предлагаю вам самим в этом убедиться :\) Получить пробную версию анализатора PVS\-Studio можно [здесь](https://pvs-studio.ru/ru/pvs-studio/try-free/)\.