Die statische Code-Analyse ist am effektivsten, wenn Änderungen an einem Projekt vorgenommen werden, da Fehler in Zukunft immer schwieriger zu beheben sind, als zu verhindern, dass sie in einem frühen Stadium auftreten. Wir erweitern die Optionen für die Verwendung von PVS-Studio in kontinuierlichen Entwicklungssystemen weiter und zeigen am Beispiel des Minetest-Spiels, wie die Analyse von Pull-Anforderungen mithilfe von selbst gehosteten Agenten in Microsoft Azure DevOps eingerichtet wird.
Kurz darüber, womit wir es zu tun haben
Minetest ist eine plattformübergreifende Open-Source-Spiel-Engine, die etwa 200.000 Zeilen C-, C ++ - und Lua-Code enthält. Sie können damit verschiedene Spielmodi im Voxelraum erstellen. Unterstützt Multiplayer und viele Community Mods. Das Projekt-Repository wird hier gehostet: https://github.com/minetest/minetest .
Die folgenden Tools wurden verwendet, um eine regelmäßige Fehlersuche einzurichten:
PVS-Studio ist ein statischer Code-Analysator in C, C ++, C # und Java, um Fehler und Sicherheitsmängel zu finden.
Azure DevOps ist eine Cloud-basierte Plattform, mit der Anwendungen entwickelt, ausgeführt und Daten auf Remoteservern gespeichert werden können.
Sie können virtuelle Windows- und Linux-Maschinen verwenden, um Entwicklungsaufgaben in Azure auszuführen. Das Ausführen von Agenten auf lokaler Hardware bietet jedoch mehrere wesentliche Vorteile:
- Localhost verfügt möglicherweise über mehr Ressourcen als Azure VM.
- Der Agent "verschwindet" nicht nach Abschluss seiner Aufgabe.
- Die Möglichkeit, die Umgebung direkt anzupassen und die Erstellungsprozesse flexibler zu steuern;
- Das lokale Speichern von Zwischendateien wirkt sich positiv auf die Erstellungsgeschwindigkeit aus.
- Sie können über 30 Aufgaben pro Monat kostenlos erledigen.
Vorbereiten der Verwendung eines selbst gehosteten Agenten
Der Einstieg in Azure wird im Artikel " PVS-Studio geht in die Clouds: Azure DevOps " ausführlich beschrieben. Daher werde ich direkt einen selbst gehosteten Agenten erstellen.
Damit Agenten das Recht haben, eine Verbindung zu Projektpools herzustellen, benötigen sie ein spezielles Zugriffstoken. Sie können es auf der Seite "Persönliche Zugriffstoken" im Menü "Benutzereinstellungen" herunterladen.
Nachdem Sie auf "Neues Token" geklickt haben, müssen Sie einen Namen angeben und "Agentenpools lesen und verwalten" auswählen (möglicherweise müssen Sie die vollständige Liste über "Alle Bereiche anzeigen" erweitern).
Sie müssen das Token kopieren, da Azure es nicht erneut anzeigt und Sie ein neues erstellen müssen.
Ein auf Windows Server Core basierender Docker-Container wird als Agent verwendet. Der Host ist mein Arbeitscomputer unter Windows 10 x64 mit Hyper-V.
Zunächst müssen Sie den für Docker-Container verfügbaren Speicherplatz erweitern.
Unter Windows müssen Sie dazu die Datei 'C: \ ProgramData \ Docker \ config \ daemon.json' wie folgt ändern:
{
"registry-mirrors": [],
"insecure-registries": [],
"debug": true,
"experimental": false,
"data-root": "d:\\docker",
"storage-opts": [ "size=40G" ]
}Fügen Sie im Verzeichnis 'D: \ docker-agent' eine Docker-Datei mit folgendem Inhalt hinzu, um ein Docker-Image für Agenten mit einem Build-System und allem, was Sie benötigen, zu erstellen:
# 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.ps1Das Ergebnis wird ein MSBuild-basiertes Build-System für C ++ sein, mit Chocolatey für die Installation von PVS-Studio, CMake und Git. Zur bequemen Verwaltung der Bibliotheken, von denen das Projekt abhängt, wird Vcpkg erstellt. Außerdem wird die neueste Version des Azure Pipelines Agent heruntergeladen.
Um den Agenten aus der ENTRYPOINT Docker-Datei zu initialisieren, wird das PowerShell-Skript 'entrypoint.ps1' aufgerufen, zu dem Sie die URL der Projektorganisation, das Agentenpool-Token und die PVS-Studio-Lizenzparameter hinzufügen müssen:
$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
}Befehle zum Erstellen des Images und Starten des Agenten:
docker build -t azure-agent -m 4GB .
docker run -id --name my-agent -m 4GB --cpu-count 4 azure-agent
Der Agent wird ausgeführt und ist bereit, Aufgaben auszuführen.
Ausführen einer Analyse auf einem selbst gehosteten Agenten
Für die PR-Analyse wird eine neue Pipeline mit dem folgenden Skript erstellt:
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'Dieses Skript wird ausgeführt, wenn die PR empfangen wird, und wird auf Agenten ausgeführt, die dem Standardpool zugewiesen sind. Sie müssen ihm nur die Erlaubnis geben, mit diesem Pool zu arbeiten.
Das Skript speichert die Liste der geänderten Dateien, die mit git diff erhalten wurden. Anschließend werden die Abhängigkeiten aktualisiert, die Projektlösung über CMake generiert und erstellt.
Wenn der Build erfolgreich war, wird die Analyse der geänderten Dateien gestartet (Flag '-f diff-files.txt'), wobei die von CMake erstellten Hilfsprojekte ignoriert werden (wählen Sie nur das gewünschte Projekt mit dem Flag '-S minetest' aus). Um die Suche nach Verknüpfungen zwischen Header- und C ++ - Quelldateien zu beschleunigen, wird ein spezieller Cache erstellt, der in einem separaten Verzeichnis gespeichert wird (Flag '-DC: \ caches').
So können wir jetzt Berichte über die Analyse von Änderungen im Projekt erhalten.
Wie bereits zu Beginn des Artikels erwähnt, ist ein angenehmer Vorteil der Verwendung von selbst gehosteten Agenten eine spürbare Beschleunigung der Aufgabenausführung aufgrund der lokalen Speicherung von Zwischendateien.
Einige Fehler im Minetest gefunden
Ergebnis
überschreiben V519 Der Variablen 'color_name' werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 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;
....
}Diese Funktion sollte einen Farbnamen mit einem Transparenzparameter (z. B. Grün # 77 ) analysieren und seinen Code zurückgeben. Abhängig vom Ergebnis der Überprüfung der Bedingung wird das Ergebnis der Zeilenaufteilung oder eine Kopie des Funktionsarguments an die Variable color_name übergeben . Dann wird jedoch nicht die resultierende Zeichenfolge selbst in Kleinbuchstaben konvertiert, sondern das ursprüngliche Argument. Daher kann es im Farbwörterbuch nicht gefunden werden, wenn der Transparenzparameter vorhanden ist. Wir können diese Zeile folgendermaßen reparieren:
color_name = lowercase(color_name);Unnötige Bedingungsprüfungen V547 Der Ausdruck 'next_emergefull_d == -1' ist immer wahr. 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 ....
}Die Variable next_emergefull_d ändert sich während der Schleifenoperation nicht und ihre Überprüfung hat keinen Einfluss auf die Ausführung des Algorithmus. Entweder ist dies das Ergebnis eines ungenauen Kopierens und Einfügens, oder sie haben vergessen, einige Berechnungen damit durchzuführen.
V560 Ein Teil des bedingten Ausdrucks ist immer falsch: 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;
}
....
}Der Wert der Variablen ' y ' wird vor der nächsten Iteration der Schleife überprüft. Der nachfolgende, entgegengesetzte Vergleich gibt immer false zurück und hat im Allgemeinen keinen Einfluss auf das Ergebnis des Bedingungstests.
Verlust von
Zeigerprüfungen V595 des Zeigers 'm_client' wurde verwendet, bevor IT gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 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);
}Bevor Sie den Zugriff auf m_client Zeiger, wird geprüft , ob es nicht null ist mit der assert - Makro . Dies gilt jedoch nur für den Debug-Build. Eine solche Vorsichtsmaßnahme wird beim Einbau in die Freigabe durch einen Dummy ersetzt, und es besteht die Gefahr, dass ein Nullzeiger dereferenziert wird.
Bit oder nicht Bit?
V616 Die benannte Konstante '(FT_RENDER_MODE_NORMAL)' mit dem Wert 0 wird in der bitweisen Operation verwendet. 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; // <=
}Das Makro FT_LOAD_TARGET_NORMAL wird auf Null erweitert , und das bitweise "oder" setzt keine Flags in load_flags . Der Zweig else kann entfernt werden.
Rundung der Ganzzahldivision
V636 Der Ausdruck 'rect.getHeight () / 16' wurde implizit vom Typ 'int' in den Typ 'float' umgewandelt. Erwägen Sie die Verwendung eines expliziten Typgusses, um den Verlust eines Bruchteils zu vermeiden. Ein Beispiel: 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 gibt einen ganzzahligen Wert zurück. Das Ergebnis der Division von ganzen Zahlen wird in eine Gleitkommavariable geschrieben, und der Bruchteil geht verloren. Es sieht so aus, als ob diese Berechnungen nicht übereinstimmende Datentypen enthalten.
Verzweigungsanweisungen für verdächtige Sequenzen
V646 Überprüfen Sie die Logik der Anwendung. Möglicherweise fehlt das Schlüsselwort "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()) { // <=
....
}
....
}Hier sind die else-if- Sequenzen im Baumgenerierungsalgorithmus. In der Mitte befindet sich der nächste if- Block in derselben Zeile wie die schließende Klammer des vorherigen else . Vielleicht funktioniert der Code richtig: davor, wenn -a, werden die Trunk-Blöcke erstellt und dann die Blätter; oder vielleicht haben sie das andere verpasst . Dies kann sicherlich nur der Entwickler sagen.
Ungültige Speicherzuweisungsprüfung
V668 Es macht keinen Sinn, den ' Clouds' -Zeiger gegen Null zu testen, da der Speicher mit dem' neuen 'Operator zugewiesen wurde. Die Ausnahme wird im Falle eines Speicherzuordnungsfehlers generiert. 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;
}
}
}Falls new kein Objekt erstellen kann, wird eine std :: bad_alloc- Ausnahme ausgelöst , die von einem try-catch- Block behandelt werden muss. Das Einchecken in dieses Formular ist nutzlos.
Lesen eines Arrays außerhalb der
Grenzen V781 Der Wert des ' i' -Index wird nach seiner Verwendung überprüft. Möglicherweise liegt ein Fehler in der Programmlogik vor. 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);
}Auf die Array-Elemente wird zugegriffen, bevor der Index überprüft wird, was zu einem Fehler führen kann. Es könnte sich lohnen, die Schleife folgendermaßen umzuschreiben:
for (i=0; i < n; ++i) // <=
if (!array[i] || !other[i] || array[i] != other[i])
return false;Sonstige Fehler
Dieser Artikel befasst sich mit der Analyse von Pull-Anforderungen in Azure DevOps und bietet keinen detaillierten Überblick über die Fehler im Minetest-Projekt. Hier sind nur einige der Code-Schnipsel, die ich interessant fand. Wir empfehlen, dass die Autoren des Projekts diesen Artikel nicht befolgen, um Fehler zu beheben und eine gründlichere Analyse der von PVS-Studio ausgegebenen Warnungen durchzuführen.
Fazit
Dank der flexiblen Konfiguration im Befehlszeilenmodus kann die PVS-Studio-Analyse in eine Vielzahl von CI / CD-Szenarien eingebettet werden. Die richtige Nutzung der verfügbaren Ressourcen zahlt sich in einer höheren Produktivität aus.
Es ist zu beachten, dass der Pull-Request-Check-Modus nur in der Enterprise Edition des Analysators verfügbar ist. Um eine Demo Enterprise-Lizenz zu erhalten, geben Sie dies bitte in den Kommentaren an, wenn Sie eine Lizenz auf der Download-Seite anfordern . Weitere Informationen zum Unterschied zwischen Lizenzen finden Sie auf der Bestellseite von PVS-Studio .
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Übersetzungslink: Alexey Govorov. PVS-Studio: Analysieren von Pull-Anforderungen in Azure DevOps mithilfe von selbst gehosteten Agenten .