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

• Introduction
• Phased Implementation of the Analyzer
• Additional Analyzer Configuration
• Might become necessary in other projects
• View of a Report in Linux or Windows
• Examples of Errors from the Azure Service Fabric
  • Classic typos
  • Incorrect Use of Functions
  • About the Pointers and Memory
• Running the Analyzer in the Containers with Windows
• Conclusion

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.

Introduction

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.

Phased Implementation of the Analyzer

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}/$DirName

To 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+=1

The 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 -j4

The 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.

Additional Analyzer Configuration

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=/external

Warnings 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/test

Fight 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.pvsconfig

The 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:640

A 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=4

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

Might become necessary in other projects

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.

View of a Report in Linux or Windows

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.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 just open the .log file in the Standalone utility, which is included in the distribution package for Windows.

Examples of Errors from the 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 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; }

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 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.

About the 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<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.

Running the Analyzer in the Containers with Windows

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,Standalone

and run the analysis of your project:

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

Conclusion

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!