Однажды мы уже использовали trial-версию анализатора PVS-Studio для HPX, и тогда он запомнился нам своей многословностью. В последнее время появилось множество статей об этой утилите, и, т.к. прошло немало времени с момента ее использования, мы решили связаться с разработчиками с целью узнать, готовы ли они поддержать наш open-source продукт. Мы очень обрадовались, когда получили лицензию на 1 год в обмен на статью о найденных проблемах.
Эта статья была опубликована в STE||AR Group Blog. Перепечатка и перевод сделаны с разрешения авторов.
Скачав PVS-Studio V5.26, у меня не возникло проблем с установкой ее в качестве расширения для Visual Studio 2013 Professional, хотя я бы предпочел протестировать ее с версией 2015RC1 — моей основной средой разработки. К сожалению, на данный момент VS 2015 не поддерживается, но, надеюсь, это случится скоро после нового релиза у Microsoft.
Интеграция PVS-Studio в Visual Studio произвела очень хорошее впечатление. Одна дополнительная кнопка меню обеспечивает доступ ко всем командам и опциям. Все сообщения диагностики выводятся в специальное окно, из которого напрямую можно переходить к исходному коду. Также можно открыть web-based context-sensitive help, объясняющий каждую из диагностик в отдельности. Все, как и хотелось бы.
Сообщения имеют 3 различных уровня важности (severity levels): высокий, средний и низкий, и, в свою очередь, сгруппированны в 3 категории анализа: общий, оптимизационный и анализ 64-битной совместимости. Пользовательский интерфейс позволяет ограничить отображаемую диагностику и фильтровать результат. Для главного HPX-модуля анализатор сгенерировал около 70 сообщений из примерно 1000 .h и .cpp файлов (~140000 строк кода), что было, на мой взгляд, неплохо (high severity: 5, medium: 44, low: 21). Начальный анализ на моем ноутбуке занял примерно 10 минут.
Я с нетерпением хотел увидеть скрытые баги в HPX. Наша команда очень трепетна к качеству кода, и каждый пул реквест ревьюится как минимум одним разработчиком перед мерджем в главный брэнч. Так что я был уверен, что анализатор ничего не найдет.
Для начала давайте рассмотрим high-severity ошибки. Четыре из них были очень похожи и имели общую природу:
template <typename Archive>
void load(Archive& ar)
{
actions::manage_object_action_base* act = 0;
ar >> hpx::serialization::detail::raw_ptr(act);
// V522: Dereferencing of the null pointer 'act' might take place.
HPX_ASSERT(act->is_valid());
// ...
}Данный код десериализует полиморфный объект через указатель на базовый класс. Мы знаем, что raw_ptr(act) динамически создает новый объект, присваивая его адрес переданному указателю. Также мы знаем, что в случае ошибки функция бросает исключение. Все это могло быть видно статическому анализатору, т.к. весь модуль сериализации в HPX расположен в хэдерах. Анализатор, по всей видимости, не увидел это и сгенерировал ошибки. К счастью, можно явно указать PVS-Studio игнорировать данную ошибку просто одним кликом, что добавит магический комментарий вида //-V522 — очень удобно. Более того, PVS-Studio дает множество опций для подавления сообщений, основанных на файлах или директориях, на паттернах в именах файлов — все опции легкодоступны и хорошо объяснены.
Вторая ошибка меня встревожила:
#define HPX_VERSION_MAJOR 0
#define HPX_VERSION_MINOR 9
#define HPX_VERSION_SUBMINOR 11
std::string full_version_as_string()
{
// V609 Mod by zero. Denominator '0' == 0.
return boost::str(
boost::format("%d.%d.%d") %
HPX_VERSION_MAJOR % HPX_VERSION_MINOR %
HPX_VERSION_SUBMINOR);
}Чтобы понять, что хотел сказать анализатор, у меня ушло некоторое время, т.к. operator% из Boost.Format для меня выглядел совсем неприметно. Тем не менее, даже если бы operator% не был перегружен, и код в самом деле был проблематичным, сообщение об ошибке все равно было бы малопонятно. Я "решил" эту проблему тем же образом — подавив сообщение.
Последнее "high severity" сообщение было результатом оптимизации:
// V808 'hostname' object of 'basic_string' type was created
// but was not utilized.
std::string hostname = boost::asio::ip::host_name();И анализатор был прав, переменная "hostname" совсем не использовалась в данном контексте. Ни один из компиляторов, которых мы используем для регулярного тестирования на более чем 20 платформах, не обнаружил этого раньше — отличное замечание!
Сообщения среднего и низкого уровня важности были в основном связаны с доброкачественными проблемами о 32бит/64бит совместимости, как, например, неявное приведение знаковых целых в большие беззнаковые целые (например, int32_t --> uint64_t).
Но две ошибки оказались действительными багами:
int runtime_support::load_components(util::section& ini)
{
// load all components as described in the configuration information
if (!ini.has_section("hpx.components"))
{
// V601 The 'true' value is implicitly cast to the integer type.
return true; // no components to load
}
// ...
}Само сообщение прекрасно указывает на проблему: когда-то мы изменили возвращаемый тип функции с bool на int, но забыли изменить одно из return выражений. В дальнейшем это могло бы создать сложные для вопроизведения проблемы.
Другая ошибка оказалась более серьезной:
struct when_each_frame
{
// ...
private:
// V690 Copy constructor is declared as private in the
// 'when_each_frame' class, but the default '=' operator
// will still be generated by compiler. It is dangerous
// to use such a class.
when_each_frame();
when_each_frame(when_each_frame const&);
public:
// ...
};Это в самом деле отличное замечание, особенно потому что мы добавили объявление конструктора как workaround для старых версий gcc, которые некорректно инстанцировали дефолтные реализации.
В конце концов, я был рад потратить время на запуск анализатора для всей кодобазы HPX. Был рад видеть, что не было найдено серьезных проблем, как подтвержение нашей политики ревью, которую мы ввели достаточно давно. Я также решил внести PVS-Studio в наш contiguous integration процесс, запускающийся при каждом коммите.
Статический анализ определенно имеет значение. Запуск утилит вроде PVS-Studio хоть и занимает некоторое время и усилия, но, на мой взгляд, является правильным вложением времени и средств. Компиляторы не подходят для глубокого анализа, который делает PVS-Studio, т.к. в противном случае это значительно увеличило бы время компиляции.
Одной из наиболее полезных возможностей для нас оказалась легкая интеграция в CI процесс. Так как мы запускаем наши ежедневные тесты на различных платформах (включая Windows), то результаты анализа становятся доступны для разработчиков, работающих под другие ОС (Linux, macOS).
Еще одна порадовшая опция PVS-Studio — запуск анализа каждый раз после успешной сборки проекта. Удивительно, но это не прибавляет много оверхэда в обычный процесс сборки, к тому же дает фидбэк на тонкие проблемы кода на самых ранних этапах внедрения. Я оставил опцию включенной, чтобы оценить полезность инструмента в процессе.
Разбирая сгенерированные сообщения, я был удивлен, что, даже хотя утилита имеет время на как можно более глубокий анализ, PVS-Studio не смогла проанализировать перегрузки определенных операторов, чтобы определить недефолтную семантику. Пример с оператором operator% из Boost.Format продемонстрировал это. Согласен — это действительно очень тонкий момент, и я не уверен, что можно обеспечить правильную диагностику в данном случае. С другой стороны, именно здесь, в глубоком анализе семантики кода, инструмент должен иметь всю мощь.
Если вам интересен HPX, форкните билиотеку с github.