PVS-Studio:analisi delle richieste pull in Azure DevOps tramite agenti self-hosted

PVS-Studio:analisi delle richieste pull in Azure DevOps tramite agenti self-hosted

L'analisi del codice statico è più efficace quando si modifica un progetto, poiché gli errori sono sempre più difficili da correggere in futuro che in una fase iniziale. Continuiamo ad ampliare le opzioni per l'utilizzo di PVS-Studio nei sistemi di sviluppo continuo. Questa volta ti mostreremo come configurare l'analisi delle richieste pull utilizzando agenti self-hosted in Microsoft Azure DevOps, usando l'esempio del gioco Minetest.

In breve di cosa abbiamo a che fare

Minetest è un motore di gioco multipiattaforma open source contenente circa 200.000 righe di codice in C, C++ e Lua. Ti consente di creare diverse modalità di gioco nello spazio voxel. Supporta il multiplayer e molte mod dalla community. Il repository del progetto si trova qui:https://github.com/minetest/minetest.

I seguenti strumenti vengono utilizzati per configurare il rilevamento regolare degli errori:

PVS-Studio è un analizzatore di codice statico del codice scritto in C, C++, C# e Java per la ricerca di errori e difetti di sicurezza.

Azure DevOps è una piattaforma cloud che consente di sviluppare, eseguire applicazioni e archiviare dati su server remoti.

È possibile usare le macchine virtuali dell'agente Windows e Linux per eseguire attività di sviluppo in Azure. Tuttavia, l'esecuzione di agenti sull'apparecchiatura locale presenta diversi importanti vantaggi:

  • L'host locale potrebbe avere più risorse di una macchina virtuale di Azure;
  • L'agente non "scompare" dopo aver completato il suo compito;
  • Possibilità di configurare direttamente l'ambiente e una gestione più flessibile dei processi di compilazione;
  • L'archiviazione locale dei file intermedi ha un effetto positivo sulla velocità di compilazione;
  • Puoi completare più di 30 attività al mese gratuitamente.

Preparazione all'utilizzo di un agente self-hosted

Il processo per iniziare con Azure è descritto in dettaglio nell'articolo "PVS-Studio in the Clouds:Azure DevOps", quindi andrò direttamente alla creazione di un agente self-hosted.

Affinché gli agenti possano connettersi ai pool di progetti, hanno bisogno di uno speciale token di accesso. Puoi ottenerlo nella pagina "Token di accesso personale", nel menu "Impostazioni utente".

