L'analyse de code statique montre la plus grande efficacité lors de la modification d'un projet, car les erreurs sont toujours plus difficiles à corriger à l'avenir que de les empêcher de se produire au début. Nous continuons à élargir les options d'utilisation de PVS-Studio dans les systèmes de développement continu et montrons comment configurer l'analyse des pull requests à l'aide d'agents auto-hébergés dans Microsoft Azure DevOps, en utilisant l'exemple du jeu Minetest.
En bref sur ce Ă quoi nous avons affaire
Minetest est un moteur de jeu multiplateforme open source contenant environ 200 000 lignes de code C, C ++ et Lua. Il vous permet de créer différents modes de jeu dans l'espace voxel. Prend en charge le multijoueur et de nombreux mods communautaires. Le référentiel du projet est hébergé ici: https://github.com/minetest/minetest .
Les outils suivants ont été utilisés pour mettre en place une recherche d'erreur régulière:
PVS-Studio est un analyseur de code statique en C, C ++, C # et Java pour trouver les erreurs et les défauts de sécurité.
Azure DevOps est une plateforme cloud qui permet de développer, d'exécuter des applications et de stocker des données sur des serveurs distants.
Vous pouvez utiliser des machines virtuelles Windows et Linux pour effectuer des tâches de développement dans Azure. Cependant, l'exécution d'agents sur du matériel local présente plusieurs avantages importants:
- Localhost peut avoir plus de ressources que la machine virtuelle Azure;
- L'agent ne «disparaît» pas après avoir terminé sa tâche;
- La possibilité de personnaliser directement l'environnement et un contrôle plus flexible sur les processus de construction;
- Le stockage local des fichiers intermédiaires a un effet positif sur la vitesse de construction;
- Vous pouvez effectuer plus de 30 tâches par mois gratuitement.
Se préparer à utiliser un agent auto-hébergé
Le processus de mise en route dans Azure est décrit en détail dans l'article « PVS-Studio Goes to the Clouds: Azure DevOps », je vais donc passer directement à la création d'un agent auto-hébergé.
Pour que les agents aient le droit de se connecter aux pools de projets, ils ont besoin d'un jeton d'accès spécial. Vous pouvez l'obtenir sur la page "Personal Access Tokens", dans le menu "User settings".
Après avoir cliqué sur "Nouveau jeton", vous devez spécifier un nom et sélectionner Lire et gérer les pools d'agents (vous devrez peut-être développer la liste complète via "Afficher toutes les étendues").
Vous devez copier le jeton, car Azure ne l'affichera plus et vous devrez en créer un nouveau.
Un conteneur Docker basé sur Windows Server Core sera utilisé comme agent. L'hôte est mon ordinateur de travail sous Windows 10 x64 avec Hyper-V.
Tout d'abord, vous devez augmenter la quantité d'espace disque disponible pour les conteneurs Docker.
Sous Windows, pour cela, vous devez modifier le fichier 'C: \ ProgramData \ Docker \ config \ daemon.json' comme suit:
{
"registry-mirrors": [],
"insecure-registries": [],
"debug": true,
"experimental": false,
"data-root": "d:\\docker",
"storage-opts": [ "size=40G" ]
}Pour créer une image Docker pour les agents avec un système de build et tout ce dont vous avez besoin dans le répertoire 'D: \ docker-agent', ajoutez un fichier Docker avec le contenu suivant:
# 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.ps1Le résultat sera un système de construction basé sur MSBuild pour C ++, avec Chocolatey pour l'installation de PVS-Studio, CMake et Git. Pour une gestion pratique des bibliothèques dont dépend le projet, Vcpkg est construit. Et la dernière version de l'agent Azure Pipelines est également téléchargée.
Pour initialiser l'agent à partir du fichier Docker ENTRYPOINT, le script PowerShell 'entrypoint.ps1' est appelé, auquel vous devez ajouter l'URL "organisation" du projet, le jeton de pool d'agents et les paramètres de licence 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
}Commandes pour créer l'image et démarrer l'agent:
docker build -t azure-agent -m 4GB .
docker run -id --name my-agent -m 4GB --cpu-count 4 azure-agent
L'agent est en cours d'exécution et prêt à exécuter des tâches.
Exécution d'une analyse sur un agent auto-hébergé
Pour l'analyse PR, un nouveau pipeline est créé avec le script suivant:
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'Ce script s'exécutera à la réception du PR et sera exécuté sur les agents affectés au pool par défaut. Il vous suffit de lui donner la permission de travailler avec ce pool.
Le script enregistre la liste des fichiers modifiés obtenus à l'aide de git diff. Ensuite, les dépendances sont mises à jour, la solution de projet est générée via CMake et elle est générée.
Si la construction a réussi, l'analyse des fichiers modifiés est lancée (drapeau '-f diff-files.txt'), en ignorant les projets auxiliaires créés par CMake (sélectionnez uniquement le projet requis avec le drapeau '-S minetest'). Pour accélérer la recherche de liens entre l'en-tête et les fichiers source C ++, un cache spécial est créé, qui sera stocké dans un répertoire séparé (drapeau '-DC: \ caches').
Ainsi, nous pouvons désormais recevoir des rapports sur l'analyse des évolutions du projet.
Comme il a été dit au début de l'article, un avantage agréable de l'utilisation d'agents auto-hébergés est une accélération notable de l'exécution des tâches grâce au stockage local des fichiers intermédiaires.
Quelques bugs trouvés dans Minetest
RĂ©sultat de l'Ă©crasement
V519 La variable 'color_name' reçoit des valeurs deux fois de suite. C'est peut-être une erreur. Vérifiez les lignes: 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;
....
}Cette fonction doit analyser un nom de couleur avec un paramètre de transparence (par exemple, Green # 77 ) et renvoyer son code. En fonction du résultat de la vérification de la condition, le résultat de la division de ligne ou une copie de l'argument de la fonction est passé à la variable color_name . Cependant, ce n'est pas la chaîne résultante elle-même qui est convertie en minuscules, mais l'argument d'origine. Par conséquent, il ne peut pas être trouvé dans le dictionnaire des couleurs si le paramètre de transparence est présent. Nous pouvons corriger cette ligne comme ceci:
color_name = lowercase(color_name);VĂ©rifications de conditions inutiles V547 L' expression 'plus proche_emergefull_d == -1' est toujours vraie. 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 la plus proche_emergefull_d ne change pas pendant l'opération de boucle et sa vérification n'affecte pas l'exécution de l'algorithme. Soit c'est le résultat d'un copier-coller inexact, soit ils ont oublié de faire des calculs avec.
V560 Une partie de l'expression conditionnelle est toujours fausse: 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;
}
....
}La valeur de la variable « y » est vérifiée avant la prochaine itération de la boucle. La comparaison suivante, opposée, retournera toujours faux et, en général, n'affectera pas le résultat du test de condition.
Perte des vérifications de pointeur
V595 du pointeur 'm_client' WAS Utilisé avant qu'il ne soit vérifié contre nullptr. Vérifiez les lignes: 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);
}Avant d'accéder au pointeur m_client , il est vérifié s'il n'est pas nul à l'aide de la macro assert . Mais cela ne s'applique qu'à la version de débogage. Une telle précaution, lors de l'intégration dans la version, est remplacée par un mannequin, et il existe un risque de déréférencer un pointeur nul.
Peu ou pas peu?
V616 La constante nommée '(FT_RENDER_MODE_NORMAL)' avec la valeur 0 est utilisée dans l'opération au niveau du 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 développe à zéro, et le "ou" au niveau du bit ne définira aucun indicateur dans load_flags , la branche else peut être supprimée.
Arrondir la division entière
V636 L' expression 'rect.getHeight () / 16' a été implicitement convertie du type 'int' en type 'float'. Pensez à utiliser une conversion de type explicite pour éviter la perte d'une partie fractionnaire. Un exemple: double A = (double) (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);
}Getters rect renvoie une valeur entière. Le résultat de la division d'entiers est écrit dans une variable à virgule flottante et la partie fractionnaire est perdue. Il semble qu'il existe des types de données incompatibles dans ces calculs. Instructions de
branchement de séquence suspecte
V646 Envisagez d'inspecter la logique de l'application. Il est possible que le mot-clé «else» soit manquant. 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()) { // <=
....
}
....
}Voici les séquences else-if dans l'algorithme de génération d'arbre. Au milieu, le bloc if suivant est sur la même ligne avec la parenthèse fermante du bloc else précédent . Peut-être que le code fonctionne correctement: avant cela si -a, les blocs de tronc sont créés, puis les feuilles; ou peut-être qu'ils ont manqué l' autre . Cela ne peut certainement être dit que par le développeur.
Vérification d'allocation de mémoire incorrecte
V668 Il n'y a aucun sens à tester le pointeur «clouds» par rapport à null, car la mémoire a été allouée à l'aide de l'opérateur «new». L'exception sera générée en cas d'erreur d'allocation de mémoire. 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;
}
}
}Dans le cas nouveau ne parvient pas à créer un objet, un std :: bad_alloc exception sera levée et doit être manipulé par un try-catch bloc. Et l'enregistrement sous ce formulaire est inutile.
Lecture d'un tableau hors
limites V781 La valeur de l'index 'i' est vérifiée après son utilisation. Il y a peut-être une erreur dans la logique du programme. 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);
}Les éléments du tableau sont accédés avant de vérifier l'index, ce qui peut conduire à une erreur. Cela peut valoir la peine de réécrire la boucle comme ceci:
for (i=0; i < n; ++i) // <=
if (!array[i] || !other[i] || array[i] != other[i])
return false;Autres erreurs
Cet article porte sur l'analyse des demandes d'extraction dans Azure DevOps et n'est pas destiné à fournir une vue d'ensemble détaillée des erreurs dans le projet Minetest. Voici quelques extraits de code que j'ai trouvés intéressants. Nous suggérons aux auteurs du projet de ne pas suivre cet article pour corriger les erreurs et effectuer une analyse plus approfondie des avertissements que PVS-Studio émettra.
Conclusion
Grâce à la configuration flexible en mode ligne de commande, l'analyse PVS-Studio peut être intégrée dans une grande variété de scénarios CI / CD. Et utiliser correctement les ressources disponibles se traduit par une productivité accrue.
Il convient de noter que le mode de vérification de la demande d'extraction n'est disponible que dans l'édition Entreprise de l'analyseur. Pour obtenir une licence démo Enterprise, veuillez l'indiquer dans les commentaires lors de la demande de licence sur la page de téléchargement . Vous pouvez en savoir plus sur la différence entre les licences sur la page de commande PVS-Studio .
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien de traduction: Alexey Govorov. PVS-Studio: analyse des pull requests dans Azure DevOps à l'aide d'agents auto-hébergés .