Хабрахабр

Ложные срабатывания в PVS-Studio: как глубока кроличья нора

Единорог PVS-Studio и GetNamedSecurityInfo

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

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

Некоторые вопиющие ложные срабатывания мы замечаем сами, про некоторые нам пишут клиенты и бесплатные пользователи. Хотя ложные срабатывания не являются такой большой проблемой, как может показаться на первый взгляд, мы постоянно боремся с ними, совершенствуя диагностики.

Недавно один из наших клиентов написал письмо приблизительно следующего содержания:

Более того, анализатор ведёт себя странно и нестабильно на тестовом проекте: то выдаёт предупреждение, то не выдаёт. Анализатор почему-то говорит, что указатель всегда нулевой, хотя это явно не так. Синтетический пример, на котором воспроизводится ложное срабатывание:

#include <windows.h>
#include <aclapi.h>
#include <tchar.h> int main()
{ PACL pDACL = NULL; PSECURITY_DESCRIPTOR pSD = NULL; ::GetNamedSecurityInfo(_T("ObjectName"), SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, &pDACL, NULL, &pSD); auto test = pDACL == NULL; // V547 Expression 'pDACL == 0' is always true. return 0;
}

Я представляю, как выглядят подобны срабатывания со стороны наших пользователей. Сразу понятно, что функция GetNamedSecurityInfo меняет значение переменной pDACL. Неужели разработчики не смогли написать обработчик для таких простых ситуаций? Более того, непонятно, почему анализатор то выдаёт сообщение, то нет. Может быть, у них самих какой-то баг в инструменте, типа неинициализированный переменной?

Но что делать, я сам выбрал себе такую судьбу. Эх… Непростая работа — поддерживать статический анализатор кода. Засучив рукава, я приступил расследовать причину возникновения ложного срабатывания.

Вот описание 6-ого аргумента функции:
Для начала я изучил описание функции GetNamedSecurityInfo и убедился, что её вызов действительно должен привести к изменению значения переменной pDACL.

ppDacl

The returned pointer is valid only if you set the DACL_SECURITY_INFORMATION flag. A pointer to a variable that receives a pointer to the DACL in the returned security descriptor or NULL if the security descriptor has no DACL. Also, this parameter can be NULL if you do not need the DACL.

Я знаю, что анализатор PVS-Studio однозначно должен корректно обрабатывать такой простой код и не выдавать бессмысленное предупреждение. Уже в этот момент моя интуиция подсказала мне, что это будет необычный случай, на который придётся потратить время.

И так, и сяк, но анализатор молчит. Я утвердился в своих опасениях, когда не смог воспроизвести ложное срабатывание ни на текущей alpha-версии анализатора, ни на именно той версии, которая установлена у клиента.

Он сгенерировал, прислал файл, и я приступил к его детальному рассмотрению. Я попросил клиента прислать мне препроцессированный i-файл, сгенерированный для программы с синтетическим примером.

С одной стороны, это хорошо, так как баг воспроизведён. На присланном файле анализатор сразу выдал ложное срабатывание. С другой стороны, я испытал чувства, которые лучше всего описывает вот эта картинка.

шта

Я отлично знаю, как работает анализатор и диагностика V547. Почему именно эти? Ну не может быть такого срабатывания!

Ok, заварим чай и продолжим.

Вызов функции GetNamedSecurityInfo разворачивается в:

::GetNamedSecurityInfoW(L"ObjectName", SE_FILE_OBJECT, (0x00000004L), 0, 0, &pDACL, 0, &pSD);

Этот код одинаково выглядит как в моём собственном препроцессированном i-файле, так и в файле, присланном клиентом.

В своём файле я вижу: Хм… Хорошо, теперь изучим, как объявлена эта функция.

__declspec(dllimport)
DWORD
__stdcall
GetNamedSecurityInfoW( LPCWSTR pObjectName, SE_OBJECT_TYPE ObjectType, SECURITY_INFORMATION SecurityInfo, PSID * ppsidOwner, PSID * ppsidGroup, PACL * ppDacl, PACL * ppSacl, PSECURITY_DESCRIPTOR * ppSecurityDescriptor );

Всё логично, всё понятно. Ничего неожиданного.

Далее я заглядываю в файл клиента и…

шта???

Там я вижу что-то из параллельной реальности:

__declspec(dllimport)
DWORD
__stdcall GetNamedSecurityInfoW( LPCWSTR pObjectName, SE_OBJECT_TYPE ObjectType, SECURITY_INFORMATION SecurityInfo, const PSID * ppsidOwner, const PSID * ppsidGroup, const PACL * ppDacl, const PACL * ppSacl, PSECURITY_DESCRIPTOR * ppSecurityDescriptor );

Обратите внимание, что формальный аргумент ppDacl помечен как const.

WTF? WAT? WTF? WAT?

Откуда он здесь!? Что за const!?

