Мне не нравится, когда кто-то пытается использовать созданные вручную примеры кода для оценки возможностей статического анализатора кода. Сейчас на конкретном примере я продемонстрирую, почему негативно отношусь к синтетическим тестам.
Не так давно Bill Torpey написал в своем блоге заметку "Even Mo' Static", где рассказал, как, на его взгляд, показали себя инструменты Cppcheck и PVS-Studio при анализе проекта itc-benchmarks. Проект itc-benchmarks — это static analysis benchmarks from Toyota ITC.
Мне не понравилось, что после прочтения статьи создается впечатление, что анализаторы Cppcheck и PVS-Studio приблизительно равны в своих возможностях. Из статьи следует, что один анализатор показывает себя лучше в одном, второй в другом, но в целом их диагностические возможности похожи.
Я думаю, что это не так. Мое мнение — наш анализатор PVS-Studio в несколько раз мощнее, чем Cppcheck. И вообще, это не «мнение», я знаю это!
Однако, раз со стороны не видно, что PVS-Studio в 10 раз лучше Cppcheck, то надо попытаться понять причину. Я решил посмотреть на этот самый itc-benchmarks и разобраться, почему PVS-Studio показал себя на этой тестовой базе не лучшим образом.
Чем дальше я разбирался, тем большее раздражение я испытывал. А один пример совсем вывел меня из равновесия, и о нем я расскажу чуть ниже. Мои выводы такие: у меня нет претензий к Bill Torpey. Он написал хорошую, честную статью. Спасибо, Bill. Зато у меня есть претензии к Toyota ITC. Мое личное мнение: их тестовая база — хрень. Это, конечно, громкое заявление, но я считаю, что имею достаточную квалификацию и опыт, чтобы рассуждать о статических анализаторах кода и способах их оценки. По моему мнению, itc-benchmarks не может быть использован для адекватной оценки возможностей того или иного анализатора.
А вот собственно тест, который окончательно вывел меня из душевного равновесия.
Это тест на разыменование нулевого указателя:
void null_pointer_001 ()
{
int *p = NULL;
*p = 1; /*Tool should detect this line as error*/
/*ERROR:NULL pointer dereference*/
}Анализатор Cppcheck заявляет, что нашел ошибку в этом коде:
Null pointer dereference: pАнализатор PVS-Studio на этом коде молчит, хотя для таких случаев в нём существует диагностика V522.
Так что же получается, PVS-Studio на этом примере слабее чем Cppcheck? Нет, он как раз сильнее!
Анализатор PVS-Studio понимает, что этот код написан сознательно и никакой ошибки здесь нет.
Бывают ситуации, когда аналогичный код пишут специально, чтобы добиться возникновения исключения при разыменовании нулевого указателя. Такое можно встретить в тестах или в специфических участках кода. Подобный код мы встречали неоднократно. Вот, например, как такое может выглядеть в реальном проекте:
void GpuChildThread::OnCrash() {
LOG(INFO) << "GPU: Simulating GPU crash";
// Good bye, cruel world.
volatile int* it_s_the_end_of_the_world_as_we_know_it = NULL;
*it_s_the_end_of_the_world_as_we_know_it = 0xdead;
}Поэтому в анализаторе PVS-Studio в диагностике V522 реализовано несколько исключений, чтобы как раз не ругаться на такой код. Анализатор видит, что null_pointer_001 является ненастоящей функцией. Не бывает в реальном коде ошибок в функциях, когда ноль записывают в указатель и сразу его разыменовывают. Да и имя функции говорит анализатору, что «нулевой указатель» здесь неспроста.
Для подобных случаев, в диагностике V522 реализовано исключение A6. Под него же попадает и синтетическая функция null_pointer_001. Вот как опасно исключение A6:
Разыменование переменной находится в функции, в названии которой есть одно из слов:
- error
- default
- crash
- null
- test
- violation
- throw
- exception
При этом, переменной присваивается 0 строчкой выше.
Синтетический тест полностью подошел под это исключение. Во-первых, в названии функции есть слово «null». Во-вторых, присваивание нуля переменной происходит как раз на предыдущей строке. Исключение выявило ненастоящий код. И код действительно не настоящий, это синтетический тест.
Вот из-за подобных нюансов я и не люблю синтетические тесты!
У меня есть к itc-benchmarks и другие претензии. Например, всё в том же файле, мы можем видеть вот такой тест:
void null_pointer_006 ()
{
int *p;
p = (int *)(intptr_t)rand();
*p = 1; /*Tool should detect this line as error*/
/*ERROR:NULL pointer dereference*/
}Функция rand может вернуть 0, который затем превратится в NULL. Анализатор PVS-Studio пока не знает, что может вернуть rand и поэтому не видит в этом коде ничего подозрительного.
Я попросил коллег научить анализатор лучше понимать, что из себя представляет функция rand. Деваться некуда, придётся подточить напильником анализатор, чтобы он лучше себя показывал на рассматриваемой тестовой базе. Это вынужденная мера, раз для оценки анализаторов используют подобные наборы тестов.
Но не бойтесь. Я заявляю, что мы по-прежнему будем работать над настоящими хорошими диагностиками, а не заниматься подгонкой анализатора под тесты. Пожалуй, мы немного подретушируем PVS-Studio для itc-benchmarks, но в фоновом режиме и только в тех местах, которые имеют хоть какой-то смысл.
Я хочу, чтобы разработчики понимали, что пример с rand ничего на самом деле не оценивает. Это синтетический тест, который высосан из пальца. Не пишут так программы. Нет таких ошибок.
Кстати, если функция rand вернёт не 0, а 1400 лучше не будет. Все равно такой указатель разыменовывать нельзя. Так что разыменование нулевого указателя какой-то странный частный случай совершенно некорректного кода, который просто был выдуман и который не встречается в реальных программах.
Мне известны настоящие программистские беды. Например, это опечатки, которые мы выявляем сотнями, скажем, с помощью диагностики V501. Что интересно, в itc-benchmarks я не заметил ни одного теста, где проверялось, может ли анализатор обнаружить опечатку вида «if (a.x == a.x)». Ни одного теста!
Таким образом, itc-benchmarks игнорирует возможности анализаторов по поиску опечаток. А читатели наших статей знают, насколько это распространенные ошибки. Зато содержит, на мой взгляд, дурацкие тестовые кейсы, которые в реальных программах не встречаются. Я не могу представить, что в настоящем серьезном проекте можно повстречать вот такой код, который приводит к выходу за границу массива:
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];
}Пожалуй, такое можно встретить разве только в лабораторных работах студентов.
При этом я знаю, что в серьезном проекте легко встретить опечатку вида:
return (!strcmp (a->v.val_vms_delta.lbl1,
b->v.val_vms_delta.lbl1)
&& !strcmp (a->v.val_vms_delta.lbl1,
b->v.val_vms_delta.lbl1));Эту ошибку анализатор PVS-Studio нашел в коде компилятора GCC. Два раза сравниваются одни и те же строки.
Получается, что тесты на поиск экзотического кода с rand есть, а на классические опечатки нет.
Я могу продолжать и дальше, но, пожалуй, остановлюсь. Я выговорился и мне стало легче. Спасибо за внимание. Теперь у меня есть статья-ответ, с моим мнением о синтетических базах ошибок.
Предлагаю всем установить и попробовать мощнейший анализатор кода PVS-Studio.
Дополнительные ссылки:
- Диагностические возможности PVS-Studio.
- База реальных ошибок, найденных с помощью PVS-Studio в открытых проектах.
- Мифы о статическом анализе. Миф пятый – можно составить маленькую программу, чтобы оценить инструмент.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Why I Dislike Synthetic Tests
Комментарии (193)
ggrnd0
06.02.2017 12:06-10На мой взгляд, в null_pointer_006 анализатор должен ругаться в первую очередь на приведение int -> intptr_t
Все таки intptr_t является указателем, в то время как int — это просто говно, в котором может быть все что угодно.
Любые преобразования говно -> указатель и обратно, независимо от реального типа должны детектироваться как минимум как подозрительное место...
Кстати, если функция rand вернёт не 0, а 4400 лучше не будет.
Рад что вы наконец это поняли, я крайне удивлен, что у вас до сих пор нет такой диагностики…
Казалось бы, каст типов данных сам по себе небезопасен, тем более с указателями...
А читатели наших статей знают, насколько это распространенные ошибки.
Да, большей частью вы их и публикуете, и я начинаю догадываться почему в их сторону такой перевес...
overrun_st_014
Похоже мои догадки верны…
К черту rand(), там может быть произвольная функция произвольной библиотеки.
Как может анализатор c/c++ пропустить проверку нарушения границ массива?!
Да, судя по всему ваш анализатор хорошо ищет опечатки.
И это полезно. Например, в дополнение к CppCheck, потому что segfault вы не найдете...
Andrey2008
06.02.2017 12:45+7Комментарий в духе «Не пробовал, но не одобряю».
Класть указатель в int, это не всегда неверно. В теоретические споры давать не буду, но если мы тупо начнём ругаться на преобразование int->pointer и т.п., то количество желающих использовать наш анализатор явно уменьшится :). Надо быть тоньше и умнее. И наш анализатор такой и есть. У него есть ряд диагностик, относящихся в основном к 64-битным проверкам, которые предупреждают о возможной потери старших бит или например о ситуациях вида "Красивая 64-битная ошибка на языке Си".
По поводу разыменования нулевых указателей и опечаток. В наших статьях много и о разыменовании нулевых указателей. Так что речь идёт об однобокости тестов.ggrnd0
06.02.2017 13:25+2Использование int вместо size_t, указателей и прочего, не более чем наследие.
Согласен, что такого кода полно, и многим просто лень переписывать свой код.
Однако как минимум предупреждать все же следует.
Вы (компания) сами признаете, что многие ваши проверки генерируют ложные срабатывания, и даже разделили их по уровням критичности и в каждой статье об этом упоминаете…
Так что генерация еще большего количества ложных срабатываний не может быть оправданием.
Несмотря на большое количество работающего кода, подобное приведение не является правильным.
Andrey2008
06.02.2017 14:41+1Предупреждаем, не бойтесь. Мы с 64-битных проблем начинали. Все что нужно, там есть. Я хотел сказать, что ругаться надо разумно. Ведь в 32-битной программе size_t может быть объявлен через unsigned int. А значит всё хорошо.
P.S. Всех интересующихся проблемой 64-битности отсылаю к своим работам:
- Коллекция примеров 64-битных ошибок в реальных программах
- Уроки разработки 64-битных приложений на языке Си/Си++
Давно их не упоминал. Возможно новичкам будет интересно почитать этот материал.ggrnd0
06.02.2017 14:58+1Ведь в 32-битной программе size_t может быть объявлен через unsigned int. А значит всё хорошо.
Это вы сейчас про каст signed <-> unsigned типов говорите "А значит все хорошо"???
Andrey2008
06.02.2017 15:06Ой, ну давайте посоветуйте искать предобразования int <-> unsigned int. Всем кто думает, что это хорошая идея, предлагаю создать свой собственный анализатор с подобными правилами и попробовать его продать. :)
Satus
07.02.2017 14:40+1Справедливости ради, много проектов с которыми я работал, включают это предупреждение, даже те, в которых warnings = errors. И всегда это оказывается к лучшему. А не смешивать разные типы int'ов привыкнуть несложно.
Ddnn
06.02.2017 17:52+2Интересный факт: std::basic_string::npos — получается методом signed -> unsigned преобразования. Из cppreference:
static const size_type npos = -1;
Так что категорично ругаться на все такие преобразования не стоит точно)Satus
07.02.2017 14:58+1Вот только в стандартной библиотеке явно приводят один тип к другому.
// STATIC npos OBJECT template<class _Elem, class _Traits, class _Alloc> _PGLOBAL const typename basic_string<_Elem, _Traits, _Alloc>::size_type basic_string<_Elem, _Traits, _Alloc>::npos = (typename basic_string<_Elem, _Traits, _Alloc>::size_type)(-1);
А когда просто используют неявное преобразование, то это может быть и ошибкой.
ggrnd0
06.02.2017 13:43-5По вашей ссылке.
Не знаю, почему код компилируется, или же почему компилятор неожиданно думает, что там будет int?
Наверно это legacy от которого почему то до сих пор не избавились, несмотря на всякие с++0x с++11 с++14 и прочие, не так?
Я читал с чего ваша компания началась, как раз с 64битных ошибок.
И вполне логично что вы просто забыли о поддержке анализа legacy кода…
Но неужели за 8 лет, вы впервые столкнулись с такой проблемой?
На каждом углу, адепты высокоурневых языков кричат, что массивы, указатели и, не дай бог, арифметика указателей в c/c++ небезопасны...mayorovp
06.02.2017 13:58+3Почему-почему, да потому что Си без плюсов от 89го года! В этом языке любая неизвестная функция считается обладательницей вот такой сигнатуры:
int UnknownFunction(...);
То есть принимает любое число параметров и возвращает
int.ggrnd0
06.02.2017 14:08-3Это попадает под мой ответ "legacy"?
Как думаете, следовало бы избавиться от такого поведения?mayorovp
06.02.2017 14:22+3Компилятор должен компилировать старые программы независимо от того сколько лет прошло! Поэтому по умолчанию (без указания новой версии стандарта ключами компилятора) подобная "фича" будет работать для любых программ на Си.
ggrnd0
06.02.2017 14:27-4Да, это как раз legacy.
Не должен он, ничего он не должен.
Это пользователи не хотят код переписывать ("работает — не трожь"), вот и требуют этого от компилятора.
Так как ожидать от компилятора, что он будет сам анализировать подобный код нельзя, так как пользователи наврядли изменятся, то это должны делать анализаторы.
В любом случае, любое действие должно контролироваться.
Если действие не контролируется копилятором, то кроме анализатора уже ничто не спасет...
khim
06.02.2017 19:17Как думаете, следовало бы избавиться от такого поведения?
В какой-то момент — несомненно. Так, GCC больше не требует поддержки фишек, которые C89 выкинул из C. Кажется начиная с версии 5, вышедшей в прошлом году.
Legacy — потому и legacy, что остаётся с нами долгие годы.
Tujh
06.02.2017 12:54+2После прочтения у меня возник вопрос…
Почемув диагностике V522 реализовано исключение A
номы по-прежнему будем работать над настоящими хорошими диагностиками, а не заниматься подгонкой анализатора под тесты
Разве исключение из диагностики это уже не некоторая подгонка под конкретный проект или тест? Понизьте приоритет диагностики с «критического» на «предупреждение», к примеру, с соответствующим комментарием, но не проглатывайте диагностику совсем. Это более правильное решение, на мой взгляд.
И я правильно понимаю, что А6 — это уже шестое исключение из правила? Если так, то ценность диагностик действительно падает, так как случайно (жизнь полна случайностей) может так произойти, что реальная проблема будет «проглочена» исключением, ну например:
void* GetMemoryBlockOrReturnNull( const size_t nSize, void *pStart ) { void *pStartLocal = NULL; pStartLocal = pStartLocal + 1; // хотели pStart но IntelliSense подставил локальную переменную ... return pStartLocal; }
Я не претендую, что этот пример «сломает» диагностику, но это возможно в определённых ситуациях, как мне кажется.mayorovp
06.02.2017 13:05С типом
void *ниpStartLocal = pStartLocal + 1, ни*pStartLocal = *pStartLocal + 1не скомпилируются :)
Andrey2008
06.02.2017 14:50+4Разве исключение из диагностики это уже не некоторая подгонка под конкретный проект или тест?
Нет. Это подгонка под определённые принятые паттерны программирования. Например, нигде не написано, что выражение «if ((a = b))» не содержит ошибку. Однако, почти все компиляторы и анализаторы молчат на такой код, так как двойные скобки считаются дефакто подсказкой, что всё хорошо.
Ценность диагностик действительно падает.
Нет, ценность диагностики растёт. Чем больше ложного удается отсечь, тем лучше. Тогда среди мусора не затеряется настоящие ошибки. С++ такой язык, что можно выдавать предупреждение на каждую строчку, и не ошибешься. Вот только толку от такого анализатора не будет.
Объяснить подробно — надо писать целую статью. Будем читать, что я в этом вопросом просто буду давить авторитетом. :) Поверьте, исключения это очень важная и самая ценная часть анализатора. Многие диагностики начитывают десятки исключений.Tujh
07.02.2017 08:29+1Про паттерн из двойных скобок не знал, интересно, спасибо.
Про исключения я понимаю вашу позицию, что множество ложных срабатываний снижает уровень доверия к анализатору. Но ведь всегда найдётся реальный пример, когда исключение может скрыть реальную проблему, как бороться с этим? Или Вы просто полагаетесь, что вероятность подобного стечения обстоятельств ничтожно мала и проще скрыть пару тысяч ложных срабатываний, чем отобразить их ради одного реального?Andrey2008
07.02.2017 09:20+5Лучше потерять 1 полезное сообщение, чем при этом показать за компанию ещё 1000.
В этом нет ничего ужасного. Полнота выявления ошибок не единственный критерий полезности анализатора. Не менее важен ещё и баланс между полезными и бесполезными сообщениями.
Для тех, кто переживает о потерянном предупреждении хочу напомнить, что с другими технологиями поиска ошибок все обстоит точно также. И точно также приходится идти на компромисс. Возьмем, например, юнит-тесты. Можно пропустить ошибку, не покрыв тестами 100% кода. Но с практической точки зрения разумно остановиться, скажем на 80%. Потому, что покрытия 100% кода столь сложная задача, что на неё уйдёт неразумное количество времени и сил. И даже покрыв 100% кода тестами вновь нет гарантии, что мы проверили все случаи входных данных и можно продолжать усложнять и усложнять тесты.
Аналогично и со статическим анализом. С практической точки зрения лучше рассматривать отчет с 30 ошибками и 50 ложными срабатываниями, чем с 32 ошибками и 500 ложными срабатываниями. Почему? Потому, что рассматривая первый отчёт вы поправите больше ошибок, чем во втором случае. Второй отчет вы будете изучать гораздо более невнимательно, если вообще станете.
Внимательность быстро теряется. Просматривая отчет с большим количеством ложных срабатываний, человек начинает очень невнимательно относиться к предупреждениям и пропускает многие ошибки, помечая их как не ошибки.
Кто-то скажет, но вот я то, буду смотреть все все! Дайте мне все предупреждения!
Нет, не будете. Или покажите мне как у вас уже на 100% код покрыт тестами. Не на 100%? А почему? Там ведь может быть ошибка! :)
P.S. Сделать шумный анализатор (без исключений) очень просто. Однако мы тратим много сил как раз на эти самые исключения, так как точно знаем, что без этого пользоваться анализатором невозможно.Tujh
07.02.2017 11:48+3Я понял Вашу точку зрения и в целом согласен.
Но есть один момент, который мне, как зануде со стажем, не даёт покоя. А есть ли возможность увидеть эти скрытые предупреждения, принудительно?
Вот из Вашего же примера — первичный отчёт выдал 30 ошибок и 50 ложных предупреждений. Я их все просмотрел, ошибки исправил, ложные добавил в исключения для этого конкретного проекта (а так вообще можно?) и у меня есть время для «we need to go deeper». Я хочу включить более «шумный» вариант анализа, принудительно, понимая, что я получаю в итоге, и этот «шумный вариант» выключен по умолчанию, именно для того, что бы получить те самые, оставшиеся 2 ошибки и 450 ложных срабатываний. Это возможно?
SvyatoslavMC
07.02.2017 12:05ошибки исправил, ложные добавил в исключения
Где-то в комментариях Andrey2008 уже сделал акцент на том, чтобы не путали исключения анализатора с подавлением ложных срабатываний.
1. Исключения анализатора внутренние, по их разработке и обоснованию проделана большая работа.
2. Если вы видите ложные срабатывания, значит исключения не сработали. Такие срабатывания можно разметить как ложные и больше не видеть. К ложным срабатываниям можно вернуться позже при желании.Tujh
07.02.2017 14:25я говорил не о ложных срабатываниях, а о реальных, но скрытых из-за исключений.
EvgeniyRyzhkov
07.02.2017 15:47+3Если у вас есть понимание, что статический анализатор полезный инструмент — то исправьте все сообщения уровня High (самые крутые). Если вам кажется, что это у вас получилось очень легко и принесло пользу — исправьте теперь сообщения уровня Medium. Наконец, если вы вдруг еще готовы работать и дальше — то исправьте сообщения уровня Low.
Если и после этого у вас осталось желания посмотреть сообщения, которые отвалились как исключения, то я вам просто не поверю, что вы уже исправили все три предыдущих уровня :-)Tujh
07.02.2017 16:42+1А теперь представьте такую ситуацию, что этим занимается специально выделенный для этого разработчик. Он не пишет продукт, он тестирует и исправляет ошибки. DevOps с уклоном в разработку, скажем.
EvgeniyRyzhkov
07.02.2017 16:43+1Вопрос — у него работы нет что-ли? Он реально исправил все три уровня сообщений и хочет ЧЕГО-ТО ЕЩЕ поправить?
Это какое-то теоретизирование. Нет смысла НАСТОЛЬКО увлекаться этим процессом. Всякие 80/20 никто не отменял.
Andrey2008
07.02.2017 12:07Если Вы про базу разметки, то Святослав уже ответил.
Если про возможность отключить исключения, то нет.
И смысла нет. Я, например, знаю, как выглядит работа с проектом в 10 млн. строк кода. Поверьте, в таких проектах не до «we need to go deeper». Там даже особенно нет времени для изучения предупреждений 3-его уровня достоверности. Так что желание посмотреть всё-всё, может быть только в маленьких проектах. Но вопрос, а нужен ли там так сильно статический анализатор? Можно устроить code-review? Или поиграться с другими инструментами, раз есть время. :)
HighPredator
07.02.2017 12:00+1Может быть я вашу мысль немного не так понял, но я как разработчик получу в рассмотрение не 30 ошибок и 50 ложных срабатываний (или 32 ошибки и 500 ложными срабатываниями), а условно 3 ошибки и 6 ложных срабатываний. Или же 4 ошибки и 60 ложных срабатываний во втором случае. Как и каждый из разработчиков в команде. Мне представляется маловероятным, что задача на фикс проблем, найденных анализатором, не будет распределена по команде. И в этом контексте вариант с большим числом предупреждений более полезен с практической точки зрения.
Andrey2008
07.02.2017 12:02+2вариант с большим числом предупреждений более полезен с практической точки зрения.
Нет.
Если честно, я не ожидал такого всплеска комментариев на тему того, что анализатор может очень редко, но не выдать предупреждение на ошибку из-за реализованных в нём механизмов отсечения ложных срабатываний. Борьба с ложными срабатываниями настолько большая составляющая любого статического анализатора, что как-то даже не понятно, что тут собственно обсуждать. Это надо делать и всё. И естественно такие механизмы существуют не только в нашем анализаторе, но и в других анализаторах/компиляторах.
Вот представите другой сценарий и попробуйте его прочувствовать. Допустим, команда использует нашу утилиту BlameNotifier, которая рассылает по почте предупреждения на тот код, который написал определенный человек. Если ему ничего не приходит или приходит одно-два сообщения, он их смотрит. Если ему придёт в течении недели 4 сообщения, одно из которых будет на настоящую ошибку, то код будет исправлен. Все хорошо.
А теперь представьте, что система каждый день будет присылать ему по 8-10 бессмысленных сообщений. Я уверен, что уже через несколько дней он просто перестанет их внимательно смотреть и начнет размечать на автомате код как безопасный. Вероятность что он пропустит настоящую ошибку увеличивается в несколько раз.
Т.е. большое количество сообщений позволит находить, скажем на 0.5% больше ошибок. Но при этом вероятность, что ошибка будет не замечена и пропущена, увеличивается на 500%. 500% это ещё хорошо. По факту может быть бесконечность, потому что, некоторые разработчики просто перестают вообще обращать внимание на предупреждения.splav_asv
07.02.2017 14:06+1Бурю эмоций вызывает нежелание сделать галочку для отключения этих эвристик. Даже если вашим реальным заказчикам это не нужно, наверняка это не так уж и сложно. Можно ее очень глубоко спрятать, но параноиков это успокоит. Какой никакой, а репутационный плюс от этой фичи точно будет.
datacompboy
07.02.2017 14:26+4Я думаю нужна кнопка, включающая генерацию:
PARANOID: This line contains code. Please check it.
на каждую не пустую строчку.
Andrey2008
07.02.2017 15:24+6Весь мной многолетний опыт подсказывает, что это галочка не имеет отношение к реальности. Прежде чем думать о таком, можно победить предупреждения 3-его уровня. Многие с ними не работают и это нормально, так как трудозатраты от их просмотр значительны. Собственно, по умолчанию, мы 3 уровень отключаем.
(P.S. Вы уже добились 100% покрытия кода юнит-тестами? А почему?)
Если сделать такую галочку, то смотреть результаты при её включении будет нереально. Я знаю, о чем пишу. Никому весь этот мусор не нужен. Мы ведь не монетку бросаем, чтобы решить делать исключение или нет. Мы изучаем поведение диагностики и исключений на 150 проектах и проверяем, чтобы убедить, что не уберем что-то хорошее.
Возьмем скажем диагностику V670. Инициализация членов класса в неправильном порядке. Если в секции инициализации один членов зависит от другого, то необходимо проверить соответствие порядка в секции инициализации, порядку их объявления.
Казалось бы, ну какие здесь исключения? Легко. Ведь понятие «зависит один от другого» понятие растяжимое. Выражения бывают разные. Пример:
class Foo{ X x; Y y; Foo(): x(sizeof(y)), y(1) {} };
В выражении для инициализации 'x' используется ещё неинициализированный член 'y'. Но ошибки нет, так как это мы просто считаем размер члена.
Вы можете получить пользу если выключить исключение? Нет.
Если кто-то возразит, а вдруг там что-то другое хотели написать. Возможно. Но это можно про любую строчку в программе сказать, что «вдруг хотели написать что-то другое». И тут мы возвращаемся к идее идеального анализатора, который просто ругается подряд на все строки в программе. Даже на пустые. Вдруг неправильно, что она пустая. Вдруг там надо было написать a=b. :)splav_asv
07.02.2017 17:15Зато имея такой режим, его можно включать для синтетических тестов. Можно автоматически (как тут уже ниже вспоминали VW) при превышении некоторой плотности ошибок, отсекаемых вашим эвристическим фильтром. Или писать сообщение вида: «Возможно, вы запустили анализатор на синтетическом тесте. Если вы хотите видеть все ошибки, сделайте следующее...»
Отключать можно не случаи, как вы только что привели пример, а как в статье — явная грубая ошибка, скорее всего сделанная намеренно. Тут конечно работы поболее, отличать одно от другого.
С точки зрения практики, вероятно вы правы. Но бодаться из принципа (с теми же тестами), если есть другие пути — занятие странное. В существующими эвристиками всё ок, просто не хватает еще одной.EvgeniyRyzhkov
08.02.2017 08:49+4Берем PC-lint. Запускаем на «hello world». Получаем 100 сообщений анализатора. И что, радуемся?
splav_asv
08.02.2017 09:17-5Простите, но не вижу связи между моим комментарием и вашим ответом на него.
И всё же:
1)Берём и начинаем разрабатывать проект с 0. Включаем в число проверок для PR PVS-Studio. Требуем отсутствия предупреждений — либо чиним, либо обоснованно глушим. Итого проект всего чисто собирается без предупреждений. Для таких условий вполне можно время от времени смотреть, какие еще подозрительные места в коде он бы нашел без данных эвристик.
2) Если часть ваших потенциальных клиентов использует синтетические тесты(Не такой уж странное желание в конце концов, потому как лучше тестов вроде бы нет. Может вы на основе собранных случаев когда нибудь лучше тесты сделаете..), то почему бы вам, если это не слишком сложно, не сделать соответствующий режим для их прохождения? Игнорируемые по умолчанию ошибки можно добавить в отдельный 4ый уровень — предположительно намеренные ошибки. Я говорю сейчас именно про ошибки — UB и т.д.EvgeniyRyzhkov
08.02.2017 09:24+41. Проекты с нуля не разрабатываются. Они всегда идут на основе какой-то кодовой базы, чаще всего большой.
2. Никому не интересно в реальной жизни как работает инструмент на абстрактном проекте. Всегда интересно как он работает ТОЛЬКО на моем конкретном проекте.splav_asv
08.02.2017 09:25Вы в это верите. Я сталкивался с обратным. Ваше право.
khim
08.02.2017 13:14Не подскажите — какая это была фирма и сколько человек на ней работало примерно? Хотя бы порядок — тысяча человек, десять тысяч?
Я сталкивался с обратным — но никогда у практиков, которые могли бы купить подобные инструменты. Только у всегда всем недовольных теоретиков.
splav_asv
08.02.2017 13:49Фирма мало известная, практики, но область инженерно-научная. Сотрудников около 500.
Я лишь утверждаю, что выбирать инструмент без тестов не логично. Если инструмент игнорирует тесты — его тяжело оценить. Говорить, что надо только на реальном проекте проверять можно, но хочется знать в принципе на что он способен. Для этого принято использовать тесты.
Tujh
07.02.2017 14:47-6я не ожидал такого всплеска комментариев на тему того, что анализатор может очень редко, но не выдать предупреждение на ошибку из-за реализованных в нём механизмов отсечения ложных срабатываний
Потому что эти исключения не очевидны для большинства разработчиков и выглядят как «затыкание дыр» на примере нескольких проектов/тестов.
Если бы исключения (а для такого семейства языков, как С/С++ их априори полно) были формализованы в стандарте, или хотя бы в книгах K&R, Стауструпа, Майерса и Ко, считающимися едва ли не обязательными учебными материалами, ни кто бы и слова, я думаю, не сказал Вам.
За более чем восемь лет постоянной практики С++ я ни разу не встречал ни в одном документе, даже в гугловских рекомендациях по оформлению кода описания, что:
Сигнализирует не об опечатке, а об осмысленном действии. Хотя конструкций вида if((a=b)!=0) видел огромное множество.if ((a=b)) { ... }
Нет такого правила, Вы их увидели в нескольких проектах и посчитали общепринятыми… но это не так.
Точно так же, и остальные исключения. Вы обвиняете синтетический тест в том, что он подпадает под Ваши исключения (вычисленные эмпирическим путём и не имеющие прообразов ни в одном стандартном документе) и Вам не кажется это, скажем так, странным?khim
07.02.2017 21:42+4За более чем восемь лет постоянной практики С++ я ни разу не встречал ни в одном документе, даже в гугловских рекомендациях по оформлению кода описания, что:
Круто, чё.
Сигнализирует не об опечатке, а об осмысленном действии.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 игнорировала бы подобные вещи…Tujh
08.02.2017 11:09-3Я правильно понимаю что нынче можно изучать как работать с микроволновкой не по инструкции к микроволновке, а по телепередачам лучших поваров?
В итоге то что, шашечки или ехать? То читаем инструкцию, то «кому инструкции нужны».
…
Стандарты — это что, священные крижали?
А я их увидел в документации на компилятор… -Wparentheses за последние 10 лет ничуть не изменился).
Не вижу там про двойные скобки ни слова.
В каком документе описывается подобное и при чём тут -Wparentheses?if ( a = b ) { // тут PVS выдаст предупреждение ... if (( a = b )) { // а тут промолчитTujh
08.02.2017 11:37ОК, про «Wparentheses» я понял, согласен, протупил. Но получается, что эта техника заставляет проглотить предупреждения для GCC, возможно для clang.
khim
08.02.2017 13:23+1Но получается, что эта техника заставляет проглотить предупреждения для GCC, возможно для clang.
GCC, clang, coverity, PVC-Studio и много кого ещё. Даже в линтерах таких языков как JavaScript эта конструкция поддерживается (вот тут, например — conditionalAssign). Я же сказал — чтобы выяснить когда и кто ввёл это правило впервые археологи нужны будут. Не удивлюсь, если окажется что и оригинальный lint это умел
khim
08.02.2017 13:29+4То читаем инструкцию, то «кому инструкции нужны».
Никто не читает инструкции™ — и это нормально.
Ненормально — рассказывать сказки про свою дотошность и готовность изучать тысячи ложных сообщений обшибках. Особенно на фоне того, что при всей своей постулируемой супердотошности выясняется что за восемь лет вы так и не удосужились прочитать не то, что полную инструкцию к компилятору, а, я извиняюсь, краткую справкуman gcc.
Извините, но… не верю™.Tujh
08.02.2017 15:41Не путайте тёплое с мягким. Даже по описанию из справки не сразу очевидно применение двойных скобок. И, как я уже писал, на тех проектах, что я видел/участвовал, писали более очевидную запись:
if ( ( a = b ) != 0 ) { ...
Antervis
09.02.2017 06:39это субъективно. Лично для меня (и я точно не один такой) true и != 0 настолько тождественны, что ваш вариант ничуть не очевиднее, даже наоборот: видя двойные скобки, я сразу понимаю, что они там не просто так, а вот != 0 мой мозг старательно игнорирует. А в погоне за очевидностью можно писать
a = b; if (a) { ...
В с++17 можно будет писатьif (a = b; a) { ...
maaGames
06.02.2017 13:02+4Я бы хотел, чтобы описанные ошибки всё-таки PVS обнаруживал. Сейчас есть очень много ложных срабатываний и ещё парочка не помешает, зато и «рейтинг» анализатора повысит и даст нерадивому программисту лишний повод задуматься, действительно ли он написал именно то, что хотел?
Выход за границы массива, в общем случае, анализатором проверить невозможно. И пихать кучу проверок 99% программистов перед обращением не будут. С другой стороны, те же 99% программистов будут использовать vector, а не массив и ошибка будет найдена в рантайме. Тут стоит учесть, что обычно запустить программу и словить сегфолт или ещё что-нибудь оказывается гораздо быстрее, чем ждать окончание работы анализатора. Зато опечатки, копипасты и ряд других ошибок простым запуск-падением обнаружить невозможно.
В общем, пользуюсь PVS и радуюсь, что кроме ложных срабатываний почти ничего не находит. Значит, не сильно говнокодю.)Tujh
06.02.2017 13:12+2В общем, пользуюсь PVS и радуюсь, что кроме ложных срабатываний почти ничего не находит. Значит, не сильно говнокодю.)
Вы не поверите, но в «говнокоде» анализатор тоже не найдёт ошибок. Они там не синтаксические, а логические :)maaGames
06.02.2017 13:15Не совсем так. Некоторые логические ошибки он тоже неплохо находит, а вот алгоритмических, надеюсь, никогда находить не будет. А то можно сразу выкидывать программиста и нанимать статический анализатор.)
Andrey2008
06.02.2017 14:54+2Я бы хотел, чтобы описанные ошибки всё-таки PVS обнаруживал.
Как я сказал, подточим, но без фанатизма. Rand() уже поддержали, даже научили анализатор доставать правильную константу RAND_MAX из заголовочных файлов.
Однако, это не отменяет что тест плох и ориентироваться на него я не хочу. Я ведь только чуть-чуть покритиковал, но у меня есть и другие претензии к нему.
BekzhanKassenov
06.02.2017 20:04С другой стороны, те же 99% программистов будут использовать vector, а не массив и ошибка будет найдена в рантайме.
Стандарт C++ гарантирует, что лишьvector::at(насколько часто он встречается?) выбросит исключение (пруф). В случае же сvector::operator []будет обычный UB.mayorovp
07.02.2017 08:45С ключом _DEBUG стандартная библиотека от Microsoft проверяет выход за границы в обоих случаях. Вот в релизе — да, там проверки из
operator []пропадают.
maaGames
07.02.2017 09:11-1Про дебаг уже написали, а использование [], вместо at — преждевременная оптимизация.
vadim_ig
06.02.2017 13:44+15А мне кажется, что тут скорее ваш провтык. Продукт нужно разрабатывать не только с прицелом на качественную работу, но и на продажи, т.е. он должен уметь показать себя в тот момент, когда клиент выбирает. Наверняка было известно, что тестовые базы существуют, и уж совсем ничего удивительного в том, что их используют для быстрой проверки анализаторов. Очень круто было бы, если бы анализатор шел со включенными по умолчаниями предупреждениями об откровенной синтетике, причем в явном виде. Т.е. с сообщениями вида «Разыменование нулевого указателя (Это что, синтетический тест?)». Если человек сделат выбор в вашу пользу, он найдет, как отключить лишний шум, а вот не увидевший нужной функции будет потерян навсегда.
khim
06.02.2017 19:25+1Вы абсолютно правы — такое везде происходит. Я общался с разработчиками V8, например — у них та же беда: Subspider уже настолько устарел, что многие решения, ускоряющие реальные программы замедляют его искуственные тестовые примеры.
Решение — регулярно гонять Subspider и следить за тем, чтобы он не сильно «проседал» в новых версиях.
То же самое и разработчики PVS-Studio должны делать. А куда деваться?
raacer
07.02.2017 15:17+1И вообще, надо брать пример с дизельгейта. Пусть параноидальный режим автоматически включается на тестовых проектах.
EaE
06.02.2017 13:54+4Из параллельной вселенной тулзов для оценки покрытия кода тестами: в спорных ситуациях, когда человек «явно имел в виду намеренное использование проблемы» (например, когда в коде есть ветка «если винда, то делай А, если не винда, то делай Б») — просто помечается соответствующая строчка понятным проверяющей тулзе тегом в комментарии (вида " /* code_coverage_ignore */ "). Это позволяет избежать игры в угадайку с одной стороны, и раздражения от тонны мешающих варнингов с другой стороны.
Just sayin'.Andrey2008
06.02.2017 14:58+1И? Предположим есть проект на 10 миллионов строк кода. И код такой, какой есть. Анализатор должен просить в начале его разметить? Вписать подсказки? Ну хорошо, предположим программисты скучают от безделья и готовы это делать: куда и что им вписывать? Это не серьезно, анализатор должен работать сразу. И игра в угадайку неизбежна.
EaE
06.02.2017 15:07+3Анализатор должен просить в начале его разметить? Вписать подсказки?
Я надеюсь, мы сейчас о реальной ситуации говорим, а не синтезируем ее спора ради (вроде только что статью прочитали о вреде этого). Так вот в реальной ситуации анализатор кода даст на 10 миллионах строк кода, ну, положим, 250к варнингов. Из них, положим, 1% ложно-положительных — тех, которые можно было бы угадать, но мы предпочтем дать юзерам самим решать, проблема это или так и предполагалось. С оставшимися 99% — 247500 варнингами — все равно ведь что-то придется сделать (если конечно вся эта катавасия с анализом делалась не для красивого отчета совету директоров). Если из 100 варнингов 99 вы поправите путем исправления бага, а оставшуюся одну проблему пометите (ПОСЛЕ прогона анализатора, а не до, разумеется) правильным тегом «такинадо» — производительность труда от этого не упадет, а потенциально-проблемное место в коде будет гарантировано прокомментировано, что проблема известна и более того специально тут сидит. Чтобы следующий разработчик на проекте, например, не «почистил» ее от большого ума.Andrey2008
06.02.2017 15:12По необходимость подсказок и разных разметок это понятно. У нас масса механизмов на эту тему. Но это не отменяет необходимости гадания на кофейной гуще. Так как если не гадать, сообщений будет скажем не 250k а, 400k.
EaE
06.02.2017 15:15Ну, я же и не говорю, что это плохой механизм, просто он в статье подан как… единственно правильный, что ли. Кстати, вот вы разрабатываете этот инструмент, наверняка же гоняли его на реальных непуганых кодебазах и владеете статистикой: сколько там вылезает в среднем проблем на тот же миллион строк кода? И сколько из них — угадываемые псевдо-проблемы? Без подвоха спрашиваю, любопытно просто, для общего образования.
Andrey2008
06.02.2017 15:22Статистику не ведем. Могу предложить походить по нашим статьям. В некоторых из них приведены какие-то числа. Например, предыдущая статья: https://habrahabr.ru/company/pvs-studio/blog/320778/
Только учитывайте, что перед нами не стоит задача найти как можно больше ошибок. Задача — найти достаточно, чтобы написать статью.
mayorovp
06.02.2017 15:27+2Вы же сами рассказывали про чудесный механизм маскировки срабатываний, позволяющий замаскировать все старое и сообщать только о новых подозрительных местах.
А вообще — да, надо размечать все старые костыли. Хотя бы для того чтобы самим не забыть.
iUser
07.02.2017 06:13+1Вы, вероятно, удивитесь, на что готовы люди, которым реально необходимо верифицировать код :)
Поглядите, например, на инструменты типа Frama-C, на ANSI/ISO C Specification Language.
И пишут, и аннотируют — надо, значит надо. Ничего страшного в этом нет.
AllexIn
06.02.2017 14:03+2void 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();
Это тоже только в лабораторных у студентов?Andrey2008
06.02.2017 15:01Как я уже писал выше, поддержали полноценно rand(). Такая ошибка теперь найдется. Но это не отменяет моего мнения, что подобный код мало реален.
kekekeks
06.02.2017 14:23+12Вы лучше чем ругать чужие наборы тестов тесты, опубликовали бы свои. И не просто так, а как открытый проект, куда другие могут слать пул-реквесты. Ну и раз в три месяца прогонять оценку имеющихся на рынке анализаторов.
Примерно так в проекте FrameworkBenchmarks происходит с веб-фреймворками.
Andrey2008
06.02.2017 15:02+6Бессмысленно. Всё равно каждый мимо проходящий будет упрекать нас в том, что мы создали такую базу, на которой лучше всего показываем себя. И непонятно как оспорить. Так что разработкой тестов в любом случае должна заниматься третья сторона. Только тогда можно говорить о какой-то адекватности и честности.
DarkEld3r
06.02.2017 15:27+1И откуда такая третья сторона возьмётся? Ну оплатит, например, Coverity кому-то эту работу и что дальше? И ведь даже если кто-то действительно "независимый" (опять же, личную предвзятость никто не отменял) этим займётся, то точно так же будут спекуляции, что мол проплатили.
Я, конечно, понимаю, что для вас создание такой базы — это дополнительная работа (особенно в плане дальнейшей поддержки), причём с непонятным профитом. Хотя польза вполне могла бы быть: как минимум, можно козырять тем, что подход более основательный, чем у описанных в статье тестов.
kekekeks
06.02.2017 17:15+14Всё равно каждый мимо проходящий будет упрекать нас в том, что мы создали такую базу, на которой лучше всего показываем себя. И непонятно как оспорить.
Предлагать этим мимопроходящим добавить свои тесты.
staticlab
07.02.2017 10:36Но ведь они добавят такие же синтетические тесты, как те, которые описаны в посте — которые игнорируются PVS-Studio.
mayorovp
07.02.2017 11:46+1Это решается правилом — "любой тест должен быть куском кода из реального проекта с указанием источника".
Критика набора синтетических тестов целиком и правда смотрится как отмазки — но синтетический тест в набор из тестов, основанных на реальном коде довольно просто.
misha_shar53
06.02.2017 14:52+2Хотел попробовать вашу студию в Linux. Но оказывается нужна лицезия. Средствами не распологаю.
SvyatoslavMC
06.02.2017 14:56+2Предлагаю попробовать :-) Как использовать PVS-Studio бесплатно
misha_shar53
06.02.2017 17:48-1Обязательно попробую в ближайшее время. Требования считаю вполне обоснованными. Мне надо еще разобраться как все это присобачить к NetBeans.
olekl
06.02.2017 16:55Подтверждаю, в продакшн коде встречал «принудительный крэш» с записыванием числа в нулевой указатель… Хоть и не сторонник такого кода…
homm
06.02.2017 17:29+3Андрей, при всем уважении, считаю что про null-указатель вы неправы. Давайте разберемся.
Бывают ситуации, когда аналогичный код пишут специально, чтобы добиться возникновения исключения при разыменовании нулевого указателя.
Бывает, что такое выражение пишут специально, а бывает, что не специально. Вы желали облегчить жизнь тем разработчикам, которые пишут его специально и дать возможность игнорировать его, поэтому придумали критерий, согласно которому эта проверка глушится. Но давайте посмотрим на критерий:
Разыменование переменной находится в функции, в названии которой есть одно из слов:
error, default, crash, null, test, violation, throw, exceptionВы действительно считаете надежным критерием для пропуска проверки наличие, например, слова default в названии функции? Нет же, вы просто где-то встречали код, где разыменовывание было в функции с таким словом в названии. Значит ли это, что разыменовывания в во всех функциях, где в названии есть слово default можно считать безопасными? Да нет конечно, с какой стати. То, что вы называете достаточно умной проверкой, я бы назвал достаточно сломанной проверкой.
На мой взгляд, в данном случае может быть только один надежный критерий отключения этой проверки: когда программист говорит: «эй, анализатор, я знаю, что тут разыменовывание нулевого указателя и это точно не ошибка». Я не знаком с вашим продуктом (хотя регулярно читаю ваши посты), но думаю у вас есть механизм общения программиста с анализатором через комментарии в коде. Вот его нужно оставить, а проверку на «слово в названии функции» выкинуть.
Andrey2008
06.02.2017 17:39+3Прошу посмотреть описание в статье более подробно. Критерием является присутствие в названии слова И странный код. Под странным кодом понимается, когда СРАЗУ присваивают NULL и тут-же его разыменовывают. Уже самое написание:
T *x = 0; *x = 0;
Выглядит не естественным. Поймите, что не делают таких ошибок. Если присваивание разнесено по телу функции, то да, это может быть ошибка. И мы найдём такой случай. Но нельзя ошибиться, разыменовывая указатель на следующей строке.
Впрочем, мы даже на такое готовы и в качестве подстраховки дополнительно анализируем ещё и название функции.
Меня можно переубедить, только приведя реальный пример, вот такой ошибки где сразу присвоили и разменивали, хотя не хотели этого делать. И чтоб имя функции при этом было, скажем crash. Вот только нет такого примера в природе.
datacompboy
06.02.2017 17:40+3но ведь написано же, что это ОДИН из критериев. исключение, когда ВСЕ истина. То есть И имя с подсказкой, И нулевой прямо на предыдущей строке.
то есть в случае
int* aaa = 0;
int* aaaa = b;
*aaa = 5;
ошибка НЕ будет подавлена же!
homm
06.02.2017 17:40+2Поправка: понял, что пропустил дополнительное условие «При этом, переменной присваивается 0 строчкой выше». Так намного лучше для «надежного критерия», но все равно не понимаю, зачем в списке default.
Andrey2008
06.02.2017 17:44Дефолтные обработчики. Встречается в программах, активно работающими с указателями на функции. По умолчанию, указателю присваивается не NULL, а указатель на дефолтную функцию. И если её случайно позвать, то пишется отчет в лог и разыменовывается нулевой указатель, чтобы упасть.
nothern_wind
06.02.2017 17:53Почему бы в анализатор не добавить функцию по поиску деверсий или саботажа в коде. ПРосто эта фишка с рандомом и указателем достаточно не плохой способ нагадить перед увольнением или если заказчик кинул на денежку.
Andrey2008
06.02.2017 17:56Для этого есть специальный класс инструментов. Здесь мне стоит сделать отсылку к Эшелону.
Призываю в тред: alexdorofeeff facet npoechelon :)
Lauren
06.02.2017 19:07+3Насколько я понял дискуссию в комментариях, в pvs не хватает опции отключения эмпирической обработки. Если пользователь хочет найти шлак, пусть ищет, его право.
Andrey2008
06.02.2017 20:02Для некоторых диагностик у нас имеются специализированные настройки. Однако, этот тот случай, когда такая настройка не нужна, так как и нет реальных ошибок, которые могут быть пропущены из-за исключения A6. А нереальные, настоящим пользователям не нужны. Впрочем, если кто-то из клиентов напишет и попросит, мы сделаем такую настройку. Я уверен, что такого запроса не будет.
Tiendil
08.02.2017 09:16-1Всё-таки ограничение поведения программы частным (в смысле не общим/универсальным) подходом на основе субъективного мнения (а объективного тут быть не может) — не правильно. Не соответствует оно лучшим практикам.
— Сделать опцию — ок.
— Сделать опцию, включенную по-умолчанию — ок.
— Запретить проведение более полного анализа — не ок.
Любое допущение, основанное на мнении человека, имеет вероятность оказаться неверным. Значит, в данном случае, имеется вероятность пропустить ошибку. А цена ошибки может быть очень большой.
Будет крайне неприятно, если, например, какой-нибудь спутник сойдёт с орбиты из-за того, что в названии одной из функций попалось слово, которое заставило чекер молча проглотить ошибку.EvgeniyRyzhkov
08.02.2017 09:25Будет крайне неприятно, если анализатор выдал ошибку, но из-за того, что он еще 100 сообщений рядом выдал, эту ошибку не заметили.
Andrey2008
08.02.2017 09:27+1Мы по кругу пошли. Нас не слышат, так как никогда не использовали подобные инструменты в больших проектах. Думаю стоит закругляться с дискуссиями по это теме.
Tiendil
08.02.2017 09:32Большие — это какие?
khim
08.02.2017 13:34+2Достаточно большие для того, чтобы быть интересными PVS-Studio как потенциальные клиенты. Можно посчитать. Грубо. 100 строк в день, скажем, 100 человек работает и, пусть они проработают год и ? всего, что они наработают «протухнет». В любом случае у нас останется миллион строк кода.
Вот примерно отсюда и начинаются большие проекты…
0xd34df00d
15.02.2017 19:27100 человек, работающих над одним проектом, без всякого разделения на относительно слабосвязанные компоненты? У них проблемы начнутся сильно раньше, чем через год, миллиона строк не получится.
khim
15.02.2017 22:00100 человек, работающих над одним проектом, без всякого разделения на относительно слабосвязанные компоненты?
Ну разумеется речь не идёт о том, что все 100 человек работают с одним файлом в миллион строк. Какое-то разделение есть.
У них проблемы начнутся сильно раньше, чем через год, миллиона строк не получится.
Вот прямо даже так? Вот вам один такой проект, другой, третий… Тысячи их!
Да и какой-нибудь Android за счёт repo к этому приближается.
Речь ведь не идёт о том, чтобы в миллионе строк не было никакого порядка, а о том, чтобы всё это вместе было одним проектом. Где вам не нужно было бы получать согласования на пяти страницах на изменение API другого компонента. Сделали topic, меняющий API в 100500 файлах и 1050 подпроектах, залили, поехали дальше. Вот там инструменты типа PVS-Studio — востребованы как воздух. Так как там люди часто вносят изменения в код, о котором они буквально не знают ничего (ну кроме того, что этот код каким-то боком вызвывает их API).
Tiendil
08.02.2017 09:32-1Именно поэтому это делать надо опцией + возможность отключить каждое срабатывание на уровне код. Большинству не надо, а параноики спасибо скажут.
4144
06.02.2017 21:19Случай a.x == a.x может найти обычный компилятор
вот пример: https://godbolt.org/g/6NMDe6
Если есть перегруженный оператор сравнения, уже не найдет.Andrey2008
06.02.2017 21:19Какое это отношение имеет к качеству тестов?
4144
06.02.2017 22:20+1Зачем тестировать то, что может находить компилятор? А сложных опечаток в тестах возможно нет потому, что тесты проверяют совсем простые случаи.
А про nulll pointer, возможно вам стоит добавить еще один уровень предупреждений, которые по вашему мнению не интересны пользователям, но находят проблемы.Andrey2008
06.02.2017 22:30Что значит может? Чуть более сложную ситуацию GCC уже не находит. В статье как раз приведён пример из его кода. И перегруженные операторы тут ни при чём. Более того, эта диагностика и некоторые другие появлялись в GCC недавно, причем они подозрительно напоминают некоторые наши диагностики :).
Ну ладно, простые случаи он может находить с недавнего времени. Что дальше? Мы должны отказаться от выявления таких ситуаций? Или такие проверки надо удалять из тестов для оценки качества анализаторов?4144
06.02.2017 22:46+1Я писал про тесты а не про анализатор.
Тесты в этом репозитории только для простых случаев. А ваш анализатор не только для простых.
Gcc также может многое, что могли бы находить анализаторы, на пример ошибки форматной строки, такие как переполнение буфера назначения. Или забытые скобочки, на основе анализа форматирования кода…
Я не противопоставляю gcc и pvs studio. Просто разработчики редко даже включают и проверяют предупреждения компилятора, не то что использует статические анализаторы. Если бы все исправляли все предупреждения компилятора, то и вы бы находили меньше проблем :)
stranger777
07.02.2017 09:33-6Анализатор PVS-Studio понимает, что этот код написан сознательно и никакой ошибки здесь нет.
Святая обязанность анализатора — предупреждать, а не понимать. Ниже — подробнее.
«Понимают» люди, но они могут не понимать, с каким клиентом имеют дело.
Что, если я — богатенький новичок, который просто пишет что попало и хочет получить все возможные пинки?
Что, если я — тестер-критик, как Билл?
Что, если я — опытный программист, опытный, разумно-ленивый и уставший, по запарке начинающий нести чушь в коде и желающий эту чушь увидеть быстро? А анализатор — молчит…
Что, если я пишу сложный генератор кода, качество которого нужно быстро проверять?
Что, если я — учитель и понадеялся в образовательном процессе на предупредительность анализатора? Ту самую, которой внезапно не оказалось…
Что, если я — «хреновый» набор тестов?
И так далее…
Этой статьёй вы теряете несколько потенциальных клиентов, а могли бы «приобрести».khim
07.02.2017 22:03+2Этой статьёй вы теряете несколько потенциальных клиентов, а могли бы «приобрести».
Вряд ли. Люди, которые поднимают тут шум и отчаянно «спорят о вкусе устриц и кокосовых орехов с теми, кто их ел» мало интересны кому-либо, кроме них самих. Хотя их вопли кого-то могут, в принципе, отпугнуть. Сама статья — вряд ли…stranger777
09.02.2017 20:48Разъясню позицию, чтобы быть понятым.
Если бы статья писалась в ключе «Теперь мы ловим даже самые сумасшедшие ошибки!», то лучше было бы всем. А от негативного субъективизма хорошо ещё никому не было.
Мне нравится, что делает PVS-Studio, (и особенно тот факт, что они не жадничают и за упоминание в комментариях готовы предоставить бесплатную лицензию некоммерческим проектам. Большое человеческое спасибо им!), недаром также подписан на посты; но…
Вот конкретно такой подход печалит. Я хочу, чтобы компания цвела и пахла, а не жаловалась на «хреновые тесты». И это я процитировал. Автор в статье так прямо и говорит, что тестовая база — хрень. И о том, какой же слабый CPPCheck и какой сильный PVS, вместо того, чтобы чуть допилить анализатор. Как говорится, «спасись сам — и вокруг тебя спасутся многие».
Гораздо конструктивнее было бы просто добавить фичу и предлагать её тем, кто в ней нуждается, а не объяснять, почему такой фичи нет… Я вот об этом.Antervis
10.02.2017 04:22ну, значит вы проигнорировали мнение автора, которое можно вольно сформулировать как «наш анализатор слишком взрослый, чтобы играться с той детской тестовой базой»
Am0ralist
07.02.2017 22:54+4Этой статьёй вы теряете несколько потенциальных клиентов, а могли бы «приобрести».
Этой статьей они точно не потеряют адекватных клиентов.
И вряд ли им нужны пользователи, которые не представляют, что такое разработка больших и сложных реальных проектов…stranger777
09.02.2017 21:02-1адекватных клиентов
Конечно, вопрос адекватности довольно важен, что касается консультативной поддержки, (если она включена в лицензию), но в общем и целом один из основных принципов: «Кто платит, тот и музыку заказывает». Звучит довольно цинично, но так и есть… Какая разница, что представляет и не представляет себе клиент, если у него есть деньги и он готов заплатить?khim
09.02.2017 23:11+1Какая разница, что представляет и не представляет себе клиент, если у него есть деньги и он готов заплатить?
Самая прямая и очень серьёзная. Количество попросов в техподдержку очень негативно коррелирует с адекватностью, а готовность платить, наоборот — позитивно. Или, по рабоче-крестьянски: один дурак может задать столько вопросов что и сто мудрецов не смогут на них ответить.
А поскольку компании, почему-то, интересуют не просто деньги, а прибыль, то вопрос адекватности клиентов — очень и очень важен.stranger777
09.02.2017 23:19Это понятно.
Думаю, эти вопросы решаются ещё на стадии заключения договора, когда разработчик сразу оговаривает, какие вопросы он решает и какие нет (что далеко ходить, примером — правила хабра). Так отсекается большое число неадекватных запросов. Хотя, конечно, всё равно тратит время службы поддержки и её ресурс.
linux_art
07.02.2017 12:47+1В конце статьи написано, что можно скачать и попробовать анализатор. Собственно скачал версию под линукс, попробовать не могу т.к. он требует лицензии :)
Andrey2008
07.02.2017 12:50+1Напишите нам письмо и мы отправим Вам временную лицензию.
P.S. Быть может, Вас ещё заинтересует такой вариант лицензирования: Как использовать PVS-Studio бесплатно.
withkittens
08.02.2017 01:02+5Как использовать PVS-Studio бесплатно.
Это одна из лучших штук, что вы придумали. Спасибо!
Andrey2008
07.02.2017 21:27+5Все, кто пишет, что хочет ещё больше неочишенных сообщений от статических анализаторов кода, никогда этими самыми анализаторами кода не пользовались. Ну или пробовали их на игрушечных проектах. В любом настоящем проекте всегда стоит проблема, как разобраться с имеющимися ложными срабатываниями. Это большая сложная задача, над которой приходится работать как разработчикам анализаторов, так и их пользователям. Куда уж ещё больше предупреждений! :)
Причем, PVS-Studio тут ни причем. То, что я написал относится к любому статическому, будь то Cppcheck или Coverity. Интегрируйте их в проект, размером в 4-5 миллиона строк кода для регулярного использования, а потом можно продолжить дискуссию о необходимости добавки. :)stranger777
11.02.2017 10:55-4Понимаю, что для больших, всяко-разно тестируемых проектов с конвенцией кодирования, разработанной на UML моделью, отдельным архитектором, отдельными квалифицированными программистами с их отдельными задачами, системами сборки, баг-трекерами и конкретным ТЗ «вот это вот всё» целесообразно отключить. Это всё очевидно.
Не понимаю, зачем «взрослому анализатору» (как где-то здесь было написано) жаловаться на детей и пинать их вместо того, чтобы просто сделать функцию и к ней кнопку «вкл/выкл». Хочешь — пользуешься, не хочешь, не нужно — не пользуйся. Да, это работа. Но и да, это ваша работа. И да, спасибо вам за работу.
Killy
08.02.2017 21:09Как насчёт добавить отдельный класс сообщений «синтетические тесты»? Ну или отдельную диагностику «запуск на синтетическом тесте».
С тем, чтобы в описании дать ссылку на пояснение своей позиции по этому вопросу. Ну или собрать отзывы от тех, кто найдёт «синтетические» ошибки у себя в проекте (if any).SvyatoslavMC
08.02.2017 22:59+2Напомнило демонстрационный режим у современных телевизоров, который включают на витрине и показывают синтетические
тестыкартинки :DKilly
09.02.2017 16:38-1Задача демо-режима — покрасоваться перед пользователем.
А у вас тут есть шанс достучаться до потенциального клиента, который пытается делать выбор в условиях неполноты информации. И вы можете показать ему, что знаете, что он сейчас пытается сделать и как он может ошибиться в результате.
Killy
09.02.2017 16:46Про отключение эмпирической обработки:
Если рассмотреть такую возможность, то я бы добавил кнопку «Мне нечем заняться. Покажи что-нибудь». И показывал порционно, и только на проектах, где других сообщений не осталось.Andrey2008
09.02.2017 16:58Когда нечем заняться, а предупреждений больше нет, то рациональнее будет попробовать какой-то другой инструмент или написать новых юнит-тестов, а не выжимать ещё одну каплю. Впрочем, не думаю, что такая ситуация встречается у наших клиентов. Им и некогда и всегда есть Low уровень предупреждений. :)
Killy
09.02.2017 17:23+1Этот тред вырисовал психологическую проблему:
Программист, IT-шник, как привило — control freak.
И тут ему говорят, что решение за него принимает не формально верифицируемый алгоритм, а эмпирический. Как же можно! Контроль отобрали и лазеек не оставили!
Мой коммент — не очень серьёзая попытка адресовать эту проблему, предложить морковку на верёвочке…khim
09.02.2017 18:55И тут ему говорят, что решение за него принимает не формально верифицируемый алгоритм, а эмпирический. Как же можно! Контроль отобрали и лазеек не оставили!
Ну прямо как Google и Яндекс. Ваш «сферический IT-шник» не плачет в подушку по ночам оттого, что Alta Vista умерла?
Статический анализатор — это, собственно, от начала до конца «эмпирический неверефицируемый алгоритм». Если вам такие не подходят — значит вам, собственно, статический анализатор и не нужен. Либо вы пишите идельный код (да, такие люди встречаются — но редко), либо (что гораздо чаще) — вы реально ни разу ни один анализатор на уровне «параноика» не запускали и с соответствующими сообщениями не боролись.Killy
09.02.2017 19:13-2Вы уверены, что со мной спорите?
Killy
10.02.2017 18:58+1Да, резко получилось.
Я понимаю и разделяю позицию Andrey2008 и khim. Но, видимо, из моих комментариев сложилось другое впечатление.
Мне интересны причины, почему не удалось донести позицию до значительного числа читателей, и что с этим можно сделать.
Очевидно, что не все могут переварить предложенное объяснение, пока не имеют соответствующего опыта. Апеллировать к авторскому опыту не слишком помогает. Отсюда и зациклившееся обсуждение. Либо надо искать другие способы донесения своего опыта, дать почувствовать себя на месте авторов, либо искать обходные манёвры.Am0ralist
11.02.2017 00:02+1Выпустить игру «Отлови баги»! От первого лица!
Сюжет:
Ты — программист, который должен отлавливать все баги за целым отделом <говно-кодеров>.
перед тобой монитор с простой IDE. И периодически раздаются сигналы об изменении куска какого-то кода и появление новой ошибки.
Смотришь код, если не видишь ошибку — требуешь подсказку (а-ля помощь гуру, которую можно вызывать с разной периодичностью, в зависимости от уровня).
Если не верно исправил — штраф (пока сумма штрафа не превысит зп и тебя не уволят).
Плюс таймер и минимум возможностей сохраниться (не во время правки бага)
Ну и уровни от изи, где лишь явные, до маньяка, где аж до синтетики из статьи.
Этакий тетрис для программистов. Без явной победы, просто на рекорды.
EvgeniyRyzhkov
09.02.2017 17:03+6кнопку «Мне нечем заняться. Покажи что-нибудь»
… которая просто вызывает начальника :-)
meduzik
Два приведенных примера с разыменованием NULL и rand — неопределенное поведение в стандарте. То есть компилятор в полном праве выкинуть их из кода (или заменить на еще какую-нибудь ерунду). Соответственно, на мой взгляд, статический анализатор должен предупреждать об этом, даже если пользователь что-то там имел в виду и написал *NULL сознательно. Просто потому, что пользователь в данном случае не прав.
Ситуация, когда анализатор может быть "немного прав", не выдавая предупреждение — если он точно знает, что в конкретном компиляторе, который используется для сборки кода, данная ситуация является определенной и документированной. И даже в этом случае, у анализатора должна быть уверенность, что это проект не собирается одновременно еще и другим компилятором (скажем, приведенный код находится в #ifdef _MSC_VER блоке), или в следующей версии поведение не изменят. Осуществляет ли PVS-Studio подобные проверки?
Andrey2008
Не понял суть вопроса. Анализатор находит ошибки, в том числе и неопределенное поведение. Однако в нем есть много исключений, когда определенные паттерны кода он считает допустимыми. Здесь дело практики. Можно сколько угодно рассуждать о потенциальной корректности/некорректности, но если ругаться на такие ситуации никто спасибо всё равно не скажет, так как люди пишут такой код осознанно. Более того, будут просить избавить их от ненужных предупреждений.
meduzik
Вот что clang делает вместо разыменовывания нулевого указателя, если записать его немного по разному: https://godbolt.org/g/NiJXiL. Замечу, делает на практике, а не в теории. На сколько из этих функций ваш анализатор выдаст предпреждение о неопределенном поведении?
Andrey2008
На какие-то ругнется, на какие-то нет. Не проверял. Поймите, что Вы тоже написали те-же самые бессмысленные синтетические тесты :). Анализатор будет на них сознательно молчать. Вредно ругаться на такой код. На практике в 99,9% случаев (то есть, всегда), такой код написан сознательно и все равно не угадаешь, что хотел человек. И даже не важно, есть там volatile или нет. Быть может человек хочет словить исключение. А быть может проверяет, будет исключении или нет. А быть может это микроконтроллер, где действительно надо записать по нулевому указателю. В любом случае это код написан сознательно. Случайно он получиться не может. Ну нельзя ошибиться, назвав функцию null1 и явно разменивать в ней 0. :) Да ещё комментарий рядом "//invalid opcode". А мы кстати подобные комментарии тоже иногда учитываем.
meduzik
Это — бессмысленный, неверный (в языке С и С++) код, который не должен появляться в программе. Что бы программист не думал, что бы он не писал в комментарии рядом, разыменование нулевого указателя всегда будет ошибкой, до тех пор пока у компилятора нет флага вроде --define-null-pointer-dereference-semantics.
Если я вас правильно понял, вы ищете только опечатки, но не ошибки по незнанию или непониманию языка, с которым программист работает. В таком случае, не стоит говорить, что ваш анализатор "в 10 раз лучше" — у вас разные цели и вывод в оригинальной статье абсолютно верный.
SvyatoslavMC
Не только опечатки, смотрите табличку.
khim
meduzik
За ссылку на опцию спасибо, с микроконтроллерами работать не довелось. Но про то, что такого флага нет — я не говорил. Посмотрите, пожалуйста, первое мое сообщение. Я спрашивал, проверяет ли PVS-Studio, что имеет дело с ситуацией, когда поведение в этом случае документировано, или же эвристика глобальная. Судя по ответам Andrey2008 — таких проверок нет.
Andrey2008
Это эвристика, которой достаточно с практической точки зрения.
Antervis
а если программист просто забыл инициализировать указатель?
Andrey2008
Где и что он здесь забыл??? :-D
Antervis
мало ли
zagayevskiy
А кто вам сказал, что они на это не будут кидать ворнингов?
staticlab
А если функция называется Contract::getNullifiers() или City::getViolationStats(), ваш анализатор тоже отключит предупреждения? Nullifier — лицо, имеющее право аннулировать договор.
Думаю, стоило бы в таких случаях всё-таки отключать предупреждения комментариями. Пусть программист сам указывает анализатору, что он знает, что пишет.
Andrey2008
Наличие Null в названии является необходимым, но недостаточным условием. Ещё должно быть присваивание 0 и его разыменование в соседней строчке. Вероятность такого события стремится к нулю и лучше не ругаться.
Я понимаю, что всегда можно придумать теоретический вариант, где анализатор даст сбой и не выдаст полезное срабатывание. Но это не интересно с практической точки зрения. Ещё раз повторю пример с двойными скобками: if ((a = b)). Да, здесь может быть ошибка. Но ругаться здесь не следует. И таким паттернов тьма и хороший анализатор должен их учитывать.
Lachezis
Lachezis
Конечно, ложные срабатывания очень нервируют, но я лучше самостоятельно проверю их чем буду надеятся на эвристику самого анализатора.
khim
Впервые встречаю мазохиста, который хочет разрбираться в сотнях сообщений об ошибках на каждую строчку программы.
Супер-программа
уже содержит сотни «подозрительных» мест, которые, формально, нарушают стандарт. И далеко не все они специальным образом помечены.
А как только вы напишите ещё что-то, что реально инстанциирует всю эту машинерию — сразу получите ещё кучу сообщений.
Вы, вообще, свой код хотя бы через все -Wall -Wextra -Wxxx (которых десятки НЕ включены ни в -Wall, ни в -Wextra именно чтобы не заваливать пользователя кучей сообщений) прогоняете? Или ваша любовь к статическим анализаторам сугубо платоническая?
Lachezis
Я удивляюсь как вы не определили цвет обоев в моем офисе, они красные если вам интересно.
Естественно я не хочу что бы анализатор кидал сообщение на каждый чих, но инструмент становится менее предсказуем когда начинает додумывать за программиста в спорных моментах.
Andrey2008
Lachezis
Везде нужен баланc :)
Andrey2008
Ой. Это я не так понял комментарий! Ааа не читайте мой комментарий. Мне показалось, что я отвечаю на «Естественно я хочу что бы анализатор кидал сообщение на каждый чих».
DarkEld3r
Ну уж стандартные хедеры можно и исключать из анализа?
khim
Нельзя. В стандартных хедерах — туева хуча макросов и inline-функций, если вы всё это проигнорируете, то кучу вещей можете не заметить.
DarkEld3r
Можно пример? Потому что мне кажется, что у статического анализатора есть достаточный контекст, чтобы понимать когда надо ругаться на стандартные штуки, а когда нет.
Andrey2008
На системные заголовочные файлы мы не ругаемся. А вот #define, да, это сущности, которые генерируют шум и надо обучать анализатор распознавать определённые паттерны. С макросами связана часть исключений.
meduzik
Про rand я был не прав, я согласен с вами, что сам тест неверен. Можно придумать (очень извращенный, но корректный) пример его использования в соответствии со стандартом:
mayorovp
Из-за UB оптимизатор имеет право считать, к примеру, содержимое по нулевому адресу неиспользуемым (не специально, а в результате случайного совпадения нескольких эвристик). В итоге код, который должен был по задумке падать с ошибкой — корректно завершится!
В приведенном вами реальном примере эту проблему обошли через
volatile. А код из синтетического теста я бы все же посчитал ошибочным.UPD: выше об этом уже написали, и со ссылкой. Как я и предполагал, надо обязательно добавлять
volatile(что довольно очевидно) и надо обязательно использовать запись вместо чтения (что совсем не очевидно)Andrey2008
Вот только невозможно угадать что хотел автор. Быть может он как раз хотело узнать ведёт себя код одинаково с volatile или нет. Я серьезно. Я насмотрелся удивительнейших проверок в коде. Даже в духе if (sizeof(int) != sizeof(unsigned int)). Поэтому я и говорю, что лучше всего молчать. Это уроки практики.
Antervis
так они не обязаны быть равны. Где-то sizeof(unsigned int) == 8, sizeof(int) == 4
znsoft
Ого, а можно реальный кейс когда они не равны? Я не гуру, поэтому интересно.
ruzzz
Да больше на бред похоже, unsigned перепутали с long. Иначе буду сильно удивлен.
Andrey2008
Ой, да ладно. Я даже где-то и if (sizeof(char) == 1) видел. Вы не поверите, какие программисты творческие натуры.
splav_asv
Исходники раритетного Word, если память не изменяет. Это в те времена, когда соответствие компиляторов стандартам оставляло желать много лучшего, чем сейчас.
brain_tyrin
Стандарт C99 (пункт 6.2.5.6) говорит, что
В стандартах C++ фраза повторена почти дословно (есть пункт про «same amount of
storage»).
Так что это какие-то странные реализации.
Antervis
да. В стандарте С89 такого требования нет
brain_tyrin
В ISO/IEC 9899:1990 (который, фактически, и есть нынешний C89) такое требование есть (пункт 6.1.2.5)
Antervis
вопрос скорее в другом. Зачем явно вызывать разыменование нулевого указателя (которое компилятор может попросту выкинуть) вместо std::raise(SIGSEGV), throw std::runtime_error или std::exit(<какой-нибудь код>) в зависимости от ситуации? Это И явно, И не UB
Andrey2008
Ну пишут вот так… Что я теперь сделать могу… :)
Но надо учиться с этим как-то жить.
klirichek
В низкоуровневом коде для микроконтроллера, например.
CTAPOBEP
> Более того, будут просить избавить их от ненужных предупреждений.
В CppCheck на это случай есть возможность указать прямо в коде, что данную строку следует проигнорировать. Вы хотите сказать, что у вас такой возможности нет?
Andrey2008
Происходит некоторая путаница между явным подавлением предупреждений и исключениями для сокращения числа ложных срабатываний. Давайте я ещё раз попробую объяснить.
В PVS-Studio существует несколько механизмов для устранения ложных срабатываний, которые в любом случае неизбежны:
Выключать предупреждения или подавлять предупреждения в макросах, можно также используя конфигурационные файлы (см. про pvsconfig).
Отдельно следует выделить систему массовой разметки предупреждений с помощью специальной базы. Это позволяет быстро интегрировать анализатор в процесс разработки больших проектов.
Есть ещё некоторые возможности, но, пожалуй, нет смысл повторять документацию.
Так вот, все это относится к явному указанию, что не считать ошибками. Однако, это не снимает задачу минимизации предупреждений с помощью специальных исключений. Ценность анализатора не в том, что он ругается на всё подряд, а в том, как много он знает ситуаций, когда ругаться не надо.
Примеров, столь много, что надо писать цикл статей на эту тему. Поэтому опишу только одно, из последней диагностики V779 — Недостижимый код.
Исключение:
Не надо ругаться, если блок заканчивается конструкцией 'no_return_statement; return;'. Многие так подавляют предупреждения компилятора. Пример:
Теоретически здесь надо ругаться, что «return» является недостижимым кодом. Однако этот return появился в коде вынужденно, как борьба с предупреждениями компиляторов или других анализаторов. В коде иногда есть соответствующий комментарий на эту тему. Так вот, никакой практической пользы от предупреждения на такой return нет. Мы проверяем как работают диагностики на 154 открытых проектах. И ни в одном, не было ошибки. Всегда было видно, что это ложное срабатывание. Всего их несколько десятков. Нет смысла оставлять их из-за гипотетической вероятности, что будет пропущена настоящая ошибка. Если бы мы оставляли такие ложные срабатывания, анализатором уже давно бы было невозможно пользоваться.
CTAPOBEP
Спасибо за развернутый ответ — теперь я понял в чем наша с вами проблема. Смотрите — когда я прихожу в магазин, чтобы купить нечто, в названии чего есть слово 'молоток', я ожидаю получить предмет, обладающий определенным набором свойств (скажем, я хочу задействовать ящик восьмисантиметровых гвоздей, доставшихся мне в наследство от деда). И неважно, как этот 'молоток' будет выглядеть — как стальной брусок на деревянной палке или бегающий по стенам агрегат, читающий мои мысли по поводу того, где должны быть забиты гвозди. И если в результате окажется, что этот навороченный агрегат не может забивать восьмисантиметровые гвозди (так как по собранной производителем статистике за последние 5 лет ни одна из крупнейших гвоздезабивающих компаний ни забила ни одного такого гвоздя), с моей точки зрения это будет выглядеть как ОБМАН.
Возвращаемся к статическим анализаторам. Ну да, синтетические тесты. Или Вы всерьез считаете, что я, будучи инженером, поверю Вам на слово? Да, когда у меня наконец-то дойдут руки до PVS-Studio, я первым делом соберу тестовый проект из (утрированных) ошибок, которые я постоянно совершаю (спасибо CppCheck за собранный список) и опробую его на нем.
Важно вот что — любой инструмент должен по умолчанию работать ожидаемым образом, а всякие интеллектуальные фичи должны включаться уже при необходимости. Может, конечно, я один такой, но для меня очевидное использование статического анализатора (с практической точки зрения) выглядит следующим образом:
1. Запустил проверку проекта с настройками по умолчанию — получил миллион предупреждений.
2. Постепенно повышая уровень ранжирования, нашел состояние, при котором число предупреждений становится разумным (скажем, 50 и при этом это должны быть самые 'страшные' баги).
3. Исправил самые опасные места, снизил планку, повторил, остановившись на том уровне, который меня на данный момент устраивает.
P.S. Пожалуйста, не нужно пытаться меня переубедить — если я не являюсь вышей целевой аудиторией, просто проигноруйте.
ns5d
полностью согласен, х знает чего заминусовали.
CTAPOBEP
Да уж) Я им пытаюсь сказать, что у них есть проблема, мешающая 'делать бизнес', а они меня минусуют и даже в карму плюют. Что тут можно сказать — успехов в семейной и личной жизни)
Andrey2008
Вот делать нам больше нечего, минусовать, а тем более карму портить. Возможно это намек со стороны сообщества, что Вы слишком категоричны и стоит попробовать осмыслить альтернативную точку зрения.
Andrey2008
(del)
ns5d
Вы с пеной у рта пытаетесь доказать что Ваш продукт неебически крут и рядом не стоит с другими, даже если это так, это не Вам судить. Если Вам в комментариях ни один, а несколько человек говорят, что по умолчанию замалчивать не нужно, а предоставлять этот выбор программисту, значит к их мнению нужно прислушиваться наверное?! Вы для кого продукт делаете? И для чего? Для тех кто любит копипастить? Или тех кто еще не до конца понял грабли языка?! Пусть он показывает хоть 100млн., хоть 1000 сообщений, главное, чтобы он их ранжировал по степени важности, а я сам как нибудь решу исправлять или нет, и с какого конца начинать, хоть выбор очевиден, НО не нужно умалчивать о проблемах, какими бы они наигранными не были. А не так: "Эти ошибки жизненные, их постоянно делают, вы закостылим, а вот эти наигранные, мы будем руками разводить" — такая система гроша ломанного не стоит, а чтобы продукт покупали, он должен быть на порядок лучше, а то и более. И когда минусуют логичные вещи, в голову приходит только одно, что специально несколько человек, которые находятся в Вашем распоряжении, сядят тут, минусуют, чтобы Ваше мнение хоть чего-то стоило, — да бредятина, а что я должен еще думать?! Когда говоришь, что белое — это белое, а черное — это черное, а на тебя смотрят как на долбаеба. Кармодрочерыебучие.
Am0ralist
Продукт делается для тех, кто умеет и работает над большими и сложными проектами. Над многолетними проектами. В которых синтетических случаев не будет, либо они не окажут какого-либо существенного влияния.
Можно подсветить миллионы строчек кода якобы сомнительного содержания и это ни на шаг не приведет к качественному продукту.
А можно выделить 500 точно сомнительных мест, на исправление которых имеет смысл направить силы.
Максимализм хорош в утопиях. В реальности же, заглянув в это болото программисты только демотивируются разгребать действительно проблемные места.
А ваши сообщения больше напоминают картинку про белок истеричек. Держите себя уже в руках.
EvgeniyRyzhkov
Вы знаете, делать продукт «по комментариям» — это путь в никуда. Мнение нескольких анонимов я просто НЕ ИМЕЮ права ставить во главу угла при определении куда и как развивать продукт.
Вы может быть удивитесь, но даже мнение платящих клиентов иногда нужно игнорировать. Так как видение продукта должно и может быть у команды (неважно, правильное или не правильно), но у сотен комментаров видения быть не может.
ns5d
Вы специалисты по правому уху, в левом нихуя не понимаете!