Зачем нужен динамический анализ кода: на примере проекта PVS-Studio

В мае 2021 года CppCast записал подкаст под названием «Стабильность ABI» (CppCast # 300). В этом подкасте Маршалл Клоу и ведущие обсудили довольно старую новость — компиляторы Visual Studio поддерживают инструмент AddressSantitzer. Мы уже давно интегрировали ASan в нашу систему тестирования. Теперь мы хотим рассказать вам о паре интересных ошибок, которые он нашел.

Текстовая трансляция Cppcast 300 находится здесь.

AddressSanitizer — один из модулей динамического анализа из компилятора LLVM-rt. ASan ловит ошибки или неправильное использование памяти. Например: выход за границы выделенной памяти, использование освобожденной памяти, двойное или некорректное освобождение памяти. В блоге PVS-Studio мы пишем о статическом анализе по понятным причинам. Однако мы не можем игнорировать, насколько полезен динамический анализ для контроля корректности программ.

Введение

Несколько слов о том, как мы тестируем анализатор C++. На билд-сервере во время ночного запуска анализатор проходит несколько этапов тестирования:

Различные компиляторы (MSVC, GCC, Clang) проверяют возможность сборки ядра pvs-studio, утилит pvs-studio-analyzer и plog-converter. Компиляторы проверяют их в различных конфигурациях, таких как Debug или Release, для Windows, Linux и macOS.
Модульные и интеграционные тесты проверяют как фрагменты тестового кода, так и сценарии использования утилит. Тесты основаны на платформе GoogleTest.
Специальная программа запускает анализатор C++ через выборку проектов с открытым исходным кодом на всех поддерживаемых платформах. Мы называем эту программу SelfTester. SelfTester запускает анализатор для проекта и сравнивает результат запуска с эталонными результатами.
PVS-Studio проводит статическую «самоанализ» для себя. Кстати, в статьях и на конференциях нас часто спрашивают, анализирует ли PVS-Studio себя.
Модульные и интеграционные тесты выполняют динамический анализ.

Разработчики также запускают первые четыре этапа локально на своих машинах.

На самом деле, мы используем динамический анализ уже более 5 лет в Linux. Впервые мы добавили его, когда портировали PVS-Studio на Linux. Тестов никогда не бывает слишком много, верно? Так как код проекта в нашей тестовой базе существенно отличается от одной ОС к другой, мы решили дополнительно запустить динамический анализ на Windows. Кроме того, код анализатора немного отличается для каждой системы.

Есть ли в PVS-Studio ошибки?

Ошибки не существует, пока не доказано обратное. Шутки. Как говорят медики: «Здоровых людей нет, есть недообследованные». То же самое и с разработкой программного обеспечения. Однажды ваши инструменты радостно сообщат, что все в порядке. Затем на днях вы пробуете что-то новое или обновляете что-то старое — и задаете себе вопрос: «Как ваш код вообще мог работать раньше?» К сожалению, мы не исключение. Но так оно и есть, и это нормально.

А если серьезно, то и статический, и динамический анализ имеют свои сильные и слабые стороны. И нет смысла пытаться выбрать что-то одно. Они прекрасно дополняют друг друга. Как видите, для проверки кода PVS-Studio мы используем как статический, так и динамический анализ. А далее в этой статье мы покажем вам преимущества разнообразия.

Средства отладки из стандартной библиотеки

Прежде чем перейти непосредственно к ASan, укажу на одну полезную настройку. Эта настройка также является механизмом динамического анализа и уже есть под рукой. Мы отмечаем эту настройку, потому что без нее проект с ASan не собирается. Речь идет о проверках, встроенных в реализацию стандартной библиотеки компилятора. В режиме отладки MSVS по умолчанию включены следующие макросы: _HAS_ITERATOR_DEBUGGING=1, _ITERATOR_DEBUG_LEVEL=2 и _SECURE_SCL=1. Во время проверки программы эти макросы активируют проверку на некорректную работу с итераторами и другими стандартными библиотечными классами. Такие проверки позволяют отловить множество случайно допущенных банальных ошибок.

Однако большое количество проверок может мешать, резко замедляя процесс отладки. Поэтому разработчики обычно отключают их и включают ночью на тестовом сервере. Ну, это было на бумаге. На самом деле эта настройка исчезла из сценария тестового запуска на сервере Windows… Соответственно, когда мы настроили проект под санитайзер, всплыла пачка накопившихся сюрпризов:

Например, такие сообщения MessageBox возникали из-за некорректной инициализации переменной типа std::Optional:

Если функция StringToIntegral не смогла проанализировать число, управляющее включенными диагностическими группами, она вернет std::nullopt. После этого код должен получить группу путем преобразования буквенный код. Однако разработчик добавил дополнительную звездочку в выражение сброса значения groupIndicator. Таким образом, мы получили неопределенное поведение, потому что метод доступа был вызван для неинициализированного std::Optional. Это похоже на разыменование нулевого указателя.

Еще одна проблема с std::Optional заключалась в неправильной логике обработки «виртуальных значений» размера массива:

Здесь объединяются виртуальные значения, полученные путем объединения путей выполнения кода. Термин «виртуальное значение» означает определенный диапазон значений, в который попадает значение переменной в соответствующем программном месте. Если нам удалось определить значения на обеих ветвях выполнения (оба значения не содержат std::nullopt), мы вызываем метод Union. Если значение неизвестно на одном из путей выполнения, вам необходимо установить его на известное значение из другой ветки. Но исходный алгоритм не был рассчитан на сценарий, когда обе ветки выполнения выдают неизвестные значения. Алгоритм по-прежнему вызывает для них метод Union, как если бы оба значения были известны. Это вызывает проблему, аналогичную той, что была в предыдущем примере. См. исправленный фрагмент кода ниже — он ничего не делает, когда оба значения неизвестны:

if (other.m_arraySizeInterval && m_arraySizeInterval)
{
  res.m_arraySizeInterval = m_arraySizeInterval
                            ->Union(*other.m_arraySizeInterval);
  res.m_elementSize = m_elementSize;
}
else if (!other.m_arraySizeInterval && m_arraySizeInterval)
{
  res.m_intervalSizeIsNotPrecise = false;
  res.m_arraySizeInterval = m_arraySizeInterval;
  res.m_elementSize = m_elementSize;
}
else if (!m_arraySizeInterval && other.m_arraySizeInterval)
{
  res.m_intervalSizeIsNotPrecise = false;
  res.m_arraySizeInterval = other.m_arraySizeInterval;
  res.m_elementSize = other.m_elementSize;
}

Следующий неудачный тест показывает пример последствий рефакторинга:

Когда-то переменная str была простым указателем на массив символов, который, очевидно, заканчивался нулевым терминалом. Затем str был заменен на std::string_view без включения нулевого терминала. Однако не все места, где используется эта переменная, были изменены на использование std::string_view. В этом фрагменте кода алгоритм, обрабатывающий содержимое строки, продолжает искать ее конец, ожидая нулевой терминал. Технически ошибки нет (не считая ненужной итерации), так как в конце строки стоит ноль в памяти. Но нет никакой гарантии, что этот ноль в конце строки останется там навсегда. Итак, ограничим цикл методом size:

for (size_t i = 1; i < str.size(); ++i)
{
  bool isUp = VivaCore::isUpperAlpha(name[i + pos]);
  allOtherIsUpper &= isUp;
  oneOtherIsUpper |= isUp;
}

Другой пример выхода за границу строки выглядит как некорректное поведение. Мы нашли его в диагностике V624, которая проверяет правильность написания некоторых констант и предлагает заменить их на более точные аналоги из стандартной библиотеки:

Из строки sampleStr мы получаем символ по индексу checkLen. Символ должен быть цифрой из числового литерала. Однако в этом случае индекс указывает на нулевой терминал. Индекс получается следующим образом:

const size_t maxDigits = 19;
size_t n; // Numbers after dot to check
switch (literalType)
{
case ST_FLOAT:
  n = 6;
  break;
case ST_DOUBLE:
  n = 14;
  break;
default:
  n = maxDigits;
}
const size_t checkLen = min(n, testStr.length());          // <=
size_t dist = GetEditDistance(testStr.substr(0, checkLen),
                              sampleStr.substr(0, checkLen));

Значение checkLen устанавливается в зависимости от типа константы с плавающей запятой и длины строки со ссылочным значением константы. При этом не учитывается длина числового литерала проверяемой константы. В результате диагностика может некорректно работать на коротких номерах. Правильный фрагмент кода:

const size_t checkLen = min(n, min(sampleStr.size() - 1, testStr.size()));

Последняя ошибка, обнаруженная при проверках из стандартной библиотеки, была в диагностике V1069. Эта диагностика ищет конкатенацию различных типов строковых литералов.

Лямбда-выражение compareWithPattern использует std::equal для сравнения префиксов фрагментов строкового литерала. Сравнение происходит в обратном порядке (это необходимо!) через реверсивные итераторы. Проблема здесь в том, что используемая перегрузка алгоритма std::equal сравнивает включение элементов одного контейнера в другой поэлементно. Он не проверяет длину контейнеров заранее. Эта перегрузка просто проходит через итератор, пока не достигнет последнего итератора первого контейнера. Если первый контейнер длиннее второго, мы выходим за границу второго контейнера. В нашем случае мы искали подстроку «u8» в префиксе «u». Чтобы убедиться, что мы не выходим за границы контейнеров, мы можем использовать правильную перегрузку. Он проверяет конечные итераторы обоих контейнеров. Но std::equal возвращает true, даже если контейнеры имеют разную длину и их элементы совпадают. Вот почему нам нужно использовать std::mismatch и проверять оба результирующих итератора:

StringLiteralType GetPattern(const SubstringView& element)
{
  auto rElementItBegin = element.RBeginAsString();
  auto rElementItEnd = element.REndAsString();
  .... // 'rElementItBegin' modification
  const auto compareWithPattern =
  [&rElementItBegin, &rElementItEnd](const auto &el)
  {
    const auto &pattern = el.second;
    auto [first, second] = std::mismatch(pattern.rbegin(), pattern.rend(),
                                         rElementItBegin, rElementItEnd);
    return first == pattern.rend() || second == rElementItEnd;
  };
  const auto type = std::find_if(Patterns.begin(), Patterns.end(),
                                 compareWithPattern);
  return type != Patterns.end() ? type->first : StringLiteralType::UNKNOWN;
}

Это была последняя обнаруженная ошибка.

Где Асан?

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

Чтобы включить AddressSanitizer для своего проекта, сначала установите соответствующий компонент в Visual Studio.

Стандартные библиотечные проверки должны быть включены в конфигурации Debug (они не нужны в конфигурации Release). Кроме того, нам нужно добавить флаг компиляции /fsanitize=address в свойствах проекта.

Мы можем легко включить флаг /fsanitize=address с помощью скрипта CMake, но нам нужно удалить конфликтующие флаги /RTC из компилятора:

if (PVS_STUDIO_ASAN)
  if (MSVC)
    add_compile_options(/fsanitize=address)
    string(REGEX REPLACE "/RTC(su|[1su])" ""
           CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}")
  endif ()
endif ()

Раз поправили мелкие испытания — пришло время «тяжелой артиллерии». Соберем ядро в конфигурации Release с включенным ASan и запустим SelfTester.

Ладно, тестирование заняло в 10 раз больше времени, чем тестирование обычного ядра. Время ожидания одного из проектов истекло, и он потерпел неудачу через 5 часов. Когда мы запускали этот проект отдельно, мы не обнаружили никаких проблем. Вы не можете толкнуть его на ночной пробег, но: «Очевидно, что он что-то делает!» :) В итоге ASan нашел 2 одинаковые ошибки в 6 разных файлах.

