После некоторых поисков серьёзного вызова для анализатора PVS-Studio выбор пал на открытую коллекцию компиляторов GCC. Да, это уже не первая по счёту проверка этого проекта. Однако поддерживаемые этой коллекцией языки программирования не стоят на месте, и их постоянное развитие влечёт за собой соответствующее постоянное усложнение кода GCC. Таким образом цель — обнаружить ошибки в коде GCC с помощью анализатора PVS-Studio.
Коллекция компиляторов GCC ведёт свою историю с 1987 года, т.е. 36 лет, и занимает очень важную роль в жизни проектов с открытым исходным кодом. Сначала она включала в себя только компилятор для языка С, однако позднее была добавлена поддержка таких языков, как C++, Objective-C, Java, Фортран, Ada, Go, GAS и D. На момент написания статьи самой новой мажорной стабильной версией GCC являлась версия 13. Конкретно в данной статье рассматривается версия 13.1. За свою долгую историю развития GCC обзавёлся большим количеством разработчиков и ещё большим количеством пользователей. Соответственно, проект очень хорошо тестируется, и найти ошибку в таком отлаженном и проверенном коде — это очень интересная задача.
Код компилятора написан по определённому соглашению с обильным использованием макросов, что делает его чтение настоящим кошмаром для неподготовленного пользователя. Это также усложняет работу статическому анализатору PVS-Studio, но не останавливает его. Ниже я рассмотрю 10 фрагментов кода, которые меня заинтересовали в процессе беглого просмотра предупреждений. Более подробный анализ отчёта, к сожалению, затруднён в силу уже упомянутых макросов и требует предварительной настройки анализатора, что выходит за рамки простого написания статьи. Если вы желаете ознакомиться с более ранней проверкой проекта GCC, то вы можете перейти по ссылке.
static ctf_id_t
gen_ctf_function_type (ctf_container_ref ctfc,
dw_die_ref function,
bool from_global_func)
{
....
ctf_funcinfo_t func_info; // <=
....
{
....
if (....)
{
do
{
....
if (....)
....
else if (....)
{
func_info.ctc_flags |= CTF_FUNC_VARARG; // <=
....
}
}
}
....
}
....
}Предупреждение анализатора PVS-Studio: V614 Uninitialized variable 'func_info.ctc_flags' used. gcc/dwarf2ctf.cc 676
Объект func_info не инициализируется при объявлении. Затем ниже по коду в одной из веток происходит выставление флага CTF_FUNC_VARARG в поле ctc_flags, однако оно не было проинициализировано.
static struct gcov_info *
read_gcda_file (const char *filename)
{
....
curr_gcov_info = obj_info =
(struct gcov_info *) xcalloc (sizeof (struct gcov_info) +
sizeof (struct gcov_ctr_info) * GCOV_COUNTERS, 1);
obj_info->version = version;
obj_info->filename = filename;
....
}Предупреждение анализатора PVS-Studio: V522 There might be dereferencing of a potential null pointer 'obj_info'. Check lines: 290, 287. libgcov-util.c 290. libgcov-util.c 287
Анализатор PVS-Studio обнаружил отсутствие проверки на валидность указателя obj_info после аллокации. Известно, что функции xmalloc, xcalloc и xrealloc всегда возвращают валидный указатель, а в случае ошибки программа прервёт своё выполнение. Когда я увидел это предупреждение, то отнёс его к ложным и решил подготовить минимально воспроизводимый пример для будущей отладки. Однако открыв препроцессированный файл, я обнаружил в соответствующей строке вызов обычного calloc. Вот во что на самом деле раскрывается макрос xcalloc:
// libgcc/libgcov.h
....
/* work around the poisoned malloc/calloc in system.h. */
#ifndef xmalloc
#define xmalloc malloc
#endif
#ifndef xcalloc
#define xcalloc calloc
#endifПредвосхищая будущие комментарии, хочу привести ссылку на статью своего коллеги. В ней он приводил доводы к тому, зачем стоит проверять результат этих функций.
static void
generate_results (const char *file_name)
{
....
json::object *root = new json::object ();
root->set ("format_version", new json::string ("1"));
root->set ("gcc_version", new json::string (version_string));
if (....)
root->set ("current_working_directory", new json::string (bbg_cwd));
root->set ("data_file", new json::string (file_name));
json::array *json_files = new json::array ();
root->set ("files", json_files);
....
if (....)
{
if (....)
{
root->dump (stdout);
printf ("\n");
}
else
{
pretty_printer pp;
root->print (&pp);
....
}
}
}Предупреждение анализатора PVS-Studio: V773 The function was exited without releasing the 'root' pointer. A memory leak is possible. gcov.cc 1525
Создаваемый json-объект root забыли удалить, в итоге получилась утечка памяти. Скорее всего, такой вывод JSON происходит единоразово, и эта ошибка фактически ни на что не повлияет. Решил рассмотреть этот случай, чтобы показать возможности PVS-Studio в выявлении утечек памяти.
char m_flag_chars[256];
....
void
flag_chars_t::add_char (char ch)
{
int i = strlen (m_flag_chars);
m_flag_chars[i++] = ch;
m_flag_chars[i] = 0;
}Предупреждение анализатора PVS-Studio: V557 Array overrun is possible. The value of 'i' index could reach 256. c-format.cc 1994
На данном участке кода может произойти выход за границу массива при записи терминального нуля, если длина строки m_flag_chars до вызова функции add_char была равна 255.
....
/* Return True when the format flag CHR has been used. */
bool get_flag (char chr) const
{
unsigned char c = chr & 0xff;
return (flags[c / (CHAR_BIT * sizeof *flags)]
& (1U << (c % (CHAR_BIT * sizeof *flags))));
}
....
static fmtresult
format_floating (const directive &dir, const HOST_WIDE_INT prec[2])
{
....
unsigned flagmin = (1 /* for the first digit */
+ (dir.get_flag ('+') | dir.get_flag (' ')));
....
}Предупреждение анализатора PVS-Studio: V792 The 'get_flag' function located to the right of the operator '|' will be called regardless of the value of the left operand. Perhaps, it is better to use '||'. gimple-ssa-sprintf.cc 1227
Функция get_flag возвращает объект типа bool. Результат двух вызовов функции объединяется оператором побитового "ИЛИ". Опасности такой код не содержит, однако побитовый оператор будет вычислять оба операнда. Воспользовавшись логическим "ИЛИ", можно избавиться от лишнего вычисления, если первый вызов вернёт true:
unsigned flagmin = (1 /* for the first digit */
+ (dir.get_flag ('+') || dir.get_flag (' ')));struct gcov_info *
gcov_read_profile_dir (const char* dir_name,
int recompute_summary ATTRIBUTE_UNUSED)
{
char *pwd;
int ret;
read_profile_dir_init ();
if (access (dir_name, R_OK) != 0)
{
fnotice (stderr, "cannot access directory %s\n", dir_name);
return NULL;
}
pwd = getcwd (NULL, 0); // <=
gcc_assert (pwd);
ret = chdir (dir_name);
if (ret != 0)
{
fnotice (stderr, "%s is not a directory\n", dir_name);
return NULL; // <=
}
#ifdef HAVE_FTW_H
ftw (".", ftw_read_file, 50);
#endif
chdir (pwd);
free (pwd); // <=
return gcov_info_head;;
}Предупреждение анализатора PVS-Studio: V773 The function was exited without releasing the 'pwd' pointer. A memory leak is possible. libgcov-util.c 450, libgcov-util.c 434
Обратите внимание на вызов функции getcwd. Если ей первым аргументом передаётся нулевой указатель, функция динамически выделяет память под буфер необходимой длины и записывает туда текущую рабочую директорию. Освобождение памяти остаётся на совести разработчика. Как видно, в самом конце функции разработчик освободил память, но забыл сделать это в одной из веток кода с ранним возвратом. В итоге в коде содержится потенциальная, хоть и несущественная, утечка памяти. О способности PVS-Studio обнаруживать утечки памяти можно подробнее почитать здесь.
Исправленный код:
....
ret = chdir (dir_name);
if (ret != 0)
{
fnotice (stderr, "%s is not a directory\n", dir_name);
free(pwd);
return NULL;
}
....static conversion *
build_aggr_conv (tree type,
tree ctor,
int flags, // <=
tsubst_flags_t complain)
{
unsigned HOST_WIDE_INT i = 0;
conversion *c;
tree field = next_aggregate_field (TYPE_FIELDS (type));
tree empty_ctor = NULL_TREE;
hash_set<tree, true> pset;
flags = LOOKUP_IMPLICIT|LOOKUP_NO_NARROWING; // <=
....
}Предупреждение анализатора PVS-Studio: V763 Parameter 'flags' is always rewritten in function body before being used. call.cc 994, call.cc 981
Принимаемый по копии параметр функции flags всегда перезаписывается. Возможно, разработчик планировал дополнительно установить флаги в эту переменную или должна была использоваться другая переменная.
void
gt_pch_p_13ctf_container (ATTRIBUTE_UNUSED void *this_obj,
void *x_p,
ATTRIBUTE_UNUSED gt_pointer_operator op,
ATTRIBUTE_UNUSED void *cookie)
{
struct ctf_container * x ATTRIBUTE_UNUSED = (struct ctf_container *)x_p;
{
....
size_t l0 = (size_t)(0); // <=
....
if ((*x).ctfc_vars_list != NULL)
{
size_t i0;
for (i0 = 0; i0 != (size_t)(l0) // <=
&& ((void *)(*x).ctfc_vars_list == this_obj); i0++)
{
if ((void *)((*x).ctfc_vars_list) == this_obj)
op (&((*x).ctfc_vars_list[i0]), NULL, cookie);
}
....
}
}
....
}Предупреждение анализатора PVS-Studio: V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. gtype-desc.cc 11865
Переменная l0, от которой зависит состояние цикла, так и осталась равной нулю, в связи с чем цикл никогда не начнёт своего выполнения.
Анализатор PVS-Studio также выдал ещё несколько похожих предупреждений. Вот только некоторые из них:
static bool
combine_reaching_defs (ext_cand *cand,
const_rtx set_pat,
ext_state *state)
{
....
while ( REG_P (SET_SRC (*dest_sub_rtx))
&& (REGNO (SET_SRC (*dest_sub_rtx)) == REGNO (SET_DEST (set))))
{
....
if (....)
break;
if (....)
break;
....
break;
}
}Предупреждение анализатора PVS-Studio: V612 An unconditional 'break' within a loop. ree.cc 985
Анализатор PVS-Studio обнаружил, что данный цикл безусловно прерывается на первой итерации. При дальнейшем рассмотрении мы также обнаруживаем множество других break под условиями. Возможно, это способ избежать написания оператора goto, что, однако, выглядит странно, учитывая его постоянное открытое использование в целом по проекту.
void
const_fn_result_svalue::dump_to_pp(pretty_printer *pp,
bool simple) const
{
if (simple)
{
pp_printf (pp, "CONST_FN_RESULT(%qD, {", m_fndecl);
for (unsigned i = 0; i < m_num_inputs; i++)
{
if (i > 0)
pp_string (pp, ", ");
dump_input (pp, i, m_input_arr[i], simple);
}
pp_string (pp, "})");
}
else
{
pp_printf (pp, "CONST_FN_RESULT(%qD, {", m_fndecl);
for (unsigned i = 0; i < m_num_inputs; i++)
{
if (i > 0)
pp_string (pp, ", ");
dump_input (pp, i, m_input_arr[i], simple);
}
pp_string (pp, "})");
}
}Предупреждение анализатора PVS-Studio: V523 The 'then' statement is equivalent to the 'else' statement. svalue.cc 2009, svalue.cc 1998
Обнаружено полное дублирование логики в условных ветках. Я решил поискать причину такого дублирования и воспользовался командой git log –follow -p ./gcc/analyzer/svalue.cc в корне git проекта GCC, чтобы посмотреть разницу между коммитами, однако ничего там не обнаружил. Получается, файл был написан с этой ошибкой изначально.
После изучения кода, настолько активно использующего макросы, очень быстро накатывает усталость. Но в этом случае усталость компенсируется удовлетворением от факта работы над таким серьезным проектом, как GCC. PVS-Studio подтвердил свою способность находить малозаметные ошибки даже в хорошо отлаженных кодовых базах, и даже завеса из макросов не стала для него препятствием.
Напоследок хочется пожелать всем допускать как можно меньше ошибок и не плодить баги в релиз :) Ну и пользуясь случаем, приглашаю попробовать триальную версию анализатора и подписаться на дайджест наших статей, чтобы не пропустить интересное.