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

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

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

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


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

>
>
>
Статический анализ следует применять ре…

Статический анализ следует применять регулярно

20 августа 2012 г.

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

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

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

Статический анализ следует применять регулярно, так как:

  • В проекте появляется новый код. Если его не проверить сразу, многие ошибки будут исправляться медленно. О них будет сообщать отдел тестирования или клиенты.
  • Инструменты учатся выявлять всё новые и новые паттерны ошибок.

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

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

Посмотрим, что удалось заметить интересного в коде. В основном подозрительные фрагменты связаны с операторами сдвига.

int64_t DataExtractor::getSLEB128(....) const {
  int64_t result = 0;
  ...
  // Sign bit of byte is 2nd high order bit (0x40)
  if (shift < 64 && (byte & 0x40))
    result |= -(1 << shift);
  ...
}

PVS-Studio: V629 Consider inspecting the '1 << shift' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. dataextractor.cpp 171

Судя по проверке "shift < 64", число 1 может быть сдвинуто влево на [0..63] позиций. Однако этот код может привести к неопределенному поведению (undefined behaviour). Подробнее, почему здесь возможно неопределенное поведение, можно познакомиться в статье "Не зная брода, не лезь в воду. Часть третья". Коварство подобных дефектов заключается в том, что программа может долгое время делать вид, что корректно работает. Сбои могут возникать при смене версий компилятора, при изменении ключей оптимизации и после рефакторинга кода.

Код станет безопасным, если число 1 будет представлено 64-битным беззнаковым типом данных. Тогда его можно будет смело сдвигать на 63 разряда. Безопасный код:

result |= -(1ui64 << shift);

К сожалению, я не соображу, как лучше поступить со знаком минус.

Рассмотрим другой пример, содержащий подозрительную операцию сдвига:

void EmitVBR64(uint64_t Val, unsigned NumBits) {
  if ((uint32_t)Val == Val)
    return EmitVBR((uint32_t)Val, NumBits);

  uint64_t Threshold = 1U << (NumBits-1);
  ...
}

PVS-Studio: V629 Consider inspecting the '1U << (NumBits - 1)' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. bitstreamwriter.h 173

Если аргумент 'NumBits' может быть больше 32, то функция будет работать некорректно. Как и в предыдущем примере при сдвиге '1U' на большие значения, будет возникать неопределенное поведение программы. На практике неопределенное поведение, скорее всего, будет проявлять себя как помещение в переменную 'Threshold' бессмысленных значений.

Безопасный вариант:

uint64_t Threshold = 1UI64 << (NumBits-1);

Рассмотренные выше примеры будут приводить к ошибкам, только в том случае, если сдвиг осуществляется на большое количество бит. Но есть фрагменты, которые всегда приводят к неопределенному поведению. Например, это сдвиг отрицательных чисел.

int find_next(unsigned Prev) const {
  ...
  // Mask off previous bits.
  Copy &= ~0L << BitPos;
  ...
}

PVS-Studio: V610 Undefined behavior. Check the shift operator '<<. The left operand '~0L' is negative. bitvector.h 175

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

Есть и другие потенциально опасные операции сдвига. Они однотипны, поэтому не будем их рассматривать подробнее. Просто перечислю их расположение в коде:

  • V610 Undefined behavior. Check the shift operator '<<=. The left operand 'Val' is negative. pointerintpair.h 139
  • V610 Undefined behavior. Check the shift operator '<<. The left operand '~0L' is negative. bitvector.h 454
  • V610 Undefined behavior. Check the shift operator '<<. The left operand '~0L' is negative. sparsebitvector.h 161
  • V610 Undefined behavior. Check the shift operator '<<=. The left operand 'Val' is negative. pointerintpair.h 144
  • V610 Undefined behavior. Check the shift operator '<<=. The left operand 'Val' is negative. densemapinfo.h 35
  • V610 Undefined behavior. Check the shift operator '<<=. The left operand 'Val' is negative. densemapinfo.h 40
  • V629 Consider inspecting the '1U << (NumBits - 1)' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. bitstreamreader.h 362
  • V629 Consider inspecting the 'Bit->getValue() << i' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. record.cpp 248

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

bool ObjCARCOpt::VisitBottomUp(....) {
  ...
  for (BBState::edge_iterator SI(MyStates.succ_begin()),
       SE(MyStates.succ_end()); SI != SE; ++SI)
  {
    const BasicBlock *Succ = *SI;
    DenseMap<const BasicBlock *, BBState>::iterator I =
      BBStates.find(Succ);
    assert(I != BBStates.end());
    MyStates.InitFromSucc(I->second);
    ++SI;
    for (; SI != SE; ++SI) {
      Succ = *SI;
      I = BBStates.find(Succ);
      assert(I != BBStates.end());
      MyStates.MergeSucc(I->second);
    }
    break;
  }
  ...
}

PVS-Studio: V612 An unconditional 'break' within a loop. objcarc.cpp 2763

Обратите внимание на последний оператор 'break'. Перед ним нет никакого условия, и он всегда останавливает цикл. Таким образом, выполняется только одна итерация цикла.

Аналогичные странные фрагменты кода:

  • V612 An unconditional 'break' within a loop. objcarc.cpp 2948
  • V612 An unconditional 'break' within a loop. undefinedassignmentchecker.cpp 75
  • V612 An unconditional 'break' within a loop. bugreporter.cpp 1095

Заключение

Диагностики V610, V612, V629 являются новыми и поэтому, позволили найти что-то новое и интересное. Если вы проверяли свой проект год назад, это ничего не значит. Вообще ничего. Вы написали новый непроверенный код. В анализаторе появились новые диагностические возможности. И продолжают появляться каждый месяц. Начните использовать статический анализ регулярно, и вы существенно сократите усилия на поиск и устранения многих ошибок.

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

Дата: 22.10.2018

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

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

Дата: 21.11.2018

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

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

Дата: 19.05.2017

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

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

Дата: 14.04.2016

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

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

Дата: 31.07.2017

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

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

Дата: 20.03.2017

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

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

Дата: 30.01.2019

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

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

Дата: 31.05.2014

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

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

Дата: 16.10.2017

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

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

Дата: 17.01.2019

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

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

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

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

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