Время от времени читатели наших статей по проверке open-source проектов замечают, что статический анализатор кода PVS-Studio выявляет большой процент ошибок, которые незначительны или вообще не влияют на работу приложения.
Это верно.
Большинство важных ошибок уже исправлено благодаря ручному тестированию, отзывам пользователей и другим дорогостоящим методам.
При этом многие из этих ошибок можно было обнаружить еще на этапе кодирования и исправить с минимальными потерями времени, репутации и денег.
В этой статье будет приведено несколько примеров реальных ошибок, которые были бы немедленно исправлены, если бы авторы проектов использовали статический анализ кода.
Идея очень проста.
Давайте посмотрим на GitHub примеры пул-реквестов, в комментариях к которым указано, что это исправление ошибки.
И попробуем найти эти ошибки с помощью статического анализатора кода PVS-Studio. Если исправленная ошибка обнаружена анализатором, это означает, что ее можно было исправить еще на этапе кодирования.
И чем раньше ошибка будет исправлена, тем дешевле это обойдется.
К сожалению, GitHub нас подвел и не позволил сделать отличную-прекрасную статью на эту тему.
В самом GitHub также есть глюк (или функция), который не позволяет искать комментарии в пул-реквестах в проектах, написанных только на определенных языках программирования.
Ну, или я не «умею его готовить».
Независимо от того, указываю ли я поиск комментариев в проектах C++, C# или Java, результаты возвращаются для всех языков, включая PHP, Python, JavaScript и т. д. В результате поиск подходящих кейсов оказался крайне утомительным занятием, и я ограничусь лишь несколькими примерами.
Однако их достаточно, чтобы продемонстрировать полезность инструментов статического анализа кода при регулярном использовании.
Что, если ошибка была обнаружена раньше? Ответ прост: программистам не нужно было бы ждать, пока она проявится, а затем искать и исправлять дефектный код. Давайте посмотрим на ошибки, которые PVS-Studio смог сразу обнаружить: Первый пример взято из проекта SatisfactoryModLoader. До исправления ошибки код выглядел так:
В этом коде была ошибка, о которой PVS-Studio сразу же выдавал предупреждение: В591 Непустая функция должна возвращать значение.// gets an API function from the mod handler SML_API PVOID getAPIFunction(std::string name) { bool found = false; for (Registry reg : modHandler.APIRegistry) { if (reg.name == name) { found = true; } } if (!found) { std::string msg = .; MessageBoxA(NULL, msg.c_str(), "SatisfactoryModLoader Fatal Error", MB_ICONERROR); abort(); } }
ModFunctions.cpp 44 Вышеупомянутая функция не имеет возвращаться , поэтому он вернет формально неопределенное значение.
Программист не использовал анализатор кода, поэтому ему пришлось искать ошибку самому.
Функция после редактирования: // gets an API function from the mod handler
SML_API PVOID getAPIFunction(std::string name) {
bool found = false;
PVOID func = NULL;
for (Registry reg : modHandler.APIRegistry) {
if (reg.name == name) {
func = reg.func;
found = true;
}
}
if (!found) {
std::string msg = .
;
MessageBoxA(NULL,
msg.c_str(),
"SatisfactoryModLoader Fatal Error",
MB_ICONERROR);
abort();
}
return func;
}
Любопытно, что в коммите автор указал ошибку как критическую: " зафиксированный критическая ошибка где функции API не были возвращены ".
В секунду совершить Из истории проекта mc6809 были внесены исправления в следующий код: void mc6809dis_direct(
mc6809dis__t *const dis,
mc6809__t *const cpu,
const char *const op,
const bool b16
)
{
assert(dis != NULL);
assert(op != NULL);
addr.b[MSB] = cpu->dp;
addr.b[LSB] = (*dis->read)(dis, dis->next++);
.
if (cpu != NULL) { .
}
}
Автор исправил только одну строчку.
Он заменил выражение addr.b[MSB] = cpu->dp;
к выражению addr.b[MSB] = cpu != NULL ? cpu->dp : 0;
В старой версии кода никакой проверки не проводилось.
Процессор для равенства нулевому указателю.
Если вдруг окажется, что в качестве второго аргумента функции mc6809dis_direct Если передан нулевой указатель, он будет разыменован в теле функции.
Результат грустно и непредсказуемо .
Разыменование нулевого указателя — один из наиболее распространенных шаблонов, о которых нам говорят: «Это не критическая ошибка.
Ну оно в коде живет и живет, а если произойдет разыменование, то программа спокойно упадет и все».
Странно и грустно слышать это от программистов C++, но жизнь есть жизнь.
В любом случае, в этом проекте такое разыменование превратилось в баг, о чем нам сообщает заголовок коммита: " Исправлена ошибка: разыменование NULL. ".
Если бы разработчик проекта использовал PVS-Studio, он смог бы проверить свой код два с половиной месяца назад (именно столько времени прошло с момента появления ошибки) и обнаружить предупреждение: В595 Указатель «cpu» использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 1814, 1821. mc6809dis.c 1814. Таким образом, ошибка будет устранена в момент ее появления, что сэкономит время и, возможно, нервы разработчика :).
Еще один интересный пример правки был найден в проекте libmorton.
Исправленный код: template<typename morton>
inline bool findFirstSetBitZeroIdx(const morton x,
unsigned long* firstbit_location)
{
#if _MSC_VER && !_WIN64
// 32 BIT on 32 BIT
if (sizeof(morton) <= 4) {
return _BitScanReverse(firstbit_location, x) != 0;
}
// 64 BIT on 32 BIT
else {
*firstbit_location = 0;
if (_BitScanReverse(firstbit_location, (x >> 32))) { // check first part
firstbit_location += 32;
return true;
}
return _BitScanReverse(firstbit_location, (x & 0xFFFFFFFF)) != 0;
}
#elif _MSC_VER && _WIN64
.
#elif __GNUC__ .
#endif
}
В своей правке программист заменяет выражение " firstbit_location += 32 " на " * firstbit_location += 32 ".
Программист ожидал, что число 32 будет добавлено к значению переменной, к которой привязан указатель firstbit_location , но он был добавлен непосредственно к указателю.
Измененное значение указателя больше нигде не использовалось, а ожидаемое значение переменной осталось неизменным.
PVS-Studio выдаст предупреждение для этого кода: В1001 Переменная firstbit_location назначается, но не используется в конце функции.
morton_common.h 22 Казалось бы, что может быть страшного в выражении, которое было изменено, но в дальнейшем не использовалось? Диагностика V1001 явно не похожа на то, что она предназначена для обнаружения особо опасных ошибок.
Несмотря на это, она обнаружила важную ошибку, которая повлияла на логику программы.
Более того, оказалось, что эту ошибку найти не так-то просто! Он не только был в программе с самого момента создания файла, но и пережил множество правок в соседних строках и просуществовал в проекте аж 3( ! ) года! Все это время логика программы была нарушена, и она работала не совсем так, как ожидали разработчики.
Если бы они использовали PVS-Studio, ошибка была бы обнаружена гораздо раньше.
Наконец, давайте посмотрим на еще один красивый пример.
Пока я собирал исправления ошибок на GitHub, я несколько раз натыкался на исправление с помощью с этим контентом .
Исправленная ошибка была здесь: int kvm_arch_prepare_memory_region(.
) { .
do { struct vm_area_struct *vma = find_vma(current->mm, hva); hva_t vm_start, vm_end; .
if (vma->vm_flags & VM_PFNMAP) { .
phys_addr_t pa = (vma->vm_pgoff << PAGE_SHIFT) + vm_start - vma->vm_start; .
} .
} while (hva < reg_end); .
}
PVS-Studio выдал предупреждение на этот участок кода: В629 Рассмотрите возможность проверки файла 'vma-> vm_pgoff << 12' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. mmu.c 1795
Я посмотрел на объявления переменных, используемые в выражении " phys_addr_t pa = (vma-> vm_pgoff << PAGE_SHIFT) + vm_start — vma-> vm_start; ", и обнаружил, что приведенный выше код похож на следующий синтетический пример: void foo(unsigned long a, unsigned long b)
{
unsigned long long x = (a << 12) + b;
}
Если значение 32-битной переменной а больше, чем 0xFFFFF , то 12 старших битов будут иметь хотя бы одно ненулевое значение.
После применения операции сдвига влево к этой переменной эти значащие биты будут потеряны, что приведет к Икс Неверная информация будет записана.
Чтобы исключить потерю старших битов, необходимо сначала произнести а печатать беззнаковый длинный длинный и только после этого выполнить операцию сдвига: pa = (phys_addr_t)vma->vm_pgoff << PAGE_SHIFT;
pa += vm_start - vma->vm_start;
Затем в год правильное значение всегда будет записано.
Все бы ничего, но этот баг, как и первый пример из статьи, тоже оказался критическим, о чем автор коммита написал отдельно в своем комментарии.
Более того, эта ошибка появилась в огромном количестве проектов.
Чтобы в полной мере оценить масштаб трагедии, Я предлагаю вам взглянуть по количеству результатов при поиске этого исправления на GitHub. Страшно, не так ли? 
Итак, я применил новый подход, чтобы продемонстрировать преимущества регулярного использования статического анализатора кода.
Надеюсь, вам понравилось.
Скачайте и попробуйте Статический анализатор кода PVS-Studio для проверки собственных проектов.
На момент написания в нем реализовано около 700 диагностических правил для обнаружения самых разных шаблонов ошибок.
Поддерживаемые языки: C, C++, C# и Java. 
Если вы хотите поделиться этой статьей с англоязычной аудиторией, воспользуйтесь ссылкой для перевода: Георгий Грибков.
Ошибки, которые не находит статический анализ кода, поскольку он не используется.
Теги: #github #программирование #C++ #pvs-studio #bugs #статический анализ кода #статический анализатор кода
-
Ближе К Людям.
19 Oct, 24 -
Вы Доверяете Своему Провайдеру?
19 Oct, 24 -
Озера И Океаны На Xkcd
19 Oct, 24 -
Топ-100 Ведущих Веб-Студий России-2011
19 Oct, 24
