PVS-Studio och Continuous Integration: TeamCity. Analys av projektet Open RollerCoaster Tycoon 2

En av de mest aktuella användningsscenarierna för PVS-Studio-analyzatorn är dess integration med CI-system. Även om analysen av projektet med PVS-Studio kan integreras i nästan vilket som helst continuous integration-system med bara några få kommandon, fortsätter vi att göra denna process ännu bekvämare. PVS-Studio har fått stöd för att konvertera analyzatorns output till TeamCity-format – TeamCity Inspections Type. Låt oss titta på hur detta fungerar.

Information om den använda programvaran

PVS-Studio – en statisk analysator för C, C++, C# och Java-kod, avsedd för att underlätta sökandet och åtgärdandet av olika typer av fel. Analyzatorn kan användas i Windows, Linux och macOS. I denna artikel kommer vi att använda inte bara själva analyzatorn utan även vissa verktyg från dess distribution.

CLMonitor – är en övervakningsserver som spårar körningar av kompilatorer. Den måste startas direkt innan byggandet av ditt projekt inleds. I övervakningsläge kommer servern att fånga upp körningar av alla stödda kompilatorer. Observera att detta verktyg endast kan användas för analys av C/C++-projekt.

PlogConverter – ett verktyg för att konvertera analyzatorns rapport till olika format.

Information om det undersökta projektet

Låt oss prova denna funktionalitet med ett praktiskt exempel – vi ska analysera projektet OpenRCT2.

OpenRCT2 – en öppen implementation av spelet RollerCoaster Tycoon 2 (RCT2), som utökar det med nya funktioner och åtgärdar fel. Spelmekaniken kretsar kring byggandet och skötseln av en nöjespark där det finns attraktioner, butiker och objekt. Spelaren måste sträva efter att göra en vinst och upprätthålla ett gott rykte för parken, samtidigt som gästerna hålls nöjda. OpenRCT2 tillåter att spela antingen i scenarieläget eller i sandlådan. Scenarier kräver att spelaren fullföljer en viss uppgift inom en angiven tidsram, medan sandlådan tillåter spelaren att bygga en mer flexibel park utan några begränsningar eller ekonomiska hinder.

Inställning

För att spara tid kommer jag att hoppa över installationsprocessen och börja från det ögonblicket då servern TeamCity är igång på min dator. Vi behöver gå till: localhost:{angett under installationsprocessen port}(i mitt fall, localhost:9090) och ange autentiseringsuppgifter. Efter inloggning möts vi av:

Klicka på knappen Skapa projekt. Därefter väljer vi Manuellt och fyller i fälten.

Efter att ha tryckt på knappen Skapa, möts vi av ett fönster med inställningar.

Klicka Skapa byggkonfiguration.

Fyll i fälten, klicka på Skapa. Vi ser ett fönster med förslag på versionskontrollsystem. Eftersom källkoden redan finns lokalt, klicka på Hoppa över.

Till sist går vi vidare till projektsinställningarna.

Lägg till byggelement, för detta klicka på: Byggelement -> Lägg till byggelement.

Här väljer vi:

• Körartyp -> Kommando Rad
• Kör -> Anpassad skript

Eftersom vi kommer att genomföra analysen under projektkompileringen, bör bygg och analys vara ett steg, så vi fyller i fältet Anpassad skript:

Vi kommer att stanna vid separata steg senare. Det är viktigt att laddningen av analysatorn, byggandet av projektet, dess analys, rapportgenerering och formatering tar totalt elva rader kod.

Sista steget är att ställa in miljövariabler, för vilka jag har angett vissa vägar för att förbättra deras läsbarhet. För detta går vi till: Parametrar -> Lägg till ny parameter och lägger till tre variabler:

Det återstår bara att klicka på knappen Run i det övre högra hörnet. Medan byggandet och analysen av projektet pågår, ska jag berätta om skriptet.

Själva skriptet

För att börja måste vi ladda ner den senaste versionen av PVS-Studio. För detta använder vi paketchefen Chocolatey. För dem som vill veta mer finns det motsvarande artikeln:

choco install pvs-studio -y

Därefter startar vi verktyget för att spåra byggprojektet CLMonitor.

%CLmon% monitor –-attach

Sedan kommer vi att bygga projektet, som miljövariabeln MSB representerar vägen till rätt version av MSBuild som jag behöver för att bygga

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

Vi anger användarnamn och licensnyckel för PVS-Studio:

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

Efter bygget kör vi CLMonitor igen för att generera preprocessade filer och statisk analys:

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

Efteråt använder vi ännu ett verktyg från vårt distributionspaket. PlogConverter konverterar rapporten från standardformat till TeamCity-specifikt format. Tack vare detta kan vi titta på den direkt i byggfönstret.

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

Som sista åtgärd skriver vi ut den formaterade rapporten i stdout, där den plockas upp av TeamCity-parsern.

type "C:tempptest.plog_TeamCity.txt"

Fullständig kod av skriptet:

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"

Under tiden har byggandet och analysen av projektet slutförts framgångsrikt, så vi kan gå till fliken Projekt och bekräfta detta.

