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

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

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

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

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

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Ваше сообщение отправлено.

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


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

>
>
>
Топ 10 ошибок в C++ проектах за 2020 год

Топ 10 ошибок в C++ проектах за 2020 год

18 Дек 2020

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

0784_Top_10_CPP_Bugs_2020_ru/image1.png

Стоит отметить, что прошедший год ознаменовался большим количеством новых диагностических правил, срабатывания которых позволили им попасть в данный топ. Также мы продолжаем улучшать ядро анализатора и добавлять новые сценарии его использования, обо всём этом можно почитать в нашем блоге. Если вам интересны другие поддерживаемые нашим анализатором языки (C, C# и Java), обратите внимание на статьи моих коллег. Теперь же перейдём непосредственно к самым запомнившимся мне багам, найденным PVS-Studio за прошедший год.

Десятое место: Деление по модулю на единицу

V1063 The modulo by 1 operation is meaningless. The result will always be zero. llvm-stress.cpp 631

void Act() override {
  ....
  // If the value type is a vector, and we allow vector select,
  // then in 50% of the cases generate a vector select.
  if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 1)) {
    unsigned NumElem =
        cast<FixedVectorType>(Val0->getType())->getNumElements();
    CondTy = FixedVectorType::get(CondTy, NumElem);
  }
  ....
}

Разработчик хотел получить случайное значение в диапазоне от 0 до 1, использовав деление по модулю. Однако операция вида X%1 всегда вернёт 0. В данном случае правильно было бы переписать условие следующим образом:

if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 2))

Эта ошибка вошла в топ из статьи: "Проверка Clang 11 с помощью PVS-Studio".

Девятое место: Четыре проверки

На следующий участок кода PVS-Studio выдал четыре предупреждения:

  • V560 A part of conditional expression is always true: x >= 0. editor.cpp 1137
  • V560 A part of conditional expression is always true: y >= 0. editor.cpp 1137
  • V560 A part of conditional expression is always true: x < 40. editor.cpp 1137
  • V560 A part of conditional expression is always true: y < 30. editor.cpp 1137
int editorclass::at( int x, int y )
{
  if(x<0) return at(0,y);
  if(y<0) return at(x,0);
  if(x>=40) return at(39,y);
  if(y>=30) return at(x,29);

  if(x>=0 && y>=0 && x<40 && y<30)
  {
      return contents[x+(levx*40)+vmult[y+(levy*30)]];
  }
  return 0;
}

Все предупреждения относятся к последнему if-выражению. Проблема в том, что все четыре проверки, которые в нём выполняются, всегда будут возвращать true. Не сказал бы, что это серьезная ошибка, но получилось довольно забавно. В общем, данные проверки являются избыточными и их можно убрать.

Эта ошибка вошла в топ из статьи: "VVVVVV??? VVVVVV!!!".

Восьмое место: delete вместо delete[]

V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] poke_data;'. CCDDE.CPP 410

BOOL Send_Data_To_DDE_Server (char *data, int length, int packet_type)
{
  ....
  char *poke_data = new char [length + 2*sizeof(int)]; // <=
  ....
  if(DDE_Class->Poke_Server( .... ) == FALSE) {
    CCDebugString("C&C95 - POKE failed!\n");
    DDE_Class->Close_Poke_Connection();
    delete poke_data;                                  // <=
    return (FALSE);
  }

  DDE_Class->Close_Poke_Connection();

  delete poke_data;                                    // <=

  return (TRUE);
}

Анализатор обнаружил ошибку, связанную с тем, что память выделена и освобождена несовместимыми между собой способами. Для освобождения памяти, выделенной под массив, следует использовать оператор delete[], а не delete.

Эта ошибка вошла в топ из статьи: "Код игры Command & Conquer: баги из 90-х. Том второй".

Седьмое место: Выход за границу буфера

Рассмотрим функцию net_hostname_get, которая будет использоваться дальше.

#if defined(CONFIG_NET_HOSTNAME_ENABLE)
const char *net_hostname_get(void);
#else
static inline const char *net_hostname_get(void)
{
  return "zephyr";
}
#endif

В данном случае при препроцессировании выбирался вариант, относящийся к ветке #else. То есть, в препроцессированном файле функция реализуется так:

