Нам регулярно поступают предложения и рекомендации по улучшению диагностических возможностей анализатора. Большинство предложений мы помещаем во внутренний todo-список и со временем реализовываем. Мы благодарны нашим пользователям за присылаемые отзывы и предложения и пользуясь случаям, я хочу сказать им спасибо. Спасибо! Итак, многое мы реализуем, но не всё. Дело в том, что некоторые виды предлагаемых диагностик не укладываются в философию разрабатываемого нами анализатора PVS-Studio. Чтобы нашим пользователям была понятна наша позиция и как мы приходим к заключению, что можно реализовывать, а что нет, я решил написать эту подробную заметку.
Есть два философских подхода в реализации статических анализаторов кода:
Первый подход позволяет обнаружить больше ошибок, чем второй. Однако, мы считаем, что это путь в никуда, так как количество ложных срабатываний превращает анализатор в инструмент, которым невозможно пользоваться. Разрабатывая PVS-Studio, мы придерживаемся второго подхода и ругаемся только в случае, когда для этого есть основания или подозрения.
К нам поступают предложения по диагностикам следующего вида:
По отдельности каждое из таких предупреждений выглядит разумным и полезным. Но вместе они убьют анализатор кода. Каждая диагностика, реализованная подобным образом, порождает большое количество ложных срабатываний. Конечно, если реализовать только одну диагностику, беды не случится. Однако, если мы реализуем поиск делений, в которых не уверены, то почему мы должны не реализовывать поиск недостоверных sqrt? Нет границы или критерия где надо остановиться, реализуя подобные диагностики.
Здесь проявляет себя теория разбитых окон. Стоит сделать пару диагностик в духе философии "ругаемся на всё, про что не можем сказать, что оно правильное" и процесс будет необратим. Подобные диагностики открывают ящик Пандоры. Например, непонятно как отказаться от реализации поиска сложения двух переменных "A+B" типа int, если нет уверенности, что не возникнет переполнение. И так далее и так далее.
Идя по этому пути, анализатор начнёт выдавать предупреждение на код:
void Foo(int &i)
{
i++; // Возможно переполнение.
}Формально всё верно и диагностика полезна. Переполнение переменной типа int приводит к неопределённому поведению. Анализатор должен ругаться, если не может убедиться, что безопасны все диапазоны значения переменной, переданной в функцию.
В итоге, мы получим анализатор, который ругается на каждую пятую строчку в программе и этот анализатор можно смело выбросить на свалку.
Читатель может возразить: вы преувеличиваете и доводите до абсурда, давайте сделаем диагностику, которая ругается на использование непроверенного указателя и остановимся.
В том-то всё и дело, что остановиться не получится. Если мы сделаем для одного пользователя поиск указателей, то как мы обоснуем другому пользователю, что не будем искать непроверенные деления? Разыменовывание нулевого указателя не более серьезно, чем деление на 0.
Означает ли это, что анализатор PVS-Studio не ищет разыменование нулевых указателей или деление на ноль? Конечно же ищет.
Но поиск ошибок реализован согласно философии "ругаемся на то, которое по какой-то причине скорее всего неправильное". Другими словами, должны существовать некие признаки, которые указывают на то, что код может содержать ошибку.
Давайте рассмотрим это на примерах.
void F(int *P)
{
*P = 1;
}Анализатор PVS-Studio не ругается на этот код, так как у него нет для этого оснований. То, что указатель P используется без проверки, вовсе не означает, что программа содержит ошибку.
Анализатору нужна дополнительная информация, которая прямо или косвенно будет указывать на ошибку. Самый очевидный случай, когда анализатор заметит явный некорректный вызов такой функции.
void F(int *P)
{
*P = 1;
}
void Foo()
{
F(0);
}PVS-Studio: V522 Dereferencing of the null pointer 'P' might take place. The null pointer is passed into 'F' function. Inspect the first argument. Check lines: 'simple.cpp:69'. simple.cpp 64
Здесь все понятно, раз явно передаём в функцию NULL, то это ошибка. Конечно, на практике подобный код практически не встречается, поэтому рассмотрим что-то более приближённое к реальности:
void F(int *P)
{
*P = 1;
}
void Foo()
{
int *X = (int *)malloc(sizeof(int));
F(X);
free(X);
}PVS-Studio: V522 Dereferencing of the null pointer 'P' might take place. The potential null pointer is passed into 'F' function. Inspect the first argument. Check lines: 'simple.cpp:70'. simple.cpp 64
Анализатор вновь выдал предупреждение, но теперь речь идёт о "потенциально нулевом указателе". Имеется в виду, что функция malloc может вернуть NULL, и возвращаемый указатель надо обязательно проверить перед использованием.
Это хороший пример, когда анализатор использует дополнительную информацию, чтобы ругаться, только когда код действительно опасен.
Например, если мы изменим код следующим образом, то анализатор вновь будет молчать:
void F(int *P)
{
*P = 1;
}
void Foo()
{
int *X = (int *)malloc(sizeof(int));
if (!X)
return;
F(X);
free(X);
}Теперь все хорошо и предупреждений нет.
А есть ли другие ситуации, когда анализатор выдаст предупреждения? Да. Для демонстрации я выбрал короткий пример из списка ошибок, найденных в открытых проектах с помощью диагностики V595.
FName UKismetNodeHelperLibrary::GetEnumeratorName(
const UEnum* Enum, uint8 EnumeratorValue)
{
int32 EnumeratorIndex = Enum->GetIndexByValue(EnumeratorValue);
return (NULL != Enum) ?
Enum->GetEnum(EnumeratorIndex) : NAME_None;
}PVS-Studio: V595 The 'Enum' pointer was utilized before it was verified against nullptr. Check lines: 146, 147. kismetnodehelperlibrary.cpp 146
Что заставляет анализатор PVS-Studio выдать предупреждение на разыменование указателя? То, что после разыменования этот указатель проверяется на равенство NULL. Наличие такой проверки является поводом выдать предупреждение.
Рассмотрим другой пример.
void F(char *A, char *B)
{
strcpy(A, B);
}Следует выдавать здесь предупреждение? С нашей точки зрения нет. Использование функции strcpy само по себе не является ошибкой.
Если хочется проверить корректность вызова всех таких функций, то анализатор PVS-Studio тут ни при чём. Можно просто сделать поиск в программе всех strcpy и изучить соответствующий код.
Конечно, искать вручную вызов функций strcpy и аналогичных не удобно. Поэтому Visual C++ предупреждает о наличии таких функций и предлагает заменить их на безопасные аналоги. Для приведённого выше кода, он выдаст:
warning C4996: 'strcpy': This function or variable may be unsafe. Consider using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
Когда анализатор PVS-Studio выдаёт предупреждение на strcpy? Когда для этого есть повод. Пример:
void F()
{
size_t bad_buf_size = strlen("string"); // забыли + 1
char *A = (char *)malloc(bad_buf_size);
if (A)
strcpy(A, "string");
free(A);
}V512 A call of the 'strcpy' function will lead to overflow of the buffer 'A'. consoleapplication1.cpp 14
Надеюсь мне удалось пояснить нашу философию. Исходя из неё, мы делим предложенные диагностики на две категории:
Важно не выдать как можно больше предупреждений. В этом как раз нет какой-то особенной заслуги, так как не понятно, как все эти предупреждения потом просматривать и что с ними делать. Важно помочь пользователю найти ошибки, и мы совершенствуем PVS-Studio в этом направлении.