Если вы следили за новостями о последних разработках в области инструментов анализа 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: %sn", 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 zeron",
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, но, как я понимаю, достаточно просто написать разработчикам, и они помогут вам с настройкой под эту операционную систему :)
Автор: PVS-Studio