static inline const char *net_hostname_get(void)
{
  return "zephyr";
}

Функция возвращает указатель на массив из 7 байт (учитываем терминальный ноль в конце строки).

Теперь рассмотрим код, приводящий к выходу за границу массива.

static int do_net_init(void)
{
  ....
  (void)memcpy(hostname, net_hostname_get(), MAX_HOSTNAME_LEN);
  ....
}

Предупреждение PVS-Studio: V512 [CWE-119] A call of the 'memcpy' function will lead to the 'net_hostname_get()' buffer becoming out of range. log_backend_net.c 114

После препроцессирования MAX_HOSTNAME_LEN раскрывается следующим образом:

(void)memcpy(hostname, net_hostname_get(),
    sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx"));

Соответственно, при копировании данных возникает выход за границу строкового литерала. Как это скажется на выполнении программы - предсказать сложно, так как это приводит к неопределённому поведению.

Эта ошибка вошла в топ из статьи: "Исследуем качество кода операционной системы Zephyr".

Шестое место: Что-то очень странное

static char *mntpt_prepare(char *mntpt)
{
  char *cpy_mntpt;

  cpy_mntpt = k_malloc(strlen(mntpt) + 1);
  if (cpy_mntpt) {
    ((u8_t *)mntpt)[strlen(mntpt)] = '\0';
    memcpy(cpy_mntpt, mntpt, strlen(mntpt));
  }
  return cpy_mntpt;
}

Предупреждение PVS-Studio: V575 [CWE-628] The 'memcpy' function doesn't copy the whole string. Use 'strcpy / strcpy_s' function to preserve terminal null. shell.c 427

Кто-то пытался сделать аналог функции strdup, но у него это не получилось.

Начнём с предупреждения анализатора. Он сообщает, что функция memcpy копирует строчку, но не скопирует терминальный ноль, и это очень подозрительно.

Кажется, что этот терминальный 0 копируется здесь:

((u8_t *)mntpt)[strlen(mntpt)] = '\0';

Но нет! Здесь опечатка, из-за которой терминальный ноль копируется сам в себя! Обратите внимание, что запись происходит в массив mntpt, а не в cpy_mntpt. В итоге функция mntpt_prepare возвращает строку, незавершенную терминальным нулём.

На самом деле программист хотел написать так:

((u8_t *)cpy_mntpt)[strlen(mntpt)] = '\0';

Однако всё равно не понятно, зачем сделано так сложно! Этот код можно упростить до следующего варианта:

static char *mntpt_prepare(char *mntpt)
{
  char *cpy_mntpt;

  cpy_mntpt = k_malloc(strlen(mntpt) + 1);
  if (cpy_mntpt) {
    strcpy(cpy_mntpt, mntpt);
  }
  return cpy_mntpt;
}

Эта ошибка вошла в топ из вышеупомянутой статьи: "Исследуем качество кода операционной системы Zephyr".

Пятое место: Неправильная защита от переполнения

V547 [CWE-570] Expression 'rel_wait < 0' is always false. Unsigned type value is never < 0. os_thread_windows.c 359

static DWORD
get_rel_wait(const struct timespec *abstime)
{
  struct __timeb64 t;
  _ftime64_s(&t);
  time_t now_ms = t.time * 1000 + t.millitm;
  time_t ms = (time_t)(abstime->tv_sec * 1000 +
    abstime->tv_nsec / 1000000);

  DWORD rel_wait = (DWORD)(ms - now_ms);

  return rel_wait < 0 ? 0 : rel_wait;
}

В данном случае переменная rel_wait имеет беззнаковый тип DWORD. А значит, сравнение rel_wait < 0 не имеет смысла, так как результатом всегда является ложь.

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

Ошибка же вошла в топ из статьи: "Статический анализ кода коллекции библиотек PMDK от Intel и ошибки, которые не ошибки".

Четвёртое место: Не пиши в std, брат

V1061 Extending the 'std' namespace may result in undefined behavior. sized_iterator.hh 210

// Dirty hack because g++ 4.6 at least wants
// to do a bunch of copy operations.
namespace std {
inline void iter_swap(util::SizedIterator first,
                      util::SizedIterator second)
{
  util::swap(*first, *second);
}
} // namespace std

В статье, из которой взято срабатывание: "Анализ кода проекта DeepSpeech или почему не стоит писать в namespace std" подробно описано, почему не стоит поступать подобным образом.

Третье место: Скроллбар, который не смог

V501. There are identical sub-expressions to the left and to the right of the '-' operator: bufferHeight - bufferHeight TermControl.cpp 592

bool TermControl::_InitializeTerminal()
{
  ....
  auto bottom = _terminal->GetViewport().BottomExclusive();
  auto bufferHeight = bottom;

  ScrollBar().Maximum(bufferHeight - bufferHeight);
  ScrollBar().Minimum(0);
  ScrollBar().Value(0);
  ScrollBar().ViewportSize(bufferHeight);
  ....
}

Это, что называется, "срабатывание с историей". В данном случае из-за ошибки не работал скроллбар в Windows Terminal. По мотивам данного бага написана целая статья, в которой мой коллега провёл исследование и разобрался почему так случилось. Заинтересовались? Вот она: "Скроллбар, который не смог".

Второе место: перепутали радиус и высоту

И опять речь пойдёт о нескольких предупреждениях анализатора:

  • V764 Possible incorrect order of arguments passed to 'CreateWheel' function: 'height' and 'radius'. StandardJoints.cpp 791
  • V764 Possible incorrect order of arguments passed to 'CreateWheel' function: 'height' and 'radius'. StandardJoints.cpp 833
  • V764 Possible incorrect order of arguments passed to 'CreateWheel' function: 'height' and 'radius'. StandardJoints.cpp 884

Привожу вызовы функции:

NewtonBody* const wheel = CreateWheel (scene, origin, height, radius);

А так выглядит её объявление:

static NewtonBody* CreateWheel (DemoEntityManager* const scene,
  const dVector& location, dFloat radius, dFloat height)

При вызовах функций аргументы были перепутаны местами.

Эта ошибка вошла в топ из статьи: "Повторная проверка Newton Game Dynamics статическим анализатором PVS-Studio".

Первое место: Затирание результата

V519 The 'color_name' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 621, 627. string.cpp 627

static bool parseNamedColorString(const std::string &value,
                                  video::SColor &color)
{
  std::string color_name;
  std::string alpha_string;

  size_t alpha_pos = value.find('#');
  if (alpha_pos != std::string::npos) {
    color_name = value.substr(0, alpha_pos);
    alpha_string = value.substr(alpha_pos + 1);
  } else {
    color_name = value;
  }

  color_name = lowercase(value); // <=

  std::map<const std::string, unsigned>::const_iterator it;
  it = named_colors.colors.find(color_name);
  if (it == named_colors.colors.end())
    return false;
  ....
}

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

Однако затем в функции lowercase() в нижний регистр переводится не сама полученная строка, а исходный аргумент функции. В результате мы просто потеряем цвет, который должна была вернуть parseNamedColorString().

color_name = lowercase(color_name);

Эта ошибка вошла в топ из статьи: "PVS-Studio: Анализ pull request-ов в Azure DevOps при помощи self-hosted агентов".

Заключение

За прошедший год мы нашли много ошибок в open source проектах. Это были привычные ошибки copy-paste, ошибки в константах, утечки памяти и множество других проблем. Наш анализатор не стоит на месте и в топе присутствует несколько срабатываний новых диагностик, написанных в этом году.

Надеюсь, вам понравились собранные ошибки. Лично мне они показались достаточно интересными. Но, конечно, ваше видение может отличаться от моего, поэтому вы можете составить свой "Tоп 10...", почитав статьи из нашего блога или посмотрев список ошибок, найденных PVS-Studio в open source проектах.

Также предлагаю вашему вниманию статьи с топ 10 C++ ошибок прошлых лет: 2016, 2017, 2018, 2019.

Популярные статьи по теме
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

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

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

Дата: 22 Окт 2018

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

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

Дата: 27 Июн 2017

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

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

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

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

Дата: 31 Июл 2017

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

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
Эффект последней строки

Дата: 31 Май 2014

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

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

Дата: 22 Дек 2018

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

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

Дата: 17 Янв 2019

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

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

Дата: 19 Май 2017

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

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

Дата: 16 Окт 2017

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

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

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

Следующие комментарии

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