Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо
to the top
>
>
>
Теперь PVS-Studio ещё лучше знает, что …

Теперь PVS-Studio ещё лучше знает, что за зверь такой – strlen

27 Апр 2021

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

0824_DataFlow_And_Strlen_ru/image1.png

Всё началось с твита от JetBrains CLion IDE

На днях я увидел в Twitter пост от JetBrains про новые возможности статического анализатора, встроенного в CLion.

0824_DataFlow_And_Strlen_ru/image2.png

Поскольку мы скоро планируем выпустить плагин PVS-Studio для CLion, то я не мог пройти мимо и не написать, что мы тоже не лыком шиты. И что есть смысл попробовать PVS-Studio как плагин для CLion, чтобы находить ещё больше ошибок.

0824_DataFlow_And_Strlen_ru/image3.png

Ну и ещё немного с ними мило попереписывался:

После всего этого я подумал. А ведь они молодцы! Улучшили анализ потока данных и рассказывают миру. А мы чем хуже? Мы ведь тоже постоянно что-то улучшаем внутри анализатора, в том числе и тот же механизм анализа потока данных. И вот я уже пишу эту заметку.

А что у нас интересного с Data Flow

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

bool foo()
{
  unsigned N = 2;
  for (unsigned i = 0; i < N; ++i)
  {
    bool stop = (i - 1 == N);
    if (stop)
      return true;
  }
  return false;
}

Анализатор не мог понять, что переменной stop всегда присваивается значение false.

Почему false? Давайте быстро посчитаем:

  • диапазон значения переменной i = [0; 1];
  • возможные значения выражения i-1 = [0; 0] U [UINT_MAX; UINT_MAX];
  • значение переменной N, равное двойке, не входит в множество { 0, UINT_MAX };
  • условие всегда ложно.

Примечание. Неопределённого поведения здесь нет, так как происходит переполнение (wrap) при работе с беззнаковым типом.

Теперь мы научили PVS-Studio правильно работать с такими выражениями и выдавать соответствующее предупреждение. Что интересно, это изменение повлекло каскад других доработок.

Например, возникли ложные срабатывания, связанные с обработкой длины строк. Борьба с ними привела к новым улучшениям и обучению анализатора лучше понимать, как и зачем используют такие функции, как strlen. Сейчас мы на практике покажем, о каких улучшениях идёт речь.

В тестовой базе открытых проектов, на которой мы регулярно проводим регрессионное тестирование ядро анализатора, есть эмулятор FCEUX. После проделанных улучшений мы смогли найти в коде интересную ошибку в функции Assemble.

int Assemble(unsigned char *output, int addr, char *str) {
  output[0] = output[1] = output[2] = 0;
  char astr[128],ins[4];
  if ((!strlen(str)) || (strlen(str) > 0x127)) return 1;
  strcpy(astr,str);
  ....
}

Видите ошибку? Если честно, мы не сразу её заметили и думали, что что-то поломали. А когда поняли, в чём суть, то в очередной раз восхитились тем, как статический анализ может быть полезен.

Предупреждение PVS-Studio: V512 A call of the 'strcpy' function will lead to overflow of the buffer 'astr'. asm.cpp 21

Всё равно не видите ошибку? Давайте внимательно разберём код. Для начала уберём всё не относящееся к делу:

int Assemble(char *str) {
  char astr[128];
  if ((!strlen(str)) || (strlen(str) > 0x127)) return 1;
  strcpy(astr,str);
  ....
}

Есть локальный массив из 128 байт, в который планируется скопировать строчку, переданную в качестве аргумента. Копирование не должно выполняться, если строка пустая или содержит более 127 символов (не считая терминальный ноль).

Пока всё логично и правильно? На первый взгляд, да. Но что это?! Что это за константа 0x127?!

Это вовсе не 127. Совсем не 127 :)

Константа задана в шестнадцатеричной системе. Если перевести в десятичную, то получается 295.

Итак, написанный код эквивалентен следующему:

int Assemble(char *str) {
  char astr[128];
  if ((!strlen(str)) || (strlen(str) > 295)) return 1;
  strcpy(astr,str);
  ....
}

Как видите, проверка никак не защищает от переполнения буфера, и анализатор совершенно правильно предупреждает о проблеме.

Раньше анализатор не мог найти ошибку, будучи не в состоянии понять, что две функции strlen работают с одной строкой. И эта строка не меняется между двумя вызовами strlen. С точки зрения программиста, всё это очевидно, а вот анализатор нужно учить всё это понимать :).

Теперь PVS-Studio выводит из выражения, что длина строки str лежит в диапазоне [1..295], а значит, может возникнуть выход за границу массива, если попытаться его скопировать в буфер astr.

0824_DataFlow_And_Strlen_ru/image4.png

Новые вызовы

Описанная ошибка присутствует и в текущей версии кодовой базы проекта FCEUX. Но мы её не найдём, так как код изменился и теперь длина строки сохраняется в переменной. Это разрывает взаимосвязь между строкой и её длиной. Анализатор пока, к сожалению, молчит на новый вариант кода:

int Assemble(unsigned char *output, int addr, char *str) {
  output[0] = output[1] = output[2] = 0;
  char astr[128],ins[4];
  int len = strlen(str);
  if ((!len) || (len > 0x127)) return 1;
  strcpy(astr,str);
  ....
}

Человеку такой код может показаться даже проще, но, с точки зрения статического анализа, он труден для отслеживания значений. Нужно учитывать, что значение переменной len является длиной строки str. Дополнительно требуется аккуратно отслеживать, когда разорвётся эта взаимосвязь при модификации содержимого строки или переменной len.

Пока это анализатор PVS-Studio делать не умеет. Зато видно, куда можно и нужно развиваться! Со временем научимся находить ошибку и в этом новом коде.

Кстати, читатель может задаться вопросом, а почему мы анализируем старый код проектов и не обновляем их регулярно? Всё просто. Если обновлять проекты, мы не сможем проводить регрессионное тестирование. Будет непонятно, появились изменения в работе анализатора из-за правки кода самого анализатора или из-за правки в коде проверяемого проекта. Поэтому мы не обновляем открытые проекты, используемые в качестве базы для тестирования.

А чтобы тестировать анализатор на современном коде, написанном на C++14, C++17 и т.д., мы постепенно пополняем базу новыми проектами. Например, относительно недавно мы добавили коллекцию header-only C++ библиотек (awesome-hpp).

Заключение

Развивать механизмы анализа потока данных интересно и полезно. А если вам интересней в целом больше узнать о работе статических анализаторов кода, то предлагаем вашему вниманию следующие наши публикации:

Приглашаю скачать анализатор PVS-Studio и проверить свои проекты.

Популярные статьи по теме


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

Следующие комментарии next comments
close comment form
close form

Заполните форму в два простых шага ниже:

Ваши контактные данные:

Шаг 1
Поздравляем! У вас есть промокод!

Тип желаемой лицензии:

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

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

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

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

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

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


Если вы так и не получили ответ, пожалуйста, проверьте, отфильтровано ли письмо в одну из следующих стандартных папок:

  • Промоакции
  • Оповещения
  • Спам