PVS-Studio goes to the clouds: CircleCI

We continue the series of articles on the use of the PVS-Studio static analyzer in cloud CI systems. Today we are considering another service - CircleCI. This time, the Kodi media player will act as a project for analysis, in the source code of which we will try to find interesting places.



Note. Other articles on integrating PVS-Studio into cloud CI systems can be found here:

• PVS-Studio goes to the clouds: Azure DevOps
• PVS-Studio goes to the clouds - launch analysis on Travis CI

Before we proceed directly to setting up and analyzing analyzer warnings, let us say a few words about the software used and analyzed.



CircleCI is a cloud-based CI service for automating the assembly, testing, and publishing of software. Supports the assembly of projects both in containers and in virtual machines running Windows, Linux and macOS.



Kodi is a free, open source cross-platform media player. Allows you to play audio and video files located both on your home computer and on a local network or the Internet. It supports themes and functionality by installing plugins. Available for Windows, Linux, macOS and Android.



PVS-Studio is a static code analyzer for searching for errors and potential vulnerabilities in code written in C, C ++, C # and Java. Works under Windows, Linux and macOS.

Customization

Go to the CircleCI home page and click on the “Sign Up” button

On the next page, we will be asked to authenticate with a GitHub or Bitbucket account. Select GitHub and get to the CircleCI application authorization page.

We authorize the application (green button “Authorize circleci”) and redirect us to the welcome page “Welcome to CircleCI!”

On this page, we can immediately configure which projects will be assembled in CircleCI. We mark our repository and click “Follow”.



After adding the repository, CircleCI will automatically start the build, but since there is no configuration file in the repository yet - the build task will fail.

Before adding a configuration file, we add to the project variables containing license data for the analyzer. To do this, click on “Settings” in the left pane, then select “Projects” in the “ORGANIZATION” group and click on the gear to the right of the project we need. A settings window will open.

We are interested in the section "Environment Variables". We go into it and create the PVS_USERNAME and PVS_KEY variables containing the username and license key for the analyzer.

When starting a project build, CircleCI reads the task configuration from a file in the repository along the path .circleci / config.yml. Add it.



First, we indicate the image of the virtual machine where the analyzer will start. A complete list of images is available here .

version: 2 jobs: build: machine: image: ubuntu-1604:201903-01
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

Next, add the necessary repositories to apt and install the project dependencies:

 steps: - checkout - run: sudo -- sh -c " add-apt-repository -y ppa:team-xbmc/xbmc-ppa-build-depends && add-apt-repository -y ppa:wsnipex/vaapi && add-apt-repository -y ppa:pulse-eight/libcec && apt-get update" - run: sudo apt-get install -y automake autopoint build-essential cmake curl default-jre gawk gdb gdc gettext git-core gperf libasound2-dev libass-dev libbluray-dev libbz2-dev libcap-dev libcdio-dev libcec4-dev libcrossguid-dev libcurl3 libcurl4-openssl-dev libdbus-1-dev libegl1-mesa-dev libfmt3-dev libfontconfig-dev libfreetype6-dev libfribidi-dev libfstrcmp-dev libgif-dev libgl1-mesa-dev libglu1-mesa-dev libiso9660-dev libjpeg-dev liblcms2-dev libltdl-dev liblzo2-dev libmicrohttpd-dev libmysqlclient-dev libnfs-dev libpcre3-dev libplist-dev libpng-dev libpulse-dev libsmbclient-dev libsqlite3-dev libssl-dev libtag1-dev libtinyxml-dev libtool libudev-dev libusb-dev libva-dev libvdpau-dev libxml2-dev libxmu-dev libxrandr-dev libxrender-dev libxslt1-dev libxt-dev mesa-utils nasm pmount python-dev python-imaging python-sqlite rapidjson-dev swig unzip uuid-dev yasm zip zlib1g-dev wget
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

Add the PVS-Studio repository and install the analyzer:

 - run: wget -q -O - https://files.viva64.com/etc/pubkey.txt | sudo apt-key add - && sudo wget -O /etc/apt/sources.list.d/viva64.list https://files.viva64.com/etc/viva64.list - run: sudo -- sh -c "apt-get update && apt-get install pvs-studio -y"
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

Let's collect the project dependencies:

 - run: sudo make -C tools/depends/target/flatbuffers PREFIX=/usr/local
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

Generate Makefiles in the assembly directory:

 - run: mkdir build && cd build && cmake -DCMAKE_BUILD_TYPE=Debug ..
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

The next step is to configure and run a static analysis of our project.



