GDB - это инструмент, без которого трудно жить. Конечно, как говорит Брукс: «Качество разработки программного обеспечения достигается за счет правильного проектирования, а не бесконечного тестирования». Однако правильный дизайн не защищает от логических ошибок, опечаток, нулевых указателей и т. Д. Вот почему на помощь приходят различные инструменты отладки, такие как GDB. Моя цель - показать, что статические анализаторы кода также являются очень полезными инструментами, которые помогают обнаруживать ошибки на ранних этапах разработки. Намного лучше, если ошибка будет исправлена в коде до этапа тестирования и отладки. Чтобы продемонстрировать преимущества статического анализа кода, давайте углубимся в GDB и поищем ошибки с помощью PVS-Studio.
Введение
Уже написав статью о проверке GCC, я решил написать статью и о GDB. Но на этот раз сделать это было намного сложнее. Видимо, свою роль сыграл размер проектов. Однако сравнить размер базы кода не так-то просто. В обоих проектах есть файлы, содержащие большие таблицы данных. Они значительно влияют на размер кода и количество строк в нем. Например, в проекте GDB есть файл i386-tbl.h, размером 5Мб, с такой таблицей:
Я думаю, что реального кода в GCC в несколько раз больше, чем размер кода GDB. Проверяя GCC, я мог легко получить приличное количество ошибок для написания статьи, просто просматривая код и не копаясь в подозрительных частях, но было трудно понять фрагменты кода. В случае с GDB мне пришлось очень внимательно посмотреть, и я все же смог найти очень мало подозрительных мест.
Анализ
Я проверил исходный код GDB версии 7.11.1. Код проверялся на версии PVS-Studio, работающей под Linux.
Краткий справочник. - коммерческий статический анализатор, который обнаруживает ошибки в исходном коде, написанном на языках C, C ++ и C #. Он работает как в среде Linux, так и в среде Windows.
Чтобы проверить GDB с помощью статического анализатора кода PVS-Studio, нам нужно выполнить несколько простых шагов.
0) Прочтите документацию: Как запустить PVS-Studio в Linux. Я выбрал способ, позволяющий проверять проект без интеграции анализатора в систему сборки.
1) Загрузите последнюю версию исходного кода из официального репозитория:
$ git clone git://sourceware.org/git/binutils-gdb.git
2) Измените конфигурационный файл PVS-Studio.cfg, а именно параметры output-file и sourcetree-root. В моем случае:
exclude-path = /usr/include/ exclude-path = /usr/lib64/ lic-file = /home/andr/PVS-Studio.lic output-file = /home/andr/gdb.log sourcetree-root = /home/andr/binutils-gdb
3) Заходим в скачанный каталог:
$ cd binutils-gdb
4) Создайте Makefile:
$ ./configure
Запускаем сборку gdb и анализатор PVS-Studio:
$ pvs-studio-analyzer trace -- make -j3
6) Запустить анализ (указав путь к конфигурационному файлу PVS-Studio.cfg)
$ pvs-studio-analyzer analyze --cfg /home/andr/PVS-Studio.cfg
После успешного завершения анализа в домашнем каталоге появится файл журнала gdb.log, который можно просмотреть в Windows с помощью утилиты Standalone. Я сделал это именно так, мне это было очень удобно.
Если вы хотите просмотреть отчет в Linux, вам поможет утилита-конвертер (plog-converter); исходный код также включен в дистрибутив PVS-Studio. Утилита может конвертировать файлы * .plog в разные форматы (см. Документацию). Теперь вы можете настроить конвертер так, чтобы он отвечал вашим требованиям.
Важно. Пожалуйста, не пытайтесь открывать * .log в текстовом редакторе. Это будет ужасно. Этот файл содержит много ненужной и повторяющейся информации; вот почему эти файлы такие большие. Например, если какое-то предупреждение связано с h-файлом, вы все равно увидите его столько раз, сколько этот h-файл включен в cpp-файлы. Когда вы используете PVS-Studio Standalone или plog-converter, эти инструменты автоматически удаляют такие дубликаты.
Допустим, вам нравится просматривать отчет в Qt Creator, преобразовывая файл * .log в формат Файл списка задач Qt. Затем мы должны использовать утилиту plog-converter следующим образом:
$ plog-converter -t tasklist -o /home/andr/gdb.tasks -r /home/andr/binutils-gdb/ -a GA:1,2,3 /home/andr/gdb.log
Хотя для начала лучше было бы использовать GA: 1,2. Начинать знакомство с анализатором с включения всех трех уровней предупреждений - не лучший вариант.
После того, как вы запустите эту команду, в домашнем каталоге появится файл отчета gdb.tasks, который можно просмотреть с помощью Qt Creator:
Просмотр опций конвертера:
$ plog-converter --help
Результаты анализа
Как я уже сказал, на этот раз мне удалось найти всего несколько ошибок, демонстрирующих возможности PVS-Studio. Причина этого - высокое качество исходного кода проекта GDB и тот факт, что он очень хорошо протестирован большим количеством пользователей, которые сами являются программистами, а это означает, что они более требовательны и внимательны, чем среднестатистические пользователи программы. .
Посмотрим, какие интересные ошибки мне удалось найти. Начнем с ошибки в функции сравнения. Я могу назвать это новым паттерном ошибок. Я сталкиваюсь с такими ошибками в большом количестве проектов и в ближайшее время планирую написать новую статью на эту тему, которая будет напоминать Эффект последней строчки.
Неправильная функция сравнения
static int
psymbol_compare (const void *addr1, const void *addr2, int length)
{
struct partial_symbol *sym1 = (struct partial_symbol *) addr1;
struct partial_symbol *sym2 = (struct partial_symbol *) addr2;
return (memcmp (&sym1->ginfo.value, &sym1->ginfo.value,
sizeof (sym1->ginfo.value)) == 0
&& sym1->ginfo.language == sym2->ginfo.language
&& PSYMBOL_DOMAIN (sym1) == PSYMBOL_DOMAIN (sym2)
&& PSYMBOL_CLASS (sym1) == PSYMBOL_CLASS (sym2)
&& sym1->ginfo.name == sym2->ginfo.name);
}
Предупреждение PVS-Studio: V549 Первый аргумент функции memcmp равен второму аргументу. psymtab.c 1580
Первый и второй аргументы - это функция memcmp (),, и они одинаковы. Видимо, программист хотел написать:
memcmp (&sym1->ginfo.value,
&sym2->ginfo.value,
sizeof (sym1->ginfo.value))
Неверный код, который работает правильно
Статические анализаторы кода работают с исходным кодом программ и могут находить такие фрагменты, которые, безусловно, являются ошибкой с точки зрения человека. Интересно, что, несмотря на эту ошибку в коде, и благодаря счастливой случайности, код может работать полностью корректно. Давайте посмотрим на один из этих интересных случаев.
struct event_location *
string_to_explicit_location (const char **argp, ....)
{
....
/* It is assumed that input beginning with '-' and a non-digit
character is an explicit location. "-p" is reserved, though,
for probe locations. */
if (argp == NULL
|| *argp == '\0'
|| *argp[0] != '-'
|| !isalpha ((*argp)[1])
|| ((*argp)[0] == '-' && (*argp)[1] == 'p'))
return NULL;
....
}
Предупреждение PVS-Studio: V528 Странно, что указатель на тип char сравнивается со значением \ 0. Вероятно имелось ввиду: ** argp == ‘\ 0’. location.c 527
Нас интересует следующий фрагмент кода:
.... const char **argp ....
if (argp == NULL
|| *argp == '\0'
|| *argp[0] != '-'
Литерал ‘\ 0’ - это конечный нуль, который используется, когда необходимо проверить, пуста ли строка или нет. Для этого программист проверяет первый элемент буфера, содержащий строку, и если есть конечный ноль, то строка считается пустой. Именно этого и хотел здесь программист. Но при этом не учитывалось, что переменная argp является не указателем на символы, а указателем на указатель.
Следовательно, правильная проверка должна быть такой:
*argp[0] == '\0'
Или вот так:
**argp == '\0'
Однако, если мы напишем такой код
if (argp == NULL
|| *argp[0] == '\0'
|| *argp[0] != '-'
тогда это опасно. Нам нужно добавить еще одну проверку к нулевому указателю:
if (argp == NULL
|| *argp == NULL
|| *argp[0] == '\0'
|| *argp[0] != '-'
Теперь код правильный. Но имейте в виду, что это избыточно. Если первый символ не является тире «-», то не имеет значения, какой это символ. Нет никакой разницы, есть ли терминальный нуль или любой другой символ. Вот почему мы можем упростить код следующим образом:
if (argp == NULL
|| *argp == NULL
|| *argp[0] != '-'
Обратите внимание на то, что этот правильный код эквивалентен исходному коду:
if (argp == NULL
|| *argp == '\0'
|| *argp[0] != '-'
Разница в том, как написано 0. В первом случае это NULL. Во втором - ‘\ 0’. В общем, это то же самое, и код ведет себя точно так же.
Достаточно забавно. Несмотря на то, что код был написан некорректно, он работает абсолютно корректно.
Неправильная оценка размера буфера
extern void
read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
void
java_value_print (....)
{
....
gdb_byte *buf;
buf = ((gdb_byte *)
alloca (gdbarch_ptr_bit (gdbarch) / HOST_CHAR_BIT));
....
read_memory (address, buf, sizeof (buf));
....
}
Предупреждение PVS-Studio: V579 Функция read_memory принимает в качестве аргументов указатель и его размер. Возможно, это ошибка. Изучите третий аргумент. jv-valprint.c 111
Эта ошибка, скорее всего, произошла во время рефакторинга. Рискну предположить, что в какой-то момент код был примерно таким:
gdb_byte buf[gdbarch_ptr_bit (gdbarch) / HOST_CHAR_BIT)]; .... read_memory (address, buf, sizeof (buf));
Оператор sizeof () правильно оценил размер буфера. Затем программист начал выделять память для буфера с помощью функции alloca (). В результате оператор sizeof (buf) оценивает не размер буфера, а размер указателя.
Думаю, правильный код должен быть таким:
gdb_byte *buf; const size_t size = gdbarch_ptr_bit (gdbarch) / HOST_CHAR_BIT; buf = ((gdb_byte *) alloca (size)); .... read_memory (address, buf, size);
Но это еще не все, самое смешное еще впереди. Я просто решил объяснить суть этой ошибки и способ ее появления. Все становится намного интереснее, если взглянуть на несколько строк кода:
read_memory (address, buf, sizeof (buf));
address += gdbarch_ptr_bit (gdbarch) / HOST_CHAR_BIT;
/* FIXME: cagney/2003-05-24: Bogus or what. It
pulls a host sized pointer out of the target and
then extracts that as an address (while assuming
that the address is unsigned)! */
element = extract_unsigned_integer (buf, sizeof (buf),
byte_order);
Как видите, я не первый, кто заметил, что с этим кодом что-то не так. Ошибка существует в этом коде по крайней мере с 2003 года. Действительно непонятно, почему она до сих пор не исправлена.
Насколько я понимаю, комментарий связан со строкой:
element = extract_unsigned_integer (buf, sizeof (buf),
byte_order);
При вызове функции extract_unsigned_integer () была сделана та же ошибка, что и описанная выше.
PVS-Studio выдает предупреждение для этой строки: V579 Функция extract_unsigned_integer получает указатель и его размер в качестве аргументов. Возможно, это ошибка. Изучите второй аргумент. jv-valprint.c 117
Анализатор выдает еще два предупреждения для кода функций java_value_print ():
- V579 Функция read_memory получает указатель и его размер в качестве аргументов. Возможно, это ошибка. Изучите третий аргумент. jv-valprint.c 123
- V579 Функция extract_unsigned_integer принимает указатель и его размер в качестве аргументов. Возможно, это ошибка. Изучите второй аргумент. jv-valprint.c 129
Двойное задание
FILE *
annotate_source (Source_File *sf, unsigned int max_width,
void (*annote) (char *, unsigned int, int, void *),
void *arg)
{
....
bfd_boolean new_line;
....
for (i = 0; i < nread; ++i)
{
if (new_line)
{
(*annote) (annotation, max_width, line_num, arg);
fputs (annotation, ofp);
++line_num;
new_line = FALSE;
}
new_line = (buf[i] == '\n');
fputc (buf[i], ofp);
}
....
}
Предупреждение PVS-Studio: V519 Переменной new_line дважды подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 253, 256. source.c 256
New_line string = FALSE; Нет смысла. Сразу после этого значение переменной new_line перезаписывается другим значением. Итак, этот фрагмент кода крайне подозрительный:
new_line = FALSE; } new_line = (buf[i] == '\n');
Видимо, здесь есть логическая ошибка. Или первое назначение просто избыточно, и мы можем его удалить.
Опечатка
int
handle_tracepoint_bkpts (struct thread_info *tinfo, CORE_ADDR stop_pc)
{
int ipa_trace_buffer_is_full;
CORE_ADDR ipa_stopping_tracepoint;
int ipa_expr_eval_result;
CORE_ADDR ipa_error_tracepoint;
....
if (ipa_trace_buffer_is_full)
trace_debug ("lib stopped due to full buffer.");
if (ipa_stopping_tracepoint)
trace_debug ("lib stopped due to tpoint");
if (ipa_stopping_tracepoint)
trace_debug ("lib stopped due to error");
....
}
Предупреждение PVS-Studio: V581 Условные выражения операторов if, расположенных рядом друг с другом, идентичны. Проверить строки: 4535, 4537. tracepoint.c 4537
Если переменная ipa_stopping_tracepoint имеет значение ИСТИНА, будут напечатаны два сообщения отладки:
lib stopped due to tpoint lib stopped due to error
Я не знаком с принципом работы кода, но похоже, что в последнем случае в условии следует использовать переменную ipa_error_tracepoint, а не ipa_stopping_tracepoint. Тогда код будет таким:
if (ipa_trace_buffer_is_full)
trace_debug ("lib stopped due to full buffer.");
if (ipa_stopping_tracepoint)
trace_debug ("lib stopped due to tpoint");
if (ipa_error_tracepoint)
trace_debug ("lib stopped due to error");
Заявление о забытом перерыве
Классическая ошибка. Оператор Break был забыт внутри переключателя в одном фрагменте.
static debug_type
stab_xcoff_builtin_type (void *dhandle, struct stab_handle *info,
int typenum)
{
....
switch (-typenum)
{
....
case 8:
name = "unsigned int";
rettype = debug_make_int_type (dhandle, 4, TRUE);
break;
case 9:
name = "unsigned";
rettype = debug_make_int_type (dhandle, 4, TRUE);
case 10:
name = "unsigned long";
rettype = debug_make_int_type (dhandle, 4, TRUE);
break;
....
}
....
}
Предупреждение PVS-Studio: V519 Переменной name два раза подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 3433, 3436. stabs.c 3436
Независимо от того, что мы работаем с «unsigned» или «unsigned long», мы присвоим типу имя «unsigned long».
Правильный код:
case 9: name = "unsigned"; rettype = debug_make_int_type (dhandle, 4, TRUE); break;
Сложный случай
В данном коде переменной alt дважды присваивается значение из-за отсутствия оператора break между двумя case. Но согласно комментарию , программист не использует break специально. Давайте взглянем на код, который меня сбивает.
static int
putop (const char *in_template, int sizeflag)
{
int alt = 0;
....
switch (*p)
{
....
case '{':
alt = 0;
if (intel_syntax)
{
while (*++p != '|')
if (*p == '}' || *p == '\0')
abort ();
}
/* Fall through. */
case 'I':
alt = 1;
continue;
....
}
}
Предупреждение PVS-Studio: V519 Переменной alt два раза подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 14098, 14107. i386-dis.c 14107
Итак, комментарий / * проваливается. * / говорит, что оператор break здесь вообще не нужен. Но тогда непонятно, почему переменной alt присваивается значение 0. В любом случае значение переменной заменяется единицей. Между этими двумя переменными назначение alt никоим образом не используется. Просто непонятно ...
Здесь либо логическая ошибка, либо нужно убрать первое присвоение.
Вывод
Да здравствует PVS-Studio для Linux! Как видите, теперь я могу не только показать преимущества статического анализа для открытых проектов Windows, но и помочь Linux-сообществу открывать программы. Думаю, что скоро наш список статей о проверенных проектах будет включать большое количество статей о программах из мира Linux.
Я также приглашаю вас подписаться на меня в Твиттере, чтобы вы не пропустили что-нибудь интересное @Code_Analysis.
Андрей Карпов
