Комментарии 194
Два приведенных примера с разыменованием NULL и rand — неопределенное поведение в стандарте. То есть компилятор в полном праве выкинуть их из кода (или заменить на еще какую-нибудь ерунду). Соответственно, на мой взгляд, статический анализатор должен предупреждать об этом, даже если пользователь что-то там имел в виду и написал *NULL сознательно. Просто потому, что пользователь в данном случае не прав.
Ситуация, когда анализатор может быть "немного прав", не выдавая предупреждение — если он точно знает, что в конкретном компиляторе, который используется для сборки кода, данная ситуация является определенной и документированной. И даже в этом случае, у анализатора должна быть уверенность, что это проект не собирается одновременно еще и другим компилятором (скажем, приведенный код находится в #ifdef _MSC_VER блоке), или в следующей версии поведение не изменят. Осуществляет ли PVS-Studio подобные проверки?
Вот что clang делает вместо разыменовывания нулевого указателя, если записать его немного по разному: https://godbolt.org/g/NiJXiL. Замечу, делает на практике, а не в теории. На сколько из этих функций ваш анализатор выдаст предпреждение о неопределенном поведении?
Это — бессмысленный, неверный (в языке С и С++) код, который не должен появляться в программе. Что бы программист не думал, что бы он не писал в комментарии рядом, разыменование нулевого указателя всегда будет ошибкой, до тех пор пока у компилятора нет флага вроде --define-null-pointer-dereference-semantics.
В любом случае это код написан сознательно. Случайно он получиться не может.
Если я вас правильно понял, вы ищете только опечатки, но не ошибки по незнанию или непониманию языка, с которым программист работает. В таком случае, не стоит говорить, что ваш анализатор "в 10 раз лучше" — у вас разные цели и вывод в оригинальной статье абсолютно верный.
Что бы программист не думал, что бы он не писал в комментарии рядом, разыменование нулевого указателя всегда будет ошибкой, до тех пор пока у компилятора нет флага вроде --define-null-pointer-dereference-semantics.Тот факт, что вы не знаете как флаг называется не означает, что его нет. Да, такой флаг, конечно же, есть, только называется он -fno-delete-null-pointer-checks. Обратите, кстати, внимание на то, что на AVR и CR16 (это как раз микроконтроллеры, о которых речь шла выше) эта опция включена по умолчанию.
За ссылку на опцию спасибо, с микроконтроллерами работать не довелось. Но про то, что такого флага нет — я не говорил. Посмотрите, пожалуйста, первое мое сообщение. Я спрашивал, проверяет ли PVS-Studio, что имеет дело с ситуацией, когда поведение в этом случае документировано, или же эвристика глобальная. Судя по ответам Andrey2008 — таких проверок нет.
Я понимаю, что всегда можно придумать теоретический вариант, где анализатор даст сбой и не выдаст полезное срабатывание. Но это не интересно с практической точки зрения. Ещё раз повторю пример с двойными скобками: if ((a = b)). Да, здесь может быть ошибка. Но ругаться здесь не следует. И таким паттернов тьма и хороший анализатор должен их учитывать.
Вероятность такого события стремится к нулю и лучше не ругаться.
Очень спорное утверждение для статического анализатора.
Конечно, ложные срабатывания очень нервируют, но я лучше самостоятельно проверю их чем буду надеятся на эвристику самого анализатора.
Супер-программа
#include <iostream>
уже содержит сотни «подозрительных» мест, которые, формально, нарушают стандарт. И далеко не все они специальным образом помечены.
А как только вы напишите ещё что-то, что реально инстанциирует всю эту машинерию — сразу получите ещё кучу сообщений.
Вы, вообще, свой код хотя бы через все -Wall -Wextra -Wxxx (которых десятки НЕ включены ни в -Wall, ни в -Wextra именно чтобы не заваливать пользователя кучей сообщений) прогоняете? Или ваша любовь к статическим анализаторам сугубо платоническая?
Я удивляюсь как вы не определили цвет обоев в моем офисе, они красные если вам интересно.
Естественно я не хочу что бы анализатор кидал сообщение на каждый чих, но инструмент становится менее предсказуем когда начинает додумывать за программиста в спорных моментах.
Ну уж стандартные хедеры можно и исключать из анализа?
Можно пример? Потому что мне кажется, что у статического анализатора есть достаточный контекст, чтобы понимать когда надо ругаться на стандартные штуки, а когда нет.
Про rand я был не прав, я согласен с вами, что сам тест неверен. Можно придумать (очень извращенный, но корректный) пример его использования в соответствии со стандартом:
void null_pointer_006 ()
{
int *p;
p = (int *)(intptr_t)rand();
*p = 1; /*Tool should detect this line as error*/
/*ERROR:NULL pointer dereference*/
}
void foo(){
int* mem = new int(0);
for ( int i = 0; ; i++ ){
srand(i);
if ( (intptr_t)rand() == (intptr_t)mem ){
srand(i);
null_pointer_006();
}
}
}
Из-за UB оптимизатор имеет право считать, к примеру, содержимое по нулевому адресу неиспользуемым (не специально, а в результате случайного совпадения нескольких эвристик). В итоге код, который должен был по задумке падать с ошибкой — корректно завершится!
В приведенном вами реальном примере эту проблему обошли через volatile
. А код из синтетического теста я бы все же посчитал ошибочным.
UPD: выше об этом уже написали, и со ссылкой. Как я и предполагал, надо обязательно добавлять volatile
(что довольно очевидно) и надо обязательно использовать запись вместо чтения (что совсем не очевидно)
Даже в духе if (sizeof(int) != sizeof(unsigned int))
так они не обязаны быть равны. Где-то sizeof(unsigned int) == 8, sizeof(int) == 4
a char on the target (16 or 32 bits) differs from the size of a byte on the host (8 bits).»
http://www.ti.com/lit/an/spra757/spra757.pdf
6.5.3.4 The sizeof operator
…
3 When applied to an operand that has type char, unsigned char or signed char (or a qualified version thereof) the result is 1.
В C89 аналогично. Так что, даже если размер char 16 бит, не понятно, что должен возвращать sizeof. С одной стороны размер в байтах, с другой для char строго 1.
Но не стоит забывать что первый стандарт вышел в 89м, а, скажем, Turbo C — в 1987м. И был он далеко не первым компилятором и даже не 10м.
For each of the signed integer types, there is a corresponding (but different) unsigned
integer type (designated with the keyword unsigned) that uses the same amount of
storage (including sign information) and has the same alignment requirements.
В стандартах C++ фраза повторена почти дословно (есть пункт про «same amount of
storage»).
Так что это какие-то странные реализации.
В PVS-Studio существует несколько механизмов для устранения ложных срабатываний, которые в любом случае неизбежны:
- Отключение диагностики с помощью настроек или специальных комментариев.
- Изменение настроек некоторых диагностик с помощью комментариев специального вида (про них говорится в описании конкретных диагностик).
- Далее, то о чём Вы говорите — подавление предупреждения в конкретной строке с помощью комментариев.
- Или в макросе (см. там-же). Или в строке кода, содержащую определённую последовательность символов.
Выключать предупреждения или подавлять предупреждения в макросах, можно также используя конфигурационные файлы (см. про pvsconfig).
Отдельно следует выделить систему массовой разметки предупреждений с помощью специальной базы. Это позволяет быстро интегрировать анализатор в процесс разработки больших проектов.
Есть ещё некоторые возможности, но, пожалуй, нет смысл повторять документацию.
Так вот, все это относится к явному указанию, что не считать ошибками. Однако, это не снимает задачу минимизации предупреждений с помощью специальных исключений. Ценность анализатора не в том, что он ругается на всё подряд, а в том, как много он знает ситуаций, когда ругаться не надо.
Примеров, столь много, что надо писать цикл статей на эту тему. Поэтому опишу только одно, из последней диагностики V779 — Недостижимый код.
Исключение:
Не надо ругаться, если блок заканчивается конструкцией 'no_return_statement; return;'. Многие так подавляют предупреждения компилятора. Пример:
int foo()
{
....
exit(1);
return 0;
}
Теоретически здесь надо ругаться, что «return» является недостижимым кодом. Однако этот return появился в коде вынужденно, как борьба с предупреждениями компиляторов или других анализаторов. В коде иногда есть соответствующий комментарий на эту тему. Так вот, никакой практической пользы от предупреждения на такой return нет. Мы проверяем как работают диагностики на 154 открытых проектах. И ни в одном, не было ошибки. Всегда было видно, что это ложное срабатывание. Всего их несколько десятков. Нет смысла оставлять их из-за гипотетической вероятности, что будет пропущена настоящая ошибка. Если бы мы оставляли такие ложные срабатывания, анализатором уже давно бы было невозможно пользоваться.
полностью согласен, х знает чего заминусовали.
похоже на троллинг, но с чем-то согласен)
Класть указатель в int, это не всегда неверно. В теоретические споры давать не буду, но если мы тупо начнём ругаться на преобразование int->pointer и т.п., то количество желающих использовать наш анализатор явно уменьшится :). Надо быть тоньше и умнее. И наш анализатор такой и есть. У него есть ряд диагностик, относящихся в основном к 64-битным проверкам, которые предупреждают о возможной потери старших бит или например о ситуациях вида "Красивая 64-битная ошибка на языке Си".
По поводу разыменования нулевых указателей и опечаток. В наших статьях много и о разыменовании нулевых указателей. Так что речь идёт об однобокости тестов.
P.S. Всех интересующихся проблемой 64-битности отсылаю к своим работам:
- Коллекция примеров 64-битных ошибок в реальных программах
- Уроки разработки 64-битных приложений на языке Си/Си++
Давно их не упоминал. Возможно новичкам будет интересно почитать этот материал.
Этот каст хотя бы обратим.
static const size_type npos = -1;
Так что категорично ругаться на все такие преобразования не стоит точно)
Почему-почему, да потому что Си без плюсов от 89го года! В этом языке любая неизвестная функция считается обладательницей вот такой сигнатуры:
int UnknownFunction(...);
То есть принимает любое число параметров и возвращает int
.
Компилятор должен компилировать старые программы независимо от того сколько лет прошло! Поэтому по умолчанию (без указания новой версии стандарта ключами компилятора) подобная "фича" будет работать для любых программ на Си.
Как думаете, следовало бы избавиться от такого поведения?В какой-то момент — несомненно. Так, GCC больше не требует поддержки фишек, которые C89 выкинул из C. Кажется начиная с версии 5, вышедшей в прошлом году.
Legacy — потому и legacy, что остаётся с нами долгие годы.
Почему
в диагностике V522 реализовано исключение Aно
мы по-прежнему будем работать над настоящими хорошими диагностиками, а не заниматься подгонкой анализатора под тесты
Разве исключение из диагностики это уже не некоторая подгонка под конкретный проект или тест? Понизьте приоритет диагностики с «критического» на «предупреждение», к примеру, с соответствующим комментарием, но не проглатывайте диагностику совсем. Это более правильное решение, на мой взгляд.
И я правильно понимаю, что А6 — это уже шестое исключение из правила? Если так, то ценность диагностик действительно падает, так как случайно (жизнь полна случайностей) может так произойти, что реальная проблема будет «проглочена» исключением, ну например:
void* GetMemoryBlockOrReturnNull( const size_t nSize, void *pStart ) {
void *pStartLocal = NULL;
pStartLocal = pStartLocal + 1; // хотели pStart но IntelliSense подставил локальную переменную
...
return pStartLocal;
}
Я не претендую, что этот пример «сломает» диагностику, но это возможно в определённых ситуациях, как мне кажется.
С типом void *
ни pStartLocal = pStartLocal + 1
, ни *pStartLocal = *pStartLocal + 1
не скомпилируются :)
Разве исключение из диагностики это уже не некоторая подгонка под конкретный проект или тест?
Нет. Это подгонка под определённые принятые паттерны программирования. Например, нигде не написано, что выражение «if ((a = b))» не содержит ошибку. Однако, почти все компиляторы и анализаторы молчат на такой код, так как двойные скобки считаются дефакто подсказкой, что всё хорошо.
Ценность диагностик действительно падает.
Нет, ценность диагностики растёт. Чем больше ложного удается отсечь, тем лучше. Тогда среди мусора не затеряется настоящие ошибки. С++ такой язык, что можно выдавать предупреждение на каждую строчку, и не ошибешься. Вот только толку от такого анализатора не будет.
Объяснить подробно — надо писать целую статью. Будем читать, что я в этом вопросом просто буду давить авторитетом. :) Поверьте, исключения это очень важная и самая ценная часть анализатора. Многие диагностики начитывают десятки исключений.
Про исключения я понимаю вашу позицию, что множество ложных срабатываний снижает уровень доверия к анализатору. Но ведь всегда найдётся реальный пример, когда исключение может скрыть реальную проблему, как бороться с этим? Или Вы просто полагаетесь, что вероятность подобного стечения обстоятельств ничтожно мала и проще скрыть пару тысяч ложных срабатываний, чем отобразить их ради одного реального?
В этом нет ничего ужасного. Полнота выявления ошибок не единственный критерий полезности анализатора. Не менее важен ещё и баланс между полезными и бесполезными сообщениями.
Для тех, кто переживает о потерянном предупреждении хочу напомнить, что с другими технологиями поиска ошибок все обстоит точно также. И точно также приходится идти на компромисс. Возьмем, например, юнит-тесты. Можно пропустить ошибку, не покрыв тестами 100% кода. Но с практической точки зрения разумно остановиться, скажем на 80%. Потому, что покрытия 100% кода столь сложная задача, что на неё уйдёт неразумное количество времени и сил. И даже покрыв 100% кода тестами вновь нет гарантии, что мы проверили все случаи входных данных и можно продолжать усложнять и усложнять тесты.
Аналогично и со статическим анализом. С практической точки зрения лучше рассматривать отчет с 30 ошибками и 50 ложными срабатываниями, чем с 32 ошибками и 500 ложными срабатываниями. Почему? Потому, что рассматривая первый отчёт вы поправите больше ошибок, чем во втором случае. Второй отчет вы будете изучать гораздо более невнимательно, если вообще станете.
Внимательность быстро теряется. Просматривая отчет с большим количеством ложных срабатываний, человек начинает очень невнимательно относиться к предупреждениям и пропускает многие ошибки, помечая их как не ошибки.
Кто-то скажет, но вот я то, буду смотреть все все! Дайте мне все предупреждения!
Нет, не будете. Или покажите мне как у вас уже на 100% код покрыт тестами. Не на 100%? А почему? Там ведь может быть ошибка! :)
P.S. Сделать шумный анализатор (без исключений) очень просто. Однако мы тратим много сил как раз на эти самые исключения, так как точно знаем, что без этого пользоваться анализатором невозможно.
Но есть один момент, который мне, как зануде со стажем, не даёт покоя. А есть ли возможность увидеть эти скрытые предупреждения, принудительно?
Вот из Вашего же примера — первичный отчёт выдал 30 ошибок и 50 ложных предупреждений. Я их все просмотрел, ошибки исправил, ложные добавил в исключения для этого конкретного проекта (а так вообще можно?) и у меня есть время для «we need to go deeper». Я хочу включить более «шумный» вариант анализа, принудительно, понимая, что я получаю в итоге, и этот «шумный вариант» выключен по умолчанию, именно для того, что бы получить те самые, оставшиеся 2 ошибки и 450 ложных срабатываний. Это возможно?
ошибки исправил, ложные добавил в исключения
Где-то в комментариях Andrey2008 уже сделал акцент на том, чтобы не путали исключения анализатора с подавлением ложных срабатываний.
1. Исключения анализатора внутренние, по их разработке и обоснованию проделана большая работа.
2. Если вы видите ложные срабатывания, значит исключения не сработали. Такие срабатывания можно разметить как ложные и больше не видеть. К ложным срабатываниям можно вернуться позже при желании.
Если и после этого у вас осталось желания посмотреть сообщения, которые отвалились как исключения, то я вам просто не поверю, что вы уже исправили все три предыдущих уровня :-)
Если про возможность отключить исключения, то нет.
И смысла нет. Я, например, знаю, как выглядит работа с проектом в 10 млн. строк кода. Поверьте, в таких проектах не до «we need to go deeper». Там даже особенно нет времени для изучения предупреждений 3-его уровня достоверности. Так что желание посмотреть всё-всё, может быть только в маленьких проектах. Но вопрос, а нужен ли там так сильно статический анализатор? Можно устроить code-review? Или поиграться с другими инструментами, раз есть время. :)
вариант с большим числом предупреждений более полезен с практической точки зрения.
Нет.
Если честно, я не ожидал такого всплеска комментариев на тему того, что анализатор может очень редко, но не выдать предупреждение на ошибку из-за реализованных в нём механизмов отсечения ложных срабатываний. Борьба с ложными срабатываниями настолько большая составляющая любого статического анализатора, что как-то даже не понятно, что тут собственно обсуждать. Это надо делать и всё. И естественно такие механизмы существуют не только в нашем анализаторе, но и в других анализаторах/компиляторах.
Вот представите другой сценарий и попробуйте его прочувствовать. Допустим, команда использует нашу утилиту BlameNotifier, которая рассылает по почте предупреждения на тот код, который написал определенный человек. Если ему ничего не приходит или приходит одно-два сообщения, он их смотрит. Если ему придёт в течении недели 4 сообщения, одно из которых будет на настоящую ошибку, то код будет исправлен. Все хорошо.
А теперь представьте, что система каждый день будет присылать ему по 8-10 бессмысленных сообщений. Я уверен, что уже через несколько дней он просто перестанет их внимательно смотреть и начнет размечать на автомате код как безопасный. Вероятность что он пропустит настоящую ошибку увеличивается в несколько раз.
Т.е. большое количество сообщений позволит находить, скажем на 0.5% больше ошибок. Но при этом вероятность, что ошибка будет не замечена и пропущена, увеличивается на 500%. 500% это ещё хорошо. По факту может быть бесконечность, потому что, некоторые разработчики просто перестают вообще обращать внимание на предупреждения.
PARANOID: This line contains code. Please check it.
на каждую не пустую строчку.
(P.S. Вы уже добились 100% покрытия кода юнит-тестами? А почему?)
Если сделать такую галочку, то смотреть результаты при её включении будет нереально. Я знаю, о чем пишу. Никому весь этот мусор не нужен. Мы ведь не монетку бросаем, чтобы решить делать исключение или нет. Мы изучаем поведение диагностики и исключений на 150 проектах и проверяем, чтобы убедить, что не уберем что-то хорошее.
Возьмем скажем диагностику V670. Инициализация членов класса в неправильном порядке. Если в секции инициализации один членов зависит от другого, то необходимо проверить соответствие порядка в секции инициализации, порядку их объявления.
Казалось бы, ну какие здесь исключения? Легко. Ведь понятие «зависит один от другого» понятие растяжимое. Выражения бывают разные. Пример:
class Foo{
X x;
Y y;
Foo(): x(sizeof(y)), y(1) {}
};
В выражении для инициализации 'x' используется ещё неинициализированный член 'y'. Но ошибки нет, так как это мы просто считаем размер члена.
Вы можете получить пользу если выключить исключение? Нет.
Если кто-то возразит, а вдруг там что-то другое хотели написать. Возможно. Но это можно про любую строчку в программе сказать, что «вдруг хотели написать что-то другое». И тут мы возвращаемся к идее идеального анализатора, который просто ругается подряд на все строки в программе. Даже на пустые. Вдруг неправильно, что она пустая. Вдруг там надо было написать a=b. :)
Отключать можно не случаи, как вы только что привели пример, а как в статье — явная грубая ошибка, скорее всего сделанная намеренно. Тут конечно работы поболее, отличать одно от другого.
С точки зрения практики, вероятно вы правы. Но бодаться из принципа (с теми же тестами), если есть другие пути — занятие странное. В существующими эвристиками всё ок, просто не хватает еще одной.
И всё же:
1)Берём и начинаем разрабатывать проект с 0. Включаем в число проверок для PR PVS-Studio. Требуем отсутствия предупреждений — либо чиним, либо обоснованно глушим. Итого проект всего чисто собирается без предупреждений. Для таких условий вполне можно время от времени смотреть, какие еще подозрительные места в коде он бы нашел без данных эвристик.
2) Если часть ваших потенциальных клиентов использует синтетические тесты(Не такой уж странное желание в конце концов, потому как лучше тестов вроде бы нет. Может вы на основе собранных случаев когда нибудь лучше тесты сделаете..), то почему бы вам, если это не слишком сложно, не сделать соответствующий режим для их прохождения? Игнорируемые по умолчанию ошибки можно добавить в отдельный 4ый уровень — предположительно намеренные ошибки. Я говорю сейчас именно про ошибки — UB и т.д.
2. Никому не интересно в реальной жизни как работает инструмент на абстрактном проекте. Всегда интересно как он работает ТОЛЬКО на моем конкретном проекте.
Я сталкивался с обратным — но никогда у практиков, которые могли бы купить подобные инструменты. Только у всегда всем недовольных теоретиков.
Я лишь утверждаю, что выбирать инструмент без тестов не логично. Если инструмент игнорирует тесты — его тяжело оценить. Говорить, что надо только на реальном проекте проверять можно, но хочется знать в принципе на что он способен. Для этого принято использовать тесты.
я не ожидал такого всплеска комментариев на тему того, что анализатор может очень редко, но не выдать предупреждение на ошибку из-за реализованных в нём механизмов отсечения ложных срабатыванийПотому что эти исключения не очевидны для большинства разработчиков и выглядят как «затыкание дыр» на примере нескольких проектов/тестов.
Если бы исключения (а для такого семейства языков, как С/С++ их априори полно) были формализованы в стандарте, или хотя бы в книгах K&R, Стауструпа, Майерса и Ко, считающимися едва ли не обязательными учебными материалами, ни кто бы и слова, я думаю, не сказал Вам.
За более чем восемь лет постоянной практики С++ я ни разу не встречал ни в одном документе, даже в гугловских рекомендациях по оформлению кода описания, что:
if ((a=b)) { ... }
Сигнализирует не об опечатке, а об осмысленном действии. Хотя конструкций вида if((a=b)!=0) видел огромное множество.Нет такого правила, Вы их увидели в нескольких проектах и посчитали общепринятыми… но это не так.
Точно так же, и остальные исключения. Вы обвиняете синтетический тест в том, что он подпадает под Ваши исключения (вычисленные эмпирическим путём и не имеющие прообразов ни в одном стандартном документе) и Вам не кажется это, скажем так, странным?
За более чем восемь лет постоянной практики С++ я ни разу не встречал ни в одном документе, даже в гугловских рекомендациях по оформлению кода описания, что:Круто, чё.
Сигнализирует не об опечатке, а об осмысленном действии.if ((a=b)) { ... }
Если бы исключения (а для такого семейства языков, как С/С++ их априори полно) были формализованы в стандарте, или хотя бы в книгах K&R, Стауструпа, Майерса и Ко, считающимися едва ли не обязательными учебными материалами, ни кто бы и слова, я думаю, не сказал Вам.Я правильно понимаю что нынче можно изучать как работать с микроволновкой не по инструкции к микроволновке, а по телепередачам лучших поваров?
Нет такого правила, Вы их увидели в нескольких проектах и посчитали общепринятыми… но это не так.А я их увидел в документации на компилятор (только не нужно про то, что это какое-то нововведение про которое Майерс ничего не знает: -Wparentheses за последние 10 лет ничуть не изменился).
Обратите, кстати, внимание, на формулировку: warn if parentheses are omitted in certain contexts, such as when there is an assignment in a context where a truth value is expected, а когда это предпреждение срабатывает, то выглядит это как
test.c:3:3: warning: suggest parentheses around assignment used as truth value [-Wparentheses] if (i = 5) {
Нафига вам Майерс, если вам прямо компилятор говорит что и как делать?
Точно так же, и остальные исключения. Вы обвиняете синтетический тест в том, что он подпадает под Ваши исключения (вычисленные эмпирическим путём и не имеющие прообразов ни в одном стандартном документе) и Вам не кажется это, скажем так, странным?А почему это должно казаться странным? Стандарты — это что, священные крижали?
Данное правило в момент выхода GCC 2.95 больше 15 лет назад было уже твёрдо установленной практикой, а кто и когда его ввёл — это ареологов вызывать надо. Было бы странно, если бы PVS-Studio игнорировала бы подобные вещи…
Я правильно понимаю что нынче можно изучать как работать с микроволновкой не по инструкции к микроволновке, а по телепередачам лучших поваров?В итоге то что, шашечки или ехать? То читаем инструкцию, то «кому инструкции нужны».
…
Стандарты — это что, священные крижали?
А я их увидел в документации на компилятор… -Wparentheses за последние 10 лет ничуть не изменился).
Не вижу там про двойные скобки ни слова.
if ( a = b ) { // тут PVS выдаст предупреждение
...
if (( a = b )) { // а тут промолчит
В каком документе описывается подобное и при чём тут -Wparentheses?Но получается, что эта техника заставляет проглотить предупреждения для GCC, возможно для clang.GCC, clang, coverity, PVC-Studio и много кого ещё. Даже в линтерах таких языков как JavaScript эта конструкция поддерживается (вот тут, например — conditionalAssign). Я же сказал — чтобы выяснить когда и кто ввёл это правило впервые археологи нужны будут. Не удивлюсь, если окажется что и оригинальный lint это умел
То читаем инструкцию, то «кому инструкции нужны».Никто не читает инструкции™ — и это нормально.
Ненормально — рассказывать сказки про свою дотошность и готовность изучать тысячи ложных сообщений обшибках. Особенно на фоне того, что при всей своей постулируемой супердотошности выясняется что за восемь лет вы так и не удосужились прочитать не то, что полную инструкцию к компилятору, а, я извиняюсь, краткую справку
man gcc
.Извините, но… не верю™.
if ( ( a = b ) != 0 ) { ...
a = b;
if (a) {
...
В с++17 можно будет писать
if (a = b; a) { ...
Выход за границы массива, в общем случае, анализатором проверить невозможно. И пихать кучу проверок 99% программистов перед обращением не будут. С другой стороны, те же 99% программистов будут использовать vector, а не массив и ошибка будет найдена в рантайме. Тут стоит учесть, что обычно запустить программу и словить сегфолт или ещё что-нибудь оказывается гораздо быстрее, чем ждать окончание работы анализатора. Зато опечатки, копипасты и ряд других ошибок простым запуск-падением обнаружить невозможно.
В общем, пользуюсь PVS и радуюсь, что кроме ложных срабатываний почти ничего не находит. Значит, не сильно говнокодю.)
В общем, пользуюсь PVS и радуюсь, что кроме ложных срабатываний почти ничего не находит. Значит, не сильно говнокодю.)Вы не поверите, но в «говнокоде» анализатор тоже не найдёт ошибок. Они там не синтаксические, а логические :)
Я бы хотел, чтобы описанные ошибки всё-таки PVS обнаруживал.
Как я сказал, подточим, но без фанатизма. Rand() уже поддержали, даже научили анализатор доставать правильную константу RAND_MAX из заголовочных файлов.
Однако, это не отменяет что тест плох и ориентироваться на него я не хочу. Я ведь только чуть-чуть покритиковал, но у меня есть и другие претензии к нему.
С другой стороны, те же 99% программистов будут использовать vector, а не массив и ошибка будет найдена в рантайме.
Стандарт C++ гарантирует, что лишь
vector::at
(насколько часто он встречается?) выбросит исключение (пруф). В случае же с vector::operator []
будет обычный UB.Решение — регулярно гонять Subspider и следить за тем, чтобы он не сильно «проседал» в новых версиях.
То же самое и разработчики PVS-Studio должны делать. А куда деваться?
Just sayin'.
Анализатор должен просить в начале его разметить? Вписать подсказки?
Я надеюсь, мы сейчас о реальной ситуации говорим, а не синтезируем ее спора ради (вроде только что статью прочитали о вреде этого). Так вот в реальной ситуации анализатор кода даст на 10 миллионах строк кода, ну, положим, 250к варнингов. Из них, положим, 1% ложно-положительных — тех, которые можно было бы угадать, но мы предпочтем дать юзерам самим решать, проблема это или так и предполагалось. С оставшимися 99% — 247500 варнингами — все равно ведь что-то придется сделать (если конечно вся эта катавасия с анализом делалась не для красивого отчета совету директоров). Если из 100 варнингов 99 вы поправите путем исправления бага, а оставшуюся одну проблему пометите (ПОСЛЕ прогона анализатора, а не до, разумеется) правильным тегом «такинадо» — производительность труда от этого не упадет, а потенциально-проблемное место в коде будет гарантировано прокомментировано, что проблема известна и более того специально тут сидит. Чтобы следующий разработчик на проекте, например, не «почистил» ее от большого ума.
Только учитывайте, что перед нами не стоит задача найти как можно больше ошибок. Задача — найти достаточно, чтобы написать статью.
Вы же сами рассказывали про чудесный механизм маскировки срабатываний, позволяющий замаскировать все старое и сообщать только о новых подозрительных местах.
А вообще — да, надо размечать все старые костыли. Хотя бы для того чтобы самим не забыть.
Поглядите, например, на инструменты типа Frama-C, на ANSI/ISO C Specification Language.
И пишут, и аннотируют — надо, значит надо. Ничего страшного в этом нет.
void overrun_st_014 ()
{
int buf[5];
int index;
index = rand();
buf[index] = 1; /*Tool should detect this line as error*/
/*ERROR: buffer overrun */
sink = buf[idx];
}
Пожалуй, такое можно встретить разве только в лабораторных работах студентов.
да ладно?
а если такое:
int gradient[GRADIENT_WIDTH];
...
int index;
index = rand() % GRADIENT_WIDTH;
из-за опечатки превратилось в:
int gradient[GRADIENT_WIDTH];
...
int index;
index = rand();
Это тоже только в лабораторных у студентов?
Вы лучше чем ругать чужие наборы тестов тесты, опубликовали бы свои. И не просто так, а как открытый проект, куда другие могут слать пул-реквесты. Ну и раз в три месяца прогонять оценку имеющихся на рынке анализаторов.
Примерно так в проекте FrameworkBenchmarks происходит с веб-фреймворками.
И откуда такая третья сторона возьмётся? Ну оплатит, например, Coverity кому-то эту работу и что дальше? И ведь даже если кто-то действительно "независимый" (опять же, личную предвзятость никто не отменял) этим займётся, то точно так же будут спекуляции, что мол проплатили.
Я, конечно, понимаю, что для вас создание такой базы — это дополнительная работа (особенно в плане дальнейшей поддержки), причём с непонятным профитом. Хотя польза вполне могла бы быть: как минимум, можно козырять тем, что подход более основательный, чем у описанных в статье тестов.
Всё равно каждый мимо проходящий будет упрекать нас в том, что мы создали такую базу, на которой лучше всего показываем себя. И непонятно как оспорить.
Предлагать этим мимопроходящим добавить свои тесты.
"в 10 раз лучше" https://www.youtube.com/watch?v=FkdTW4QaDiQ
Андрей, при всем уважении, считаю что про null-указатель вы неправы. Давайте разберемся.
Бывают ситуации, когда аналогичный код пишут специально, чтобы добиться возникновения исключения при разыменовании нулевого указателя.
Бывает, что такое выражение пишут специально, а бывает, что не специально. Вы желали облегчить жизнь тем разработчикам, которые пишут его специально и дать возможность игнорировать его, поэтому придумали критерий, согласно которому эта проверка глушится. Но давайте посмотрим на критерий:
Разыменование переменной находится в функции, в названии которой есть одно из слов:
error, default, crash, null, test, violation, throw, exception
Вы действительно считаете надежным критерием для пропуска проверки наличие, например, слова default в названии функции? Нет же, вы просто где-то встречали код, где разыменовывание было в функции с таким словом в названии. Значит ли это, что разыменовывания в во всех функциях, где в названии есть слово default можно считать безопасными? Да нет конечно, с какой стати. То, что вы называете достаточно умной проверкой, я бы назвал достаточно сломанной проверкой.
На мой взгляд, в данном случае может быть только один надежный критерий отключения этой проверки: когда программист говорит: «эй, анализатор, я знаю, что тут разыменовывание нулевого указателя и это точно не ошибка». Я не знаком с вашим продуктом (хотя регулярно читаю ваши посты), но думаю у вас есть механизм общения программиста с анализатором через комментарии в коде. Вот его нужно оставить, а проверку на «слово в названии функции» выкинуть.
T *x = 0;
*x = 0;
Выглядит не естественным. Поймите, что не делают таких ошибок. Если присваивание разнесено по телу функции, то да, это может быть ошибка. И мы найдём такой случай. Но нельзя ошибиться, разыменовывая указатель на следующей строке.
Впрочем, мы даже на такое готовы и в качестве подстраховки дополнительно анализируем ещё и название функции.
Меня можно переубедить, только приведя реальный пример, вот такой ошибки где сразу присвоили и разменивали, хотя не хотели этого делать. И чтоб имя функции при этом было, скажем crash. Вот только нет такого примера в природе.
то есть в случае
int* aaa = 0;
int* aaaa = b;
*aaa = 5;
ошибка НЕ будет подавлена же!
Поправка: понял, что пропустил дополнительное условие «При этом, переменной присваивается 0 строчкой выше». Так намного лучше для «надежного критерия», но все равно не понимаю, зачем в списке default.
Призываю в тред: alexdorofeeff facet npoechelon :)
— Сделать опцию — ок.
— Сделать опцию, включенную по-умолчанию — ок.
— Запретить проведение более полного анализа — не ок.
Любое допущение, основанное на мнении человека, имеет вероятность оказаться неверным. Значит, в данном случае, имеется вероятность пропустить ошибку. А цена ошибки может быть очень большой.
Будет крайне неприятно, если, например, какой-нибудь спутник сойдёт с орбиты из-за того, что в названии одной из функций попалось слово, которое заставило чекер молча проглотить ошибку.
Вот примерно отсюда и начинаются большие проекты…
100 человек, работающих над одним проектом, без всякого разделения на относительно слабосвязанные компоненты?Ну разумеется речь не идёт о том, что все 100 человек работают с одним файлом в миллион строк. Какое-то разделение есть.
У них проблемы начнутся сильно раньше, чем через год, миллиона строк не получится.Вот прямо даже так? Вот вам один такой проект, другой, третий… Тысячи их!
Да и какой-нибудь Android за счёт repo к этому приближается.
Речь ведь не идёт о том, чтобы в миллионе строк не было никакого порядка, а о том, чтобы всё это вместе было одним проектом. Где вам не нужно было бы получать согласования на пяти страницах на изменение API другого компонента. Сделали topic, меняющий API в 100500 файлах и 1050 подпроектах, залили, поехали дальше. Вот там инструменты типа PVS-Studio — востребованы как воздух. Так как там люди часто вносят изменения в код, о котором они буквально не знают ничего (ну кроме того, что этот код каким-то боком вызвывает их API).
вот пример: https://godbolt.org/g/6NMDe6
Если есть перегруженный оператор сравнения, уже не найдет.
А про nulll pointer, возможно вам стоит добавить еще один уровень предупреждений, которые по вашему мнению не интересны пользователям, но находят проблемы.
Ну ладно, простые случаи он может находить с недавнего времени. Что дальше? Мы должны отказаться от выявления таких ситуаций? Или такие проверки надо удалять из тестов для оценки качества анализаторов?
Тесты в этом репозитории только для простых случаев. А ваш анализатор не только для простых.
Gcc также может многое, что могли бы находить анализаторы, на пример ошибки форматной строки, такие как переполнение буфера назначения. Или забытые скобочки, на основе анализа форматирования кода…
Я не противопоставляю gcc и pvs studio. Просто разработчики редко даже включают и проверяют предупреждения компилятора, не то что использует статические анализаторы. Если бы все исправляли все предупреждения компилятора, то и вы бы находили меньше проблем :)
Анализатор PVS-Studio понимает, что этот код написан сознательно и никакой ошибки здесь нет.Святая обязанность анализатора — предупреждать, а не понимать. Ниже — подробнее.
«Понимают» люди, но они могут не понимать, с каким клиентом имеют дело.
Что, если я — богатенький новичок, который просто пишет что попало и хочет получить все возможные пинки?
Что, если я — тестер-критик, как Билл?
Что, если я — опытный программист, опытный, разумно-ленивый и уставший, по запарке начинающий нести чушь в коде и желающий эту чушь увидеть быстро? А анализатор — молчит…
Что, если я пишу сложный генератор кода, качество которого нужно быстро проверять?
Что, если я — учитель и понадеялся в образовательном процессе на предупредительность анализатора? Ту самую, которой внезапно не оказалось…
Что, если я — «хреновый» набор тестов?
И так далее…
Этой статьёй вы теряете несколько потенциальных клиентов, а могли бы «приобрести».
Этой статьёй вы теряете несколько потенциальных клиентов, а могли бы «приобрести».Вряд ли. Люди, которые поднимают тут шум и отчаянно «спорят о вкусе устриц и кокосовых орехов с теми, кто их ел» мало интересны кому-либо, кроме них самих. Хотя их вопли кого-то могут, в принципе, отпугнуть. Сама статья — вряд ли…
Если бы статья писалась в ключе «Теперь мы ловим даже самые сумасшедшие ошибки!», то лучше было бы всем. А от негативного субъективизма хорошо ещё никому не было.
Мне нравится, что делает PVS-Studio, (и особенно тот факт, что они не жадничают и за упоминание в комментариях готовы предоставить бесплатную лицензию некоммерческим проектам. Большое человеческое спасибо им!), недаром также подписан на посты; но…
Вот конкретно такой подход печалит. Я хочу, чтобы компания цвела и пахла, а не жаловалась на «хреновые тесты». И это я процитировал. Автор в статье так прямо и говорит, что тестовая база — хрень. И о том, какой же слабый CPPCheck и какой сильный PVS, вместо того, чтобы чуть допилить анализатор. Как говорится, «спасись сам — и вокруг тебя спасутся многие».
Гораздо конструктивнее было бы просто добавить фичу и предлагать её тем, кто в ней нуждается, а не объяснять, почему такой фичи нет… Я вот об этом.
адекватных клиентовКонечно, вопрос адекватности довольно важен, что касается консультативной поддержки, (если она включена в лицензию), но в общем и целом один из основных принципов: «Кто платит, тот и музыку заказывает». Звучит довольно цинично, но так и есть… Какая разница, что представляет и не представляет себе клиент, если у него есть деньги и он готов заплатить?
Какая разница, что представляет и не представляет себе клиент, если у него есть деньги и он готов заплатить?Самая прямая и очень серьёзная. Количество попросов в техподдержку очень негативно коррелирует с адекватностью, а готовность платить, наоборот — позитивно. Или, по рабоче-крестьянски: один дурак может задать столько вопросов что и сто мудрецов не смогут на них ответить.
А поскольку компании, почему-то, интересуют не просто деньги, а прибыль, то вопрос адекватности клиентов — очень и очень важен.
Думаю, эти вопросы решаются ещё на стадии заключения договора, когда разработчик сразу оговаривает, какие вопросы он решает и какие нет (что далеко ходить, примером — правила хабра). Так отсекается большое число неадекватных запросов. Хотя, конечно, всё равно тратит время службы поддержки и её ресурс.
P.S. Быть может, Вас ещё заинтересует такой вариант лицензирования: Как использовать PVS-Studio бесплатно.
Причем, PVS-Studio тут ни причем. То, что я написал относится к любому статическому, будь то Cppcheck или Coverity. Интегрируйте их в проект, размером в 4-5 миллиона строк кода для регулярного использования, а потом можно продолжить дискуссию о необходимости добавки. :)
Не понимаю, зачем «взрослому анализатору» (как где-то здесь было написано) жаловаться на детей и пинать их вместо того, чтобы просто сделать функцию и к ней кнопку «вкл/выкл». Хочешь — пользуешься, не хочешь, не нужно — не пользуйся. Да, это работа. Но и да, это ваша работа. И да, спасибо вам за работу.
С тем, чтобы в описании дать ссылку на пояснение своей позиции по этому вопросу. Ну или собрать отзывы от тех, кто найдёт «синтетические» ошибки у себя в проекте (if any).
А у вас тут есть шанс достучаться до потенциального клиента, который пытается делать выбор в условиях неполноты информации. И вы можете показать ему, что знаете, что он сейчас пытается сделать и как он может ошибиться в результате.
Если рассмотреть такую возможность, то я бы добавил кнопку «Мне нечем заняться. Покажи что-нибудь». И показывал порционно, и только на проектах, где других сообщений не осталось.
Программист, IT-шник, как привило — control freak.
И тут ему говорят, что решение за него принимает не формально верифицируемый алгоритм, а эмпирический. Как же можно! Контроль отобрали и лазеек не оставили!
Мой коммент — не очень серьёзая попытка адресовать эту проблему, предложить морковку на верёвочке…
И тут ему говорят, что решение за него принимает не формально верифицируемый алгоритм, а эмпирический. Как же можно! Контроль отобрали и лазеек не оставили!Ну прямо как Google и Яндекс. Ваш «сферический IT-шник» не плачет в подушку по ночам оттого, что Alta Vista умерла?
Статический анализатор — это, собственно, от начала до конца «эмпирический неверефицируемый алгоритм». Если вам такие не подходят — значит вам, собственно, статический анализатор и не нужен. Либо вы пишите идельный код (да, такие люди встречаются — но редко), либо (что гораздо чаще) — вы реально ни разу ни один анализатор на уровне «параноика» не запускали и с соответствующими сообщениями не боролись.
Я понимаю и разделяю позицию Andrey2008 и khim. Но, видимо, из моих комментариев сложилось другое впечатление.
Мне интересны причины, почему не удалось донести позицию до значительного числа читателей, и что с этим можно сделать.
Очевидно, что не все могут переварить предложенное объяснение, пока не имеют соответствующего опыта. Апеллировать к авторскому опыту не слишком помогает. Отсюда и зациклившееся обсуждение. Либо надо искать другие способы донесения своего опыта, дать почувствовать себя на месте авторов, либо искать обходные манёвры.
кнопку «Мне нечем заняться. Покажи что-нибудь»
… которая просто вызывает начальника :-)
Продолжение про Toyota ITC спустя несколько лет: Что там у PVS-Studio c покрытием Toyota ITC Benchmark?
Почему я не люблю синтетические тесты