First, create a file with the analyzer license. The second command starts the compilation of the project assembly trace.



After tracing, we start directly the static analysis. When using a trial license, the analyzer must be started with the parameter:



--disableLicenseExpirationCheck .



The last command converts the file with the results of the analyzer to an html report:

 - run: pvs-studio-analyzer credentials -o PVS.lic ${PVS_USER} ${PVS_KEY} - run: pvs-studio-analyzer trace -- make -j2 -C build/ - run: pvs-studio-analyzer analyze -j2 -l PVS.lic -o PVS-Studio.log --disableLicenseExpirationCheck - run: plog-converter -t html -o PVS-Studio.html PVS-Studio.log
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

After completing the tests, save the analyzer reports:

 - run: mkdir PVS_Result && cp PVS-Studio.* ./PVS_Result/ - store_artifacts: path: ./PVS_Result
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

Full text of the .circleci / config.yml file:

 version: 2.1 jobs: build: machine: image: ubuntu-1604:201903-01 steps: - checkout - run: sudo -- sh -c " add-apt-repository -y ppa:team-xbmc/xbmc-ppa-build-depends && add-apt-repository -y ppa:wsnipex/vaapi && add-apt-repository -y ppa:pulse-eight/libcec && apt-get update" - run: sudo apt-get install -y automake autopoint build-essential cmake curl default-jre gawk gdb gdc gettext git-core gperf libasound2-dev libass-dev libbluray-dev libbz2-dev libcap-dev libcdio-dev libcec4-dev libcrossguid-dev libcurl3 libcurl4-openssl-dev libdbus-1-dev libegl1-mesa-dev libfmt3-dev libfontconfig-dev libfreetype6-dev libfribidi-dev libfstrcmp-dev libgif-dev libgl1-mesa-dev libglu1-mesa-dev libiso9660-dev libjpeg-dev liblcms2-dev libltdl-dev liblzo2-dev libmicrohttpd-dev libmysqlclient-dev libnfs-dev libpcre3-dev libplist-dev libpng-dev libpulse-dev libsmbclient-dev libsqlite3-dev libssl-dev libtag1-dev libtinyxml-dev libtool libudev-dev libusb-dev libva-dev libvdpau-dev libxml2-dev libxmu-dev libxrandr-dev libxrender-dev libxslt1-dev libxt-dev mesa-utils nasm pmount python-dev python-imaging python-sqlite rapidjson-dev swig unzip uuid-dev yasm zip zlib1g-dev wget - run: wget -q -O - https://files.viva64.com/etc/pubkey.txt | sudo apt-key add – && sudo wget -O /etc/apt/sources.list.d/viva64.list https://files.viva64.com/etc/viva64.list - run: sudo -- sh -c "apt-get update && apt-get install pvs-studio -y" - run: sudo make -C tools/depends/target/flatbuffers PREFIX=/usr/local - run: mkdir build && cd build && cmake -DCMAKE_BUILD_TYPE=Debug .. - run: pvs-studio-analyzer credentials -o PVS.lic ${PVS_USER} ${PVS_KEY} - run: pvs-studio-analyzer trace -- make -j2 -C build/ - run: pvs-studio-analyzer analyze -j2 -l PVS.lic -o PVS-Studio.log --disableLicenseExpirationCheck - run: plog-converter -t html -o PVS-Studio.html PVS-Studio.log - run: mkdir PVS_Result && cp PVS-Studio.* ./PVS_Result/ - store_artifacts: path: ./PVS_Result
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

We upload the file to the repository and CircleCI will automatically begin the assembly of the project.

After the end of the task, the files with the results of the analyzer can be downloaded on the “Artifacts” tab.

Analysis results

Well, now let's take a look at some warnings issued by the analyzer during the work.



PVS-Studio Warning : V504 It is highly probable that the semicolon ';' is missing after 'return' keyword. AdvancedSettings.cpp: 1476

 void CAdvancedSettings::SetExtraArtwork(const TiXmlElement* arttypes, std::vector<std::string>& artworkMap) { if (!arttypes) return artworkMap.clear(); const TiXmlNode* arttype = arttypes->FirstChild("arttype"); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

Judging by the formatting of the code, the following execution logic was assumed:

• if arttypes is a null pointer, complete the method;
• if arttypes is a non-zero pointer, clear the artworkMap vector, and then perform some other actions.

However, the missing character ';' made adjustments; as a result, the execution logic does not correspond to formatting at all. As a result, it becomes the following:

• if arttypes is a null pointer, the artworkMap vector is cleared and the method exits;
• if arttypes is a non-zero pointer, further actions are performed, but the artworkMap vector is not cleaned up.

In general, it is very unlikely that there is no mistake. And it is unlikely that someone would write expressions in the spirit of return artworkMap.clear (); :).



