Проверка проекта Samba с помощью PVS-Studio под Linux

Если вы следили за новостями о последних разработках в области инструментов анализа C/C++ кода, то, должно быть, слышали про инструмент PVS-Studio. Я узнал о нем благодаря статьям, которые разработчики публикуют на своем сайте и в которых они рассказывают о проверках проектов с открытым кодом. К настоящему времени уже проверено внушительное число проектов, включая ядро Linux, Qt, Unreal и т.д., и каждый раз им удается находить интересные ошибки, подолгу живущие в коде, никем не обнаруженные. Опечатки, неаккуратное копирование, неопределенное поведение, бессмысленный код, синтаксические ошибки, которые чудесным образом пропускаются компилятором…

Как сказал Джон Кармак, "Все, что является допустимым с точки зрения синтаксиса и пропускается компилятором, в конце концов окажется в вашей кодовой базе".

К сожалению, в качестве поддерживаемой операционной системы заявлена только Windows. Инструмент доступен в виде плагина для Visual Studio или в качестве отдельного приложения, если у вас не установлена Visual Studio. Мой первый опыт работы с анализатором пришелся на 2014 год, когда я проверял с его помощью относительно объемную базу C++ кода, которая использовалась для внутренних нужд лабораторией компьютерной графики в моем университете в Лионе (LIRIS). Разработку мы тогда вели в Visual Studio (обычно я им не пользуюсь), и я подумал, почему бы не попробовать анализатор в деле. Я остался доволен результатами и продолжил следить за появлением новых статей на сайте PVS-Studio.

Два года спустя (за это время вышло несколько статей PVS-Studio) я начал работу над проектом Samba. Его суммарный размер — около 2 миллионов строк кода на языке C, и я подумал, что было бы интересно проверить его с помощью PVS-Studio. В коде статического анализатора в принципе не должно быть много платформенно-зависимых участков, так что я стал искать способ, как реализовать такую проверку. Анализатор работает с препроцессированным кодом, поэтому ему необходимо прогнать исходный код через препроцессор, а для этого ему нужно знать обо всех флагах препроцессора, макросах и путях включаемых файлов. Автоматический сбор таких данных может оказаться трудоемкой задачей. Для ее решения я написал небольшой скрипт на основе strace, чтобы он отслеживал вызовы компилятора — в этом случае можно осуществлять проверку независимо от используемого инструмента сборки. Последнюю версию моего решения можно найти на github.

Я отправил этот скрипт разработчикам PVS-Studio, и после небольшой переписки они прислали мне экспериментальную сборку PVS-Studio под Linux (за что им еще раз спасибо!). Теперь скрипт осуществляет все этапы анализа: от сбора информации о ключах компиляции до непосредственно анализа, отображения и фильтрации результатов.

А теперь давайте разберемся, как работать с этим скриптом.

Чтобы не пришлось при каждом запуске указывать расположение файла лицензии и исполняемого файла, можно настроить переменные окружения.

$ export PVS_LICENSE=~/prog/pvs/PVS-Studio.lic $ export PVS_BIN=~/prog/pvs/PVS-Studio

Перейдите в директорию проекта и сгенерируйте файл конфигурации для своего C++11-проекта.

$ pvs-tool genconf  -l C++11 pvs.cfg

При необходимости настройте параметры сборки до начала компиляции, после чего включите отслеживание текущей сборки (команду сборки следует указать после символов —).

$ pvs-tool trace    -- make -j8

После этого будет сгенерирован файл «strace_out», в котором содержится вся необходимая информация. На этапе анализа из этого файла будут извлечены все единицы компиляции и флаги препроцессора, после чего они будут проверены в PVS-Studio.

$ pvs-tool analyze  pvs.cfg pvs-tool: deleting existing log pvs.log... 001/061 [ 0%] analyzing /hom../rtags/src/ClangIndexer.cpp... 002/061 [ 1%] analyzing /hom../rtags/src/CompilerManager.cpp... 003/061 [ 3%] analyzing /hom../rtags/src/CompletionThread.cpp... 004/061 [ 4%] analyzing /hom../rtags/src/DependenciesJob.cpp... <...> 061/061 [98%] analyzing /hom../rtags/src/rp.cpp... pvs-tool: analysis finished pvs-tool: cleaning output... pvs-tool: done (2M -> 0M)

