Comments 29
Я правильно понимаю, что так называемая "настройка анализатора" заключается в массовом отключении выдачи предупреждений для некоторых типов и макросов, в том числе выдачи определённых типов предупреждений в принципе (505,581,592,677,707)?
То, что надо отключать диагностики, это нормально. Не все диагностики подходят под определённый стиль написания кода. Например, вот прямо сейчас коллега делает настройку, которая будет указывать анализатору, что функция malloc никогда не вернёт NULL (нет, это не связано с Tizen). Странный подход, но с желанием потенциального клиента спорить мы не будем. Это не совсем тоже, что полное отключение диагностики, но близко.
Из полного отключения диагностик как раз можно привести упомянутую V505. Кто-то считает каждый килобайт на стеке и такая диагностика просто манна небесная. А для другого бестолковое предупреждение, которое не имеет смысла, так как не было ещё случая, чтобы стековой памяти не хватило.
Однако, тут же вспоминается история с программой aeolus, которая вдруг перестала работать из-за того, что её создатель решил сделать не слишком большой стек для создаваемых потоков. Вот так годами программа работала, а потом вдруг перестала из-за того, что в другой библиотеке потребление памяти на стеке немного возросло. Так что не надо считать каждый килобайт на стеке, чтобы в какой-то день обнаружить, что alloca внутри цикла это не самая лучшая идея. Особенно если речь идёт о библиотеке, которая может использоваться другими программами в самых разных условиях.
Если ваша цель — поиск настоящих ошибок, то зачем подавлять такие предупреждения?
То, что является «настоящей ошибкой» зависит как от проекта, так и от программистов. Возможно, PVS взяли, чтобы проверить проект, запускающийся всегда на строго определённом железе с известным и достаточно большим размером стека, тогда как malloc() нет вообще. Или программист искренне считает, что малый размер стека является «не его» проблемой и ему проще не купить PVS, чем переписать код.
Вот смотрите, авторы EFL считают естественным не проверять указатель после malloc. А я тут ещё буду к ним с alloca приставать. :)
Почему странный? В Neovim память выделяется именно так: xmalloc() либо вернёт не‐NULL, либо аварийно завершит программу. Правда, у нас именно xmalloc() — обёртка над malloc(), которая попытается почистить память, если NULL всё же вернули. Причина — всё тот же overcommit: зачем зря тратить время на обработку NULL, если вы скорее получите крэш, чем NULL, а большие куски памяти редактору выделять практически никогда не нужно (во всяком случае, не с тем, что (Neo)Vim использует для файлов из памяти)?
Кстати, а не подскажете, почему у нас на строке 389 анализатор находит V779: недостижимый код? Это одно из двух сообщений, которые я не замолчал, т.к. оно выглядит как ошибка анализатора, а не ложное срабатывание.
Про V779 ничего сказать не могу. Мало данных. Прошу написать, что такое cur.aobj->type. А ещё лучше прислать нам на почту i-файл, мы посмотрим. Выглядит очень странно.
Обёртка нужна, если память освобождать и пытаться ещё раз. Или если хочется сделать mock в юнит тестах подменой указателей на malloc() (есть и другие способы, но с luajit ffi так проще всего). А если ничего из этого не хочется, то при наличии понимания последствий использования overcommit на крэши с NULL при библиотечных функциях как‐то всё равно.
Нравится побочный эффект анализатора — повышение читаемости кода.
Читаемость кода часто действительно становится лучше. На мой взгляд хороший и простой пример, это уделанные проверок перед освобождением указателя. Я описывал это в статье про микрооптимизации (см. V809: The 'if (ptr != NULL)' check can be removed).
Вот такой подход — это очень здорово! Спасибо.
it's not a bug. it's a piece of history
Прекрасно.
[Disclaimer] Холиварный комментарий для любителей поминусовать. Я не являюсь программистом C/С++.[Disclaimer]
А все-таки, что ни говори, хоть C/С++ и признаны официальными стандартами низкоуровневого программирования (да и не только низкоуровнего, вобщем-то), и история их развития заслуживает большого уважения, в 2017-м году пора бы уже "отходить" от этих языков с замысловатым и непрозрачным синтаксисом, с помощью которого можно порождать огромное многообразие корректных и некорректных ситуаций, которые на первый взгляд могут казаться "примерно одинаковыми кусками кода" и надо еще поломать голову, чтобы понять, это бага, фича или просто "работает-не-трогай" (как в статье выше).
На сегодняшний день есть уже "более современные" инструменты, не являющиеся хайпом. Это и Rust, и Go, да и C# на худой конец, хоть вы и скажете, что C# совсем из другой степи. Начинать сейчас новый проект на C/С++ можно только разве что из-за наследия кодовой базы старых библиотек и кусков других более старых проектов. Но в целом идея писать что-то на С в 2017-м году почему-то мне представляется примерно такой же, как писать что-то на ассемблере в начале 2000-х.
Да, С компилируется на любую платформу. Да, С это стандарт. Но в настоящий момент эти свойства данного языка не являются прямо уж такими уникальными, из-за чего на вопрос "на каком языке будем разрабатывать" стоило бы сказать "конечно только на C/С++". Хотя конечно всё это depends...
В некоторых областях выбора просто нет. Разве что сейчас rust постепенно становится альтернативой, но ещё не очень дозрел. Посмотрим, что будет через пару лет. D ещё обещал подтянуться, но, насколько я слышал, пока без GC ему сложновато (в частности, стандартная библиотека предполагает наличие GC и он требуются даже для работы исключений и строк).
Java/Go/C# — языки с GC и рантаймом, так что они неприменимы для тех областей, где царят плюсы и си (начиная от варианта, когда нужна предсказуемость и заканчивая полным отсутствием стандартной libc и абстракций ОС, т. е. системным уровнем).
Прикладное ПО некритичное к производительности сейчас повально пишут на go, python, C# (хотя там можно выжать больше с использованием unsafe, как и в java при желании через jni/jna).
Давайте на примере одного из доменов, которые мне интересны: bare metal на микроконтроллерах (ниша си и в меньшей степени плюсов):
- не стабилизирована
asm!(т. е. de facto необходимо пользоваться nightly); - не стабилизированы аллокаторы и, соответственно, поддержка их в libcollection (над этим работали довольно активно);
- не поставляются pre-built версии стандартной libcore и всего такого для тех же arm cortex-m, но подвижки были, чтобы для части target'ов поставлять всё (libstd и всё остальное), а для части — libcore/liballoc/libcollection; сейчас вполне решается с помощью
xargoот japaric'а; - crate libc не разделен на две части (ctypes, которые нужны для ffi и, собственно, биндинги для реализации libc);
- не появилось пока нормального type-safe подхода для работы с прерываниями (+ у некоторых архитектур abi для обработчиков довольно необычное);
- нет мало-мальски универсальных crate'ов для периферии (api-часть, чтобы, скажем, драйвер отдельного чипа мог использовать ту реализацию spi, которая есть для данного контроллера).
- жизнь в
no_stdпока требует featurelang_items(т. е. nightly) для определенияpanic_fmtиeh_personality.
Это навскидку. Если чуток покопать — найдётся куда больше.
По, например, webdev есть прекрасный сайт http://www.arewewebyet.org/, где описано текущее состояние. В основном проблема в некоторой незрелости экосистемы, но хорошие подвижки есть и включены в roadmap.
Около 8 часов мы с коллегой искали баг, который проявлялся в весьма запутанной ситуации. Когда мы его наконец нашли и тщательно побились головой об стол, проклиная свою глупость. И я, разумеется, поднимаю палец в верх и говорю своему коллеге: «А знаешь, как мы могли бы найти этот баг легко? (драматическая пауза) PVS-Studio!»
Да, я очень давно пытаюсь убедить своих коллег в необходимости использования PVS-Studio.
На этот раз мы таки взяли себя в руки, поставили, запустили проверку и… баг не нашелся. И вот это было довольно обидно, если честно.
В чем же заключался баг?
bool isDone = checkSomeCondition( first );
isDone = isDone && checkSomeCondition( second);
checkSomeCondition — функция с побочными эффектами, которая, как вы уже понимаете, не вызывалась второй раз, если первая проверка возвращала false. Этот код писал я сам (и мне стыдно) и в тот момент я просто напрочь забыл об этом свойстве оператора &&. И, формально, это в общем-то и не баг даже, подобный код можно писать намеренно.
Тем не менее, хотелось бы узнать, рассматривался ли подобный код в кандидаты на предупреждение? Можно ли как-нибудь это предупреждение получить? Допустим, если у функции нет атрибута pure?
Если это и ошибка, то в бизнесс-логике. Статанализ ищет только формальные логические ошибки. С формальной точки зрения никакой ошибки в этом коде нет.
bool okTest = Test1();
okTest = okTest && Test2();
okTest = okTest && Test3();
okTest = okTest && Test4();
if (!okTest) Error();
Я подобное встречал и даже сам писал что-то в этом духе. Всё хорошо и часто используется.
bool inited = initSome(first)
inited = inited && initSome(second)
inited = inited && initSome(third)
if(inited)
{
//blah-blah
}
deinitIfNeed(first);
deinitIfNeed(second);
deinitIfNeed(third);
После Java код на C++ кажется немного странным — не сумели определить размер по типу объекта вернули 0, обнаружили нулевой указатель вернули NULL и так далее. В Java обычно в подобных случаях бросается исключение. Хотел спросить почему в C++ так не принято.Ведь такие ситуации обычно все равно на более высоком уровне являются ошибкой.
Ну ребята… Дело ж не в Истине, дело в деньгах, как всегда. Вы можете сколько угодно доказывать, что срабатывание истинное, но чинить его от этого не побегут, и хвалить инструмент ваш тоже не обязательно побегут.
У вас bug-finding tool. Пользователь вкладывает в неё деньги (за тул) и время (разработчика, т.е. деньги всё равно менеджера) и получаешь на выходе баги (то есть багрепорты — хотя иногда и таки баги, если багрепорт был ошибочным, а его починили, или же если программист допустил ещё более серьёзную ошибку, пока фиксил не шибко критичное срабатывание).
Баги можно искать и предотвращать кучей разных способов. Другими статическими анализаторами и линтерами, динамическими инструментами, обвешиваясь автоматическими тестами и ручным тестированием, усилением ревью кода, повышением квалификации сотрудников, наконец разбором багрепортов от пользователей. Обвешиваться всеми инструментами обычно нет времени или денег.
Так вопрос-то в чём: Почему программист потратит время с большей пользой, разгребая тысячу найденных ПВС-Студией непроверенных маллоков, чем если, например, починит десяток реальных падений, уже задевших пользователя?
Если программист свободный и счастливый и обладает вечной жизнью и ему не нужно пахать на хлеб насущный, то да, он может рассуждать об истине и лжи, и думать, что вот это срабатывание истинное, вот это — ложное. Всё равно он не будет рад срабатываниям вида "Вы на воздушном шаре" (анекдот), не будет вносить premature optimizations, но в целом он будет рад повысить качество кода.
Если программист замучанный, ему каждый день говорят, что завтра релиз, разъярённые пользователи уже катят заряженные зажигательными слезами катапульты к стенам его офиса, а на нём уже и без того десятки конкретных багов, которые надо не искать а фиксить, то в его пирамиде Маслоу, увы, не найдётся места анализатору, который учит его повышать качество кода и проверять маллоки. Но в вышеупомянутой пирамиде всё равно найдётся место анализатору X, который выдаёт десяток очень серьёзных срабатываний, пусть даже в половине случаев ложных, и кроме этого ничего не говорит. Потому что анализатор X потратит очень мало его времени, а может быть как раз и вскроет причину тех дроидов багов, которых он ищет.
Поэтому мне норм называть ложными только те срабатывания, в которых анализатор несёт откровенный бред (в лучшем случае при попытке извлечь хоть какую-то пользу из symbolic execution, наверное, их и будет процентов 10-15). Но, по моим полудиванным представлениям:
- Должен быть режим, выдающий только критичные для читателя срабатывания, в которых анализатор максимально уверен, которым не требуется никакая настройка для нормальной работы. В идеале оный должен быть по умолчанию, или, во всяком случае, максимально на виду. Можно сделать много слоёв критичности или разветвлённое дерево (для кого-то code style критичнее чем security, для кого-то нет).
- Пользователь желает знать, какие сорта ложных срабатываний имеются, на каких паттернах кода они проявляются (от этих ваших макросов до, может быть (от балды, пальцем в небо говорю), недопиленной поддержки какой-нибудь конструкции из C++11 или какого-нибудь gcc extension в вашем самописном (же?) парсере/AST), чтобы прикинуть, есть ли эти паттерны в его коде. Также как подавить разные классы мешающих срабатываний. Правильный, честный (а не температура по больнице) ответ на вопрос про "процент ложных срабатываний", как мне кажется, лежит именно в этой области.
- Пользователь желает знать, какие виды багов ваш метод находит хорошо, а для каких багов лучше воспользоваться другим методом. Это вопрос про false negatives, изучать его тяжело, но вы, видимо, можете смотреть, какие баги у ваших пользователей выживают и потом всплывают другим способом.
- Как сделать программу проще для анализа? — то есть на каком коде анализатор путается и опускает руки? — вы можете это оформить это в виде советов.
Такого рода инфа в идеале отбросит скепсис и вызовет к вам кучу уважухи. Ваш агрессивный выпад "в Тайзене куча багов" порождает довольно большой спектр вопросов и недоумения, и вопрос про ложные срабатывания — это ну некий простой способ обобщить это настроение, но прямолинейный ответ "15%" явно не исчерпывает.
Менеджер, желающий внедрять анализатор, тоже в идеале должен бы не сам решать, ориентируясь только на температуру по больнице и marketing bullshit, а передать на нижние уровни иерархии вышеупомянутую инфу для осмысления и выслушать их фидбэк, нужно оно или нет.
Итого — нет смысла гордиться числом срабатываний, или даже числом истинных срабатываний (Истина мало кому нужна, увы, на хабре полностью уместна, да). Срабатываний должно быть столько, сколько пользователь сам желает увидеть.
Короче не знаю, настанет ли момент, когда я буду счастливым пользователем E, раньше благодаря вашим усилиям. Само DE очень приятно на десктопе, люблю его, но заочно — увы, много падений. Там вроде даже тайлинг милый завезли за последнее время. Вы нашли много проблем, да, стиль ужасный много где, да, для opensource это безобразие ящитаю, но не могу отделаться от ощущения, что легендарный rasterman наверняка мог бы потратить время с большей пользой. Однако не мне его судить (вас сколько угодно, хе-хе-хе), и за найденные баги спасибо.
За свою практику мы убедились, нет «точно багов». Есть просто баги, а интересны они или нет решает команда разработчиков. Поэтому невозможно выделить ядро (хотя мы и стараемся), которое точно для всех будет ошибка.
Вот, например, утечка памяти? Ошибка? Для одних — это ОШИБКА! Для других поведение «by design» и предупреждение является ложным срабатыванием.
Да, и мы ведь не заставляем править все баги. Можно отключить неуместную диагностику и радоваться жизни. Нет времен и на это? Используйте базу разметки и смотрите на предупреждения, относящиеся только к новому коду.
Поскольку NDA у меня уже закончился:
Я точно знаю, что как минимум велись работы по портированию их на RTOS. VxWorks и ThreadX как минимум. Поэтому вполне можно ожидать встречи с «древним» компилятором.
А там бы я не только new на NULL проверял. Например при портировании того же EFL на VxWorks, кажется, мне пришлось столкнуться с тем что malloc в libc BSP был реализован как nop, nop,...., nop, ret.
Собственно тогда нам упомянутый в статье Rasterman честно сказал что мы психи и оно работать не будет.
Но заработало. Как-то. ;-)
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний