Статические анализаторы кода на примере ClickHouse09.11.2017 14:18

Чуть больше месяца назад была опубликована статья, содержащая анализ исходного кода ClickHouse с помощью PVS-Studio. Статья оказалась достаточно успешной: так, ссылку на неё мне отправили по меньшей мере десять раз в день её публикации. Общий тон статьи позитивный, а посещаемость сайта clickhouse.yandex в день её выхода заметно выросла.

Я очень уважаю, когда какая-либо компания или человек делает свою работу исчерпывающим образом. Так, у PVS-Studio исчерпывающий подход к продвижению: одних только статей на Хабре 337 штук. Они проводят доклады почти на всех российских конференциях по C++. В любом случае стоит отметить: люди стараются и своим трудом приносят пользу другим людям.

Та статья пробудила в нас интерес к статическим анализаторам, и мы решили проверить работу нескольких общедоступных аналогов PVS-Studio на кодовой базе ClickHouse. В сегодняшней статье мы поделимся с вами результатами этого исследования.

В день выхода статьи мы исправили все отмеченные в ней баги. Результат можно посмотреть в этом коммите. Некоторых багов на момент публикации уже не было в master, так как мы исправили их ранее.

Сомнений в полезности статических анализаторов нет. Некоторые комментаторы отмечают, что гораздо полезнее иметь code review, тесты, запуски под sanitizer’ами. Польза этих решений очевидна. Тем более, у нас уже есть code review (посредственный), покоммитные тесты (с недостаточно хорошим покрытием) и запуски под sanitizers (покоммитно только ASan, раз в сутки Valgrind). Статические анализаторы могут находить другие баги — например, copy-paste в коде, который никогда не выполняется и который надо было удалить год назад. И что-нибудь более серьёзное тоже есть шанс найти (читайте дальше). То есть, польза от статических анализаторов ненулевая.

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

Мы решили сначала изучить несколько альтернатив PVS-Studio, в результате чего попробовали Clang-Tidy, Сppcheck, Coverity и Svace. Для каждого из них удалось составить инструкции о том, как правильно использовать их для кода ClickHouse.

CppCheck

Инструкция по использованию: cppcheck.txt.

У этого инструмента есть несколько важных достоинств: он очень прост в использовании и работает чрезвычайно быстро. Так, наш репозиторий был проанализирован за несколько секунд. Стандартный режим запуска привёл к получению лишь нескольких ложно-положительных предупреждений. В режиме --enable-all было получено около 200 предупреждений, некоторые из которых оказались содержательными:

Кто-то забыл ввести один символ. Повезло — ошибка не влияет на поведение программы.
Неиспользование функции log1p — не знал, что такая существует. В данном случае тоже не влияет на поведение программы.

Помимо этого, анализатор также нашёл некоторые неоптимальности при передаче параметров в функции, забытые explicit-определения, слишком широкие области видимости некоторых переменных и так далее.

Многие ложно-положительные срабатывания связаны с тем, что анализатор очень примитивный. По-видимому, он даже не до конца парсит C++. Например, следующий код

int x = 0;
auto lambda = [&]{ x = 1; };

рассматривается так же, как код:

int x = 0;
{ x = 1; };

Тем не менее, большое количество false positives — норма для статических анализаторов кода. Сценарий их использования состоит в том, чтобы просмотреть несколько сотен ошибок и, возможно, найти среди них несколько действительно существенных.

Clang-Tidy

Инструкция по использованию: clang-tidy.txt

Важным достоинством этого инструмента является то, что он находится в экосистеме Clang. Это значит, что, если ваша система компилируется при помощи Clang, то Clang-Tidy будет корректно учитывать все особенности сборки вашего проекта. Есть инструкция, как добавлять свои проверки.

На анализ каждого translation unit’а Clang-Tidy тратит около десятка секунд. Это достаточно медленно.

Clang-Tidy отличается от Clang static analyzer (также известного под названием scan-build) наличием проверок стиля. Некоторые из проверок стиля позволяют автоматически переписывать код. Стоит отметить, что Clang static analyzer, в отличие от Clang-Tidy, умеет генерировать HTML-страницы с результатами, тогда как Clang-Tidy лишь выводит сообщения о найденных проблемах в окно терминала.

Использовать все типы проверок нет смысла. Так, одна из проверок CERT Secure Coding Standards, C++ Core Guidelines или HTC++ говорит «не используйте арифметику указателей». Это в каком-то смысле разумно, но явно отмечать все исходники, для которых эта проверка должна быть выключена, слишком мучительно.

Полезные сообщения нашлись в группах проверок static analyzer и performance. Performance обнаружил два типа ошибок: постинкременты итераторов и передачи аргументов по значению вместо передачи по ссылке. К сожалению, проверка передачи аргументов по ссылке срабатывает даже для небольших структур, например:

struct S
{
    int x;
};

