PVS-Studio идёт в облака: CircleCI07.10.2019 15:33

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& 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 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 == 0×7fffffff' 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_MAX — true, а тернарный оператор всегда будет возвращать значение 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 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 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
  ....
}


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

Заключение


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

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

8983b65a74adb29a2113eba12fbec3f1.png

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Sergey Vasiliev, Ilya Gainulin. PVS-Studio in the Clouds: CircleCI.

© Habrahabr.ru