A technology of containerization is actively used for building and testing the software. With the emergence of PVS-Studio for Linux, the ability of adding the static analysis to other methods of testing a project on this platform, including the Docker, became available for users. The article will describe the features of the work with the PVS-Studio analyzer in Docker, which will enhance the quality of the analysis and usability. The article will also provide the errors found in the project Azure Service Fabric.
Docker is a program that enables the operating system to run processes in an isolated environment on the basis of specially created images. The technology of containerization has become very common for many tasks, including developing and testing software. Static analysis is typically performed in the same environment as the project build, so its use is very easily implemented in the already existing containers in Docker.
The examples of integration and running PVS-Studio static analyzer will be given for the Linux version. In addition, the described possibilities of the analyzer customization are recommended even on other platform. The analyzer version under macOS, which has been recently presented to public is generally identical in using PVS-Studio for Linux.
The project Azure Service Fabric was chosen as a project for integration and analyzer launch in Docker. Service Fabric is a distributed systems platform for packaging, deploying, and managing stateless and stately distributed applications and containers at large scale. Service Fabric runs on Windows and Linux, on any cloud, any datacenter, across geographic regions, or on your laptop.
To begin, let's take a look at the way the build is performed to choose the way of the analyzer integration. The order of the scripts and commands call looks like this:
The following fragment of the script build.sh where the project file is generated:
cmake ${CMakeGenerator} \
-DCMAKE_C_COMPILER=${CC} \
-DCMAKE_CXX_COMPILER=${CXX} \
-DCMAKE_BUILD_TYPE=${BuildType} \
-DBUILD_THIRD_PARTY=${BuildThirdPartyLib} \
${DisablePrecompileFlag} ${ScriptPath}/$DirNameTo analyze the project, I decided to use the method from the documentation described in the section Quick run/CMake-project:
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+=1The addition of the analyzer installation:
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
+The directory src is a part of the project and is mounted into /src. I placed the analyzer configuration file PVS-Studio.cfg in the same place. Then the analyzer call can be performed as follows:
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))I ran the analyzer before the project build. This is not the right decision, but in the script, there are many conditions under which a build is run, so I slightly simplified the task and compiled the project in advance. The developers who know better the structure of their project should integrate the analyzer after building the project.
Now it's possible to build and analyze a project by the following command:
sudo ./runbuild.sh -release -j4The first results of the analysis don't please us because of the warnings on numerous macros, nonexistent files, incorrect paths to source code files, etc. In the next section, I'll talk about the contents of the file PVS-Studio.cfg, where I added a few settings, which significantly improved the analysis.
The relative path to the directory with the source files
To view a single report on different computers, the analyzer can generate a report with relative paths to files. You can restore them on another computer using the converter.
A similar analyzer configuration has to be performed to extract a report with the correct file paths from a container.
The root directory is mounted in root, so the analyzer parameter would be as follows:
sourcetree-root=/The root directory is selected here because in a container and in a host it is the project directory.
Warnings for non-existent files
In the container an /external catalog expands, which does not exist in the repository. Most likely, some project dependencies are compiled in it and they can be simply excluded from the analysis:
exclude-path=/externalWarnings for compiler files, tests, and libraries
In Docker, a compiler can be placed in a non-standard location and its libraries might fall in a report. They must be removed as well. For this, the directory /deps and the directory with the tests are excluded from the check:
exclude-path=/deps
exclude-path=/src/prod/testFight against thousands of false positives emerging because of failed macros
The analyzer supports a configuration of different diagnostics using comments. You can read about them here and here.
You can place the settings in the project code or make a separate file, as I did:
rules-config=/src/service-fabric.pvsconfigThe content of the file 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:640A few lines of special markup remove from the report thousands of warnings for macros.
Other settings
The path to the license file and enabling only diagnostics of general purpose (to speed up the analysis):
lic-file=/src/PVS-Studio.lic
analysis-mode=4The entire PVS-Studio.cfg file
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=/Another way to test a project requires having a system utility strace. Most likely, it will not be presented in the container and you'll have to add the step of installing this utility from a repository.
The container can include a non-standard compiler, for example, cross-compiler. I've already written that it is necessary to exclude the compiler directory from the analysis, but in this case you will have to pass the analyzer the name of the new compiler as well:
pvs-studio-analyzer analyze ... --compiler COMPILER_NAME...You can duplicate a flag for specifying several compilers.
To view the analyzer report in Linux, you can add a command in a script to generate the report in the needed format.
For example, for viewing in QtCreator:
plog-converter -t tasklist -r "~/Projects/service-fabric" \
./service-fabric-pvs.log -o ./service-fabric-pvs.tasksOr in the browser:
plog-converter -t fullhtml -r "~/Projects/service-fabric" \
./service-fabric-pvs.log -o ./To view the report in Windows, you can just open the .log file in the Standalone utility, which is included in the distribution package for 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())
{
....
}
....
}The variable iter->PackageName should be compared with iter2->PackageName or 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))
{
....
}
....
}Due to Copy-Paste the size of the buffer dataSizeInRecordMetadata is not checked.
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++)
{
...
}
....
}
....
}Probably, in a condition of a nested loop the variable ix1 has to be checked instead of the ix0 one.
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;
}Most likely, a programmer wanted to take the value of the field statusDetails_ from other.statusDetails_, but made a typo.
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());
}A missing keyword return has resulted in not optimal code. Due to a typo, a quick check on the size of the collections is not working as it was intended by the author.
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;
}A typo is similar to the previous example, but it leads to a more serious error. The result of one of the comparisons is never saved. Correct code should be like this:
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);
}
....
}The analyzer has detected strange code for generating a message in the variable errorMessage. Judging by the neighboring fragments, the following has to be written here:
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);
....
}Incorrect check of the return value of the function fwrite(). The documentation for this function can be found at cppreference.com and 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);
....
}Incorrect check of the return value of a function from the OpenSSL library. This may well be a serious mistake, or even a vulnerability.
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);
}
....
}Probably a programmer wanted to call a constructor from another one. In reality, a temporary object of a class JsonBufferManager2 is created and immediately destroyed. This type of an error is described in detail in the article "Wade not in unknown waters. Part one". This article also explains how you can call one constructor from another one.
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)); // <=
}The right sizeof() is passed to the function write(), but the result of the read function, most likely, has to be compared with the size of the written object:
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;
}
}
}A classic error with the pointer globalDomain: first a dereference, then a check.
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;
}Many places were found where the memory, allocated for an array, was released incorrectly. delete[] had to be used.
In this case, run of the analyzer is not much different from the automation of analysis, for example, in Jenkins on a real computer. We ourselves use Docker for testing PVS-Studio for Windows. You can simply perform the installation of the analyzer:
START /w PVS-Studio_setup.exe /VERYSILENT /SUPPRESSMSGBOXES \
/NORESTART /COMPONENTS=Core,Standaloneand run the analysis of your project:
"C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" ...The focus of the article was placed on the exciting technology of containerization, which is not an obstacle to the integration of the static analysis to your project. Therefore, found PVS-Studio warnings were reduced in the article, but fully available to download in the format for the browser: service-fabric-pvs-studio-html .7z.
I suggest for those who are interested to try PVS-Studio on your projects. The analyzer works on Windows, Linux and macOS!