ASan завершает работу программы при обнаружении ошибки. Перед этим он выводит стек вызовов, чтобы мы могли понять, где произошла эта ошибка:

Диагностика V808 обратилась где-то из буфера памяти. Эта диагностика предупреждает, что какой-то объект был создан, а затем не использовался. Мы начали отладку ядра с включенным ASan, передав в ядро файл .cfg, на котором произошел сбой. Потом мы ждали. Мы не ожидали найти этот тип ошибки.

Диагностика V808 имеет одно исключение: символы, переданные в функцию __noop(….) компилятора MSVC, не запускают ее. Кто-то посчитал ненужной обработку этой операции как обычного вызова функции. Итак, во время парсинга исходного кода парсер просто создает конечный узел дерева. Грубо говоря, это std::string_view. Диагностика V808 анализирует его содержимое отдельно. Из-за ошибки внутри парсера алгоритм, генерирующий лист для __noop, неправильно определил конец конструкции — и захватил лишний код. Этот __noop был близок к концу файла. Итак, когда алгоритм построил строку из указателя и длины листа, ASan выдал предупреждение о выходе за границу файла. Отличный улов! После того, как мы исправили парсер, анализатор выдал дополнительные предупреждения на фрагмент кода, стоящий за функциями __noop. У нас была только одна такая проблема в нашей тестовой базе.

