PVS-Studio:análisis de solicitudes de incorporación de cambios en Azure DevOps mediante agentes autohospedados

 C Programming >> Programación C >  >> Tags >> Azure
PVS-Studio:análisis de solicitudes de incorporación de cambios en Azure DevOps mediante agentes autohospedados

El análisis de código estático es más efectivo cuando se cambia un proyecto, ya que los errores siempre son más difíciles de corregir en el futuro que en una etapa inicial. Seguimos ampliando las opciones de uso de PVS-Studio en sistemas de desarrollo continuo. Esta vez, le mostraremos cómo configurar el análisis de solicitud de incorporación de cambios mediante 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 alrededor de 200 000 líneas de código en 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 se encuentra aquí:https://github.com/minetest/minetest.

Las siguientes herramientas se utilizan para configurar la detección regular de errores:

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

Azure DevOps es una plataforma en la nube que le permite desarrollar, ejecutar aplicaciones y almacenar datos en servidores remotos.

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

  • El host local puede tener más recursos que una máquina virtual de Azure;
  • El agente no "desaparece" después de completar su tarea;
  • Capacidad para configurar directamente el entorno y una gestión más flexible de los procesos de compilación;
  • El almacenamiento local de archivos intermedios tiene un efecto positivo en la velocidad de compilación;
  • Puede completar más de 30 tareas por mes de forma gratuita.

Preparación para usar un agente autohospedado

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

Para que los agentes puedan 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".

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").

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

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

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

Para hacer esto, 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 Docker para agentes con el sistema de compilación y todo lo necesario, agreguemos un archivo Docker con el siguiente contenido en el directorio '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

El resultado es un sistema de compilación basado en MSBuild para C++, con Chocolatey para instalar PVS-Studio, CMake y Git. Vcpkg está diseñado para una gestión conveniente de las bibliotecas de las que depende el proyecto. Además, tenemos que descargar la última versión del Agente de Azure Pipelines.

Para inicializar el agente desde el archivo ENTRYPOINT Docker, 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 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
}

Comandos para construir una 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

El agente se está ejecutando y listo para realizar tareas.

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:

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 funcionará cuando se reciba un PR y se ejecutará en los agentes asignados al grupo de forma predeterminada. Solo necesita darle permiso para trabajar con este grupo.

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

Si la compilación fue exitosa, se inicia el análisis de los archivos modificados (el indicador '-f diff-files.txt'), ignorando los proyectos auxiliares creados por CMake (seleccione solo el proyecto necesario con el indicador '-S minetest'). Para agilizar la determinación de las relaciones entre el encabezado y los archivos fuente de C++, se crea un caché especial, que se almacenará en un directorio separado (el indicador '-D C:\caches').

De esta manera ahora podemos obtener informes sobre el análisis de cambios en el proyecto.

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

Algunos errores encontrados en Minetest

Sobrescribir el resultado

V519 A la variable 'color_name' se le asignan valores dos veces seguidas. Quizás esto sea un error. Comprobar 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 del color con el parámetro de transparencia (por ejemplo, Green#77 ) y devolver su código. Dependiendo del resultado de verificar la condición, el color_name La variable se pasa el resultado de dividir la cadena o una copia del argumento de la función. Sin embargo, el argumento original se convierte a minúsculas en lugar de la propia cadena resultante. 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 redundantes de condiciones

V547 La expresión 'nearest_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 ....
}

El más cercano_emergefull_d La variable no cambia durante la operación del ciclo y su verificación no afecta el progreso de la ejecución del algoritmo. O esto es el resultado de un copiado y pegado descuidado, o se olvidaron de realizar algunos cálculos con él.

V560 Una parte de la expresión condicional siempre es 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 'y ' La variable se comprueba antes de la siguiente iteración del bucle. Una comparación opuesta posterior siempre devolverá falso y en realidad no afecta el resultado de verificar la condición.

Comprobación de puntero perdida

V595 El puntero 'm_client' se utilizó antes de que se verificara con nullptr. Comprobar líneas: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 comprueba si es nulo usando el afirmar macro. Pero esto solo se aplica a la compilación de depuración. Por lo tanto, esta medida de precaución se reemplaza con un dummy cuando se construye para liberar, y existe el riesgo de desreferenciar el puntero nulo.

¿Un poco o no un poco?

V616 La constante nombrada '(FT_RENDER_MODE_NORMAL)' con el valor de 0 se usa 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; // <=
}

El FT_LOAD_TARGET_NORMAL la macro se implementa en cero, y el "O" bit a bit no establecerá ningún indicador en load_flags , el más la rama se puede eliminar.

Redondeo de división entera

V636 La expresión 'rect.getHeight() / 16' se transformó implícitamente del tipo 'int' al tipo 'float'. Considere utilizar una conversión de tipos explícita 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);
}

Recto los captadores devuelven valores enteros. El resultado de dividir números enteros se escribe en una variable de punto flotante y la parte fraccionaria se pierde. Parece que hay tipos de datos que no coinciden en estos cálculos.

Secuencia sospechosa de operadores de bifurcación

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

Hay si no secuencias en el algoritmo de generación de árboles aquí. En el medio el siguiente si el bloque está en la misma línea que la llave de cierre del else anterior declaración. Tal vez, el código funcione correctamente:antes de este if declaración, se crean bloques del tronco, seguidos de hojas. Por otro lado, es posible que else se extraña. Solo el autor puede decir esto con seguridad.

Comprobación de asignación de memoria incorrecta

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

Si nuevo no se puede crear un objeto, un std::bad_alloc se lanza una excepción, y debe ser manejada por try-catch bloquear. Un cheque como este es inútil.

Leyendo fuera del límite de la matriz

V781 El valor del índice 'i' se comprueba después de su uso. Quizás hay un error en la lógica del programa. cadenairr.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. Quizás el autor debería reescribir el bucle 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 cubre el análisis de las solicitudes de incorporación de cambios en Azure DevOps y no pretende proporcionar una descripción general detallada de los errores encontrados en el proyecto Minetest. Solo algunos fragmentos de código que encontré interesantes están escritos aquí. Sugerimos que los autores del proyecto no sigan este artículo para corregir errores, sino que realicen un análisis más exhaustivo de las advertencias que emitirá PVS-Studio.

Conclusión

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

Tenga en cuenta que el modo de verificación de solicitud de extracción solo está disponible en la versión Enterprise del analizador. Para obtener una licencia Enterprise de demostración, especifíquelo en los comentarios cuando solicite una licencia en la página de descarga. Puede obtener más información sobre la diferencia entre licencias en la página Comprar PVS-Studio.