На этапе очистки скрипт удалит дублированные строки, что значительно сократит размер файла с результатами анализа.

Теперь можно просматривать результаты, сгруппированные по файлам, к которым они относятся:

$ pvs-tool view     pvs.log

Формат выходного файла аналогичен формату gcc/make, то есть с ним можно работать «как есть». Например, его можно открыть в редакторе Emacs и применять к нему стандартные встроенные функции перехода к ошибкам (goto-error). Также можно выключать отдельные диагностики, например:

$ pvs-tool view -d V2006,V2008 pvs.log

По умолчанию отображаются предупреждения только 1 уровня, но этот параметр можно изменить с помощью команды -l.

Подробная справка вызывается командой -h.

PVS-Studio нашел много проблемных участков в Samba. Большинство из них оказались ложными срабатываниями, но это неизбежно при проверке объемной кодовой базы, каким бы анализатором вы ни пользовались. Важно то, что были найдены и настоящие ошибки. Ниже я приведу наиболее интересные из них, а также способы их исправления в формате diff-патчей.

- if (memcmp(u0, _u0, sizeof(u0) != 0)) { + if (memcmp(u0, _u0, sizeof(*u0)) != 0) {    printf("USER_MODALS_INFO_0 struct has changed!!!!\n");    return -1;   }

В этом примере неправильно поставлена закрывающая скобка. Результат сравнения sizeof с нулём был использован в качестве размера памяти для функции memcmp (всегда 1 байт). Также нас интересует размер типа, на который указывает указатель u0, а не размер самого указателя.

   handle_main_input(regedit, key);    update_panels();    doupdate(); - } while (key != 'q' || key == 'Q'); + } while (key != 'q' && key != 'Q');