Последняя ошибка, которую нам помог найти динамический анализ, была связана с использованием освобожденной памяти:

Одной из техник, которую мы используем для анализа программ, является анализ потока данных.

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

pair<optional<IntegerVirtualValue>, optional<IntegerVirtualValue>>
PreciseListVirtualValue::SizeFromCondition(
  BinaryOperator op,
  const IntegerVirtualValue& value,
  const IntegerInterval &sizeInterval) const
{
  Pool pool{};
  pair<optional<IntegerVirtualValue>, optional<IntegerVirtualValue>> res;
  auto length = GetLengthVirtual()
                .value_or(IntegerVirtualValue(sizeInterval, false));
  ....
  auto getResForCond = [](const VirtualValueOpt& value)
    -> std::optional<IntegerVirtualValue>
  {
    if (!value)
    {
      return nullopt;
    }
    if (const IntegerVirtualValue *val = get_if<IntegerVirtualValue>(&*value))
    {
      return *val;                         // <=
    }
    return nullopt;
  };
  ....
  switch (op)
  {
    case .... :
      // for example
      res.first = getResForCond(length.Intersection(pool, value));
      res.second = getResForCond(length.Complement(pool, value));
    ....
  }
  return { res.first, res.second };
}

В лямбда-выражении getResForCond создается оболочка над ссылками на виртуальные значения. Затем ссылки обрабатываются в зависимости от типа операции в операторе switch. Функция SizeFromCondition завершает работу, оболочка возвращается, а ссылки внутри нее продолжают указывать на значения из пула, удаленного с помощью RAII. Чтобы исправить код, нам нужно возвращать копии объектов, а не ссылки. В данном случае нам повезло: причина ошибки и ее следствие оказались близки друг к другу. В противном случае это была бы долгая и мучительная отладка.

Вывод

Динамический анализ — мощный инструмент. Основное его преимущество – принципиальное отсутствие ложных срабатываний. Например, если ASan предупреждает о выходе за границу буфера, значит, это произошло во время выполнения с указанными исходными данными. За исключением эффекта бабочки (когда проблема возникает в начале выполнения программы, а проявляется значительно позже), отладке будет достаточно информации о том, что произошло и где исправить ошибку.

К сожалению, это работает и в обратную сторону. Если возможна ошибка, но выполнение программы успешно прошло по краю, то ASan молчит, т.е. динамический анализ не может показать потенциальные ошибки. Можно написать тесты, проверяющие все пограничные случаи в некоторых программах. Однако для PVS-Studio это означает создание кодовой базы, содержащей все возможные программы на C++.

Подробнее о плюсах и минусах динамического анализа вы можете прочитать в следующей статье: «Что толку в динамическом анализе, когда у вас есть статический анализ?»