Мы уже давно хотели проверить библиотеку Boost. У нас не было уверенности, что результатов проверки хватит на статью. Однако, желание не пропадало. Два раза мы пытались сделать это, но отступали, не разобравшись, как заменить вызов компилятора на вызов PVS-Studio.exe. Теперь мы вооружились новым инструментарием, и третья попытка оказалась удачной. Итак, возможно ли найти в Boost ошибки?
Анализ на основе имеющихся препроцессированных файлов больше не поддерживается. Используйте режим отслеживания запусков компилятора, позволяющий автоматически генерировать и анализировать препроцессированные файлы в ходе сборки проекта.
Boost — собрание свободно распространяемых библиотек, расширяющих функциональность C++. Проект был создан после принятия стандарта C++, когда многие были недовольны, что в стандарт не были включены некоторые библиотеки. Проект является своего рода "испытательным полигоном" для различных расширений языка, и часть библиотек являются кандидатами на включение в следующие стандарты C++. Ссылки:
Boost представляет собой "тяжёлый" код, в котором активно используются сложные шаблоны. Эта библиотека в каком-то смысле является тестом для компиляторов. Часто бывает, что какой-то компилятор способен скомпилировать только часть проектов из современной библиотеки Boost.
Однако, у нас проблемы возникли не с разбором кода. В крайнем случае, анализатор может позволить себе тихо пропустить совсем уж запутанную конструкцию. Нам оказалось сложно собственно интегрироваться в сборку.
Напомню существующие варианты проверки проектов с помощью PVS-Studio.
Здесь всё просто. Анализ проекта может быть выполнен непосредственно из среды разработки. Другой вариант - запустить PVS-Studio из командной строки и на выходе получить файл с результатами проверки. Этот режим удобно использовать в системах непрерывной интеграции (например Cruise Control, Draco.NET или Team Foundation Build). Подробнее этот режим описан в документации. О взаимодействии с системами непрерывной интеграции можно посмотреть здесь.
В этом случае нужно создать режим сборки, когда вместо компилятора (или вместе с ним) запускается анализатор кода. На выходе также получается файл с отчётом. Необходимые магические заклинания также описаны в документации. При этом маг должен быть очень внимателен, подробно изучить руководство и не забыть ни одного символа.
Именно этот подход нам и следовало использовать, чтобы проверить Boost. Жаль, мы оказались магами недостаточного уровня или просто ленивыми. Не удавалось разобраться в системе сборки, чтобы передать консольной версии анализатора все необходимые параметры.
О этом новом режиме уже упоминал мой коллега в недавней заметке "Из подвала секретной лаборатории разработчиков PVS-Studio". Не надо полноценно интегрироваться в систему сборки. Достаточно просто получить препроцессированные *.i файлы. Это намного проще. Именно так мы и поступили.
Затем, используя прототип нового инструмента (PVS-Studio Standalone), мы выполнили анализ всех *.i файлов и получили долгожданный отчёт. В этой же программе можно удобно работать со списком ошибок и вносить исправления в код.
Я надеюсь, мы включим этот инструмент в состав дистрибутива через несколько версий. Возможно, это произойдет в PVS-Studio 5.10.
Мы потихоньку подбираемся к отслеживанию действий компилятора. Этот режим также будет относиться к инструменту PVS-Studio Standalone. Будут отслеживаться все запуски компилятора и собираться ключи его запуска. Таким образом, будет достаточно выполнить следующие действия. Приказать - "следи". Собрать проект с помощью любой системы сборки. Сказать - "готово". И анализатор будет знать, как теперь проверять проект. Конечно, если структура проекта или параметры изменится, этот процесс придётся повторить. Ладно, помечтали, а теперь вернемся к проверке Boost.
Я был готов, что статью про Boost написать не получится. У этого грустного предположения были следующие предпосылки.
Boost собирается большим количеством компиляторов. Какими-то частично, какими-то полностью. Я не проводил изучения этого вопроса. Но, как я понимаю, Boost неплохо собирается с помощью Visual C++, Intel C++, Sun Studio, Compaq C++, GCC, Clang. Каждый компилятор обладает своими уникальными диагностическими способностями. А их суммарное использование должно обеспечить очень высокое качество кода. Один компилятор найдёт ошибку A, второй ошибку B и так далее.
Более того, библиотека Boost является своего рода полигоном испытания различных инструментов и статических анализаторов кода. Поскольку в Boost активно используются различные современные возможности языка Си++, то всегда интересно знать сможет ли инструмент разобраться с таким кодом. В результате, Boost проверен и перепроверен различными анализаторами кода.
Искать ошибки и опечатки после такого количества компиляторов и других инструментов, дело почти безнадежное.
Библиотека Boost используется во многих проектах. Одно время мы и сами использовали Boost в проекте PVS-Studio (тогда ещё Viva64). Использовались механизмы для работы с регулярными выражениями, с конфигурационными файлами и пара других мелочей. Потом мы поняли, что регулярные выражения – это тупиковый путь, и постепенно они исчезли из кода. Таскать Boost только из-за конфигурационных файлов было сомнительно. Тем более выяснились некоторые неприятные недостатки. Например, в имени файла нельзя использовать '#'. Этот символ считается за начало комментария. В нашем конкретном случае Boost не прижился, однако, это, безусловно, очень полезная библиотека.
Так как библиотека широко используется, ошибки должны быстро обнаруживаться программистами. Ошибки могут оставаться только в редко используемых фрагментах кода или в экзотических подсистемах, у которых мало пользователей.
В Boost много шаблонных классов. Их почти невозможно проверить, если они не инстанцируются. Например ,Visual C++ вообще не разбирает шаблонные классы, если они не используются. В неиспользуемом классе можно написать любую белиберду, и файл успешно скомпилируется. Достаточно, чтобы соблюдалось количество открывающихся и закрывающихся скобок и кавычек (), <>, {}, [], "", ''.
Анализируя шаблонный класс, приходится идти на компромисс между количеством ложных срабатываний и пропуском ошибок. Поясню на простом примере, в чем состоит сложность.
template <typename T>
bool IsNAN(T x) { return x != x; }Эта функция определяет, является ли значение переменной "не числом" (Not-a-Number). Эта функция имеет смысл для типов float/double/long double. Для целочисленных типов такое сравнение бессмысленно, и указывает на наличие ошибки.
Как быть, если тип неизвестен? Неразрешимый вопрос. Для полноценной проверки все шаблоны должны использоваться во всех возможных вариантах. Более того. Шаблоны нужно ещё разобрать. А это сложная задачка. Например, у PVS-Studio есть ряд проблем с этим. Что-то он может разобрать и даже попытаться инстанцировать, а что-то нет.
В любом случае анализ шаблонов очень неблагодарное дело. А в Boost их полным-полно.
Оценивая и размышляя обо всём вышесказанном, я был настроен пессимистично. Я предполагал, что может вообще не найтись ничего дельного. Или найдется одна ошибка, из которой всё равно статью не сделаешь.
Найти 3-4 ошибки в Boost я посчитал бы огромным достижением.
Давайте посмотрим, что удалось найти PVS-Studio 5.06 в Boost 1.55 (скачана версия на этапе разработки).
Ошибок или подозрительных мест, конечно, найдено немного. Но я считаю результат анализа просто великолепным.
point3D operator/(const point3D &p1, const point3D &p2)
{
return point3D( p1.x/p2.x , p1.y/p2.y , p1.z/p1.z );
}Диагностическое сообщение PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '/' operator: p1.z / p1.z lorenz_point.cpp 61
В третьем аргументе функции, переменная 'p1.z' делится на саму себя. Скорее всего, она должна делиться на 'p2.z'.
BOOST_UBLAS_INLINE
compressed_matrix_view(const compressed_matrix_view& o) :
size1_(size1_), size2_(size2_),
nnz_(nnz_),
index1_data_(index1_data_),
index2_data_(index2_data_),
value_data_(value_data_)
{}Первое диагностическое сообщение PVS-Studio (остальные приводить нет смысла): V546 Member of a class is initialized by itself: 'size1_(size1_)'. sparse_view.hpp 193
Члены класса инициализируются сами собой. И получается, что остаются неинициализированными. Наверное, должны были использоваться данные из объекта 'o'. Мне кажется, конструктор должен был выглядеть так:
BOOST_UBLAS_INLINE
compressed_matrix_view(const compressed_matrix_view& o) :
size1_(o.size1_), size2_(o.size2_),
nnz_(o.nnz_),
index1_data_(o.index1_data_),
index2_data_(o.index2_data_),
value_data_(o.value_data_)
{}static std::basic_string<wchar_t> get(char const* source = "")
{
....
std::auto_ptr<wchar_t> result (new wchar_t[len+1]);
....
}Диагностическое сообщение PVS-Studio: V554 Incorrect use of auto_ptr. The memory allocated with 'new []' will be cleaned using 'delete'. tree_to_xml.ipp 71
Контейнер 'std::auto_ptr' является неподходящим типом для хранения указателя на массив объектов. Для уничтожения массива будет использоваться оператор 'delete', а не 'delete []'. Скорее всего не фатальная, но самая настоящая ошибка. Опасность таких ошибок хорошо описана в статье "delete, new[] в C++ и городские легенды об их сочетании".
Аналогичная ошибочка присутствует здесь: generate_static.hpp 53.
По-моему, редко можно встретить проект, где нет хотя бы одной ошибки, связанный с использованием типа SOCKET. Напомню суть беды. Нередко статус операции проверяют следующим образом:
SOCKET s = Foo();
if (s < 0) { Error(); }Такая проверка незаконна. Следует сравнивать переменную с константой SOCKET_ERROR. Однако, программисты часто ленятся и пишут "socket < 0" или "socket >= 0".
В Linux тип SOCKET знаковый и подобная халтурная работа сходит с рук. В Windows тип SOCKET является беззнаковым. В результате, условия всегда ложны и ошибочные ситуации никак не обрабатываются.
Нашлась такая ошибка и в Boost.
typedef SOCKET socket_type;
class socket_holder
{
....
socket_type socket_;
....
socket_type get() const { return socket_; }
....
};
template <typename Socket>
boost::system::error_code accept(....)
{
....
// On success, assign new connection to peer socket object.
if (new_socketnew_socket.get() >= 0)
{
if (peer_endpoint)
peer_endpoint->resize(addr_len);
if (!peer.assign(impl.protocol_, new_socket.get(), ec))
new_socket.release();
}
return ec;
}Диагностическое сообщение PVS-Studio: V547 Expression 'new_socket.get() >= 0' is always true. Unsigned type value is always >= 0. win_iocp_socket_service.hpp 436
В Windows этот фрагмент кода будет работать не так, как ожидал программист. Условие "new_socketnew_socket.get() >= 0" выполняется всегда.
void set_duration_style(duration_style style)
{
duration_style_ == style;
}Диагностическое сообщение PVS-Studio: V607 Ownerless expression 'duration_style_ == style'. base_formatter.hpp 51
Думаю особенно комментировать здесь нечего. Полагаясь на название функции, мне кажется, здесь должно быть написано: "duration_style_ = style". Просто опечатка.
Посмотрите на эту ошибку и представьте, сколько нервов и времени может экономить PVS-Studio при регулярном использовании. Мы все постоянно делаем такие ляпы. Потом ищем и правим их. Мы не помним про них, так как они слишком мелкие. Но они складываются в часы и дни, бессмысленно потраченные программистом. PVS-Studio хорошо ищет опечатки. Попробуйте режим инкрементального анализа. И когда инструмент покажет вам тройку таких ляпов сразу после компиляции, вы полюбите его.
Вы не делаете таких ошибок? Ещё раз посмотрите этот пример (и разные другие примеры). Думаю, этот код писал не студент. Просто мозг человек не совершенен, и мы все допускаем ошибки. Это нормально и нет ничего плохого, чтобы подстраховаться, используя различные вспомогательные инструменты: статические анализаторы кода, методологию TDD, обзоры кода.
template< typename CharT >
basic_settings< CharT > parse_settings(std::basic_istream< CharT >&
strm)
{
....
string_type line;
while (!strm.eof())
{
std::getline(strm, line);
const char_type* p = line.c_str();
parser.parse_line(p, p + line.size());
line.clear();
++line_number;
}
....
}Диагностическое сообщение PVS-Studio: V663 Infinite loop is possible. The 'cin.eof()' condition is insufficient to break from the loop. Consider adding the 'cin.fail()' function call to the conditional expression. settings_parser.cpp 285
Этот код делает, что должен делать - читает данные из файла. Анализатору не нравится, что код может привести к зацикливанию. Я не знаю, как точно смоделировать опасную ситуацию, но попробую предположить. Пусть файл находится на сетевом диске. В момент чтения, соединение разрывается. Функция 'eof()' будет возвращать 'false'. Ведь файл не прочитан до конца. Чтобы отлавливать такие ситуации рекомендуется использовать функцию 'eof()' в паре с функцией 'fail()'. В приведенном фрагменте функция 'fail()' нигде не зовется, а значит возможна неприятная ситуация при работе программы. Из таких мелочей и складывается надежность программ и их устойчивость к ошибкам.
И ещё одна потенциально опасное место: V663 Infinite loop is possible. The 'cin.eof()' condition is insufficient to break from the loop. Consider adding the 'cin.fail()' function call to the conditional expression. adjacency_list_io.hpp 195
template<>
struct identity_element<boost::gregorian::date_duration>
{
static boost::gregorian::date_duration value()
{
return
boost::gregorian::date(boost::gregorian::min_date_time) -
boost::gregorian::date(boost::gregorian::min_date_time);
}
};Сообщение анализатора PVS-Studio: V501 There are identical sub-expressions 'boost::gregorian::date(boost::gregorian::min_date_time)' to the left and to the right of the '-' operator. gregorian.hpp 57
Мне кажется, или эта функция всегда будет возвращать 0?
Я считаю, что анализатор PVS-Studio великолепно показал себя. Найти хоть что-то в Boost это огромное достижение. И анализатор сделал это!