Мы продолжаем цикл статей по использованию статического анализатора 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»
На следующей странице нам предложат аутентифицироваться с учетной записью GitHub либо Bitbucket. Выбираем GitHub и попадаем на страницу авторизации приложения CircleCI.
Авторизуем приложение (зеленая кнопка «Authorize circleci») и нас перенаправит на приветственную страницу «Welcome to CircleCI!»
На этой странице мы можем сразу настроить, какие проекты будут собираться в CircleCI. Отмечаем наш репозиторий и нажимаем «Follow».
После добавления репозитория, CircleCI автоматически запустит сборку, но т.к. в репозитории еще нет файла конфигурации – задачи сборки завершится с ошибкой.
Перед добавлением файла с конфигурацией добавим в проект переменные, содержащие лицензионные данные для анализатора. Для этого на левой панели нажимаем «Settings», потом в группе «ORGANIZATION» выбираем пункт «Projects» и нажимаем на шестерёнку справа от нужного нам проекта. Откроется окно с настройками.
Нас интересует раздел «Environment Variables». Переходим в него и создаем переменные PVS_USERNAME и PVS_KEY, содержащие имя пользователя и лицензионный ключ для анализатора.
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 автоматически начнет сборку проекта.
После окончания задачи файлы с результатами работы анализатора можно скачать на вкладке «Artifacts».
Результаты анализа
Что ж, теперь давайте взглянем на некоторые предупреждения, выданные анализатором в ходе работы.
Предупреждение 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_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<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.
