Технологии контейнеризации активно используются для сборки и тестирования программного обеспечения. С появлением PVS-Studio для Linux, пользователям стала доступна возможность добавить статический анализ к другим методам тестирования своего проекта на этой платформе, в том числе в Docker. В статье будут описаны особенности работы с анализатором PVS-Studio в Docker, которые повысят качество анализа и удобство использования. А также будут приведены ошибки, найденные в проекте Azure Service Fabric.
Docker — программа, позволяющая операционной системе запускать процессы в изолированном окружении на базе специально созданных образов. Технология контейнеризации стала очень распространённой для многих задач, включая разработку и тестирование программного обеспечения. Статический анализ обычно выполняется в том же окружении, что и сборка проекта, поэтому его использование в Docker очень просто реализуется в уже существующих контейнерах.
Примеры интеграции и запуска статического анализатора PVS-Studio будут приведены для Linux версии. Но описанные возможности настройки анализатора возможны и даже рекомендуются на любой платформе. Версия анализатора под macOS, которая недавно была представлена общественности, вообще идентична в использовании PVS-Studio для Linux.
В качестве проекта для интеграции и запуска анализатора в Docker выбран Azure Service Fabric. Service Fabric - это платформа для распределенных систем, предназначенная для развертывания и управления масштабируемыми и высоконадежными распределенными приложениями. Service Fabric работает на Windows и Linux, в любом облаке, любом дата-центре, любом регионе и даже на ноутбуке.
Для начала посмотрим, как выполняется сборка проекта, чтобы выбрать способ интеграции анализатора. Порядок вызова скриптов и команд выглядит следующим образом:
Ниже представлен фрагмент скрипта build.sh, где генерируется проектный файл:
cmake ${CMakeGenerator} \
-DCMAKE_C_COMPILER=${CC} \
-DCMAKE_CXX_COMPILER=${CXX} \
-DCMAKE_BUILD_TYPE=${BuildType} \
-DBUILD_THIRD_PARTY=${BuildThirdPartyLib} \
${DisablePrecompileFlag} ${ScriptPath}/$DirName
Для анализа проекта я решил воспользоваться способом из документации, описанном в разделе Быстрый старт/CMake-проект:
diff --git a/src/build.sh b/src/build.sh
index 290c57d..5901fd6 100755
--- a/src/build.sh
+++ b/src/build.sh
@@ -179,6 +179,7 @@ BuildDir()
-DCMAKE_CXX_COMPILER=${CXX} \
-DCMAKE_BUILD_TYPE=${BuildType} \
-DBUILD_THIRD_PARTY=${BuildThirdPartyLib} \
+ -DCMAKE_EXPORT_COMPILE_COMMANDS=On \
${DisablePrecompileFlag} ${ScriptPath}/$DirName
if [ $? != 0 ]; then
let TotalErrors+=1
Добавление установки анализатора:
diff --git a/src/build.sh b/src/build.sh
index 290c57d..581cbaf 100755
--- a/src/build.sh
+++ b/src/build.sh
@@ -156,6 +156,10 @@ BuildDir()
CXX=${ProjRoot}/deps/third-party/bin/clang/bin/clang++
fi
+ dpkg -i /src/pvs-studio-6.23.25754.2246-amd64.deb
+ apt -f install -y
+ pvs-studio --version
+
Каталог src является частью проекта и монтируется в /src. Там же я разместил конфигурационный файл анализатора PVS-Studio.cfg. Тогда вызов анализатора можно выполнить следующим образом:
diff --git a/src/build.sh b/src/build.sh
index 290c57d..2a286dc 100755
--- a/src/build.sh
+++ b/src/build.sh
@@ -193,6 +193,9 @@ BuildDir()
cd ${ProjBinRoot}/build.${DirName}
+ pvs-studio-analyzer analyze --cfg /src/PVS-Studio.cfg \
+ -o ./service-fabric-pvs.log -j4
+
if [ "false" = ${SkipBuild} ]; then
if (( $NumProc <= 0 )); then
NumProc=$(($(getconf _NPROCESSORS_ONLN)+0))
Запуск анализатора я сделал перед сборкой проекта. Это не является правильным решением, но в скрипте очень много условий, при которых запускается сборка проекта, поэтому я немного упростил себе задачу и скомпилировал проект заранее. Разработчикам, которые лучше знают структуру своего проекта, следует интегрировать анализатор после сборки проекта.
Теперь собрать и проанализировать проект можно следующей командой:
sudo ./runbuild.sh -release -j4
Первые результаты анализа расстраивают предупреждениями на многочисленные макросы, несуществующие файлы, неверные пути до файлов исходного кода т.п. В следующем разделе я расскажу о содержимом файла PVS-Studio.cfg, где я добавил несколько настроек, значительно улучшивших анализ.
Относительный путь до каталога с исходниками
Для просмотра одного отчёта на разных компьютерах анализатор умеет генерировать отчёт с относительными путями к файлам. Восстановить их можно на другом компьютере с помощью конвертера.
Аналогичную настройку анализатора необходимо выполнить, чтобы извлечь из контейнера отчёт с правильными путями к файлам. Корневой каталог проекта монтируется в root, поэтому параметр анализатора будет выглядеть следующим образом:
sourcetree-root=/
Предупреждения на несуществующие файлы
В контейнере разворачивается каталог /external, который отсутствует в репозитории. Скорее всего, в нём компилируются какие-то зависимости проекта и их можно просто исключить из анализа:
exclude-path=/external
Предупреждения на файлы компилятора, тестов и библиотек
В Docker компилятор может размещаться в нестандартном месте и его библиотеки могут попадать в отчёт. Их тоже необходимо исключить. Для этого из проверки исключается каталог /deps и заодно каталог с тестами:
exclude-path=/deps
exclude-path=/src/prod/test
Борьба с тысячами ложных срабатываний, возникающих из-за неудачных макросов
Анализатор поддерживает настройку разных диагностик с помощью комментариев. Про них можно почитать здесь и здесь.
Настройки можно размещать в коде проекта или вынести в отдельный файл, как это сделал я:
rules-config=/src/service-fabric.pvsconfig
Содержимое файла service-fabric.pvsconfig:
#V501
//-V:CODING_ERROR_ASSERT:501
//-V:TEST_CONFIG_ENTRY:501
//-V:VERIFY_IS_TRUE:501
//-V:VERIFY_ARE_EQUAL:501
//-V:VERIFY_IS_FALSE:501
//-V:INTERNAL_CONFIG_ENTRY:501
//-V:INTERNAL_CONFIG_GROUP:501
//-V:PUBLIC_CONFIG_ENTRY:501
//-V:PUBLIC_CONFIG_GROUP:501
//-V:DEPRECATED_CONFIG_ENTRY:501
//-V:TR_CONFIG_PROPERTIES:501
//-V:DEFINE_SECURITY_CONFIG_ADMIN:501
//-V:DEFINE_SECURITY_CONFIG_USER:501
//-V:RE_INTERNAL_CONFIG_PROPERTIES:501
//-V:RE_CONFIG_PROPERTIES:501
//-V:TR_INTERNAL_CONFIG_PROPERTIES:501
#V523
//-V:TEST_COMMIT_ASYNC:523
#V640
//-V:END_COM_INTERFACE_LIST:640
Несколько строк особой разметки удаляют из отчёта тысячи предупреждений на макросы.
Другие настройки
Путь к файлу лицензии и включение только диагностик общего назначения (для ускорения анализа):
lic-file=/src/PVS-Studio.lic
analysis-mode=4
Весь файл PVS-Studio.cfg
lic-file=/src/PVS-Studio.lic
rules-config=/src/service-fabric.pvsconfig
exclude-path=/deps
exclude-path=/external
exclude-path=/src/prod/test
analysis-mode=4
sourcetree-root=/
Другой способ проверки проекта требует наличия системной утилиты strace. Скорее всего, в контейнере она будет отсутствовать и в скрипт необходимо добавить шаг установки этой утилиты из репозитория.
В контейнер могут подложить компилятор с нестандартным названием например, кросс-компилятор. Я уже писал, что директорию компилятора необходимо исключать из анализа, но в этом случае дополнительно придётся передать анализатору и имя нового компилятора:
pvs-studio-analyzer analyze ... --compiler COMPILER_NAME...
Можно дублировать флажок для указания нескольких компиляторов.
Для просмотра отчёта анализатора в Linux, можно добавить в скрипт команду генерации отчёта в нужном формате.
Например, для просмотра в QtCreator:
plog-converter -t tasklist -r "~/Projects/service-fabric" \
./service-fabric-pvs.log -o ./service-fabric-pvs.tasks
Или в браузере:
plog-converter -t fullhtml -r "~/Projects/service-fabric" \
./service-fabric-pvs.log -o ./
Для просмотра отчёта в Windows можно просто открыть .log файл в утилите Standalone, которая входит в дистрибутив для Windows.
V501 CWE-571 There are identical sub-expressions to the left and to the right of the '==' operator: iter->PackageName == iter->PackageName DigestedApplicationDescription.cpp 247
ErrorCode
DigestedApplicationDescription::ComputeAffectedServiceTypes(....)
{
....
if (iter->PackageName == iter->PackageName &&
originalRG != this->ResourceGovernanceDescriptions.end() &&
targetRG != targetDescription.ResourceGovernanceDes....end())
{
....
}
....
}
Переменная iter->PackageName должна сравниваться с iter2->PackageName или codePackages.
V501 CWE-571 There are identical sub-expressions '(dataSizeInRecordIoBuffer > 0)' to the left and to the right of the '&&' operator. OverlayStream.cpp 4966
VOID
OverlayStream::AsyncMultiRecordReadContextOverlay::FSMContinue(
__in NTSTATUS Status
)
{
ULONG dataSizeInRecordMetadata = 0;
ULONG dataSizeInRecordIoBuffer = 0;
....
if ((dataSizeInRecordIoBuffer > 0) &&
(dataSizeInRecordIoBuffer > 0))
{
....
}
....
}
Из-за Copy-Paste не проверяется размер буфера dataSizeInRecordMetadata.
V534 CWE-691 It is likely that a wrong variable is being compared inside the 'for' operator. Consider reviewing 'ix0'. RvdLoggerVerifyTests.cpp 2395
NTSTATUS
ReportLogStateDifferences(....)
{
....
for (ULONG ix0=0; ix0 < RecoveredState._NumberOfStreams; ix0++)
{
KWString streamId(....);
ULONG ix1;
for (ix1 = 0; ix0 < LogState._NumberOfStreams; ix1++)
{
...
}
....
}
....
}
Вероятно, в условии вложенного цикла должна проверяться переменная ix1, а не ix0.
V570 The 'statusDetails_' variable is assigned to itself. ComposeDeploymentStatusQueryResult.cpp 49
ComposeDeploymentStatusQueryResult &
ComposeDeploymentStatusQueryResult::operator = (
ComposeDeploymentStatusQueryResult && other) // <=
{
if (this != & other)
{
deploymentName_ = move(other.deploymentName_);
applicationName_ = move(other.applicationName_);
dockerComposeDeploymentStatus_ = move(other....);
statusDetails_ = move(statusDetails_); // <=
}
return *this;
}
Скорее всего, значение поля statusDetails_ хотели взять из other.statusDetails_, но допустили опечатку.
V606 Ownerless token 'false'. CryptoUtility.Linux.h 81
template <typename TK, typename TV>
static bool MapCompare(const std::map<TK, TV>& lhs,
const std::map<TK, TV>& rhs)
{
if (lhs.size() != rhs.size()) { false; }
return std::equal(lhs.begin(), lhs.end(), rhs.begin());
}
Пропущенное ключевое слово return привело к тому, что код стал не оптимален. Из-за опечатки быстрая проверка на размер коллекций не работает так, как задумывал автор.
V607 CWE-482 Ownerless expression. EnvironmentOverrideDescription.cpp 60
bool EnvironmentOverridesDescription::operator == (....) const
{
bool equals = true;
for (auto i = 0; i < EnvironmentVariables.size(); i++)
{
equals = EnvironmentVariables[i] ==
other.EnvironmentVariables[i];
if (!equals) { return equals; }
}
this->CodePackageRef == other.CodePackageRef; // <=
if (!equals) { return equals; }
return equals;
}
Опечатка похожа на предыдущий пример, но приводит к более серьёзной ошибке. Результат одного из сравнений никогда не сохраняется. Правильный код должен быть таким:
equals = this->CodePackageRef == other.CodePackageRef;
if (!equals) { return equals; }
V521 CWE-480 Such expressions using the ',' operator are dangerous. Make sure the expression is correct. ReplicatedStore.SecondaryPump.cpp 1231
ErrorCode
ReplicatedStore::SecondaryPump::ApplyOperationsWithRetry(....)
{
....
if (errorMessage.empty())
{
errorMessage = L"error details missing: LSN={0}", operationLsn;
Assert::TestAssert("{0}", errorMessage);
}
....
}
Анализатор обнаружил странный код для формирования сообщения в переменной errorMessage. Судя по соседним фрагментам кода, здесь необходимо написать так:
WriteInfo(errorMessage, L"error ....: LSN={0}", operationLsn);
V547 CWE-570 Expression 'nwrite < 0' is always false. Unsigned type value is never < 0. File.cpp 1941
static void* ScpWorkerThreadStart(void* param)
{
....
do
{
size_t nwrite = fwrite(ptr, 1, remaining, destfile);
if (nwrite < 0)
{
pRequest->error_.Overwrite(ErrorCode::FromErrno(errno));
break;
}
else
{
remaining -= nwrite;
ptr += nwrite;
pRequest->szCopied_ += nwrite;
}
} while (remaining != 0);
....
}
Неправильная проверка возвращаемого значения функции fwrite(). Документацию по этой функции можно найти на cppreference.com и cplusplus.com.
V547 CWE-571 Expression 'len >= 0' is always true. Unsigned type value is always >= 0. Types.cpp 121
size_t BIO_ctrl_pending(BIO *b);
template <typename TBuf>
TBuf BioMemToTBuf(BIO* bio)
{
char* data = NULL;
auto len = BIO_ctrl_pending(bio);
Invariant(len >= 0);
....
}
Неверная проверка возвращаемого значения функции из библиотеки OpenSSL. Это вполне может быть серьёзной ошибкой или даже уязвимостью.
V603 CWE-665 The object was created but it is not being used. If you wish to call constructor, 'this->JsonBufferManager2::JsonBufferManager2(....)' should be used. JsonReader.h 48
class JsonBufferManager2
{
template<typename T>
friend struct JsonBufferManagerTraits;
public:
JsonBufferManager2()
{
JsonBufferManager2(nullptr, 0);
}
....
}
Вероятно, из одного конструктора хотели вызвать другой. Но на самом деле создаётся временный объект класса JsonBufferManager2 и тут же уничтожается. Подробнее этот тип ошибки описан в статье "Не зная брода, не лезь в воду: часть первая". В этой же статье рассказано, как можно вызвать один конструктор из другого.
V568 It's odd that 'sizeof()' operator evaluates the size of a pointer to a class, but not the size of the 'thisPtr' class object. TimerQueue.cpp 443
void TimerQueue::SigHandler(int sig, siginfo_t *si, void*)
{
TimerQueue* thisPtr = (TimerQueue*)si->si_value.sival_ptr;
auto written = write(thisPtr->pipeFd_[1],
&thisPtr, sizeof(thisPtr));
Invariant(written == sizeof(thisPtr)); // <=
}
В функцию write() передан правильный sizeof(), а вот результат функции чтения, скорее всего, должен сравниваться с размером записанного объекта:
Invariant(written == sizeof(*thisPtr));
V595 CWE-476 The 'globalDomain' pointer was utilized before it was verified against nullptr. Check lines: 196, 197. PlacementReplica.cpp 196
void PlacementReplica::ForEachWeightedDefragMetric(....) const
{
....
size_t metricIndexInGlobalDomain =
totalMetricIndexInGloba.... - globalDomain->MetricStartIndex;
if (globalDomain != nullptr &&
globalDomain->Metrics[metricIndexInGlobalDomain].Weight > 0)
{
if (!processor(totalMetricIndexInGlobalDomain))
{
break;
}
}
}
Классическая ошибка при работе с указателем globalDomain: сначала разыменование, потом проверка.
V611 CWE-762 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] groups;'. PAL.cpp 4733
NET_API_STATUS NetUserGetLocalGroups(....)
{
string unameA = utf16to8(UserName).substr(0, ACCT_NAME_MAX);
int ngroups = 50;
gid_t *groups = new gid_t[ngroups];
gid_t gid;
....
delete groups;
return NERR_Success;
}
Нашлось много мест, где неправильным способом освобождается память, выделенная под массив. Нужно использовать delete[].
В этом случае запуск анализатора не сильно отличается от автоматизации анализа, например, в Jenkins на реальном компьютере. Мы сами используем Docker для тестирования PVS-Studio для Windows. Достаточно выполнить установку анализатора:
START /w PVS-Studio_setup.exe /VERYSILENT /SUPPRESSMSGBOXES \
/NORESTART /COMPONENTS=Core,Standalone
и запустить анализ своего проекта:
"C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" ...
Акцент статьи был сделан на интересной технологии контейнеризации, которая не является препятствием для интеграции статического анализа в свой проект. Поэтому найденные предупреждения PVS-Studio были сокращены в статье, но полностью доступны для скачивания в формате для браузера: service-fabric-pvs-studio-html.7z.
Предлагаю всем желающим скачать и попробовать PVS-Studio на своём проекте. Анализатор работает на Windows, Linux и macOS!