Låt oss nu klicka på Inspections Total, för att gå till analysrapporten:

Varningar är grupperade efter diagnostiska regelnummer. För att navigera i koden, klicka på radnumret med varningen. Att klicka på frågetecknet i det övre högra hörnet öppnar en ny flik med dokumentation. Du kan också navigera i koden genom att klicka på radnumret med analysatorns varning. Navigering från en fjärrdator är möjlig genom att använda SourceTreeRoot markören. De som är intresserade av denna analysatormodus kan läsa avsnittet om dokumentationen.

Visa analysresultat

Efter att ha slutfört distributionen och konfigurationen av bygget tänker jag att vi kan titta på några intressanta varningar som upptäcktes i det granskade projektet.

Varning N1

V773 [CWE-401] Undantaget kastades utan att frigöra 'result'-pekaren. En minnesläcka kan vara möjlig. libopenrct2 ObjectFactory.cpp 443

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

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("Ogiltig objekttyp");
  }
  return result;
}

Analysatorn noterade ett fel, nämligen att efter dynamisk minnesallokering i CreateObject, vid uppkomsten av ett undantag städas inte minnet, vilket resulterar i en minnesläcka.

Varning N2

V501 Det finns identiska sub-uttryck '(1ULL << WIDX_MONTH_BOX)' till vänster och till höger om '|' operatorn. 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),
  ....
};

Det är få som, förutom en statisk analysator, skulle klara detta uppmärksamhetstest. Detta exempel på kopiering och klisjéer är bra just därför.

Varning N3

V703 Det är märkligt att fältet 'flags' i den härledda klassen 'RCT12BannerElement' överskrider fältet i basklassen 'RCT12TileElementBase'. Kontrollera rader: RCT12.h:570, RCT12.h:259. libopenrct2 RCT12.h 570

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

Självklart är det inte alltid ett misstag att använda en variabel med samma namn i basklassen och den härledda klassen. Men arvsteknologi förutsätter att alla fält från basklassen också finns i den härledda klassen. Genom att deklarera fält med samma namn i den härledda klassen skapar vi förvirring.

Varning N4

V793 Det är märkligt att resultatet av uttalandet 'imageDirection / 8' ingår som en del av villkoret. Kanske borde detta uttalande ha jämförts med något annat. libopenrct2 ObservationTower.cpp 38

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

Låt oss gå igenom det mer noggrant. Uttrycket imageDirection / 8 kommer att vara false om imageDirection ligger inom intervallet -7 till 7. Andra delen: (imageDirection / 8) != 3 kontrollerar imageDirection om det ligger utanför intervallet: från -31 till -24 och från 24 till 31. Jag tycker att det är ganska märkligt att kontrollera tal för att se om de ligger inom ett visst intervall på det här sättet och även om det inte finns något fel i denna kodfragment skulle jag rekommendera att omskriva dessa villkor till mer tydliga former. Det skulle avsevärt förenkla livet för dem som läser och underhåller denna kod.

Varning N5

V587 En märklig sekvens av tilldelningar av den här typen: A = B; B = A;. Kontrollera rader: 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;
      ....
  }
  ....
}

Denna kodsnutt har troligtvis erhållits genom dekompilering. Utifrån den kvarlämnade kommentaren verkar det som att en del av den icke-fungerande koden har tagits bort. Det finns dock några operationer kvar som cursorId, vilka inte heller bär någon särskild mening.

Varning N6

V1004 [CWE-476] Pekaren 'player' användes osäkert efter att den kontrollerades mot nullptr. Kontrollera rader: 2085, 2094. libopenrct2 Network.cpp 2094

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

Denna kod är ganska enkel att rätta till, antingen genom att kontrollera player mot nullpekare för tredje gången, eller genom att inkludera det i kroppen av villkorssatsen. Jag skulle föreslå det andra alternativet:

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

Varning N7

V547 [CWE-570] Uttrycket 'name == nullptr' är alltid falskt. libopenrct2 ServerList.cpp 102

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

Man kan på en gång bli av med den svårlästa kodraden och lösa problemet med kontrollen på nullptr. Jag föreslår att koden ändras på följande sätt:

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

Varning N8

V1048 [CWE-1164] Variabeln 'ColumnHeaderPressedCurrentState' tilldelades samma värde. libopenrct2ui CustomListView.cpp 510

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

Koden verkar ganska konstig. Jag tror att det kan ha varit en skrivfel antingen i villkoret eller vid den upprepade tilldelningen av variabeln ColumnHeaderPressedCurrentState värden false.

Utdata

Som vi ser, är det ganska enkelt att integrera PVS-Studio statisk analysator i sitt projekt på TeamCity. För detta räcker det att skriva en liten konfigurationsfil. Koden kommer att kontrolleras för att identifiera problem direkt efter byggprocessen, vilket hjälper till att åtgärda dem när komplexiteten och kostnaden för ändringar fortfarande är låg.

Om du vill dela denna artikel med en engelsktalande publik, vänligen använd länken till översättningen: Vladislav Stolyarov. PVS-Studio och kontinuerlig integration: TeamCity. Analys av Open RollerCoaster Tycoon 2-projektet.

Källa: habr.com