Features of setting up and running PVS-Studio in Docker using the Azure Service Fabric code example

Containerization technologies are actively used for the assembly and testing of software. With the advent of PVS-Studio for Linux, users now have the opportunity to add static analysis to other methods of testing their project on this platform, including Docker. The article will describe the features of working with the PVS-Studio analyzer in Docker, which will improve the quality of analysis and ease of use. It will also list errors found in the Azure Service Fabric project.

Introduction

Docker is a program that allows the operating system to run processes in an isolated environment based on specially created images. Containerization technology has become very common for many tasks, including software development and testing. Static analysis is usually performed in the same environment as the project assembly, so its use in Docker is very easy to implement in existing containers.

Examples of integration and launch of the PVS-Studio static analyzer will be given for the Linux version. But the described analyzer tuning options are possible and even recommended on any platform. The macOS version of the analyzer , which was recently presented to the public, is generally identical in using PVS-Studio for Linux.

Azure Service Fabric was selected as the project for integration and launch of the analyzer in Docker. Service Fabric is a distributed system platform for deploying and managing scalable and highly reliable distributed applications. Service Fabric runs on Windows and Linux, in any cloud, any data center, any region, and even on a laptop.

Phased implementation of the analyzer

First, let's see how the project is assembled in order to choose the analyzer integration method. The order of calling scripts and commands is as follows:

The following is a snippet of the build.sh script 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}/$DirName

To analyze the project, I decided to use the method from the documentation described in the Quick Start / CMake project section :

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

Adding an 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 src directory is part of the project and is mounted in / src . I also marked out the configuration file of the PVS-Studio.cfg analyzer . Then the analyzer can be called 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 started the analyzer before building the project. This is not the right decision, but there are a lot of conditions in the script under which the assembly of the project starts, so I simplified my task a bit and compiled the project in advance. Developers who better know the structure of their project should integrate the analyzer after building the project.

Now you can assemble and analyze the project with the following command:

sudo ./runbuild.sh -release -j4

The first analysis results frustrate with warnings on numerous macros, nonexistent files, incorrect paths to source code files, etc. In the next section, I will talk about the contents of the PVS-Studio.cfg file , where I added several settings that greatly improved the analysis.

Additional analyzer setup

Relative path to the source directory.

To view one 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 configuration of the analyzer must be performed in order to extract a report from the container with the correct file paths. The project root directory is mounted in root, so the analyzer parameter will look like this:

sourcetree-root=/

Warnings for nonexistent files

The container expands the / external directory , which is not in the repository. Most likely, some project dependencies are compiled in it and you can simply exclude them from the analysis:

exclude-path=/external

Warnings for Compiler Files, Tests, and Libraries

In Docker, the compiler can be placed in a non-standard place and its libraries can be reported. They also need to be excluded. To do this, the / deps directory and the test directory are excluded from the scan :

exclude-path=/deps
exclude-path=/src/prod/test

Fighting thousands of false positives arising from unsuccessful macros

The analyzer supports the configuration of various diagnostics using comments. You can read about them here and here .

Settings can be placed in the project code or put in a separate file, as I did:

rules-config=/src/service-fabric.pvsconfig

The contents of the service-fabric.pvsconfig file:

#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

Several lines of custom markup remove thousands of warnings for macros from the report.

Other settings

The path to the license file and the inclusion of only general-purpose diagnostics (to speed up the analysis):

lic-file=/src/PVS-Studio.lic
analysis-mode=4

The whole 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=/

May be needed in other projects

Another way to verify the project requires the strace system utility . Most likely, it will be absent in the container and you need to add the installation step of this utility from the repository to the script.

A compiler with a non-standard name, for example, a cross-compiler, can be placed in a container. I already wrote that the compiler directory should be excluded from the analysis, but in this case, you will also have to pass the name of the new compiler to the analyzer:

pvs-studio-analyzer analyze ... --compiler COMPILER_NAME...

You can duplicate a flag to specify multiple compilers.

View report on Linux or Windows

To view the analyzer report in Linux, you can add the report generation command in the required format to the script.

For example, to view in QtCreator:

plog-converter -t tasklist -r "~/Projects/service-fabric" \
  ./service-fabric-pvs.log -o ./service-fabric-pvs.tasks

Or in the browser:

plog-converter -t fullhtml -r "~/Projects/service-fabric" \
  ./service-fabric-pvs.log -o ./

To view the report in Windows, you can simply open the .log file in the Standalone utility , which is included in the distribution kit for Windows.

Sample Errors from Azure Service Fabric

Classic typos

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 dataSizeInRecordMetadata buffer size 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 the condition of the nested loop, the variable ix1 , not ix0 , should be checked .

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, they wanted to take the value of the statusDetails_ field from other.statusDetails_ , but they made a typo.

V606 Ownerless token 'false'. CryptoUtility.Linux.h 81

template 
static bool MapCompare(const std::map& lhs,
                       const std::map& rhs)
{
  if (lhs.size() != rhs.size()) { false; }
  return std::equal(lhs.begin(), lhs.end(), rhs.begin());
}

The missing keyword return led to the fact that the code became not optimal. Because of a typo, a quick check on the size of collections does not work as the author intended.

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 leads to a more serious error. The result of one of the comparisons is never saved. The correct code should be like this:

equals = this->CodePackageRef == other.CodePackageRef;
if (!equals) { return equals; }

Incorrect use of functions

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 detected strange code for generating a message in the errorMessage variable . Judging by the neighboring code fragments, it is necessary to write 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 fwrite () function . Documentation for this feature 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 
TBuf BioMemToTBuf(BIO* bio)
{
  char* data = NULL;
  auto len = BIO_ctrl_pending(bio);
  Invariant(len >= 0);
  ....
}

Incorrect check of the return value of the function from the OpenSSL library. This could very well be a serious mistake or even vulnerability.

About pointers and memory

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
    friend struct JsonBufferManagerTraits;
public:
    JsonBufferManager2()
    {
        JsonBufferManager2(nullptr, 0);
    }
    ....
}

Probably, they wanted to call another from one constructor. But in fact, a temporary object of the JsonBufferManager2 class is created and then destroyed. This type of error is described in more detail in the article “ Do not know the ford, do not get into the water: part one ”. The same article describes how you can call one constructor from another.

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 correct sizeof () was passed to the write () function , but the result of the read function, most likely, should be compared with the size of the recorded 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 mistake when working with the globalDomain pointer : dereferencing first, then checking.

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;
}

There were many places where the memory allocated for the array is freed in the wrong way. You need to use delete [] .

Running the analyzer in containers with Windows

In this case, starting the analyzer is not very different from automating analysis, for example, in Jenkins on a real computer. We ourselves use Docker to test PVS-Studio for Windows. It is enough to install the analyzer:

START /w PVS-Studio_setup.exe /VERYSILENT /SUPPRESSMSGBOXES \
  /NORESTART /COMPONENTS=Core,Standalone

and start an analysis of your project:

"C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" ...

Conclusion

The focus of the article was on interesting containerization technology, which is not an obstacle to the integration of static analysis in your project. Therefore, the found warnings of PVS-Studio were shortened in the article, but are fully available for download in a format for the browser: service-fabric-pvs-studio-html.7z .

I suggest everyone to download and try PVS-Studio on their project. The analyzer works on Windows, Linux and macOS!

If you want to share this article with an English-speaking audience, then please use the link to the translation: Svyatoslav Razmyslov. Features of PVS-Studio setting and running in Docker on the example of Azure Service Fabric code.