Статический анализ библиотеки HPX при помощи PVS-Studio

04 Июл 2015

Автор: Хартмут Кайзер

Общие впечатления
Примеры ошибок
Заключение

Однажды мы уже использовали 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.

#Cpp