# PVS\-Studio и RPCS3: лучшие предупреждения в один клик Best Warnings — режим анализатора, оставляющий в окне вывода 10 лучших предупреждений\. Мы предлагаем вам ознакомиться с обновлённым режимом Best Warnings на примере проверки проекта RPCS3\. ![1014_BestWarningsRPCS3_ru/image1.png](https://import.viva64.com/docx/blog/1014_BestWarningsRPCS3_ru/image1.png) ## Best Warnings Best Warnings — это специальный механизм для первого знакомства со статическим анализатором PVS\-Studio\. Полный лог анализатора может содержать тысячи предупреждений\. Поэтому если вы хотите оценить работу анализатора и не тратить время и силы на просмотр большого отчёта, выданного ещё не настроенным анализатором, то вы как раз можете воспользоваться механизмом Best Warnings\. Для этого откройте лог анализатора в плагине PVS\-Studio для Visual Studio и включите Best Warnings: ![1014_BestWarningsRPCS3_ru/image3.png](https://import.viva64.com/docx/blog/1014_BestWarningsRPCS3_ru/image3.png) При использовании Best Warnings отображаются 10 лучших предупреждений анализатора\. Вот так стало выглядеть окно вывода после включения Best Warnings: ![1014_BestWarningsRPCS3_ru/image5.png](https://import.viva64.com/docx/blog/1014_BestWarningsRPCS3_ru/image5.png) Этот режим появился год назад\. И не так давно мы поняли, что им почти никто не пользуется\. Почему? Потому что найти этот режим было проблематично\. Поэтому мы перенесли кнопку на главную строку панели и сделали ей яркую иконку\. Кроме того, теперь режим доступен в JetBrains Rider, CLion, IntelliJ IDEA, а также в C and C\+\+ Compiler Monitoring UI\. Второй проблемой были неинтересные предупреждения в окне вывода Best Warnings\. Отчасти это вызвано тем, что мы сами забыли про свою собственную фичу и не добавляли веса для новых диагностик☹ Оказалось, что 15 новых диагностик просто не участвовали в режиме Best Warnings\. Разумеется, мы это исправили\. Также мы посмотрели, какие предупреждения выдаёт режим Best Warnings на различных Open Source проектах, и изменили старые веса диагностик, чтобы предупреждения стали интереснее\. Третьей проблемой были "дубликаты"\. Не очень интересно видеть два и более предупреждений одного и того же диагностического правила в Best Warnings\. Механизм для удаления дубликатов уже был — веса второго и последующих предупреждений диагностики пропорционально уменьшались\. Мы увеличили штраф для повторных срабатываний, и теперь они появляются реже\. Более подробно о Best Warnings можете прочитать [здесь](https://pvs-studio.ru/ru/docs/manual/6532/)\. ## Немного о RPCS3 RPCS3 — это эмулятор консоли PS3\. Мы уже проверяли этот проект [ранее](https://pvs-studio.ru/ru/blog/posts/cpp/0886/)\. Кодовая база проекта составляет 300 тысяч строк кода на C\+\+, и этого вполне достаточно для демонстрации Best Warnings\. Для проверки я зафиксировал проект на коммите [e98b07d](https://github.com/RPCS3/rpcs3/tree/e98b07de03fce6aff227e677b54f1464df2b104d)\. Давайте перейдём к ошибкам\. ## Лишние вычисления при постановке таймера [V1064](https://pvs-studio.ru/ru/docs/warnings/v1064/) The 'nsec' operand of the modulo operation is less than the '1000000000ull' operand\. The result is always equal to the left operand\. Thread\.cpp 2359 ```cpp void thread_ctrl::wait_for(u64 usec, [[maybe_unused]] bool alert /* true */) { // .... if (!alert && usec > 0 && usec <= 1000 && fd_timer != -1) { struct itimerspec timeout; u64 missed; u64 nsec = usec * 1000ull; timeout.it_value.tv_nsec = (nsec % 1000000000ull); timeout.it_value.tv_sec = nsec / 1000000000ull; timeout.it_interval.tv_sec = 0; timeout.it_interval.tv_nsec = 0; timerfd_settime(fd_timer, 0, &timeout, NULL); // .... } } ``` Здесь механизм [анализа потока данных](https://pvs-studio.ru/ru/docs/manual/6521/) смог выявить интересную аномалию\. В данном фрагменте кода происходит следующее: * проверяются несколько условий, в том числе _usec <\= 1000_; * переменной _nsec_ присваивается значение _usec \* 1000_; * вычисляются выражения _nsec / 1'000'000'000_ и _nsec % 1'000'000'000_\. Из первых двух пунктов получаем, что значение переменной _nsec_ не превосходит _1'000'000_, что меньше _1'000'000'000\._ Получается, что выражение _nsec / 1'000'000'000_ всегда равно _0_, а выражение _nsec % 1'000'000'000 _равно _nsec_\. Далее вычисленные значения используются для установки таймера при вызове функции [timerfd\_settime](https://man7.org/linux/man-pages/man2/timerfd_settime.2.html)\. В функцию _wait\_for_ передают количество миллисекунд, в течение которых нужно ждать\. В переменную _nsec _записывается это же значение, только в наносекундах, поэтому умножаем на _1'000_\. Операции взятия остатка и деления на _1'000'000'000_ — выделение дробной части и целого количества секунд соответственно\. Скорее всего, в функцию _wait\_for_ должны приходить значения, не превосходящие _1'000_ миллисекунд \(т\.е\. меньше секунды\), на это указывает проверка _usec <\= 1'000_\. Поэтому выделять целую и дробную часть бессмысленно, и это лишние вычисления\. Исправленный код: ```cpp void thread_ctrl::wait_for(u64 usec, [[maybe_unused]] bool alert /* true */) { // .... if (!alert && usec > 0 && usec <= 1000 && fd_timer != -1) { struct itimerspec timeout; u64 missed; u64 nsec = usec * 1000ull; timeout.it_value.tv_nsec = nsec; timeout.it_value.tv_sec = 0; timeout.it_interval.tv_sec = 0; timeout.it_interval.tv_nsec = 0; timerfd_settime(fd_timer, 0, &timeout, NULL); // .... } } ``` ## Противоречащие проверки [V637](https://pvs-studio.ru/ru/docs/warnings/v637/) Two opposite conditions were encountered\. The second condition is always false\. Check lines: 385, 387\. lv2\_socket\_p2ps\.cpp 385 ```cpp bool lv2_socket_p2ps::handle_listening(p2ps_encapsulated_tcp* tcp_header, [[maybe_unused]] u8* data, ::sockaddr_storage* op_addr) { // .... if ( tcp_header->flags == p2ps_tcp_flags::SYN && backlog.size() < max_backlog) { if (backlog.size() >= max_backlog) { // .... } } // .... } ``` Анализатор обнаружил два противоречащих друг другу условия, вложенных одно в другое\. Действительно, условия _backlog\.size\(\) < max\_backlog_ и _backlog\.size\(\) \>\= max\_backlog_ не могут быть оба выполнены одновременно\. Поэтому выражение вложенного условия всегда будет ложным\. Скорее всего, эта ошибка возникла в результате [рефакторинга](https://github.com/RPCS3/rpcs3/commit/72b2876b6a250b4d3d7076bb42b147f5cca38f70)\. ## Забытая проверка указателя перед разыменованием [V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'm\_finfo' pointer was utilized before it was verified against nullptr\. Check lines: 5316, 5344\. SPURecompiler\.cpp 5316 ```cpp class spu_llvm_recompiler : public spu_recompiler_base , public cpu_translator { // .... function_info* m_finfo; // .... virtual spu_function_t compile(spu_program&& _func) override { // .... const u32 src = m_finfo->fn ? bb.reg_origin_abs[i] : bb.reg_origin[i]; // .... value = m_finfo && m_finfo->load[i] ? m_finfo->load[i] : m_ir->CreateLoad(regptr); // .... } } ``` В данном фрагменте кода указатель _m\_info_ сначала разыменовывается, а ниже по коду проверяется на равенство _nullptr_\. В других местах указатель _m\_info_ проверяется перед разыменованием\. Скорее всего, здесь забыли проверку\. Исправленный код: ```cpp class spu_llvm_recompiler : public spu_recompiler_base , public cpu_translator { // .... function_info* m_finfo; // .... virtual spu_function_t compile(spu_program&& _func) override { // .... const u32 src = m_info && m_finfo->fn ? bb.reg_origin_abs[i] : bb.reg_origin[i]; // .... value = m_finfo && m_finfo->load[i] ? m_finfo->load[i] : m_ir->CreateLoad(regptr); // .... } } ``` ## Дубликат в std::unordered\_map [V766](https://pvs-studio.ru/ru/docs/warnings/v766/) An item with the same key '0x120' has already been added\. evdev\_joystick\_handler\.h 135 ```cpp class evdev_joystick_handler final : public PadHandlerBase { const std::unordered_map button_list = { // .... { 0x11d , "0x11d" }, { 0x11e , "0x11e" }, { 0x11f , "0x11f" }, { BTN_JOYSTICK , "Joystick" }, { BTN_TRIGGER , "Trigger" }, { BTN_THUMB , "Thumb" }, { BTN_THUMB2 , "Thumb 2" }, { BTN_TOP , "Top" }, { BTN_TOP2 , "Top 2" }, { BTN_PINKIE , "Pinkie" }, // .... } } ``` При первом взгляде на код непонятно, какие два ключа одинаковые\. Оказывается, _BTN\_JOYSTICK_ и _BTN\_TRIGGER_ — это константы Linux API, [равные _0x120_](https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/input-event-codes.h#L365)\. При этом _BTN\_JOYSTICK_ — это стартовое значение группы констант сигналов джойстика, а _BTN\_TRIGGER_ — самая первая константа в списке этой группы\. Поэтому их значения совпадают\. Забавно, что разработчики уже сталкивались с этой проблемой и некоторые значения были закомментированы\. Буквально парой строк выше в той же хеш\-таблице можно наблюдать следующее: ```cpp // .... // { BTN_MOUSE , "Mouse" }, same as BTN_LEFT { BTN_LEFT , "Left" }, // .... ``` ## Небезопасный memset [V597](https://pvs-studio.ru/ru/docs/warnings/v597/) The compiler could delete the 'memset' function call, which is used to flush 'tmpbuf' buffer\. The memset\_s\(\) function should be used to erase the private data\. sha1\.cpp 371 ```cpp /* * SHA-1 HMAC final digest */ void sha1_hmac_finish( sha1_context *ctx, unsigned char output[20] ) { unsigned char tmpbuf[20]; sha1_finish( ctx, tmpbuf ); sha1_starts( ctx ); sha1_update( ctx, ctx->opad, 64 ); sha1_update( ctx, tmpbuf, 20 ); sha1_finish( ctx, output ); memset( tmpbuf, 0, sizeof( tmpbuf ) ); } ``` Золотая классика\. В этом фрагменте вызов функции _memset_ будет удалён компилятором в результате оптимизаций, а данные останутся лежать в памяти\. Сложно сказать, сможет ли злоумышленник воспользоваться этой потенциальной уязвимостью\. Однако на всякий случай стоит использовать функцию _memset\_s_, вызов которой компилятор не имеет права убирать\. Есть и [другие способы](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1315r7.html) добиться безопасного очищения памяти\. ## Заключение Итак, мы рассмотрели несколько предупреждений в режиме Best Warnings\. Какие из этого можно сделать выводы? Во\-первых, ошибки допускают даже разработчики таких крутых проектов, как RPCS3\. Причины могут быть разные: рефакторинг, копипаста, невнимательность\. Во\-вторых, Best Warnings оказался действительно удобным режимом\. Полный лог проверки RPCS3 содержал более 700 предупреждений 1\-го и 2\-го уровней\. На просмотр всех предупреждений мне пришлось бы потратить несколько часов\. В режиме же Best Warnings я просмотрел всего 10 предупреждений примерно за полчаса — и уже набрал материала на статью\! :\) В\-третьих, если вас заинтересовал анализатор PVS\-Studio, [скачайте](https://pvs-studio.ru/ru/pvs-studio/try-free/) его, запросите пробный ключ и попробуйте Best Warnings на своём проекте\. Если анализатор вам понравится и вы захотите внедрить его для регулярных проверок, рекомендую эту [статью](https://pvs-studio.ru/ru/blog/posts/0743/)\. Обязательно [расскажите](https://pvs-studio.ru/ru/about-feedback/) нам о своём опыте\! Безбажного вам кода :\)