Pull to refresh
584
33
Андрей Карпов @Andrey2008

Директор по маркетингу

Send message

Разница в том, что анализатор знает, что такое malloc. И к сожалению (пока) не знает, что такое getViewObject. Его можно научить с помощью углубления технологий анализа (межпроцедурный анализ, межмодульный анализ) или за счёт явной аннотации методов.

Дело не в том может или не может указатель быть нулевым. Важно, чтобы анализ был полезен. А для этого надо ругаться ТОЛЬКО там, где есть информация/повод.

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

P.S. Это тогда уж проще на каждую строчку кода на всякий случай ругаться :) "У вас строчка кода на языке C++! Она может содержать ошибку" - совершенное точное предупреждение и совершенно бесполезное :)

В первом случае, анализатор ничего не знает про указатели (межпроцедурный+межмодульный анализ не смог вычислить, есть ли вариант, когда указатель нулевой).

Во втором случае, есть артефакт.

auto oldAnchor = detail->AnchorPoint.getValue();

if (detail) {

Есть разыменование указателя. Есть его последующая проверка. Значит, программист предполагал, что указатель может быть нулевой. Подробнее: Пояснение про диагностику V595.

Если проверки if ( detail)не будет и анализатор не сможет вычислить , может ли detail быть нулевым, то предупреждения не будет.

Вопрос про malloc не понятен. Хотя, это не важно. Функция malloc может вернуть NULL, а значит указатель может быть нулевым (и его надо проверять до использования). Тут и гадать не надо :)

Ругаться, на разыменование указателей, которые предварительно не проверены на nullptr, просто. К сожалению, это путь в никуда. Если придерживаться логики "ругаться на всё, в чём нет уверенности в безопасности" приведёт к такому количеству срабатываний, что анализатором пользоваться будет невозможно. Мы придерживаемся другой философии, которую я описал в этой статье. Цитата оттуда:

Есть два философских подхода в реализации статических анализаторов кода:

  • Ругаемся на всё, про что не можем сказать, что оно правильное.

  • Ругаемся на то, которое по какой-то причине скорее всего неправильное.

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

К нам поступают предложения по диагностикам следующего вида:

  • Следует выдавать предупреждение на A / B, если нет уверенности, что B != 0.

  • Следует выдавать предупреждение, если нет уверенности, что при вызове функции strcpy(DST, SRC) буфер DST достаточного размера.

  • Следует выдавать предупреждение при разыменовывании указателя, если нет уверенности, что pointer != NULL.

  • Следует выдавать предупреждение на sqrt(X), если нет уверенности, что X >= 0.

  • И так далее.

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

Прошу пояснить, какую реакцию Вы ожидаете здесь от анализатора.

Неожиданное продолжение про простую ошибку разыменования нулевого указателя: FreeCAD и C++ код с неопределённым поведением для медитации (схожий случай).

Это было интересно. Обычно, я что-то такое пишу. А тут довелось самому поиграть в угадайку. Спасибо.

Кстати, пример с int a = index + pockets[++index]; напомнил мне про наш один вопрос - Глубина кроличьей норы или собеседование по C++ в компании PVS-Studio :)

P.S. А кому интересно узнать про глубину глубин с Unicode, приглашаю сюда - Атака Trojan Source для внедрения в код изменений, незаметных для разработчика.

Спасибо за статью. По духу близко к моей подборке вредных советов :)

Пара мыслей про описанные баги.

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

И наоборот, ошибка с исчезновением memset кажется чем-то экзотическим и редким. Но это очень распространённая потенциальная уязвимость. Мы их обнаружили 100500 штук и продолжает обнаруживать. Хотя, казалось бы на эту тему тоже уже много написано: CWE-14, Zero and forget -- caveats of zeroing memory in C, Безопасная очистка приватных данных.

Сейчас у нас нет планов по реализации поддержки Golang.

Если смотреть не из мира розовых единорогов, где весь код короткий и при этом правильный, то это плохой совет. На мой взгляд, длинный код лучше читается и более защищает от ошибок. По крайней мере в контексте сравнения с nullptr.