В данном коде необходимо выйти из цикла, если встретится буква ‘q’ в любом регистре.

  uid = request->data.auth.uid;   - if (uid < 0) { + if (uid == (uid_t)-1) {    DEBUG(1,("invalid uid: '%u'\n", (unsigned int)uid));    return -1;   }

Здесь переменная типа uid_t проверяется на отрицательное значение.

Знак переменной типа uid_t не определен в POSIX. Он определён как беззнаковый тип размером 32 бита на Linux, следовательно, проверка < 0 всегда ложна.

В случае с беззнаковой версией типа uid_t компилятор в сравнении uid == -1 всегда будет неявно приводить -1 к беззнаковому типу, в результате чего сравнение как со знаковым, так и с беззнаковым типом uid_t всегда будет давать верный результат. Я сделал преобразование явным: в нашем случае чем меньше магии, тем лучше.

  DEBUG(4,("smb_pam_auth: PAM: Authenticate User: %s\n", user));   - pam_error = pam_authenticate(pamh, PAM_SILENT | -   allow_null_passwords ? 0 : PAM_DISALLOW_NULL_AUTHTOK); + pam_error = pam_authenticate(pamh, PAM_SILENT | +  (allow_null_passwords ? 0 : PAM_DISALLOW_NULL_AUTHTOK));   switch( pam_error ){    case PAM_AUTH_ERR:     DEBUG(2, ("smb_pam_auth: PAM: ....", user));

Обычная путаница с приоритетом операторов.

  gensec_init();   dump_args();   - if (check_arg_numeric("ibs") == 0 || -     check_arg_numeric("ibs") == 0) { + if (check_arg_numeric("ibs") == 0 || +     check_arg_numeric("obs") == 0) {    fprintf(stderr, "%s: block sizes must be greater that zero\n",      PROGNAME);    exit(SYNTAX_EXIT_CODE);

В этом примере дважды проверялось одно и то же.

   if (!gss_oid_equal(&name1->gn_type, &name2->gn_type)) {     *name_equal = 0;    } else if (name1->gn_value.length != name2->gn_value.length || -      memcmp(name1->gn_value.value, name1->gn_value.value, +      memcmp(name1->gn_value.value, name2->gn_value.value,     name1->gn_value.length)) {     *name_equal = 0;    }

В этом коде функция memcmp вызывается с одним и тем же указателем в качестве ее параметров, в результате чего один и тот же блок памяти сравнивается сам с собой.

  ioctl_arg.fd = src_fd;   ioctl_arg.transid = 0;   ioctl_arg.flags = (rw == false) ? BTRFS_SUBVOL_RDONLY : 0; - memset(ioctl_arg.unused, 0, ARRAY_SIZE(ioctl_arg.unused)); + memset(ioctl_arg.unused, 0, sizeof(ioctl_arg.unused));   len = strlcpy(ioctl_arg.name, dest_subvolume,          ARRAY_SIZE(ioctl_arg.name));   if (len >= ARRAY_SIZE(ioctl_arg.name)) {

Здесь в качестве параметра memset было передано количество элементов массива, а не размер в байтах.

  if (n + IDR_BITS < 31 && -     ((id & ~(~0 << MAX_ID_SHIFT)) >> (n + IDR_BITS))) { +     ((id & ~(~0U << MAX_ID_SHIFT)) >> (n + IDR_BITS))) {    return NULL;   }

Использование отрицательных значений в качестве левого операнда операции сдвига влево ведет к неопределенному поведению согласно стандарту языка C.

  if (cli_api(cli,         param, sizeof(param), 1024, /* Param, length, maxlen */ -       data, soffset, sizeof(data), /* data, length, maxlen */ +       data, soffset, data_size, /* data, length, maxlen */         &rparam, &rprcnt,   /* return params, length */         &rdata, &rdrcnt))   /* return data, length */   {

В этом примере мы имеем дело с данными, которые когда-то были массивом, выделенным на стеке, но затем превратились в буфер, выделенный в куче, в то время как операцию sizeof подправить забыли.

   goto query;   }   - if ((p->auth.auth_type != DCERPC_AUTH_TYPE_NTLMSSP) || -     (p->auth.auth_type != DCERPC_AUTH_TYPE_KRB5) || -     (p->auth.auth_type != DCERPC_AUTH_TYPE_SPNEGO)) { + if (!((p->auth.auth_type == DCERPC_AUTH_TYPE_NTLMSSP) || +       (p->auth.auth_type == DCERPC_AUTH_TYPE_KRB5) || +       (p->auth.auth_type == DCERPC_AUTH_TYPE_SPNEGO))) {    return NT_STATUS_ACCESS_DENIED;   }

До исправления условие всегда было истинным и функция всегда возвращала статус «доступ запрещен».

- Py_RETURN_NONE;   talloc_free(frame); + Py_RETURN_NONE; }

Py_RETURN_NONE — это макрос, который скрывает оператор return. В данном фрагменте, где используется привязка к коду на Python, многие функции возвращали результат прежде, чем освобождалась динамически выделенная память. Эта проблема встречалась в десятках функций.

  int i; - for (i=0;ARRAY_SIZE(results);i++) { + for (i=0;i<ARRAY_SIZE(results);i++) {    if (results[i].res == res) return results[i].name;   }   return "*";

В данном коде условие оператора for всегда оказывалось истинным.

 int create_unlink_tmp(const char *dir)  { + if (!dir) { +  dir = tmpdir(); + } +   size_t len = strlen(dir);   char fname[len+25];   int fd;   mode_t mask;   - if (!dir) { -  dir = tmpdir(); - } -

А здесь указатель dir использовался перед проверкой на ноль.

В целом я остался доволен анализатором PVS-Studio и охотно рекомендую его к использованию. К сожалению, официально он недоступен под Linux, но, как я понимаю, достаточно просто написать разработчикам, и они помогут вам с настройкой под эту операционную систему 🙂

ссылка на оригинал статьи https://habrahabr.ru/post/280856/