Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Как один разработчик PVS-Studio защищал…

Как один разработчик PVS-Studio защищал баг в проверяемом проекте

01 Ноя 2021

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

0880_one_case_bug_ru/image1.png

Вступление

В данный момент я пишу статью про проверку проекта DuckStation. Это эмулятор консоли Sony PlayStation. Проект довольно интересный и активно развивающийся. Я нашёл там несколько занимательных багов, одним из которых хочу поделиться прямо сейчас. Он продемонстрирует:

  • как всё же невнимателен может быть человек, даже если является опытным специалистом.
  • как статический анализ подстраховывает человека от его рассеянности и невнимательности.

Показательный пример ошибки

Предупреждение анализатора PVS-Studio: V726 An attempt to free memory containing the 'wbuf' array by using the 'free' function. This is incorrect as 'wbuf' was created on stack. log.cpp 216

template<typename T>
static ALWAYS_INLINE void FormatLogMessageAndPrintW(....)
{
  ....
  wchar_t wbuf[512];
  wchar_t* wmessage_buf = wbuf;
  ....
  if (wmessage_buf != wbuf)
  {
    std::free(wbuf); // <=
  }
  if (message_buf != buf)
  {
    std::free(message_buf);
  }
  ....
}

В изначальной версии пишущейся статьи, я описал этот баг так:

Здесь анализатор обнаружил ошибочный код, в котором осуществляется попытка удаления массива, выделенного на стеке через функцию std::free. Так как память не была выделена на куче, не стоит также и вызывать каких-либо специальных функций для её очистки – она будет произведена автоматически при уничтожении объекта.

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

У нас в компании после написания статьи программистом её вычитывает опытный коллега и даёт рекомендации по её доработке. Этот случай не исключение, и вот какой комментарий дал ревьювер в процессе вычитки статьи на моё описание ошибки:

Здесь нет ошибки. Это ложное срабатывание, анализатор не осилил. Там в середине под условием есть динамическое выделение памяти под сообщение функцией malloc. Проверка if (wmessage_buf != wbuf) служит для того, чтобы определить, нужно вызывать std::free или нет.

Вам, наверное, интересно, что это за malloc и откуда он взялся. Что же, это мой недочёт и самое время его исправить. Давайте я покажу полный код функции, фрагмент из которой я уже приводил выше при описании ошибки и который как раз изучил человек, который проверял статью:

template<typename T>
static ALWAYS_INLINE void FormatLogMessageAndPrintW(
                                             const char* channelName, 
                                             const char* functionName, 
                                             LOGLEVEL level,
                                             const char* message, 
                                             bool timestamp, 
                                             bool ansi_color_code,
                                             bool newline, 
                                             const T& callback)
{
  char buf[512];
  char* message_buf = buf;
  int message_len;
  if ((message_len = FormatLogMessageForDisplay(message_buf,
                       sizeof(buf), channelName, functionName,
                       level, 
                       message, timestamp,
                       ansi_color_code, newline)) > (sizeof(buf) - 1))
  {
    message_buf = static_cast<char*>(std::malloc(message_len + 1));
    message_len = FormatLogMessageForDisplay(message_buf, 
                 message_len + 1, channelName, functionName, 
                 level, message, timestamp, ansi_color_code, newline);
  }
  if (message_len <= 0)
    return;

  // Convert to UTF-16 first so unicode characters display correctly.
  // NT is going to do it anyway...
  wchar_t wbuf[512];
  wchar_t* wmessage_buf = wbuf;
  int wmessage_buflen = countof(wbuf) - 1;
  if (message_len >= countof(wbuf))
  {
    wmessage_buflen = message_len;
    wmessage_buf = static_cast<wchar_t*>
               (std::malloc((wmessage_buflen + 1) * sizeof(wchar_t)));
  }

  wmessage_buflen = MultiByteToWideChar(CP_UTF8, 0, message_buf,
                    message_len, wmessage_buf, wmessage_buflen);

  if (wmessage_buflen <= 0)
    return;

  wmessage_buf[wmessage_buflen] = '\0';
  callback(wmessage_buf, wmessage_buflen);

  if (wmessage_buf != wbuf)
  {
    std::free(wbuf);                        // <=
  }

  if (message_buf != buf)
  {
    std::free(message_buf);
  }
}

Действительно, если длина сообщения больше или равна countof(wbuf), под него создастся новый буфер на куче.

Может показаться, что пример всё больше и больше начинает походить на ложное срабатывание. Однако, я посмотрел на код из функции несколько минут и написал такой ответ, со ссылками на репозиторий:

Категорически не cогласен, давай смотреть на код: буфер на стеке, динамическая аллокация нового буфера на куче, освобождение не того буфера.

Если в локальный буфер на стеке не помещается строчка, то мы кладём её в динамически выделенный буфер по указателю wmessage_buf. И, как видно по коду, ниже есть 2 ветки с освобождением памяти, если динамическая аллокация произошла (это делают проверками вроде wmessage_buf != wbuf). Только в первой ветке освобождается не та память, на что мы и ругаемся. А уже во второй освобождается правильный буфер, и на этот код мы как раз не ругаемся.

Действительно, ошибка всё же есть, и программисту, написавшему этот код, следовало очистить буфер wmessage_buf, по аналогии с тем, как он сделал это ниже.

Ответ коллеги был краток:

Согласен. Поспешил.

P.S. С меня пиво.

Заключение

Любой статический анализатор, к сожалению, выдаёт ложно-положительные срабатывания. Это приводит к тому, что иногда программисты слишком спешат посчитать показавшееся им неубедительное сообщение ложным. Хочется предостеречь от спешки и постараться подходить к изучению сообщений с вниманием.

Кстати, у нас уже были забавные статьи на подобную тему, например:

Приятного прочтения. И приглашаем попробовать PVS-Studio в деле.

Популярные статьи по теме
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

Автор: Андрей Карпов

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Дата: 22 Окт 2018

Автор: Андрей Карпов

PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь ок…
Зло живёт в функциях сравнения

Дата: 19 Май 2017

Автор: Андрей Карпов

Возможно, читатели помнят мою статью под названием "Эффект последней строки". В ней идёт речь о замеченной мной закономерности: ошибка чаще всего допускается в последней строке однотипных блоков текс…
Эффект последней строки

Дата: 31 Май 2014

Автор: Андрей Карпов

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
Любите статический анализ кода!

Дата: 16 Окт 2017

Автор: Андрей Карпов

Я в шоке от возможностей статического анализа кода, хотя сам участвую в разработке инструмента PVS-Studio. На днях я был искренне удивлён тому, что анализатор оказался умнее и внимательнее меня.
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

Автор: Андрей Карпов

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
PVS-Studio ROI

Дата: 30 Янв 2019

Автор: Андрей Карпов

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…
PVS-Studio для Java

Дата: 17 Янв 2019

Автор: Андрей Карпов

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

Дата: 20 Мар 2017

Автор: Андрей Карпов

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

Автор: Андрей Карпов

Краткое описание технологий, используемых в инструменте PVS-Studio, которые позволяют эффективно обнаруживать большое количество паттернов ошибок и потенциальных уязвимостей. Статья описывает реализа…

Комментарии (0)

Следующие комментарии
Этот сайт использует куки и другие технологии, чтобы предоставить вам более персонализированный опыт. Продолжая просмотр страниц нашего веб-сайта, вы принимаете условия использования этих файлов. Если вы не хотите, чтобы ваши данные обрабатывались, пожалуйста, покиньте данный сайт. Подробнее →
Принять