Очередная история, как непросто программам взаимодействовать с внешним миром. На первый взгляд, у статического анализатора никаких проблем быть не должно. Он получает на вход файлы, дополнительную информацию и должен сгенерировать отчёт. Но как всегда, дьявол кроется в деталях.
Я считаю PVS-Studio очень качественным продуктом. Мы можем почти в любой день сделать и выложить дистрибутив. У нас используется очень большое количество автоматизированных тестов различного уровня и типов. Вот описание некоторых из них: "Как мы тестируем анализатор кода". Сейчас их стало больше. Например, теперь для статического анализа мы используем не только свой собственный анализатор, но и Clang. Если исправленная версия прошла все тесты, значит ее можно смело выдавать пользователям.
К сожалению, вся красота и надежность внутреннего кода иногда разваливается из-за воздействий враждебной окружающей среды. В результате все впечатление от продукта портится. Вроде и не мы виноваты, но не работает-то именно наш продукт. Я могу привести большое количество примеров. Первое что вспоминается:
Сейчас я расскажу очередную такую историю, как легко испортить впечатление о нашем продукте, будучи невиновными.
Один из потенциальных пользователей прислал вопрос о странном поведении PVS-Studio:
Мы сейчас гоняем триальную версию и раздумываем о покупке полной. Однако при анализе наткнулись на один нюанс, ставящий справедливость выводов под сомнение.
Ниже – скриншот, на котором видно ошибку.
filePath и cachePath отмечены как неиспользуемые (предупреждение V808), хотя видно, что они используются буквально в следующей строке после объявления.
Могли бы Вы объяснить подобное поведение анализатора?
На скриншоте можно видеть код следующего вида (код изменён):
std::string Foo()
{
std::string filePath(MAX_PATH + 1, 0);
std::string cachePath = "d:\\tmp";
if (!GetTempFileName(cachePath.c_str(), "tmp", 0,
&filePath.front()))
throw MakeSystemError("...", GetLastError(), __SOURCE__);
return std::move(filePath);
}Ну что сказать. Позор анализатору. Ведь действительно сообщает глупость. Переменные filePath и cachePath явно используются. Для предупреждения нет никаких причин. И ладно бы функция была на 1000 строк. Нет, функция проста для безобразия.
Всё, первое впечатление испорчено. Теперь расскажу о результатах расследования.
Анализатор PVS-Studio использует для препроцессирования файлов компилятор Visual C++ (CL.exe) или Clang. Подробнее, о том, как мы используем Clang рассказано в заметке: "Немного о взаимодействии PVS-Studio и Clang".
Препроцессор компилятора Visual C++ работает качественно, но зато крайне медленно. Clang работает быстро, но зато многое не поддерживает или работает неправильно. Разработчики Clang заявляют, что очень хорошо совместимы с Visual C++, но это неправда. Есть множество мелочей, которые они не поддерживают или делают не так как Visual C++. Для анализатора эти мелочи бывают фатальны, как и произошло в этот раз.
По умолчанию анализатор PVS-Studio в начале пытается препроцессировать файл с помощью Clang. Однако он знает: Clang далеко не всегда может препроцессировать то, что может Visual C++. Если возникает ошибка препроцессирования, то запускается CL.exe. Так теряется немного времени на бесполезный запуск Clang, но в целом такой подход очень экономит время на получении *.i файлов.
В данном случае это не помогло. Clang "успешно" препроцессировал файл, хотя на выходе получилась абракадабра.
Причиной неправильного поведения стал макрос __SOURCE__, объявленный в коде следующим образом:
#define __SLINE_0__(_line) #_line
#define __SLINE__(_line) __SLINE_0__(_line)
#define __SOURCE__ __FILE__":"__SLINE__(__LINE__)При препроцессировании строки:
throw MakeSystemError(_T("GetTempFileName"), GetLastError(),
__SOURCE__);Она должна превратиться в:
MakeSystemError("GetTempFileName", GetLastError(),
"..path.."":""37");Именно так и поступает компилятор Visual C++. Все отлично. Анализатор корректно обработает этот код.
Если явно задать в настройках PVS-Studio всегда использовать CL.exe, то ложные срабатывания исчезнут. Однако, если будет запущен Clang, то анализатор будет иметь дело с некорректным кодом.
Clang не может осилить макросы и на выходе мы имеем:
throw MakeSystemError("GetTempFileName", GetLastError(),
"..path.."":"__SLINE__(37));Макрос __SLINE__ остался нераскрытым.
Получилась некорректная конструкция, недопустимая с точки зрения языка C++. Встретив её, анализатор PVS-Studio пытается как-то обойти некорректный код, чтобы продолжить анализ далее. Лучше что-то пропустить, чем полностью не обработать файл. Часто такие пропуски не оказывают никакого влияние на результаты анализа.
Но в этот раз обойти некорректное место безболезненно не получилось. Был выброшен весь блок:
if (!GetTempFileName(cachePath.c_str(), "tmp", 0, &filePath.front()))
throw MakeSystemError("....", GetLastError(), __SOURCE__);
return std::move(filePath);Так уж вышло... Анализатор сделал всё, что смог.
Раз этого фрагмента для анализатора не существует, то переменные не инициализированы, никак не используются. Это и приводит к выдаче ложного срабатывания.
Один из вариантов решения проблемы - это всегда использовать препроцессор от Visual C++. Но есть недостаток - медленный анализ.
Поэтому в данном случае мы выбрали иной путь. Так как компания, из которой к нам обратились, подумывает о приобретении PVS-Studio, мы рассмотрели этот частный случай и сделали в коде очередную подпорку. Это некрасиво, но практично. У нас уже много разных специальных мест, которые обходят какие-то нюансы, встречающиеся в проектах наших пользователей. Это является разновидностью поддержки.
Итак, в этот раз нас подвёл препроцессор, реализованный в Clang. Интересно, что станет причиной написать следующую подобную статью о внешних ошибках.
Вот так и живём. Спасибо за внимание.
Пробуйте наш статический анализатор кода на своих проектах и, если что-то не ладится, пишите нам. Часто мы превращаем негативный настрой в положительный.