Вот, например, как узнать что хотели проверить? Указатель или значение по указателю?

int *p = foo();
if (p) .... // невозможно сказать, верно написано или нет
if (*p) .... // невозможно сказать, верно написано или нет
if (p != nullptr) .... // скорее всего всё хорошо, хотели проверить именно указатель
if (*p != nullptr) .... // ошибка компиляции

Или вот такой реальный пример из проекта Mozilla Firefox:

const char *token = str.get();
if (token == '\0') {
  return NS_ERROR_DOM_SYNTAX_ERR; // nothing between commas
}

Если-бы человек был фанатом сокращения текста, он бы написал:

if (!token)

И тогда анализатор PVS-Studio не смог-бы найти ошибку. А за счёт дополнительной информации, что сравнивают не с абстрактным нулём, а с нулевым литералом '\0', анализатор способен предположить, что здесь опечатка:

V528 It is odd that pointer to 'char' type is compared with the '\0' value. Probably meant: *token == '\0'. svgnumberlist.cpp 96

В общем, забыли разыменовать указатель. Правильно:

if (*token == '\0')

Что вы хотите от синтетического примера? :) Его смысл показать, что предупреждение показалось ложным, а оно не ложное.

Естественно, в реальном приложении, всё это где-то глубоко в потрохах. И утечка памяти самая настоящая. Естественно, реальная программа не заканчивает сразу после этого работу, как в моём примере :).

Признайтесь, вы этот пример сами выдумали? :)

Нет. Такой простой случай сложно придумать :) Правильнее будет сказать, я его "услышал". Хотя можно без кавычек. Это в прямом смысле.

Слышу вдруг коллега в соседней секции: "Ааааа!" и прочие эмоции. Заинтригован, подхожу. Что такое? Он рассказывает и показывает. Я понимаю, что это стоит маленькой заметки :)

Про V575.

Сообщение говорит, что в оператор delete всегда передаётся нулевой указатель. Если бы он не всегда был нулевым, то и предупреждения не будет. Это ведь нормально. А вот передача явного nullptr говорит и какой-то ошибке или избыточном коде. Возможно, опечатались и не ту переменную используют. Не очень понятно, что именно сбивает с толку. на мой взгляд вполне понятное предупреждение. Хотя, конечно, у меня может быть взгляд замылен :).

Интерес к C# и Java растёт, но медленно. Думаю, мы сами в этом виноваты, так как команде не хватает сил более активно заниматься их продвижением. Про Java, например, вообще давно статей не писали :(. На тему C++ у нас куда больше разнообразного материала и активностей. В том числе больших и сильных, таких как мини-книга про вредные C++ советы.

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

CVE - как я понимаю, имеется в виду SCA (software composition analysis). Пока не понятно.

C++ остаётся основным направлением? - Да. Оно самое старое, поэтому там больше всего клиентов и поддержки. И с продвижением у нас в этом ловчее получается.

На это полагаться нельзя и даже вредно про это думать. Неизвестно как и где будет выполняться программа. Более того, код может быть заимствован для других проектов, работающих в других условиях. Всегда следует исходить из того, что new может сгенерировать исключение.

P.S. Раньше подобное приходилось про malloc писать, теперь new.... эх...

В данном случае согласен. В более общем, указатель может инициализироваться при одних условиях и оставаться нулевым в других. Программисты знают про это и пишут лишние проверки перед delete (и перед free в C). Я имел в иду, что такие проверки не нужны, так как deletefree) корректно обработают нулевой указатель.

Да, еще есть new(std::nothrow), но он тут ни при чём.

Просто интересно, а почему не упоминается статический анализ кода? По идее он идёт ещё раньше или параллельно с юнит-тестированием. Или статический анализ относится  уже не к команде тестирования? Хотя юнит-тесты, вроде тоже не задача тестировщиков.

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

Information

Rating
182-nd
Works in
Date of birth
Registered
Activity

Specialization

Specialist
C++
C
Software development