Best Warnings — режим анализатора, оставляющий в окне вывода 10 лучших предупреждений. Мы предлагаем вам ознакомиться с обновлённым режимом Best Warnings на примере проверки проекта RPCS3.
Best Warnings — это специальный механизм для первого знакомства со статическим анализатором PVS-Studio. Полный лог анализатора может содержать тысячи предупреждений. Поэтому если вы хотите оценить работу анализатора и не тратить время и силы на просмотр большого отчёта, выданного ещё не настроенным анализатором, то вы как раз можете воспользоваться механизмом Best Warnings. Для этого откройте лог анализатора в плагине PVS-Studio для Visual Studio и включите Best Warnings:
При использовании Best Warnings отображаются 10 лучших предупреждений анализатора. Вот так стало выглядеть окно вывода после включения Best Warnings:
Этот режим появился год назад. И не так давно мы поняли, что им почти никто не пользуется. Почему? Потому что найти этот режим было проблематично. Поэтому мы перенесли кнопку на главную строку панели и сделали ей яркую иконку. Кроме того, теперь режим доступен в JetBrains Rider, CLion, IntelliJ IDEA, а также в C and C++ Compiler Monitoring UI.
Второй проблемой были неинтересные предупреждения в окне вывода Best Warnings. Отчасти это вызвано тем, что мы сами забыли про свою собственную фичу и не добавляли веса для новых диагностик☹ Оказалось, что 15 новых диагностик просто не участвовали в режиме Best Warnings. Разумеется, мы это исправили. Также мы посмотрели, какие предупреждения выдаёт режим Best Warnings на различных Open Source проектах, и изменили старые веса диагностик, чтобы предупреждения стали интереснее.
Третьей проблемой были "дубликаты". Не очень интересно видеть два и более предупреждений одного и того же диагностического правила в Best Warnings. Механизм для удаления дубликатов уже был — веса второго и последующих предупреждений диагностики пропорционально уменьшались. Мы увеличили штраф для повторных срабатываний, и теперь они появляются реже.
Более подробно о Best Warnings можете прочитать здесь.
RPCS3 — это эмулятор консоли PS3. Мы уже проверяли этот проект ранее. Кодовая база проекта составляет 300 тысяч строк кода на C++, и этого вполне достаточно для демонстрации Best Warnings. Для проверки я зафиксировал проект на коммите e98b07d.
Давайте перейдём к ошибкам.
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
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);
// ....
}
}
Здесь механизм анализа потока данных смог выявить интересную аномалию. В данном фрагменте кода происходит следующее:
Из первых двух пунктов получаем, что значение переменной nsec не превосходит 1'000'000, что меньше 1'000'000'000. Получается, что выражение nsec / 1'000'000'000 всегда равно 0, а выражение nsec % 1'000'000'000 равно nsec.
Далее вычисленные значения используются для установки таймера при вызове функции timerfd_settime. В функцию wait_for передают количество миллисекунд, в течение которых нужно ждать. В переменную nsec записывается это же значение, только в наносекундах, поэтому умножаем на 1'000. Операции взятия остатка и деления на 1'000'000'000 — выделение дробной части и целого количества секунд соответственно. Скорее всего, в функцию wait_for должны приходить значения, не превосходящие 1'000 миллисекунд (т.е. меньше секунды), на это указывает проверка usec <= 1'000. Поэтому выделять целую и дробную часть бессмысленно, и это лишние вычисления.
Исправленный код:
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 Two opposite conditions were encountered. The second condition is always false. Check lines: 385, 387. lv2_socket_p2ps.cpp 385
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 не могут быть оба выполнены одновременно. Поэтому выражение вложенного условия всегда будет ложным. Скорее всего, эта ошибка возникла в результате рефакторинга.
V595 The 'm_finfo' pointer was utilized before it was verified against nullptr. Check lines: 5316, 5344. SPURecompiler.cpp 5316
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 проверяется перед разыменованием. Скорее всего, здесь забыли проверку. Исправленный код:
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);
// ....
}
}
V766 An item with the same key '0x120' has already been added. evdev_joystick_handler.h 135
class evdev_joystick_handler final : public PadHandlerBase
{
const std::unordered_map<u32, std::string> 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. При этом BTN_JOYSTICK — это стартовое значение группы констант сигналов джойстика, а BTN_TRIGGER — самая первая константа в списке этой группы. Поэтому их значения совпадают.
Забавно, что разработчики уже сталкивались с этой проблемой и некоторые значения были закомментированы. Буквально парой строк выше в той же хеш-таблице можно наблюдать следующее:
// ....
// { BTN_MOUSE , "Mouse" }, same as BTN_LEFT
{ BTN_LEFT , "Left" },
// ....
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
/*
* 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, вызов которой компилятор не имеет права убирать. Есть и другие способы добиться безопасного очищения памяти.
Итак, мы рассмотрели несколько предупреждений в режиме Best Warnings. Какие из этого можно сделать выводы?
Во-первых, ошибки допускают даже разработчики таких крутых проектов, как RPCS3. Причины могут быть разные: рефакторинг, копипаста, невнимательность.
Во-вторых, Best Warnings оказался действительно удобным режимом. Полный лог проверки RPCS3 содержал более 700 предупреждений 1-го и 2-го уровней. На просмотр всех предупреждений мне пришлось бы потратить несколько часов. В режиме же Best Warnings я просмотрел всего 10 предупреждений примерно за полчаса — и уже набрал материала на статью! :)
В-третьих, если вас заинтересовал анализатор PVS-Studio, скачайте его, запросите пробный ключ и попробуйте Best Warnings на своём проекте. Если анализатор вам понравится и вы захотите внедрить его для регулярных проверок, рекомендую эту статью. Обязательно расскажите нам о своём опыте! Безбажного вам кода :)
0