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

http://stellar-group.org/2015/07/hpx-and-pvs-studio/
  • Перевод
Данная статья является переводом блогпоста Хартмута Кайзера об опыте использования 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.
PVS-Studio 245,46
Ищем ошибки в C, C++ и C# на Windows, Linux, macOS
Поделиться публикацией
Комментарии 12
  • –4
    каждый пул реквест ревьюится как минимум одним разработчиком перед мерджем в главный брэнч

    Я всё понимаю, модно, но в русском языке есть достойный перевод этих заимствований. Ну, или хотя бы стоило написать оригинальные слова на английском.
    • 0
      А как pull request перевести?
      • 0
        Ну, или хотя бы стоило написать оригинальные слова на английском.
        • –1
          Bitbucket, к примеру, в интерфейсе с русским языком использует слово «pull-запрос», что уже наполовину переведено. Это лучше, чем гротескная конструкция вида «пул реквест».
          • 0
            Надели ли вы сегодня мокроступы?
        • 0
          Абсолютно согласен. Выглядит эта фраза уродливо. «перед мерджем в главный брэнч», my ass. Ну почему бы не сказать «перед вставкой в основную ветвь кода»? «ревьюится» ещё куда ни шло.«пул реквест» можно было перевести как «запрос на изменение». Зачем засорять наш язык ещё и кальками с английского? А за статью — спасибо ) Разработчик HPX доволен как слон, что не нашлось существенных ошибок )
          • 0
            Какую метрику вы используете для определения уродливости слов? («мердж»? «ревьются»)
            template <class T1, class T2> anglicism_comparator(const T1&, const T2&)
            {
               ???
            }
            
            • 0
              метрика скорее представляет собой «чёрный ящик» нейронной сети моего мозга и не поддаётся формализации ) хотя в данном случае можно ответить, что в русском есть словарное заимствованное слово «ревю», что хоть как-то оправдывает «ревью». Но вот «мёрдж»… Также, мне кажется, такие слова как «ревью», «баг»… ну… сильнее устоялись, что ли, в программистском жаргоне, чем этот злосчастный «мёрдж». Ну ведь не говорите Вы «давайте сделаем компаризон этих двух чисел»? Звучит достаточно дико/уродливо?
              • –1
                Словарь русского языка. Прекрасная метрика, я считаю.

                Заимствования хороши, когда их использование умеренно. Они порой как нельзя лучше отражают суть предмета, который называют. Но всё нужно использовать с умом.
                • 0
                  Ну что значит «словарь русского языка — метрика»? Определите метрику на множестве используемых англицизмов в данном посту как функцию, удовлетворяющую аксиомам отсюда, если вам действительно нечем заняться, кроме балабольства и оффтопа. В данном переводе я руководствовался теми выражениями, которые использовал в рабочем словаре с русскими коллегами, и которые, на мой взгляд, более удобны для понимания без траты время на интерпретацию. И, будучи фолловером русского языка, извольте удалить запятую после «Ну» в Вашем первом примечании.
                  • –2
                    Заимствования хороши, когда их использование умеренно. Они порой как нельзя лучше отражают суть предмета, который называют. Но всё нужно использовать с умом.
                    Я так понимаю, вы не читаете комментарии оппонента, поскольку пересказали мне суть.

                    используемых англицизмов
                    Если бы это были англицизмы, я не написал. К сожалению, это транскрипция английских слов на русском языке.

                    в рабочем словаре с русскими коллегами
                    Вы не работе и люди, читающие ваши статьи, не всегда ваши коллеги.

                    Из вышесказанного вами следует, что используемые вами «слова» являются общепризнанными англицизмами. Если так, то приведите мне в пример, пожалуйста, статьи, где подобное имеется в столь сомнительной форме. Плюс, с учётом того, сколько вы этих «слов» вставили в одном предложении.

                    извольте удалить запятую после «Ну» в Вашем первом примечании
                    Без всякого подтверждения правильности (или неправильности) это выглядит как «я буду решать, что и как вам писать».
              • +1
                Перед вставкой чего? У меня от вставки очень нехорошие ассоциации, что-то вроде git push --force.

                Может, вы там вообще черри-пиком вставляете, а не мержите, поди пойми.

            Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

            Самое читаемое