PVS-Studio и Continuous Integration: TeamCity. Анализ проекта Open RollerCoaster Tycoon 2

Один из самых актуальных сценариев использования анализатора PVS-Studio — его интеграция с CI системами. И хотя анализ проекта PVS-Studio практически из-под любой continuous integration системы можно встроить всего в несколько команд, мы продолжаем делать этот процесс ещё удобнее. В PVS-Studio появилась поддержка преобразования вывода анализатора в формат для TeamCity — TeamCity Inspections Type. Давайте посмотрим, как это работает.

Информация об используемом ПО

PVS-Studio — статический анализатор С, С++, C# и Java кода, предназначенный для облегчения задачи поиска и исправления различного рода ошибок. Анализатор можно использовать в Windows, Linux и macOS. В данной статье мы будем активно использовать не только сам анализатор, но и некоторые утилиты из его дистрибутива.

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

PlogConverter – утилита для конвертации отчёта анализатора в разные форматы.

Информация об исследуемом проекте

Давайте попробуем данную функциональность на практическом примере – проанализируем проект OpenRCT2.

OpenRCT2 — открытая реализация игры RollerCoaster Tycoon 2 (RCT2), расширяющая её новыми функциями и исправляющая ошибки. Игровой процесс вращается вокруг строительства и содержания парка развлечений, в котором находятся аттракционы, магазины и объекты. Игрок должен постараться получить прибыль и поддерживать хорошую репутацию парка, сохраняя при этом гостей счастливыми. OpenRCT2 позволяет играть как в сценарии, так и в песочнице. Сценарии требуют, чтобы игрок выполнил определенную задачу в установленное время, в то время как песочница позволяет игроку построить более гибкий парк без каких-либо ограничений или финансов.

Настройка

В целях экономии времени я, пожалуй, опущу процесс установки и начну с того момента, когда у меня на компьютере запущен сервер TeamCity. Нам нужно перейти: localhost:{указанный в процессе установки порт}(в моём случае, localhost:9090) и ввести данные для авторизации. После входа нас встретит:

Нажмём на кнопку Create Project. Далее выберем Manually, заполним поля.

После нажатия на кнопку Create, нас встречает окно с настройками.

Нажмём Create build configuration.

Заполняем поля, нажимаем Create. Мы видим окно с предложением выбора системы контроля версий. Так как исходники уже лежат локально, жмём Skip.

Наконец, мы переходим к настройкам проекта.

Добавим шаги сборки, для этого жмём: Build steps -> Add build step.

Тут выберем:

• Runner type -> Command Line
• Run -> Custom Script

Так как мы будем проводить анализ во время компиляции проекта, сборка и анализ должны быть одним шагом, поэтому заполним поле Custom Script:

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

Последнее, что нам нужно сделать, — установить переменные окружения, которыми я обозначил некоторые пути для улучшения их читабельности. Для этого перейдём: Parameters -> Add new parameter и добавим три переменные:

Остаётся нажать на кнопку Run в правом верхнем углу. Пока идёт сборка и анализ проекта расскажу вам о скрипте.

Непосредственно скрипт

Для начала нам нужно выкачать свежий дистрибутив PVS-Studio. Для этого мы используем пакетный менеджер Сhocolatey. Для тех, кто хочет узнать об этом поподробнее, есть соответствующая статья:

choco install pvs-studio -y

Далее запустим утилиту отслеживания сборки проекта CLMonitor.

%CLmon% monitor –-attach

Потом произведём сборку проекта, в качестве переменной окружения MSB выступает путь к нужной мне для сборки версии MSBuild

%MSB% %ProjPath% /t:clean
%MSB% %ProjPath% /t:rebuild /p:configuration=release
%MSB% %ProjPath% /t:g2
%MSB% %ProjPath% /t:PublishPortable

Введём логин и ключ лицензии PVS-Studio:

%PVS-Studio_cmd% credentials --username %PVS_Name% --serialNumber %PVS_Key%

После завершения сборки ещё раз запустим CLMonitor для генерации препроцессированных файлов и статического анализа:

%CLmon% analyze -l "c:ptest.plog"

После воспользуемся ещё одной утилитой из нашего дистрибутива. PlogConverter преобразует отчёт из стандартного в специфичный для TeamCity формат. Благодаря этому мы сможем посмотреть его прямо в окне сборки.

%PlogConverter% "c:ptest.plog" --renderTypes=TeamCity -o "C:temp"

Последним действием выведем форматированный отчёт в stdout, где его подхватит парсер TeamCity.

type "C:tempptest.plog_TeamCity.txt"

Полный код скрипта:

