PVS-Studio идёт в облака: CircleCI

    Picture 2

    Мы продолжаем цикл статей по использованию статического анализатора PVS-Studio в облачных CI-системах. Сегодня рассматриваем очередной сервис — CircleCI. В качестве проекта для анализа в этот раз выступит медиаплеер Kodi, в исходном коде которого постараемся найти интересные места.

    Примечание. С другими статьями по интеграции PVS-Studio в облачные CI-системы можно ознакомиться здесь:


    Прежде чем перейдём непосредственно к настройке и разбору предупреждений анализатора, скажем пару слов об используемом и анализируемом ПО.

    CircleCI – облачный CI-сервис для автоматизации сборки, тестирования и публикации программного обеспечения. Поддерживает сборку проектов как в контейнерах, так и в виртуальных машинах с ОС Windows, Linux и macOS.

    Kodi – бесплатный кроссплатформенный медиаплеер с открытым исходным кодом. Позволяет воспроизводить аудио и видеофайлы, расположенные как на домашнем компьютере, так и в локальной сети или сети интернет. Поддерживает темы оформления и расширение функционала путем установки плагинов. Доступен для ОС Windows, Linux, macOS и Android.

    PVS-Studio – статический анализатор кода для поиска ошибок и потенциальных уязвимостей в коде, написанном на C, C++, C# и Java. Работает под Windows, Linux и macOS.

    Настройка


    Переходим на главную страницу CircleCI и нажимаем кнопку «Sign Up»

    Picture 1

    На следующей странице нам предложат аутентифицироваться с учетной записью GitHub либо Bitbucket. Выбираем GitHub и попадаем на страницу авторизации приложения CircleCI.

    Picture 3

    Авторизуем приложение (зеленая кнопка «Authorize circleci») и нас перенаправит на приветственную страницу «Welcome to CircleCI!»

    Picture 4

    На этой странице мы можем сразу настроить, какие проекты будут собираться в CircleCI. Отмечаем наш репозиторий и нажимаем «Follow».

    После добавления репозитория, CircleCI автоматически запустит сборку, но т.к. в репозитории еще нет файла конфигурации – задачи сборки завершится с ошибкой.

    Picture 5

    Перед добавлением файла с конфигурацией добавим в проект переменные, содержащие лицензионные данные для анализатора. Для этого на левой панели нажимаем «Settings», потом в группе «ORGANIZATION» выбираем пункт «Projects» и нажимаем на шестерёнку справа от нужного нам проекта. Откроется окно с настройками.

    Picture 6

    Нас интересует раздел «Environment Variables». Переходим в него и создаем переменные PVS_USERNAME и PVS_KEY, содержащие имя пользователя и лицензионный ключ для анализатора.

    Picture 7

    CircleCI при запуске сборки проекта читает конфигурацию задачи из файла в репозитории по пути .circleci/config.yml. Добавим его.

    Вначале укажем образ виртуальной машины, где будут запускаться анализатор. Полный список образов доступен по ссылке.

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

    Далее добавим в apt необходимые репозитории и установим зависимости проекта:

    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

    Добавим репозиторий PVS-Studio и установим анализатор:

    - 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

    Сгенерируем Makefile-ы в сборочной директории:

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

    Следующим шагом настроим и запустим статический анализ нашего проекта.

    Вначале создадим файл с лицензией анализатора. Второй командой запускаем трассировку сборки проекта компилятором.

    После трассировки запускаем непосредственно статический анализ. При использовании ознакомительной лицензии анализатор необходимо запускать с параметром:

    --disableLicenseExpirationCheck.

    Последней командой файл с результатами работы анализатора конвертируется в html-отчет:

    - 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

    Полный текста файла .circleci/config.yml:

    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

    Загружаем файл в репозиторий и CircleCI автоматически начнет сборку проекта.

    Picture 12

    После окончания задачи файлы с результатами работы анализатора можно скачать на вкладке «Artifacts».

    Picture 11

    Результаты анализа


    Что ж, теперь давайте взглянем на некоторые предупреждения, выданные анализатором в ходе работы.

    Предупреждение PVS-Studio: 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");
      ....
    }

    Судя по форматированию кода, предполагалась следующая логика исполнения:

    • если arttypes — нулевой указатель, завершаем выполнение метода;
    • если arttypes — ненулевой указатель, очищаем вектор artworkMap, после чего выполняем ещё какие-то действия.

    Однако пропущенный символ ';' внёс свои коррективы; как результат — логика исполнения совсем не соответствует форматированию. В результате она становится следующей:

    • если arttypes — нулевой указатель, выполняется очистка вектора artworkMap и выход из метода;
    • если arttypes — ненулевой указатель, выполняются дальнейшие действия, но очистки вектора artworkMap при этом не происходит.

    В общем, очень маловероятно, что здесь нет ошибки. Да и навряд ли кто-то стал бы писать выражения в духе return artworkMap.clear(); :).

    Предупреждения PVS-Studio:

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

    Обратите внимание на места, отмеченные // <=. В переменную lastsector записывают значение 0, а после два раза используют её в качестве условного выражения оператора if. Так как ни в цикле, ни между этими присваиваниями значение переменной не изменяется, then-ветви обоих операторов if выполняться не будут.

    Возможно, правда, что необходимая функциональность попросту ещё не реализована (обратите внимание на todo).

    Кстати, как вы могли заметить, анализатор выдал сразу 3 предупреждения на этот код. Иногда, однако, даже нескольких предупреждений бывает недостаточно, и пользователи продолжают считать, что анализатор ошибается… Подробнее об этом писал коллега в заметке "Один день из поддержки пользователей PVS-Studio" :).

    Предупреждение PVS-Studio: 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();
    }

    В данном случае проверка values.size() != 2 является избыточной, так как результатом условного выражения всегда будет false. В самом деле, если исполнение зайдёт в одну из case-ветвей оператора switch, в вектор будут добавлены 2 элемента, а так как он был пустым, то и его размер станет равен двум; иначе (при исполнении ветви default) будет осуществлён выход из метода.

    Предупреждение PVS-Studio: 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);
      ....
    }

    Переменная prio инициализируется значением INT_MAX, после чего она же используется в тернарном операторе в сравнении prio == INT_MAX. Однако между местом инициализации и использования её значение не изменяется, следовательно, значение выражения prio == INT_MAXtrue, а тернарный оператор всегда будет возвращать значение 0.

    Предупреждения PVS-Studio:

    • 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); // <=
      }
      ....
    }

    Оба предупреждения имеют один и тот же паттерн — указатель, полученный как результат вызова функции malloc, используется дальше в функции memcpy без предварительной проверки на NULL.

    У кого-то может возникнуть желание возразить по поводу данных предупреждений в следующем ключе: malloc никогда не вернёт нулевой указатель, а если и вернёт, то пусть приложение лучше упадёт. Это тема для длинной дискуссии, но так или иначе предлагаю ознакомиться с заметкой моего коллеги — "Почему важно проверять, что вернула функция malloc".

    При желании можно настроить поведение анализатора таким образом, чтобы он не считал, что malloc может вернуть нулевой указатель — тогда и подобных предупреждений не будет. Подробнее про это можно прочитать здесь.

    Предупреждение PVS-Studio: 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);
      ....
    }

    Ситуация аналогична описанной выше. Полученный как результат вызова malloc указатель записывается в переменную entry, после чего она используется без проверки на NULL (entry->d_name).

    Предупреждение PVS-Studio: 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();
    }

    Указатель progressHandler содержит значение, полученное в результате вызова operator new. Однако для этого указателя не вызывается operator delete, что приводит к возникновению утечки памяти.

    Предупреждение PVS-Studio: 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;
    }

    Оператор if накладывает ограничения на размер вектора m_vecPlayerConfigs за счёт условного выражения и выхода из метода в случае его истинности. В итоге, если исполнение кода дошло до последнего оператора return, размер вектора m_vecPlayerConfigs находится в указанном диапазоне: [1; idx]. Однако парой строк ниже находится обращение по индексу idx: m_vecPlayerConfigs[idx]->m_bPlaysVideo. В итоге, если idx будет равен размеру вектора, произойдёт обращение за границу допустимого диапазона.

    И напоследок взглянем на пару предупреждений на код библиотеки Platinum.

    Предупреждение PVS-Studio: 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);
      ....
    }

    В данном случае перепутан приоритет операций. К const char* приводится не результат вычисления тернарного оператора (subscription? «S»: «Uns»), а переменная subscription. Как минимум это выглядит странно.

    Предупреждение PVS-Studio: 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
      ....
    }

    Код пробела — 0x20, код табуляции — 0x09. Следовательно, подвыражение c == '\t' будет всегда иметь значение false, так как этот случай уже покрыт выражением c < ' ' (при истинности которого будет осуществлён выход из функции).

    Заключение


    Как видно из этой статьи, на очередной CI-системе (CircleCI) удалось настроить проверку проекта с использованием PVS-Studio. Предлагаю и вам загрузить и попробовать анализатор на своём проекте. Если же возникнут какие-то вопросы по настройке или использованию анализатора — смело пишите нам, с радостью поможем.

    И, конечно, безбажного кода вам, друзья. :)



    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Sergey Vasiliev, Ilya Gainulin. PVS-Studio in the Clouds: CircleCI.
    PVS-Studio
    458,81
    Static Code Analysis for C, C++, C# and Java
    Поделиться публикацией

    Похожие публикации

    Комментарии 2

      0
      Я все жду когда PVS-Studio и Gitlab договорятся и в облачном Gitlab появится поддержка PVS-Studio из коробки без добавления лицензии PVS-Studio
        +2

        Вот буквально вчера закончил прикручивать PVS-Studio к CircleCI; все было довольно легко.
        Правда, вместо публикации артефактов просто вывожу отчет в формате errorfile в консоль (и слегка подсвечиваю руками).

        Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

        Самое читаемое