PVS-Studio: análisis de solicitudes de extracción en Azure DevOps utilizando agentes autohospedados





El análisis de código estático es más efectivo cuando se realizan cambios en un proyecto, ya que los errores siempre son más difíciles de corregir en el futuro que evitar que ocurran en las primeras etapas. Continuamos expandiendo las opciones para usar PVS-Studio en sistemas de desarrollo continuo y mostramos cómo configurar el análisis de solicitudes de extracción utilizando agentes autohospedados en Microsoft Azure DevOps, usando el ejemplo del juego Minetest.



Brevemente sobre lo que estamos tratando



Minetest es un motor de juego multiplataforma de código abierto que contiene aproximadamente 200.000 líneas de código C, C ++ y Lua. Te permite crear diferentes modos de juego en el espacio voxel. Admite multijugador y muchas modificaciones de la comunidad. El repositorio del proyecto está alojado aquí: https://github.com/minetest/minetest .



Se utilizaron las siguientes herramientas para configurar una búsqueda de errores regular:



PVS-Studio es un analizador de código estático en C, C ++, C # y Java para encontrar errores y defectos de seguridad.



Azure DevOps es una plataforma basada en la nube que brinda la capacidad de desarrollar, ejecutar aplicaciones y almacenar datos en servidores remotos.



Puede usar máquinas virtuales de Windows y Linux para realizar tareas de desarrollo en Azure. Sin embargo, ejecutar agentes en hardware local tiene varias ventajas importantes:



  • Localhost puede tener más recursos que Azure VM;
  • El agente no "desaparece" después de completar su tarea;
  • La capacidad de personalizar directamente el entorno y un control más flexible sobre los procesos de construcción;
  • El almacenamiento de archivos intermedios localmente tiene un efecto positivo en la velocidad de construcción;
  • Puede completar más de 30 tareas por mes de forma gratuita.


Preparación para utilizar un agente autohospedado



El proceso de introducción en Azure se describe en detalle en el artículo " PVS-Studio va a las nubes: Azure DevOps ", por lo que pasaré directamente a crear un agente autohospedado.



Para que los agentes tengan derecho a conectarse a grupos de proyectos, necesitan un token de acceso especial. Puede obtenerlo en la página "Tokens de acceso personal", en el menú "Configuración de usuario".



image2.png


Después de hacer clic en "Nuevo token", debe especificar un nombre y seleccionar Leer y administrar grupos de agentes (es posible que deba expandir la lista completa a través de "Mostrar todos los ámbitos").



image3.png


Debe copiar el token, ya que Azure no lo volverá a mostrar y tendrá que crear uno nuevo.



image4.png


Se utilizará como agente un contenedor Docker basado en Windows Server Core. El host es mi computadora de trabajo en Windows 10 x64 con Hyper-V.



Primero, necesita expandir la cantidad de espacio en disco disponible para los contenedores de Docker.



En Windows, debe modificar el archivo 'C: \ ProgramData \ Docker \ config \ daemon.json' de la siguiente manera:



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


Para crear una imagen de Docker para agentes con un sistema de compilación y todo lo que necesita, en el directorio 'D: \ docker-agent', agregue un archivo Docker con el siguiente contenido:



# 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


El resultado será un sistema de compilación basado en MSBuild para C ++, con Chocolatey para instalar PVS-Studio, CMake y Git. Para una gestión conveniente de las bibliotecas de las que depende el proyecto, se construye Vcpkg. Y también se descarga la última versión del Agente de Azure Pipelines.



Para inicializar el agente desde el archivo de Docker ENTRYPOINT, se llama al script de PowerShell 'entrypoint.ps1', al que debe agregar la URL de la "organización" del proyecto, el token del grupo de agentes y los parámetros de licencia de PVS-Studio:



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

$pvs_studio_user = "<  PVS-Studio>"
$pvs_studio_key = "< PVS-Studio>"

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
}


Comandos para construir la imagen e iniciar el agente:



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


image5.png


El agente está en ejecución y listo para realizar tareas.



image6.png


Ejecución de análisis en un agente autohospedado



Para el análisis de relaciones públicas, se crea una nueva canalización con el siguiente script:



image7.png


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'


Este script se ejecutará cuando se reciba el RP y se ejecutará en los agentes asignados al grupo predeterminado. Solo necesita darle permiso para trabajar con este grupo.



image8.png




image9.png


El script guarda la lista de archivos modificados obtenidos usando git diff. Luego se actualizan las dependencias, se genera la solución del proyecto a través de CMake y se construye.



Si la compilación es exitosa, se inicia el análisis de los archivos modificados (el indicador '-f diff-files.txt'), ignorando los proyectos auxiliares creados por CMake (seleccionamos solo el proyecto requerido con el indicador '-S minetest'). Para acelerar la búsqueda de enlaces entre el encabezado y los archivos fuente de C ++, se crea una caché especial, que se almacenará en un directorio separado (marca '-DC: \ caches').



Así, ahora podemos recibir informes sobre el análisis de cambios en el proyecto.



image10.png




image11.png


Como se dijo al principio del artículo, una ventaja agradable de usar agentes autohospedados es una notable aceleración de la ejecución de tareas debido al almacenamiento local de archivos intermedios.