choco install pvs-studio -y
%CLmon% monitor --attach
set platform=x64
%MSB% %ProjPath% /t:clean
%MSB% %ProjPath% /t:rebuild /p:configuration=release
%MSB% %ProjPath% /t:g2
%MSB% %ProjPath% /t:PublishPortable
%PVS-Studio_cmd% credentials --username %PVS_Name% --serialNumber %PVS_Key%
%CLmon% analyze -l "c:ptest.plog"
%PlogConverter% "c:ptest.plog" --renderTypes=TeamCity -o "C:temp"
type "C:tempptest.plog_TeamCity.txt"

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

Теперь кликнем на Inspections Total, чтоб перейти к просмотру отчёта анализатора:

Предупреждения сгруппированы по номерам диагностических правил. Для осуществления навигации по коду нужно кликнуть на номер строки с предупреждением. Нажатие на знак вопроса в правом верхнем углу откроет вам новую вкладку с документацией. Также можно осуществить навигацию по коду, нажав на номер строки с предупреждением анализатора. Навигация с удалённого компьютера возможна при применении SourceTreeRoot маркера. Тот, кому интересен данный режим работы анализатора, может ознакомиться с соответствующим разделом документации.

Просмотр результатов работы анализатора

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

Предупреждение N1

V773 [CWE-401] The exception was thrown without releasing the 'result' pointer. A memory leak is possible. libopenrct2 ObjectFactory.cpp 443

Object* CreateObjectFromJson(....)
{
  Object* result = nullptr;
  ....
  result = CreateObject(entry);
  ....
  if (readContext.WasError())
  {
    throw std::runtime_error("Object has errors");
  }
  ....
}

Object* CreateObject(const rct_object_entry& entry)
{
  Object* result;
  switch (entry.GetType())
  {
    case OBJECT_TYPE_RIDE:
      result = new RideObject(entry);
      break;
    case OBJECT_TYPE_SMALL_SCENERY:
      result = new SmallSceneryObject(entry);
      break;
    case OBJECT_TYPE_LARGE_SCENERY:
      result = new LargeSceneryObject(entry);
      break;
    ....
    default:
      throw std::runtime_error("Invalid object type");
  }
  return result;
}

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

Предупреждение N2

V501 There are identical sub-expressions '(1ULL << WIDX_MONTH_BOX)' to the left and to the right of the '|' operator. libopenrct2ui Cheats.cpp 487

static uint64_t window_cheats_page_enabled_widgets[] = 
{
  MAIN_CHEAT_ENABLED_WIDGETS |
  (1ULL << WIDX_NO_MONEY) |
  (1ULL << WIDX_ADD_SET_MONEY_GROUP) |
  (1ULL << WIDX_MONEY_SPINNER) |
  (1ULL << WIDX_MONEY_SPINNER_INCREMENT) |
  (1ULL << WIDX_MONEY_SPINNER_DECREMENT) |
  (1ULL << WIDX_ADD_MONEY) |
  (1ULL << WIDX_SET_MONEY) |
  (1ULL << WIDX_CLEAR_LOAN) |
  (1ULL << WIDX_DATE_SET) |
  (1ULL << WIDX_MONTH_BOX) |  // <=
  (1ULL << WIDX_MONTH_UP) |
  (1ULL << WIDX_MONTH_DOWN) |
  (1ULL << WIDX_YEAR_BOX) |
  (1ULL << WIDX_YEAR_UP) |
  (1ULL << WIDX_YEAR_DOWN) |
  (1ULL << WIDX_DAY_BOX) |
  (1ULL << WIDX_DAY_UP) |
  (1ULL << WIDX_DAY_DOWN) |
  (1ULL << WIDX_MONTH_BOX) |  // <=
  (1ULL << WIDX_DATE_GROUP) |
  (1ULL << WIDX_DATE_RESET),
  ....
};

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

Предупреждения N3

V703 It is odd that the 'flags' field in derived class 'RCT12BannerElement' overwrites field in base class 'RCT12TileElementBase'. Check lines: RCT12.h:570, RCT12.h:259. libopenrct2 RCT12.h 570

struct RCT12SpriteBase
{
  ....
  uint8_t flags;
  ....
};
struct rct1_peep : RCT12SpriteBase
{
  ....
  uint8_t flags;
  ....
};

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

Предупреждение N4

V793 It is odd that the result of the 'imageDirection / 8' statement is a part of the condition. Perhaps, this statement should have been compared with something else. libopenrct2 ObservationTower.cpp 38

void vehicle_visual_observation_tower(...., int32_t imageDirection, ....)
{
  if ((imageDirection / 8) && (imageDirection / 8) != 3)
  {
    ....
  }
  ....
}