Dopo aver fatto clic su "Nuovo token", è necessario specificare un nome e selezionare Leggi e gestisci pool di agenti (potrebbe essere necessario espandere l'elenco completo tramite "Mostra tutti gli ambiti").

Devi copiare il token, perché Azure non lo mostrerà più e dovrai crearne uno nuovo.

Come agente verrà utilizzato un contenitore Docker basato su Windows Server Core. L'host è il mio computer desktop su Windows 10 x64 con Hyper-V.

Innanzitutto, dovrai espandere la quantità di spazio su disco disponibile per i contenitori Docker.

Per fare ciò, in Windows, è necessario modificare il file 'C:\ProgramData\Docker\config\daemon.json' come segue:

{
  "registry-mirrors": [],
  "insecure-registries": [],
  "debug": true,
  "experimental": false,
  "data-root": "d:\\docker",
  "storage-opts": [ "size=40G" ]
}

Per creare un'immagine Docker per agenti con il sistema di build e tutto il necessario, aggiungiamo un file Docker con il seguente contenuto nella directory 'D:\docker-agent':

# escape=`

FROM mcr.microsoft.com/dotnet/framework/runtime

SHELL ["cmd", "/S", "/C"]

ADD https://aka.ms/vs/16/release/vs_buildtools.exe C:\vs_buildtools.exe
RUN C:\vs_buildtools.exe --quiet --wait --norestart --nocache `
  --installPath C:\BuildTools `
  --add Microsoft.VisualStudio.Workload.VCTools `
  --includeRecommended

RUN powershell.exe -Command `
  Set-ExecutionPolicy Bypass -Scope Process -Force; `
  [System.Net.ServicePointManager]::SecurityProtocol =
    [System.Net.ServicePointManager]::SecurityProtocol -bor 3072; `
  iex ((New-Object System.Net.WebClient)
    .DownloadString('https://chocolatey.org/install.ps1')); `
  choco feature enable -n=useRememberedArgumentsForUpgrades;
  
RUN powershell.exe -Command `
  choco install -y cmake --installargs '"ADD_CMAKE_TO_PATH=System"'; `
  choco install -y git --params '"/GitOnlyOnPath /NoShellIntegration"'

RUN powershell.exe -Command `
  git clone https://github.com/microsoft/vcpkg.git; `
  .\vcpkg\bootstrap-vcpkg -disableMetrics; `
  $env:Path += '";C:\vcpkg"'; `
  [Environment]::SetEnvironmentVariable(
    '"Path"', $env:Path, [System.EnvironmentVariableTarget]::Machine); `
  [Environment]::SetEnvironmentVariable(
    '"VCPKG_DEFAULT_TRIPLET"', '"x64-windows"',
  [System.EnvironmentVariableTarget]::Machine)

RUN powershell.exe -Command `
  choco install -y pvs-studio; `
  $env:Path += '";C:\Program Files (x86)\PVS-Studio"'; `
  [Environment]::SetEnvironmentVariable(
    '"Path"', $env:Path, [System.EnvironmentVariableTarget]::Machine)

RUN powershell.exe -Command `
  $latest_agent =
    Invoke-RestMethod -Uri "https://api.github.com/repos/Microsoft/
                          azure-pipelines-agent/releases/latest"; `
  $latest_agent_version =
    $latest_agent.name.Substring(1, $latest_agent.tag_name.Length-1); `
  $latest_agent_url =
    '"https://vstsagentpackage.azureedge.net/agent/"' + $latest_agent_version +
  '"/vsts-agent-win-x64-"' + $latest_agent_version + '".zip"'; `
  Invoke-WebRequest -Uri $latest_agent_url -Method Get -OutFile ./agent.zip; `
  Expand-Archive -Path ./agent.zip -DestinationPath ./agent

USER ContainerAdministrator
RUN reg add hklm\system\currentcontrolset\services\cexecsvc
        /v ProcessShutdownTimeoutSeconds /t REG_DWORD /d 60  
RUN reg add hklm\system\currentcontrolset\control
        /v WaitToKillServiceTimeout /t REG_SZ /d 60000 /f

ADD .\entrypoint.ps1 C:\entrypoint.ps1
SHELL ["powershell", "-Command",
       "$ErrorActionPreference = 'Stop';
     $ProgressPreference = 'SilentlyContinue';"]
ENTRYPOINT .\entrypoint.ps1

Il risultato è un sistema di compilazione basato su MSBuild per C++, con Chocolatey per l'installazione di PVS-Studio, CMake e Git. Vcpkg è costruito per una comoda gestione delle librerie da cui dipende il progetto. Inoltre, dobbiamo scaricare l'ultima versione di Azure Pipelines Agent.

Per inizializzare l'agente dal file Docker ENTRYPOINT, viene chiamato lo script di PowerShell 'entrypoint.ps1', a cui è necessario aggiungere l'URL dell'"organizzazione" del progetto, il token del pool di agenti e i parametri della licenza di PVS-Studio :

$organization_url = "https://dev.azure.com/<Microsoft Azure account>"
$agents_token = "<agent token>"

$pvs_studio_user = "<PVS-Studio user name>"
$pvs_studio_key = "<PVS-Studio key>"

try
{
  C:\BuildTools\VC\Auxiliary\Build\vcvars64.bat

  PVS-Studio_Cmd credentials -u $pvs_studio_user -n $pvs_studio_key
  
  .\agent\config.cmd --unattended `
    --url $organization_url `
    --auth PAT `
    --token $agents_token `
    --replace;
  .\agent\run.cmd
} 
finally
{
  # Agent graceful shutdown
  # https://github.com/moby/moby/issues/25982
  
  .\agent\config.cmd remove --unattended `
    --auth PAT `
    --token $agents_token
}

Comandi per creare un'immagine e avviare l'agente:

docker build -t azure-agent -m 4GB .
docker run -id --name my-agent -m 4GB --cpu-count 4 azure-agent

L'agente è in esecuzione ed è pronto per eseguire attività.

Esecuzione dell'analisi su un agente self-hosted

Per l'analisi PR, viene creata una nuova pipeline con il seguente script:

trigger: none

pr:
  branches:
    include:
    - '*'

pool: Default

steps:
- script: git diff --name-only
    origin/%SYSTEM_PULLREQUEST_TARGETBRANCH% >
    diff-files.txt
  displayName: 'Get committed files'

- script: |
    cd C:\vcpkg
    git pull --rebase origin
    CMD /C ".\bootstrap-vcpkg -disableMetrics"
    vcpkg install ^
    irrlicht zlib curl[winssl] openal-soft libvorbis ^
    libogg sqlite3 freetype luajit
    vcpkg upgrade --no-dry-run
  displayName: 'Manage dependencies (Vcpkg)'

- task: CMake@1
  inputs:
    cmakeArgs: -A x64
      -DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake
      -DCMAKE_BUILD_TYPE=Release -DENABLE_GETTEXT=0 -DENABLE_CURSES=0 ..
  displayName: 'Run CMake'

- task: MSBuild@1
  inputs:
    solution: '**/*.sln'
    msbuildArchitecture: 'x64'
    platform: 'x64'
    configuration: 'Release'
    maximumCpuCount: true
  displayName: 'Build'

- script: |
    IF EXIST .\PVSTestResults RMDIR /Q/S .\PVSTestResults
    md .\PVSTestResults
    PVS-Studio_Cmd ^
    -t .\build\minetest.sln ^
    -S minetest ^
    -o .\PVSTestResults\minetest.plog ^
    -c Release ^
    -p x64 ^
    -f diff-files.txt ^
    -D C:\caches
    PlogConverter ^
    -t FullHtml ^
    -o .\PVSTestResults\ ^
    -a GA:1,2,3;64:1,2,3;OP:1,2,3 ^
    .\PVSTestResults\minetest.plog
    IF NOT EXIST "$(Build.ArtifactStagingDirectory)" ^
    MKDIR "$(Build.ArtifactStagingDirectory)"
    powershell -Command ^
    "Compress-Archive -Force ^
    '.\PVSTestResults\fullhtml' ^
    '$(Build.ArtifactStagingDirectory)\fullhtml.zip'"
  displayName: 'PVS-Studio analyze'
  continueOnError: true

- task: PublishBuildArtifacts@1
  inputs:
    PathtoPublish: '$(Build.ArtifactStagingDirectory)'
    ArtifactName: 'psv-studio-analisys'
    publishLocation: 'Container'
  displayName: 'Publish analysis report'

Questo script funzionerà quando viene ricevuto un PR e verrà eseguito sugli agenti assegnati al pool per impostazione predefinita. Devi solo concedergli un'autorizzazione per lavorare con questo pool.

Lo script salva l'elenco dei file modificati ottenuti utilizzando git diff. Quindi le dipendenze vengono aggiornate, la soluzione del progetto viene generata tramite CMake e viene compilata.

Se la compilazione ha avuto successo, viene avviata l'analisi dei file modificati (il flag '-f diff-files.txt'), ignorando i progetti ausiliari creati da CMake (selezionare solo il progetto necessario con il flag '-S minetest '). Per rendere più veloce la determinazione delle relazioni tra l'intestazione e i file C++ di origine, viene creata una cache speciale, che verrà archiviata in una directory separata (il flag '-D C:\caches').

In questo modo ora possiamo ottenere rapporti sull'analisi delle modifiche nel progetto.

Come accennato all'inizio dell'articolo, un bel vantaggio dell'utilizzo di agenti self-hosted è una notevole accelerazione dell'esecuzione delle attività, dovuta all'archiviazione locale di file intermedi.

Alcuni errori trovati in Minetest

Sovrascrivere il risultato

V519 Alla variabile 'color_name' vengono assegnati valori due volte di seguito. Forse questo è un errore. Righe di controllo:621, 627. string.cpp 627

static bool parseNamedColorString(const std::string &value,
                                  video::SColor &color)
{
  std::string color_name;
  std::string alpha_string;

  size_t alpha_pos = value.find('#');
  if (alpha_pos != std::string::npos) {
    color_name = value.substr(0, alpha_pos);
    alpha_string = value.substr(alpha_pos + 1);
  } else {
    color_name = value;
  }

  color_name = lowercase(value); // <=

  std::map<const std::string, unsigned>::const_iterator it;
  it = named_colors.colors.find(color_name);
  if (it == named_colors.colors.end())
    return false;
  ....
}

Questa funzione dovrebbe analizzare il nome del colore con il parametro di trasparenza (ad esempio, Verde#77 ) e restituirne il codice. A seconda del risultato del controllo della condizione, il nome_colore Alla variabile viene passato il risultato della divisione della stringa o una copia dell'argomento della funzione. Tuttavia, l'argomento originale viene quindi convertito in minuscolo anziché nella stringa risultante stessa. Di conseguenza, non può essere trovato nel dizionario dei colori se è presente il parametro di trasparenza. Possiamo correggere questa linea in questo modo:

color_name = lowercase(color_name);

Controlli ridondanti delle condizioni

V547 L'espressione 'nearest_emergefull_d ==- 1' è sempre vera. clientiface.cpp 363

void RemoteClient::GetNextBlocks (....)
{
  ....
  s32 nearest_emergefull_d = -1;
  ....
  s16 d;
  for (d = d_start; d <= d_max; d++) {
    ....
      if (block == NULL || surely_not_found_on_disk || block_is_invalid) {
        if (emerge->enqueueBlockEmerge(peer_id, p, generate)) {
          if (nearest_emerged_d == -1)
            nearest_emerged_d = d;
        } else {
          if (nearest_emergefull_d == -1) // <=
            nearest_emergefull_d = d;
          goto queue_full_break;
        }
  ....
  }
  ....
queue_full_break:
  if (nearest_emerged_d != -1) { // <=
    new_nearest_unsent_d = nearest_emerged_d;
  } else ....
}

Il nearest_emergefull_d la variabile non cambia durante l'operazione di ciclo e il suo controllo non influisce sull'avanzamento dell'esecuzione dell'algoritmo. O questo è il risultato di un copia-incolla sciatto o si sono dimenticati di eseguire alcuni calcoli con esso.

V560 Una parte dell'espressione condizionale è sempre falsa:y> max_spawn_y. mapgen_v7.cpp 262

int MapgenV7::getSpawnLevelAtPoint(v2s16 p)
{
  ....
  while (iters > 0 && y <= max_spawn_y) {               // <=
    if (!getMountainTerrainAtPoint(p.X, y + 1, p.Y)) {
      if (y <= water_level || y > max_spawn_y)          // <=
        return MAX_MAP_GENERATION_LIMIT; // Unsuitable spawn point

      // y + 1 due to biome 'dust'
      return y + 1;
    }
  ....
}

Il valore di 'y ' viene controllata prima della successiva iterazione del ciclo. Un successivo confronto opposto restituirà sempre falso e in realtà non influisce sul risultato del controllo della condizione.

Mancato controllo del puntatore

V595 Il puntatore 'm_client' è stato utilizzato prima che fosse verificato rispetto a nullptr. Righe di controllo:183, 187. game.cpp 183

void gotText(const StringMap &fields)
{
  ....
  if (m_formname == "MT_DEATH_SCREEN") {
    assert(m_client != 0);
    m_client->sendRespawn();
    return;
  }

  if (m_client && m_client->modsLoaded())
    m_client->getScript()->on_formspec_input(m_formname, fields);
}

Prima di accedere a m_client puntatore, viene verificato se è nullo utilizzando assert macro. Ma questo si applica solo alla build di debug. Quindi, questa misura precauzionale viene sostituita con un manichino quando si costruisce per rilasciare e c'è il rischio di dereferenziare il puntatore nullo.

Un po' o non un po'?

V616 La costante denominata '(FT_RENDER_MODE_NORMAL)' con il valore 0 viene utilizzata nell'operazione bit per bit. CGUITTFont.h 360

typedef enum  FT_Render_Mode_
{
  FT_RENDER_MODE_NORMAL = 0,
  FT_RENDER_MODE_LIGHT,
  FT_RENDER_MODE_MONO,
  FT_RENDER_MODE_LCD,
  FT_RENDER_MODE_LCD_V,

  FT_RENDER_MODE_MAX
} FT_Render_Mode;

#define FT_LOAD_TARGET_( x )   ( (FT_Int32)( (x) & 15 ) << 16 )
#define FT_LOAD_TARGET_NORMAL  FT_LOAD_TARGET_( FT_RENDER_MODE_NORMAL )

void update_load_flags()
{
  // Set up our loading flags.
  load_flags = FT_LOAD_DEFAULT | FT_LOAD_RENDER;
  if (!useHinting()) load_flags |= FT_LOAD_NO_HINTING;
  if (!useAutoHinting()) load_flags |= FT_LOAD_NO_AUTOHINT;
  if (useMonochrome()) load_flags |= 
    FT_LOAD_MONOCHROME | FT_LOAD_TARGET_MONO | FT_RENDER_MODE_MONO;
  else load_flags |= FT_LOAD_TARGET_NORMAL; // <=
}

Il FT_LOAD_TARGET_NORMAL la macro viene distribuita a zero e "OR" bit per bit non imposterà alcun flag in load_flags , l'altro il ramo può essere rimosso.

Divisione intera arrotondata

V636 L'espressione 'rect.getHeight() / 16' è stata convertita in modo implicito dal tipo 'int' al tipo 'float'. Considerare l'utilizzo di un cast di tipo esplicito per evitare la perdita di una parte frazionaria. Un esempio:doppia A =(doppia)(X) / Y;. hud.cpp 771

void drawItemStack(....)
{
  float barheight = rect.getHeight() / 16;
  float barpad_x = rect.getWidth() / 16;
  float barpad_y = rect.getHeight() / 16;

  core::rect<s32> progressrect(
    rect.UpperLeftCorner.X + barpad_x,
    rect.LowerRightCorner.Y - barpad_y - barheight,
    rect.LowerRightCorner.X - barpad_x,
    rect.LowerRightCorner.Y - barpad_y);
}

Retta i getter restituiscono valori interi. Il risultato della divisione di numeri interi viene scritto in una variabile a virgola mobile e la parte frazionaria viene persa. Sembra che ci siano tipi di dati non corrispondenti in questi calcoli.

Sequenza sospetta di operatori ramificati

V646 Considerare di ispezionare la logica dell'applicazione. È possibile che manchi la parola chiave "altro". treegen.cpp 413

treegen::error make_ltree(...., TreeDef tree_definition)
{
  ....
  std::stack <core::matrix4> stack_orientation;
  ....
    if ((stack_orientation.empty() &&
      tree_definition.trunk_type == "double") ||
      (!stack_orientation.empty() &&
      tree_definition.trunk_type == "double" &&
      !tree_definition.thin_branches)) {
      ....
    } else if ((stack_orientation.empty() &&
      tree_definition.trunk_type == "crossed") ||
      (!stack_orientation.empty() &&
      tree_definition.trunk_type == "crossed" &&
      !tree_definition.thin_branches)) {
      ....
    } if (!stack_orientation.empty()) {                  // <=
  ....
  }
  ....
}

Ci sono altri-se sequenze nell'algoritmo di generazione dell'albero qui. Nel mezzo il prossimo se block è sulla stessa riga con la parentesi graffa di chiusura del precedente else dichiarazione. Forse il codice funziona correttamente:prima di questo se istruzione, vengono creati blocchi del tronco, seguiti da foglie. D'altra parte, è possibile che altro è mancato. Solo l'autore può dirlo con certezza.

Verifica dell'allocazione della memoria non corretta

V668 Non ha senso testare il puntatore "nuvole" rispetto a null, poiché la memoria è stata allocata utilizzando l'operatore "nuovo". L'eccezione verrà generata in caso di errore di allocazione della memoria. gioco.cpp 1367

bool Game::createClient(....)
{
  if (m_cache_enable_clouds) {
    clouds = new Clouds(smgr, -1, time(0));
    if (!clouds) {
      *error_message = "Memory allocation error (clouds)";
      errorstream << *error_message << std::endl;
      return false;
    }
  }
}

Se nuovo impossibile creare un oggetto, un std::bad_alloc viene generata un'eccezione e deve essere gestita da try-catch bloccare. Un controllo del genere è inutile.

Lettura al di fuori del limite dell'array

V781 Il valore dell'indice 'i' viene verificato dopo che è stato utilizzato. Forse c'è un errore nella logica del programma. irrString.h 572

bool equalsn(const string<T,TAlloc>& other, u32 n) const
{
  u32 i;
  for(i=0; array[i] && other[i] && i < n; ++i) // <=
    if (array[i] != other[i])
      return false;

  // if one (or both) of the strings was smaller then they
  // are only equal if they have the same length
  return (i == n) || (used == other.used);
}

È possibile accedere agli elementi dell'array prima di controllare l'indice, il che potrebbe causare un errore. Forse l'autore dovrebbe riscrivere il ciclo in questo modo:

for (i=0; i < n; ++i) // <=
  if (!array[i] || !other[i] || array[i] != other[i])
    return false;

Altri errori

Questo articolo illustra l'analisi delle richieste pull in Azure DevOps e non intende fornire una panoramica dettagliata degli errori rilevati nel progetto Minetest. Qui sono scritti solo alcuni frammenti di codice che ho trovato interessanti. Suggeriamo agli autori del progetto di non seguire questo articolo per correggere gli errori, ma di eseguire un'analisi più approfondita degli avvisi che PVS-Studio emetterà.

Conclusione

Grazie alla sua configurazione flessibile della riga di comando, l'analisi PVS-Studio può essere integrata in un'ampia varietà di scenari CI/CD. E il corretto utilizzo delle risorse disponibili ripaga aumentando la produttività.

Si noti che la modalità di verifica della richiesta pull è disponibile solo nella versione Enterprise dell'analizzatore. Per ottenere una licenza Enterprise demo, specificalo nei commenti quando richiedi una licenza nella pagina di download. Puoi saperne di più sulla differenza tra le licenze nella pagina Acquista PVS-Studio.