По крайней мере сразу понятно, что анализатор здесь не виноват и я смогу защитить его честь.

Получается, что с точки зрения анализатора функция GetNamedSecurityInfoW не может менять объект, на который ссылается указатель. Аргумент является указателем на константный объект. Следовательно, здесь:

PACL pDACL = NULL;
PSECURITY_DESCRIPTOR pSD = NULL;
::GetNamedSecurityInfo(_T("ObjectName"), SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, &pDACL, NULL, &pSD);
auto test = pDACL == NULL; // V547 Expression 'pDACL == 0' is always true.

переменная pDACL не может измениться и анализатор выдаёт обоснованное предупреждение (Expression 'pDACL == 0' is always true.).

Зато непонятно, откуда взялся этот const. Почему выдаётся предупреждение — понятно. Его просто не может там быть!

Оказывается, существует старый неправильный файл aclapi.h с некорректным описанием функции. Впрочем, есть догадка, и она подтверждается поисками в интернете. Также я нашёл в интернете две интересные ссылки:

Итак, жило-было в файле aclapi.h (6.0.6002.18005-Windows 6.0) вот такое описание функции:

WINADVAPI
DWORD
WINAPI
GetNamedSecurityInfoW( __in LPWSTR pObjectName, __in SE_OBJECT_TYPE ObjectType, __in SECURITY_INFORMATION SecurityInfo, __out_opt PSID * ppsidOwner, __out_opt PSID * ppsidGroup, __out_opt PACL * ppDacl, __out_opt PACL * ppSacl, __out_opt PSECURITY_DESCRIPTOR * ppSecurityDescriptor );

Затем кто-то захотел исправить тип формального аргумента pObjectName, но попутно испортил типы указателей, вписав const. И aclapi.h (6.1.7601.23418-Windows 7.0) стал таким:

WINADVAPI
DWORD
WINAPI
GetNamedSecurityInfoW( __in LPCWSTR pObjectName, __in SE_OBJECT_TYPE ObjectType, __in SECURITY_INFORMATION SecurityInfo, __out_opt const PSID * ppsidOwner, __out_opt const PSID * ppsidGroup, __out_opt const PACL * ppDacl, __out_opt const PACL * ppSacl, __out PSECURITY_DESCRIPTOR * ppSecurityDescriptor );

diff 1

Позднее в переписке он подтвердил эту гипотезу. Становится понятным, что именно подобный неправильный файл aclapi.h и используется у клиента. У меня используется более свежая версия, поэтому ошибка и не воспроизводилась.

3. Вот как выглядит уже исправленное описание функции в aclapi.h (6. 17415-Windows_8. 9600. 1).

WINADVAPI
DWORD
WINAPI
GetNamedSecurityInfoW( _In_ LPCWSTR pObjectName, _In_ SE_OBJECT_TYPE ObjectType, _In_ SECURITY_INFORMATION SecurityInfo, _Out_opt_ PSID * ppsidOwner, _Out_opt_ PSID * ppsidGroup, _Out_opt_ PACL * ppDacl, _Out_opt_ PACL * ppSacl, _Out_ PSECURITY_DESCRIPTOR * ppSecurityDescriptor );

diff 2

Всё вернулось на свои места, но в мире пока продолжают жить заголовочные файлы с неправильным объявлением функции. Тип аргумента pObjectName остался прежним, а вот лишние const убрали.

Он рад и доволен, что ситуация прояснилась. Всё это я рассказываю клиенту. Более того, он находит причину, почему он то видит ложное срабатывание, то нет:

В тестовом проекте Debug конфигурация настроена на Platform Toolset по умолчанию для Visual Studio 2017 — «Visual Studio 2017 (v141)», а вот Release конфигурация настроена на «Visual Studio 2015 — Windows XP (v140_xp)». Я забыл, что я когда-то на этом тестовом проекте экспериментировал с тулсетами. Вчера я просто в какой-то момент менял конфигурации, и предупреждение то появлялось, то исчезало.

В расследовании можно ставить точку. Всё. Главное, что теперь ситуация понятна. С клиентом решаем, что не будем делать какую-то специальную подпорку в анализаторе, чтобы он учитывал этот баг в заголовочном файле. Как говорится, «дело закрыто».

Заключение

Конкретно в данном случае излишняя интеллектуальность анализатора привела к тому, что из-за неправильного описания функции он начал выдавать ложное срабатывание. Анализатор PVS-Studio — сложный программный продукт, собирающий из кода программы множество информации и использующий её для различных технологий анализа.

Становитесь нашими клиентами, и вы будете получать быструю качественную поддержку от меня и моих коллег.

False Positives in PVS-Studio: How Deep the Rabbit Hole Goes. Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov.

Теги
Показать больше

Похожие статьи

Добавить комментарий

Ваш e-mail не будет опубликован. Обязательные поля помечены *

Кнопка «Наверх»
Закрыть