Данная статья является переводом блогпоста Хартмута Кайзера об опыте использования PVS-Studio над HPX — C++ библиотекой для распределенных/параллельных вычислений любого масштаба.

Однажды мы уже использовали trial-версию анализатора PVS-Studio для HPX, и тогда он запомнился нам своей многословностью. В последнее время появилось множество статей об этой утилите, и, т.к. прошло немало времени с момента ее использования, мы решили связаться с разработчиками с целью узнать, готовы ли они поддержать наш open-source продукт. Мы очень обрадовались, когда получили лицензию на 1 год в обмен на статью о найденных проблемах.

Общие впечатления

Скачав 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, Mac OS).

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

Разбирая сгенерированные сообщения, я был удивлен, что, даже хотя утилита имеет время на как можно более глубокий анализ, PVS-Studio не смогла проанализировать перегрузки определенных операторов, чтобы определить недефолтную семантику. Пример с оператором operator% из Boost.Format продемонстрировал это. Согласен — это действительно очень тонкий момент, и я не уверен, что можно обеспечить правильную диагностику в данном случае. С другой стороны, именно здесь, в глубоком анализе семантики кода, инструмент должен иметь всю мощь.

Если вам интересен HPX, форкните билиотеку с github.