Давайте разберёмся поподробнее. Выражение imageDirection / 8 будет false в том случае, если imageDirection находится в диапазоне от -7 до 7. Вторая часть: (imageDirection / 8) != 3 проверяет imageDirection на нахождение вне диапазона: от -31 до -24 и от 24 до 31 соответственно. Мне кажется довольно странным проверять числа на вхождение в определённый диапазон таким способом и, даже если в данном фрагменте кода нет ошибки, я бы рекомендовал переписать данные условия на более явные. Это существенно упростило бы жизнь людям, которые будут читать и поддерживать этот код.

Предупреждение N5

V587 An odd sequence of assignments of this kind: A = B; B = A;. Check lines: 1115, 1118. libopenrct2ui MouseInput.cpp 1118

void process_mouse_over(....)
{
  ....
  switch (window->widgets[widgetId].type)
  {
    case WWT_VIEWPORT:
      ebx = 0;
      edi = cursorId;                                 // <=
      // Window event WE_UNKNOWN_0E was called here,
      // but no windows actually implemented a handler and
      // it's not known what it was for
      cursorId = edi;                                 // <=
      if ((ebx & 0xFF) != 0)
      {
        set_cursor(cursorId);
        return;
      }
      break;
      ....
  }
  ....
}

Данный фрагмент кода, скорее всего, был получен путем декомпиляции. Затем, судя по оставленному комментарию, была удалена часть нерабочего кода. Однако осталась пара операций над cursorId, которые также не несут особого смысла.

Предупреждение N6

V1004 [CWE-476] The 'player' pointer was used unsafely after it was verified against nullptr. Check lines: 2085, 2094. libopenrct2 Network.cpp 2094

void Network::ProcessPlayerList()
{
  ....
  auto* player = GetPlayerByID(pendingPlayer.Id);
  if (player == nullptr)
  {
    // Add new player.
    player = AddPlayer("", "");
    if (player)                                          // <=
    {
      *player = pendingPlayer;
       if (player->Flags & NETWORK_PLAYER_FLAG_ISSERVER)
       {
         _serverConnection->Player = player;
       }
    }
    newPlayers.push_back(player->Id);                    // <=
  }
  ....
}

Данный код поправить довольно просто, нужно или третий раз проверять player на нулевой указатель, либо внести его в тело условного оператора. Я бы предложил второй вариант:

void Network::ProcessPlayerList()
{
  ....
  auto* player = GetPlayerByID(pendingPlayer.Id);
  if (player == nullptr)
  {
    // Add new player.
    player = AddPlayer("", "");
    if (player)
    {
      *player = pendingPlayer;
      if (player->Flags & NETWORK_PLAYER_FLAG_ISSERVER)
      {
        _serverConnection->Player = player;
      }
      newPlayers.push_back(player->Id);
    }
  }
  ....
}

Предупреждение N7

V547 [CWE-570] Expression 'name == nullptr' is always false. libopenrct2 ServerList.cpp 102

std::optional<ServerListEntry> ServerListEntry::FromJson(...)
{
  auto name = json_object_get(server, "name");
  .....
  if (name == nullptr || version == nullptr)
  {
    ....
  }
  else
  {
    ....
    entry.name = (name == nullptr ? "" : json_string_value(name));
    ....
  }
  ....
}

Можно одним махом избавиться от трудночитаемой строки кода и решить проблему с проверкой на nullptr. Предлагаю изменить код следующим образом:

std::optional<ServerListEntry> ServerListEntry::FromJson(...)
{
  auto name = json_object_get(server, "name");
  .....
  if (name == nullptr || version == nullptr)
  {
    name = ""
    ....
  }
  else
  {
    ....
    entry.name = json_string_value(name);
    ....
  }
  ....
}

Предупреждение N8

V1048 [CWE-1164] The 'ColumnHeaderPressedCurrentState' variable was assigned the same value. libopenrct2ui CustomListView.cpp 510

void CustomListView::MouseUp(....)
{
  ....
  if (!ColumnHeaderPressedCurrentState)
  {
    ColumnHeaderPressed = std::nullopt;
    ColumnHeaderPressedCurrentState = false;
    Invalidate();
  }
}

Код выглядит довольно странно. Мне кажется, имела место быть опечатка либо в условии, либо при повторном присвоении переменной ColumnHeaderPressedCurrentState значения false.

Вывод

Как мы видим, интегрировать статический анализатор PVS-Studio в свой проект на TeamCity довольно просто. Для этого достаточно написать всего один маленький файл конфигурации. Проверка кода же позволит выявлять проблемы сразу после сборки, что поможет устранять их тогда, когда сложность и стоимость правок ещё малы.

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Vladislav Stolyarov. PVS-Studio and Continuous Integration: TeamCity. Analysis of the Open RollerCoaster Tycoon 2 project.

Источник: habr.com