image13.png


Algunos errores encontrados en Minetest



Resultado de sobrescritura



V519 A la variable 'color_name' se le asignan valores dos veces seguidas. Quizás esto sea un error. Verifique las líneas: 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;
  ....
}


Esta función debe analizar el nombre de un color con un parámetro de transparencia (por ejemplo, Green # 77 ) y devolver su código. Dependiendo del resultado de verificar la condición, el resultado de la división de línea o una copia del argumento de la función se pasa a la variable color_name . Sin embargo, no la cadena resultante en sí misma se convierte a minúsculas, sino el argumento original. Como resultado, no se puede encontrar en el diccionario de colores si el parámetro de transparencia está presente. Podemos arreglar esta línea así:



color_name = lowercase(color_name);


Comprobaciones de condición



innecesarias V547 La expresión 'near_emergefull_d == -1' siempre es verdadera. 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 ....
}


La variable más cercana_emergefull_d no cambia durante la operación del bucle y su verificación no afecta la ejecución del algoritmo. O esto es el resultado de copiar y pegar incorrectamente, o se olvidaron de realizar algunos cálculos con él.



V560 Una parte de la expresión condicional es siempre 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;
    }
  ....
}


El valor de la variable ' y ' se comprueba antes de la siguiente iteración del ciclo. La comparación posterior, opuesta, siempre devolverá falso y, en general, no afecta el resultado de la prueba de condición.



Pérdida de comprobaciones de puntero



V595 del puntero 'm_client' FUE Utilizado antes de que FUE verificado contra nullptr. Líneas de verificación: 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);
}


Antes de acceder al m_client puntero, se comprobó si no es nulo utilizando la aserción macro . Pero esto solo se aplica a la compilación de depuración. Dicha precaución, cuando se incorpora a la versión, se reemplaza con un maniquí y existe el riesgo de desreferenciar un puntero nulo.



¿Poco o no poco?



V616 La constante denominada '(FT_RENDER_MODE_NORMAL)' con el valor 0 se utiliza en la operación bit a 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; // <=
}


La macro FT_LOAD_TARGET_NORMAL se expande a cero, y el bit a bit "o" no establecerá ningún indicador en load_flags , la rama else puede eliminarse.



Redondeo de la división de enteros



V636 La expresión 'rect.getHeight () / 16' se ha convertido implícitamente del tipo 'int' al tipo 'float'. Considere utilizar un molde de tipo explícito para evitar la pérdida de una parte fraccionaria. Un ejemplo: doble A = (doble) (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);
}


Los captadores rect devuelven valores enteros. El resultado de la división de números enteros se escribe en una variable de coma flotante y la parte fraccionaria se pierde. Parece que hay tipos de datos no coincidentes en estos cálculos.



Declaraciones de bifurcación de secuencias sospechosas



V646 Considere inspeccionar la lógica de la aplicación. Es posible que falte la palabra clave "más". 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()) {                  // <=
  ....
  }
  ....
}


Aquí están las secuencias else-if en el algoritmo de generación de árboles. En el medio, el siguiente bloque if está en la misma línea con el paréntesis de cierre del else anterior . Quizás el código funcione correctamente: antes de este if -a, se crean los bloques troncales, y luego las hojas; o tal vez se perdieron lo demás . Seguramente esto solo lo puede decir el desarrollador.



Verificación de asignación de memoria incorrecta



V668 No tiene sentido probar el puntero 'nubes' contra nulo, ya que la memoria se asignó utilizando el operador 'nuevo'. La excepción se generará en caso de error de asignación de memoria. game.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;
    }
  }
}


En caso de que new no pueda crear un objeto, se lanzará una excepción std :: bad_alloc y debe ser manejada por un bloque try-catch . Y comprobar en este formulario es inútil.



Leyendo una matriz fuera de los



límites V781 El valor del índice 'i' se verifica después de su uso. Quizás haya un error en la lógica del programa. 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);
}


Se accede a los elementos de la matriz antes de comprobar el índice, lo que puede provocar un error. Podría valer la pena reescribir el ciclo de esta manera:



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


Otros errores



Este artículo trata sobre el análisis de solicitudes de extracción en Azure DevOps y no pretende proporcionar una descripción general detallada de los errores en el proyecto Minetest. Estos son solo algunos de los fragmentos de código que me parecieron interesantes. Sugerimos que los autores del proyecto no sigan este artículo para corregir errores y realizar un análisis más profundo de las advertencias que emitirá PVS-Studio.



Conclusión



Gracias a la configuración flexible en el modo de línea de comandos, el análisis de PVS-Studio se puede integrar en una amplia variedad de escenarios de CI / CD. Y hacer el uso correcto de los recursos disponibles da como resultado una mayor productividad.



Cabe señalar que el modo de verificación de solicitud de extracción solo está disponible en la edición Enterprise del analizador. Para obtener una licencia Enterprise de demostración, indíquelo en los comentarios cuando solicite una licencia en la página de descarga . Puede obtener más información sobre la diferencia entre las licencias en la página de pedidos de PVS-Studio .





Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace de traducción: Alexey Govorov. PVS-Studio: análisis de solicitudes de extracción en Azure DevOps mediante agentes autohospedados .



All Articles