Разница в том, что анализатор знает, что такое 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? Нет границы или критерия где надо остановиться, реализуя подобные диагностики.
Кажется, что ошибки с забытой точкой запятой весьма распространены. Они часто упоминаются в разных статьях. Но, на самом деле, они встречаются очень редко. Более чем за 10 лет проверки открытых проектов мы нашли всего 3 таких ошибки. Думаю, сказывается, что все знают про эти ошибки и внимательны к ним. Плюс анализаторы и компиляторы, видимо, тоже хорошо помогают обнаруживать их.
Если смотреть не из мира розовых единорогов, где весь код короткий и при этом правильный, то это плохой совет. На мой взгляд, длинный код лучше читается и более защищает от ошибок. По крайней мере в контексте сравнения с 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
В общем, забыли разыменовать указатель. Правильно:
Что вы хотите от синтетического примера? :) Его смысл показать, что предупреждение показалось ложным, а оно не ложное.
Естественно, в реальном приложении, всё это где-то глубоко в потрохах. И утечка памяти самая настоящая. Естественно, реальная программа не заканчивает сразу после этого работу, как в моём примере :).
Нет. Такой простой случай сложно придумать :) Правильнее будет сказать, я его "услышал". Хотя можно без кавычек. Это в прямом смысле.
Слышу вдруг коллега в соседней секции: "Ааааа!" и прочие эмоции. Заинтригован, подхожу. Что такое? Он рассказывает и показывает. Я понимаю, что это стоит маленькой заметки :)
Сообщение говорит, что в оператор delete всегда передаётся нулевой указатель. Если бы он не всегда был нулевым, то и предупреждения не будет. Это ведь нормально. А вот передача явного nullptr говорит и какой-то ошибке или избыточном коде. Возможно, опечатались и не ту переменную используют. Не очень понятно, что именно сбивает с толку. на мой взгляд вполне понятное предупреждение. Хотя, конечно, у меня может быть взгляд замылен :).
Интерес к C# и Java растёт, но медленно. Думаю, мы сами в этом виноваты, так как команде не хватает сил более активно заниматься их продвижением. Про Java, например, вообще давно статей не писали :(. На тему C++ у нас куда больше разнообразного материала и активностей. В том числе больших и сильных, таких как мини-книга про вредные C++ советы.
MISRA - интерес есть, но у малого количества пользователей. Хорошо, что сделали, но существенно на жизнь это не повлияло. По запросам пользователей скоро будем дорабатывать это направление и делать ещё некоторые диагностики.
C++ остаётся основным направлением? - Да. Оно самое старое, поэтому там больше всего клиентов и поддержки. И с продвижением у нас в этом ловчее получается.
На это полагаться нельзя и даже вредно про это думать. Неизвестно как и где будет выполняться программа. Более того, код может быть заимствован для других проектов, работающих в других условиях. Всегда следует исходить из того, что new может сгенерировать исключение.
P.S. Раньше подобное приходилось про malloc писать, теперь new.... эх...
В данном случае согласен. В более общем, указатель может инициализироваться при одних условиях и оставаться нулевым в других. Программисты знают про это и пишут лишние проверки перед delete (и перед free в C). Я имел в иду, что такие проверки не нужны, так как delete (и free) корректно обработают нулевой указатель.
Да, еще есть new(std::nothrow), но он тут ни при чём.
Просто интересно, а почему не упоминается статический анализ кода? По идее он идёт ещё раньше или параллельно с юнит-тестированием. Или статический анализ относится уже не к команде тестирования? Хотя юнит-тесты, вроде тоже не задача тестировщиков.
Мы так никому не отвечаем. Более того, у нас есть несколько вариантов бесплатного лицензирования для студентов, открытых проектов и т.д. Вы или спутали PVS-Studio с другим анализатором или произошло ещё какое-то недоразумение/непонимание.
Разница в том, что анализатор знает, что такое
malloc
. И к сожалению (пока) не знает, что такоеgetViewObject
. Его можно научить с помощью углубления технологий анализа (межпроцедурный анализ, межмодульный анализ) или за счёт явной аннотации методов.Дело не в том может или не может указатель быть нулевым. Важно, чтобы анализ был полезен. А для этого надо ругаться ТОЛЬКО там, где есть информация/повод.
Прошу прочитать статью "Философия статического анализатора PVS-Studio". Там как раз про это. Опасность нулевого указателя ничем не отличается от опасности сложения двух переменных типа
int
. При сложении может возникнуть переполнение, приводящее к UB. Но ничего хорошего не будет, если ругаться на каждое сложение, если нет уверенности в его безопасности.P.S. Это тогда уж проще на каждую строчку кода на всякий случай ругаться :) "У вас строчка кода на языке C++! Она может содержать ошибку" - совершенное точное предупреждение и совершенно бесполезное :)
В первом случае, анализатор ничего не знает про указатели (межпроцедурный+межмодульный анализ не смог вычислить, есть ли вариант, когда указатель нулевой).
Во втором случае, есть артефакт.
auto oldAnchor = detail->AnchorPoint.getValue();
if (detail) {
Есть разыменование указателя. Есть его последующая проверка. Значит, программист предполагал, что указатель может быть нулевой. Подробнее: Пояснение про диагностику V595.
Если проверки
if ( detail)
не будет и анализатор не сможет вычислить , может лиdetail
быть нулевым, то предупреждения не будет.Вопрос про
malloc
не понятен. Хотя, это не важно. Функцияmalloc
может вернутьNULL
, а значит указатель может быть нулевым (и его надо проверять до использования). Тут и гадать не надо :)Ругаться, на разыменование указателей, которые предварительно не проверены на
nullptr
, просто. К сожалению, это путь в никуда. Если придерживаться логики "ругаться на всё, в чём нет уверенности в безопасности" приведёт к такому количеству срабатываний, что анализатором пользоваться будет невозможно. Мы придерживаемся другой философии, которую я описал в этой статье. Цитата оттуда:Прошу пояснить, какую реакцию Вы ожидаете здесь от анализатора.
Неожиданное продолжение про простую ошибку разыменования нулевого указателя: FreeCAD и C++ код с неопределённым поведением для медитации (схожий случай).
Пришло время повторить проверку: 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
.Вот, например, как узнать что хотели проверить? Указатель или значение по указателю?
Или вот такой реальный пример из проекта Mozilla Firefox:
Если-бы человек был фанатом сокращения текста, он бы написал:
И тогда анализатор 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
В общем, забыли разыменовать указатель. Правильно:
Что вы хотите от синтетического примера? :) Его смысл показать, что предупреждение показалось ложным, а оно не ложное.
Естественно, в реальном приложении, всё это где-то глубоко в потрохах. И утечка памяти самая настоящая. Естественно, реальная программа не заканчивает сразу после этого работу, как в моём примере :).
Нет. Такой простой случай сложно придумать :) Правильнее будет сказать, я его "услышал". Хотя можно без кавычек. Это в прямом смысле.
Слышу вдруг коллега в соседней секции: "Ааааа!" и прочие эмоции. Заинтригован, подхожу. Что такое? Он рассказывает и показывает. Я понимаю, что это стоит маленькой заметки :)
Про V575.
Сообщение говорит, что в оператор
delete
всегда передаётся нулевой указатель. Если бы он не всегда был нулевым, то и предупреждения не будет. Это ведь нормально. А вот передача явногоnullptr
говорит и какой-то ошибке или избыточном коде. Возможно, опечатались и не ту переменную используют. Не очень понятно, что именно сбивает с толку. на мой взгляд вполне понятное предупреждение. Хотя, конечно, у меня может быть взгляд замылен :).Интерес к C# и Java растёт, но медленно. Думаю, мы сами в этом виноваты, так как команде не хватает сил более активно заниматься их продвижением. Про Java, например, вообще давно статей не писали :(. На тему C++ у нас куда больше разнообразного материала и активностей. В том числе больших и сильных, таких как мини-книга про вредные C++ советы.
MISRA - интерес есть, но у малого количества пользователей. Хорошо, что сделали, но существенно на жизнь это не повлияло. По запросам пользователей скоро будем дорабатывать это направление и делать ещё некоторые диагностики.
CVE - как я понимаю, имеется в виду SCA (software composition analysis). Пока не понятно.
C++ остаётся основным направлением? - Да. Оно самое старое, поэтому там больше всего клиентов и поддержки. И с продвижением у нас в этом ловчее получается.
На это полагаться нельзя и даже вредно про это думать. Неизвестно как и где будет выполняться программа. Более того, код может быть заимствован для других проектов, работающих в других условиях. Всегда следует исходить из того, что
new
может сгенерировать исключение.P.S. Раньше подобное приходилось про malloc писать, теперь new.... эх...
В данном случае согласен. В более общем, указатель может инициализироваться при одних условиях и оставаться нулевым в других. Программисты знают про это и пишут лишние проверки перед
delete
(и передfree
в C). Я имел в иду, что такие проверки не нужны, так какdelete
(иfree
) корректно обработают нулевой указатель.Да, еще есть
new(std::nothrow)
, но он тут ни при чём.Просто интересно, а почему не упоминается статический анализ кода? По идее он идёт ещё раньше или параллельно с юнит-тестированием. Или статический анализ относится уже не к команде тестирования? Хотя юнит-тесты, вроде тоже не задача тестировщиков.
Спасибо. Поправил.
Мы так никому не отвечаем. Более того, у нас есть несколько вариантов бесплатного лицензирования для студентов, открытых проектов и т.д. Вы или спутали PVS-Studio с другим анализатором или произошло ещё какое-то недоразумение/непонимание.