PVS-Studio warnings :

• V547 Expression 'lastsector' is always false. udf25.cpp: 636
• V547 Expression 'lastsector' is always false. udf25.cpp: 644
• V571 Recurring check. The 'if (lastsector)' condition was already verified in line 636. udf25.cpp: 644

 int udf25::UDFGetAVDP( struct avdp_t *avdp) { .... uint32_t lastsector; .... lastsector = 0; // <= .... for(;;) { .... if( lastsector ) { // <= V547 lbnum = lastsector; terminate = 1; } else { //! @todo Find last sector of the disc (this is optional). if( lastsector ) // <= V547 lbnum = lastsector - 256; else return 0; } } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

Pay attention to the places marked with // <= . The value 0 is written to the lastsector variable, and then it is used twice as a conditional expression of the if statement . Since the value of the variable does not change either in the loop or between these assignments, then the branches of both if statements will not be executed.



Perhaps it’s true that the necessary functionality is simply not yet implemented (pay attention to todo ).



By the way, as you can see, the analyzer immediately issued 3 warnings for this code. Sometimes, however, even a few warnings are not enough, and users continue to believe that the analyzer is mistaken ... A colleague wrote more about this in the article " One day from PVS-Studio user support " :).



PVS-Studio warning : V547 Expression 'values.size ()! = 2' is always false. GUIControlSettings.cpp: 1174

 bool CGUIControlRangeSetting::OnClick() { .... std::vector<CVariant> values; SettingConstPtr listDefintion = settingList->GetDefinition(); switch (listDefintion->GetType()) { case SettingType::Integer: values.push_back(m_pSlider-> GetIntValue(CGUISliderControl::RangeSelectorLower)); values.push_back(m_pSlider-> GetIntValue(CGUISliderControl::RangeSelectorUpper)); break; case SettingType::Number: values.push_back(m_pSlider-> GetFloatValue(CGUISliderControl::RangeSelectorLower)); values.push_back(m_pSlider-> GetFloatValue(CGUISliderControl::RangeSelectorUpper)); break; default: return false; } if (values.size() != 2) return false; SetValid(CSettingUtils::SetList(settingList, values)); return IsValid(); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

In this case, checking values.size ()! = 2 is redundant, as the result of the conditional expression will always be false . In fact, if the execution goes into one of the case branches of the switch statement , 2 elements will be added to the vector, and since it was empty, then its size will become equal to two; otherwise (when executing the default branch), the method will exit.



PVS-Studio warning : V547 Expression 'prio == 0x7fffffff' is always true. DBusReserve.cpp: 57

 bool CDBusReserve::AcquireDevice(const std::string& device) { .... int prio = INT_MAX; .... res = dbus_bus_request_name( m_conn, service.c_str(), DBUS_NAME_FLAG_DO_NOT_QUEUE | (prio == INT_MAX ? 0 : DBUS_NAME_FLAG_ALLOW_REPLACEMENT), // <= error); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

The prio variable is initialized with the value INT_MAX , after which it is also used in the ternary operator in comparison prio == INT_MAX . However, between the place of initialization and use, its value does not change, therefore, the value of the expression prio == INT_MAX is true , and the ternary operator will always return 0.



PVS-Studio warnings :

• V575 The potential null pointer is passed into 'memcpy' function. Inspect the first argument. Check lines: 39, 38. DVDOverlayImage.h: 39
• V575 The potential null pointer is passed into 'memcpy' function. Inspect the first argument. Check lines: 44, 43. DVDOverlayImage.h: 44

 CDVDOverlayImage(const CDVDOverlayImage& src) : CDVDOverlay(src) { Data = (uint8_t*)malloc(src.linesize * src.height); memcpy(data, src.data, src.linesize * src.height); // <= if(src.palette) { palette = (uint32_t*)malloc(src.palette_colors * 4); memcpy(palette, src.palette, src.palette_colors * 4); // <= } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

Both warnings have the same pattern - the pointer obtained as a result of calling the malloc function is used further in the memcpy function without first checking for NULL .



Someone may want to object to these warnings in the following key: malloc will never return a null pointer, and if it does, then let the application fall better. This is a topic for a long discussion, but one way or another I propose to read my colleague’s note - " Why is it important to check that the malloc function returned ".



If desired, you can configure the analyzer to behave in such a way that it does not consider that malloc can return a null pointer - then there will be no such warnings. You can read more about this here .



PVS-Studio Warning : V522 There might be dereferencing of a potential null pointer 'entry'. Check lines: 985, 981. emu_msvcrt.cpp: 985

 struct dirent *dll_readdir(DIR *dirp) { .... struct dirent *entry = NULL; entry = (dirent*) malloc(sizeof(*entry)); if (dirData->curr_index < dirData->items.Size() + 2) { if (dirData->curr_index == 0) strncpy(entry->d_name, ".\0", 2); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

The situation is similar to that described above. The pointer obtained as a result of calling malloc is written to the entry variable, after which it is used without checking for NULL ( entry-> d_name ).



PVS-Studio Warning : V773 Visibility scope of the 'progressHandler' pointer was exited without releasing the memory. A memory leak is possible. PVRGUIChannelIconUpdater.cpp: 94

 void CPVRGUIChannelIconUpdater::SearchAndUpdateMissingChannelIcons() const { .... CPVRGUIProgressHandler* progressHandler = new CPVRGUIProgressHandler(g_localizeStrings.Get(19286)); for (const auto& group : m_groups) { const std::vector<PVRChannelGroupMember> members = group->GetMembers(); int channelIndex = 0; for (const auto& member : members) { progressHandler->UpdateProgress(member.channel->ChannelName(), channelIndex++, members.size()); .... } progressHandler->DestroyProgress(); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

The progressHandler pointer contains the value obtained by calling operator new . However, operator delete is not called for this pointer, which causes a memory leak.



PVS-Studio Warning : V557 Array overrun is possible. The 'idx' index is pointing beyond array bound. PlayerCoreFactory.cpp: 240

 std::vector<CPlayerCoreConfig *> m_vecPlayerConfigs; bool CPlayerCoreFactory::PlaysVideo(const std::string& player) const { CSingleLock lock(m_section); size_t idx = GetPlayerIndex(player); if (m_vecPlayerConfigs.empty() || idx > m_vecPlayerConfigs.size()) return false; return m_vecPlayerConfigs[idx]->m_bPlaysVideo; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

The if statement imposes restrictions on the size of the vector m_vecPlayerConfigs due to the conditional expression and exit from the method if it is true. As a result, if the code execution reached the last return statement , the size of the vector m_vecPlayerConfigs is in the specified range: [1; idx]. However, a couple of lines below is the idx call : m_vecPlayerConfigs [idx] -> m_bPlaysVideo . As a result, if idx is equal to the size of the vector, it will go over the border of the allowable range.



And finally, take a look at a couple of warnings on the Platinum library code.



PVS-Studio Warning : V542 Consider inspecting an odd type cast: 'bool' to 'char *'. PltCtrlPoint.cpp: 1617

 NPT_Result PLT_CtrlPoint::ProcessSubscribeResponse(...) { .... bool subscription = (request.GetMethod().ToUppercase() == "SUBSCRIBE"); .... NPT_String prefix = NPT_String::Format(" PLT_CtrlPoint::ProcessSubscribeResponse %ubscribe for service \"%s\" (result = %d, status code = %d)", (const char*)subscription?"S":"Uns", // <= (const char*)service->GetServiceID(), res, response?response->GetStatusCode():0); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

In this case, the priority of operations is confused. Const char * is not given the result of evaluating the ternary operator ( subscription? "S": "Uns" ), but the variable subscription . At least it looks weird.



PVS-Studio warning : V560 A part of conditional expression is always false: c == '\ t'. NptUtils.cpp: 863

 NPT_Result NPT_ParseMimeParameters(....) { .... case NPT_MIME_PARAMETER_PARSER_STATE_NEED_EQUALS: if (c < ' ') return NPT_ERROR_INVALID_SYNTAX; // END or CTLs are invalid if (c == ' ' || c == '\t') continue; // ignore leading whitespace .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

The space code is 0x20, the tab code is 0x09. Therefore, the subexpression c == '\ t' will always be false , since this case is already covered by the expression c <'' (if true, the function will be exited).

Conclusion

As you can see from this article, on the next CI system (CircleCI), we were able to configure project verification using PVS-Studio. I suggest you download and try the analyzer on your project. If you have any questions about setting up or using the analyzer, feel free to write to us , we will be happy to help.



And, of course, the code of carelessness to you, friends. :)

If you want to share this article with an English-speaking audience, then please use the link to the translation: Sergey Vasiliev, Ilya Gainulin. PVS-Studio in the Clouds: CircleCI .