ПВС-Студио и континуирана интеграција: ТеамЦити. Анализа пројекта Опен РоллерЦоастер Тицоон 2

Један од најактуелнијих сценарија за коришћење ПВС-Студио анализатора је његова интеграција са ЦИ системима. И иако се анализа пројекта ПВС-Студио из скоро сваког система континуиране интеграције може уградити у само неколико команди, ми настављамо да овај процес чинимо још погоднијим. ПВС-Студио сада има подршку за претварање излаза анализатора у формат за ТеамЦити - ТеамЦити Инспецтионс Типе. Хајде да видимо како то функционише.

Информације о коришћеном софтверу

ПВС-Студио — статички анализатор Ц, Ц++, Ц# и Јава кода, дизајниран да олакша задатак проналажења и исправљања различитих врста грешака. Анализатор се може користити на Виндовс, Линук и мацОС. У овом чланку ћемо активно користити не само сам анализатор, већ и неке услужне програме из његове дистрибуције.

ЦЛМонитор — је сервер за надгледање који прати покретање компајлера. Мора се покренути непосредно пре него што почнете да градите свој пројекат. У режиму њушкања, сервер ће пресрести покретање свих подржаних компајлера. Вреди напоменути да се овај услужни програм може користити само за анализу Ц/Ц++ пројеката.

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

Информације о пројекту који се проучава

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

ОпенРЦТ2 - отворена имплементација игре РоллерЦоастер Тицоон 2 (РЦТ2), проширујући је новим функцијама и исправљајући грешке. Игра се врти око изградње и одржавања забавног парка који садржи вожње, продавнице и објекте. Играч мора покушати да оствари профит и одржи добру репутацију парка, а да притом задржи госте срећним. ОпенРЦТ2 вам омогућава да играте иу сценарију иу сандбоку. Сценарији захтевају од играча да изврши одређени задатак у одређеном року, док Сандбок омогућава играчу да изгради флексибилнији парк без икаквих ограничења или финансија.

подешавање

Да бих уштедела време, вероватно ћу прескочити процес инсталације и почети од тренутка када на мом рачунару буде покренут ТеамЦити сервер. Морамо да одемо на: лоцалхост:{порт специфициран током процеса инсталације} (у мом случају, лоцалхост:9090) и унесемо податке за ауторизацију. Након уласка дочекаће нас:

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

Након притиска на дугме Створити, дочека нас прозор са подешавањима.

Хајде да кликнемо Креирајте конфигурацију изградње.

Попуните поља и кликните Створити. Мы видим окно с предложением выбора системы контроля версий. Так как исходники уже лежат локально, жмём Прескочити.

На крају, прелазимо на подешавања пројекта.

Добавим шаги сборки, для этого жмём: Кораци изградње -> Додај корак изградње.

Овде бирамо:

• Тип покретача -> Командна линија
• Покрени -> Прилагођена скрипта

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

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

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

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

Директно сценарио

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

choco install pvs-studio -y

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

%CLmon% monitor –-attach

Потом произведём сборку проекта, в качестве переменной окружения МСБ выступает путь к нужной мне для сборки версии 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% "c:ptest.plog" --renderTypes=TeamCity -o "C:temp"

Последним действием выведем форматированный отчёт в стдоут, где ће га покупити ТеамЦити парсер.

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"

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

Сада да кликнемо на Прегледи Укупно, чтоб перейти к просмотру отчёта анализатора:

Упозорења су груписана према бројевима дијагностичких правила. Да бисте се кретали кроз код, потребно је да кликнете на број реда са упозорењем. Кликом на упитник у горњем десном углу отвориће се нова картица са документацијом. Такође можете да се крећете кроз код тако што ћете кликнути на број реда са упозорењем анализатора. Навигација са удаљеног рачунара је могућа када се користи СоурцеТрееРоот маркера. Тот, кому интересен данный режим работы анализатора, может ознакомиться с соответствующим разделом документација.

Преглед резултата анализатора

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

Упозорење Н1

ВКСНУМКС [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;
}

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

Упозорење Н2

ВКСНУМКС 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),
  ....
};

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

Упозорења Н3

ВКСНУМКС 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;
  ....
};

Наравно, коришћење променљиве са истим именом у основној класи и у потомку није увек грешка. Међутим, сама технологија наслеђивања претпоставља да су сва поља родитељске класе присутна у подређеној класи. Проглашавањем поља са истим именом у наследнику стварамо забуну.

Упозорење Н4

ВКСНУМКС 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 биће лажно ако имагеДирецтион је у опсегу од -7 до 7. Други део: (имагеДирецтион / 8) != 3 провере имагеДирецтион за изван опсега: од -31 до -24 и од 24 до 31, респективно. Чини ми се прилично чудним да на овај начин проверавам бројеве за укључивање у одређени опсег и, чак и ако нема грешке у овом делу кода, препоручио бих да се ови услови поново напишу како би били експлицитнији. Ово би знатно олакшало живот људима који би читали и одржавали овај код.

Упозорење Н5

ВКСНУМКС 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;
      ....
  }
  ....
}

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

Упозорење Н6

ВКСНУМКС [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);                    // <=
  }
  ....
}

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

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);
    }
  }
  ....
}

Упозорење Н7

ВКСНУМКС [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));
    ....
  }
  ....
}

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

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);
    ....
  }
  ....
}

Упозорење Н8

ВКСНУМКС [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();
  }
}

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

Излаз

Као што видимо, интеграција статичког анализатора ПВС-Студио у ваш ТеамЦити пројекат је прилично једноставна. Да бисте то урадили, довољно је написати само једну малу конфигурациону датотеку. Провера кода ће вам омогућити да идентификујете проблеме одмах након склапања, што ће помоћи да их елиминишете када су сложеност и цена промена још увек ниска.

Ако желите да поделите овај чланак са публиком која говори енглески, користите линк за превод: Владислав Столиаров. PVS-Studio and Continuous Integration: TeamCity. Analysis of the Open RollerCoaster Tycoon 2 project.

Извор: ввв.хабр.цом