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

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

Introduction

Docker is a program that allows the operating system to run processes in an isolated environment based on specially crafted 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 build, so its use in Docker is very easy to implement in already existing containers.

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

The project for the integration and launch of the analyzer in Docker selected Azure Service Fabric. Service Fabric is a platform for distributed systems designed to deploy and manage scalable and highly reliable distributed applications. Service Fabric runs on Windows and Linux, in any cloud, in any data center, in any region, and even on a laptop.
')

Phased implementation of the analyzer

First, let's take a look at how the project is built to choose how to integrate the analyzer. The order of invoking scripts and commands is as follows:

Below is a fragment 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 

Add analyzer setup:

 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 on / src . I also marked out the PVS-Studio.cfg analyzer configuration file. 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 did the launch of the analyzer before building the project. This is not the right solution, but the script has a lot of conditions under which the project builds run, so I simplified my task a bit and compiled the project in advance. Developers who know the structure of their project better should integrate the analyzer after building the project.

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

 sudo ./runbuild.sh -release -j4 

The first results of the analysis are frustrated with warnings for numerous macros, non-existent 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 significantly improved the analysis.

Additional analyzer setting

Relative path to the source directory

To view a single report on different computers, the analyzer can generate a report with relative paths to the files. You can restore them on another computer using a converter.

Similar analyzer settings must be performed to extract a report from the container with the correct paths to the files. The project root directory is mounted as root, so the analyzer parameter will look like this:

 sourcetree-root=/ 

Warnings for non-existent files

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

 exclude-path=/external 

Warnings for compiler files, tests and libraries

In the Docker, the compiler can be placed in a non-standard location and its libraries can be included in the report. They also need to be deleted. To do this, the directory / deps and the directory with tests are excluded from the check:

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

Fighting thousands of false positives caused by failed macros

The analyzer supports setting up various diagnostics using comments. You can read about them here and here .

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

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

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 special markup remove thousands of warnings on macros from the report.

Other settings

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

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

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

May be needed in other projects.

Another way to check a project requires the system utility strace . 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 put into the container. I already wrote that the compiler directory should be excluded from the analysis, but in this case, in addition, you will have to pass the analyzer the name of the new compiler:

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

You can duplicate the checkbox to specify multiple compilers.

View a report in Linux or Windows

To view the analyzer report in Linux, you can add a report generation command to the script in the required 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 simply open the .log file in the Standalone utility, which is included in the distribution for Windows.

Error Examples from Azure Service Fabric

Classic typos

V501 CWE-571 operator == '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)' the operator. OverlayStream.cpp 4966

 VOID OverlayStream::AsyncMultiRecordReadContextOverlay::FSMContinue( __in NTSTATUS Status ) { ULONG dataSizeInRecordMetadata = 0; ULONG dataSizeInRecordIoBuffer = 0; .... if ((dataSizeInRecordIoBuffer > 0) && (dataSizeInRecordIoBuffer > 0)) { .... } .... } 

Because of Copy-Paste, the size of the dataSizeInRecordMetadata buffer is not checked.

V534 CWE-691 It is likely that a 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, the variable ix1 should be checked in the nested loop condition , and not 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; } 

Most likely, they wanted to take the value of the statusDetails_ field 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()); } 

The missing keyword return caused the code to become not optimal. Because of a typo, a quick check on the size of the 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:

 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 a strange code to form a message in the errorMessage variable. Judging by the adjacent 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 <typename TBuf> TBuf BioMemToTBuf(BIO* bio) { char* data = NULL; auto len = BIO_ctrl_pending(bio); Invariant(len >= 0); .... } 

Invalid check of function return value from OpenSSL library. This may well be a serious mistake or even a vulnerability.

About pointers and memory

V603 CWE-665 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 from one constructor they wanted to call another. But in fact, a temporary object of the JsonBufferManager2 class is created and immediately destroyed. This type of error is described in more detail in the article " Without knowing the ford, do not climb into the water: part one ." This article explains how to call one constructor from another.

If you’re a sizeof () ’operator evalu evalu P P P P P P P P P P P 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 () is passed to the write () function, but the result of the reading function is likely to be compared with the size of the written object:

 Invariant(written == sizeof(*thisPtr)); 

V595 CWE-476 The 'globalDomain' pointer was used 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; } } } 

The classic error when working with the globalDomain pointer: first dereference, then check.

V611 CWE-762 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 are many places where the memory allocated for the array is freed in the wrong way. It is necessary to use delete [] .

Running the analyzer in Windows containers

In this case, the launch of the analyzer is not much different from the automation of analysis, for example, in Jenkins on a real computer. We ourselves use Docker to test PVS-Studio for Windows. Just install the analyzer:

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

and run an analysis of your project:

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

Conclusion

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

I suggest everyone to download and try PVS-Studio on their project. The analyzer runs 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 the Azure Service Fabric code.