или STRONG_TYPEDEF.

В этом случае замена передачи по значению на передачу по ссылке может оказаться бесполезной или даже вредной (при условии, что функция не инлайнится и не клонируется, ведь в этих случаях никакой разницы нет). В других ситуациях проверка оказалось более полезной, см. коммит с исправлениями. Конечно, такие ошибки должны обнаруживаться на этапе code review — при условии, что вы были достаточно внимательны. Заметим, что после исправления всех предупреждений performance реальная производительность ClickHouse не изменилась. Это ожидаемо: если бы соответствующие ошибки были в узких местах, мы бы это заметили при первом же запуске sudo perf top.

Предупреждения от static analyzer весьма полезны:

Вызов виртуальной функции в конструкторе. Оказалось, что virtual написали по ошибке.
Потеря точности в вычислениях
Разыменование нулевого указателя.

Coverity

Инструкция по использованию: coverity.txt.

Coverity — закрытый коммерческий продукт, но для проектов с открытым исходным кодом его можно использовать бесплатно. В числе известных продуктов, использующих Coverity есть, например, Linux.

Схема работы: с сайта скачивается утилита, которая оборачивает любую систему сборки (вместо make просто указывается cov-build make) и перехватывает вызовы компилятора. Заставить её работать удалось не с первого раза — пришлось перебрать несколько вариантов, как указать ей, какой компилятор используется.

Для некоторых единиц трансляции время работы инструмента достигает часа, а общее время анализа всего проекта — 4 часа. Результатом является tar-архив, который можно загрузить на сайт Coverity. Архив размером 1.5 GB загрузился успешно. Можно загружать и более объёмные результаты.
Затем, чтобы получить доступ к результатам работы анализатора, необходимо пройти проверку на возможность работать с исходным кодом анализируемого проекта. Например, мне доступ предоставили в течение полутора дней.

После этого стал доступен красивый веб-интерфейс, в котором можно в интерактивном режиме изучать код с аннотациями дефектов, а также отмечать резолюции (false positive, intentional, fix required и т. п.).

Сразу же выдали ачивку, правда я не понял за что.
Нашли 312 неких дефектов.

Неинициализированная переменная, если пользователь в конфиге не укажет ни одного элемента в одной из секций.
Разыменование итератора end только для того, чтобы превратить его в адрес.
Всякая мелочь.

Некоторые обнаруженные дефекты на самом деле являются частью задуманного дизайна.

Тавтологические условия в if’ах в шаблонном коде. Это полностью нормально, является распространённой практикой и стилем кода. Более того, код пишется, исходя из предположения, что компилятор об этом прекрасно знает и удалит ненужный код.
Неинициализированный член класса в конструкторе по умолчанию. Иногда это действительно ошибка, но в некоторых случаях бывает допустимо.
Необработанное исключение в функции main в тестовой программе. Это допускает наш стиль. Если исключение вылетело из функции main, то оно будет обработано с помощью std: terminate, стандартная библиотека сделает несколько полезных вещей за вас (verbose terminate handler): выведет тип исключения, для наследников std: exception выведет what, позовёт функцию abort; программа получит сигнал abort, операционная система запишет core dump, и программа завершится с ненулевым кодом. Всё это очень полезно.

Кроме того, одна из ошибок выглядела как Unrecoverable parse error при виде std: shared_mutex. Это значит, что Coverity не поддерживает C++17.

Svace

30 ноября 2016 я попал на конференцию «Технологии Баз Данных», на которой мне удалось договориться с коллегами из ИСП РАН об использовании их инструмента статического анализа Svace. Этот инструмент внедрен в корпорации Samsung, но малоизвестен в среде отечественных программистов.

Результаты анализа доступны в удобном веб-интерфейсе с просмотром исходного кода и аннотаций к нему. Самая серьёзная из обнаруженных ошибок — отсутствие freeaddrinfo в одной из веток кода. Стоит отметить, что и результаты анализа, и выборку дефектов для изучения нам предоставили коллеги из ИСП РАН.

Выводы

Использование статических анализаторов — не первая по важности мера для улучшения кода вашего продукта. Например, если у вас нет автосборки с запуском тестов хотя бы под Address Sanitizer — полезнее будет сначала это обеспечить. Разбор результатов работы статических анализаторов требует большого количества времени, по меньшей мере, в самом начале. Правильное их использование требует наличия интеграции в систему CI. Только если вы сделали более простые вещи — можно попробовать такую замечательную вещь как статические анализаторы. Впрочем, зачем это я вас отговариваю — обязательно попробуйте!

Напоследок, хочется ещё раз поблагодарить коллег из PVS-Studio за работу, которую они делают, в частности — за статью, посвящённую ClickHouse. Они не только помогли нам, обнаружив несколько важных проблем в нашем коде, но и вдохновили исследование, которому посвящена настоящая статья.