Pull to refresh

Comments 72

«нереальный» код зачастую возникает в процессе разработки. Ну то есть сначала было только одно условие, затем добавили присваивание, и чуть позже словили баг, выяснили, что присваивание должно срабатывать не всегда и добавили ещё одно условие, вот и получили неоправданно усложнённый код. Иногда полученный код называют «Rube Goldberg Code» — тут только периодический рефакторинг, ну или дотошный ревью поможет избавиться от таких участков. Статический анализатор, указывающий на такие места, конечно помогает.
Кажется, что многие не понимают принципа «пишите скучно».
Экзотические варианты есть, но каждый такой вариант должен быть в осознаным и вызывать вопрос — а нужно ли так делать, если это вызывает такие проблемы?
А главное — иногда ты пишешь скучно, а хак получается из-за ошибки. И вот тогда супер полезно обратить на него пристальное внимание.

Я могу ошибаться, прошу пояснить. В вашем последнем примере volatile относится к указателю или к массиву, на который передаётся указатель?


Up: Сам спросил, сам отвечу :) да, если бы volatile был после *, то относился бы к самой переменной. Так что пример корректен.

Чтение по спирали это одна из самых моих нелюбимых вещей в С. Для перестраховки я предпочитаю писать что-то типа volatile * volatile uint8_t (или дважды const), чтобы уж наверняка.

Это некорректно, нужно понимать, что вы делаете. Чтобы было легче и не путать указатель на константу с константным указателем могу посоветовать такой хинт (не для продакшена): объявляйте все типы через using. А потом поиграйтесь в IDE (например CLion) с заменой типов — сразу будет понятно что и как раскрывается.
Вообще меня ещё в техникуме сильно гоняли на первом курсе по этому поводу, чтобы выработать понимание. Но с появлением auto и развитием STL всё это стало ненужно.

1. Я пишу на С.
2. Я понимаю, что это и как работает. Я не понимаю, что и как осыпается при оптимизациях компилятора.
GCC не замечает проблем на реальных адресах данных, он работает с ассоциациями — именами переменных, их типами, свойствами, и атрибутами.
Ссылки — зло. Используйте объединения.

Ну тут всё просто. Если утрировать, то в си пофиг на const в большинстве случаев. Это больше для программиста подсказки. За исключением ситуаций когда объект лежит в readonly памяти. Если вы в такое упрётесь, то скорее всего вы это уже будете знать :)
Про volatile. Если вы в теле метода не меняете значение обычной переменной, то компилятор может вычислить все выражения с ней заранее, например при вызове функции; а всё что в циклах уже игнорировать и подставлять значения. Когда же переменная не volatile то компилятор не будет делать таких допущений. Грубо говоря такой код:
void foo(int i){
I = 1;
While(I != 0){...}
}
Превратится в такой:
void foo(int i){
I = 1;
While(true){...}
}
А с точки зрения ассемблера, какой нибудь cmp заменится на безусловный goto. Что будет куда более дружелюбно для конвейера.
Вот чтобы запретить компилятору такие фокусы, используется volatile. Ну или если вы что-то низкоуровневое программируете и напрямую читаете память.

Вот чтобы запретить компилятору такие фокусы, используется volatile.
Даже с -О3 такое упрощение не будет проведено? Не помню этот кусок стандарта.
За исключением ситуаций когда объект лежит в readonly памяти. Если вы в такое упрётесь, то скорее всего вы это уже будете знать :)
Ну, обычно знаю :) По-моему, регистры иначе как константный указатель на волатильную переменную и не объяснить. Ещё константные массивы как член структуры очень удобно объявлять, когда играем в ООП.

Даже с O3, если переменная помечена как volatile. Для того он собственно и создавался. Представьте, что у вас где-то в памяти лежат данные с какой-либо железяки, которые туда попадают допустим через DMA. Без volatile было бы очень проблемно отслеживать их изменение.

Кстати, а не существует прагмы justdoit, чтобы стока или блок не могли быть удалёнными компилятором? Всё-таки извращения, когда нужно забить нулями в целях безопасности память, которая в функции больше использоваться не будет, не очень способствуют читаемости кода.
Спасибо, прощёлкал этот кусок свежего стандарта.
Даже если вторая проверка действительно нужна, код лучше переписать по возможности, так что анализатор вполне справедливо кинул предупреждение.
Если на проекте слишком много такого кода, то, боюсь, проблема не в анализаторе.
а были статьи с описанием, как ваш анализатор устроен? Как происходит сам процесс анализа, используется поиск определенных шаблонов или что-то ещё?
мне любопытно, но сам не могу полностью понять как такая штука работает.

Мне кажется, что вы под влиянием вполне понятных эмоций недооцениваете опасение разработчиков. А именно: в C++ нет простого и очевидного способа удостоверится в том, что у вызваной функции нет нежелательных побочных эффектов. Причем не обязательно эффектов, которые завязаны на многопоточность, чтение I/O портов или какой-то другой хардкорной специфики.


Вот простой пример, в котором все работает в однопоточном режиме:


class Registry {
    int * ptr_{nullptr};
public:
    void register_pointer(int * ptr) { ptr_ = ptr; }
    void remove_pointer() { ptr_ = nullptr; }
    void increment() { if(ptr_) ++(*ptr_); }
};

Registry registry;

void func_with_side_effects() {
    registry.increment();
}

int demo() {
    int a[3]{ 0, 1, 2 };

    struct RegistryModificator {
        RegistryModificator(int * ptr) {
            registry.register_pointer(ptr);
        }
        ~RegistryModificator() {
            registry.remove_pointer();
        }
    } registry_modificator{a};

    if(0 == a[0]) {
        func_with_side_effects();
        if(0 == a[0]) return -1;
    }

    return 0;
}

int main() {
    return demo();
}

После вызова func_with_side_effects повторная проверка имеет смысл. Но PVS-Studio скажет, что она избыточна.


И можно было бы кивнуть на то, что ложные срабатывания — это неизбежность. Но посмотрите на это со стороны того, кто будет сопровождать код.


Если это будет человек, который не знает тонкостей работы func_with_side_effects и не имеет возможности (желания) разобраться с этим вызовом, то он может просто поверить статическому анализатору и удалить "лишнюю" проверку. Что затем приведет к потере работоспособности участка кода.


И да, здесь нет аналогии с бензопилой и ломом. Вполне себе обычный код, без каких-либо хитрых трюков. Т.к. С++ является императивным языком, разрешающим различные побочные эффекты, к выдаче предупреждений о том, что какие-то проверки не имееют смысла, следует относится с большой осторожностью.

С другой стороны, такой пример попахивает не очень хорошо, и если анализатор подсвечивает этот code smell, в этом тоже есть определенная польза, кто-то может захотеть отрефакторить его более очевидным способом.
Потому что кто-то может удалить проверку, которая выглядит избыточной, и без подсказки анализатора.

Подобный паттерн на самом деле достаточно распространён. Например, для глобальной регистрации тест кейсов движке юнит-тестов такое будет. Или ещё какой-то реестр объектов, по результатам обхода которого формируется состояние или отчёт.
Проблема в том, что здесь мы играем в «мой data-flow-анализ против твоего data-flow-анализа», т.е. в «сможет статический анализатор использовать всю информацию, которая доступна компилятору, или нет». Понятно, что без встраивания очень глубоко в сам компилятор ответ будет «нет, не сможет», но даже тот факт, что анализатор на этот код указывает как на странный — это уже благо, потому что код этот воняет, и может сломаться от случайного рефакторинга, как уже выше написали до меня. False positive намного лучше true negative, и намного лучше незнания о том, что код может перестать работать в случайного изменения в другом месте, а вы потом потратите две недели в попытках понять, что вообще произошло, и почему у вас результат сборки в дизассемблере так сильно отличается от модели этого результата в голове.
потому что код этот воняет

И как же его переписать, чтобы не вонял?

Перепишите проверку if (0 == a[0]) так, чтобы компилятор не смог использовать оптимизацию sequential read/compare elimination, например положите его внутрь no-inline-функции с побочными эффектами, таким же образом, как вы уже сделали с func_with_side_effects(). Повторное сравнение одного и того же элемента non-volatile-массива с константой само по себе не меняет состояние абстрактной машины, которой оперирует стандарт, и потому может быть пропущено в полностью конформной программе на С и С++ благодаря as-if rule. Хотите сделать так, чтобы сравнение это туда не попало — скажите компилятору об этом явно.
Перепишите проверку if (0 == a[0]) так, чтобы компилятор не смог использовать оптимизацию sequential read/compare elimination

А почему вы решили, что компилятор здесь может использовать эту оптимизацию?

Уже объяснил, потому что повторная проверка этого условия сама по себе не меняет состояние абстрактной машины, которой оперирует стандарт. Пока ваша func_with_side_effects() трогает a[] — все скорее всего будет работать, как только при очередном рефакторинге перестанет, компилятор праве сразу же выбросить повторную проверку и начать возвращать -1 прямо из первой.

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

Как только перестанет, это уже будет другой код. И о другом коде можно будет вести другой разговор. Пока речь идет о вполне себе корректном случае, где анализатор вводит в заблуждение.


Можно оправдываться тем, что от false positive никто не застрахован. Вполне понятная позиция.

Ваша позиция тоже понятная, вопросов больше не имею. Меня спросили почему код воняет — я ответил так, как я сам это понимаю. Меня в детстве безопасники покусали, и теперь я предпочитаю дуть на воду, чем обжигаться на молоке, особенно в проектах, над которыми работает больше сотни людей каждый день, а баги в нем стоят миллионы настоящих долларов.
особенно в проектах, над которыми работает больше сотни людей каждый день, а баги в нем стоят миллионы настоящих долларов.

Звучит как "сперва добейся".

Не приведи рандом добиваться этого. Хотите считать это «сперва добейся» — ваше право, я делюсь тем опытом, который имею, и теми условиями, в которых приходится работать, если у вас другие условия — отлично, это не значит что вам нужно стремиться к моим, или мне — к вашим. В моем мире не просто «с анализатором лучше», а давно уже «без анализатора полный конец обеда», если у вас не так — я реально рад.
В моем мире не просто «с анализатором лучше», а давно уже «без анализатора полный конец обеда», если у вас не так — я реально рад.

Я наивно полагаю, что целью маркетинговой программы PVS-Studio является продвижение своего продукта не только в нишу mission-critical, где анализаторы — это и так уже устоявшаяся практика, но и в нишу обычного говнокодинга, где нет ни бюджетов в сотни миллионов долларов, ни возможностей нанимать выпускников MTI, ни проектных команд в 100500 человек.


Так что можно сделать замечательный дефектоскоп, который отыскивает микротрещены в карбоновых корпусах болидов Формулы-1, а механики формулических команд будут с удовольствием рассказывать насколько этот дефектоскоп облегичил им жизнь (и все это будет чистой правдой)… Но ссылки на опыт Формулы-1 будут мало чего значить для владельца автомастерской в условном Урюпинске.

КМК встраивание анализатора в среду разработки и прививание привычки не допускать потенциальных багов никому не помешает, даже владельцу автомастерской, скорее странно что это до сих пор не стало нормой. Но когда-то же программировали и без отладчика.

А PVS-Studio допускает бесплатное использование для некоммерческих проектов. И если некий разработчик возьмёт на свою совесть скормить анализатору кусочек коммерческого кода, в котором подозревает подвох, КМК никто не будет в обиде.

Тут речь не о том, нужен ли автомастерской в условном Урюпинске дефектоскоп или нет (скорее всего нужен). А о том, подойдет ли опыт механика из Формулы-1 в реалиях урюпинской автомастерской. Ведь одно дело обнаружить микротрещину на носовом обтекателе болида Формулы-1, где эта микротрещина может реально привести к разрушению в условиях гонки. И другое дело обнаружить микротрещину на капоте Лады Калина, с которой эта самая Лада Калина пробегает еще лет 10.


Ну и как следствие: в mission-critical софте false-posivite от анализатора может восприниматься как благо, как указание на потенциальную угрозу жизни пользователей софта. Тогда как в провинциальном говнокодинге аналогичный false-posivite — это отвлечение внимания разработка и удлинение сроков разработки (соответственно, увеличение стоимости).

Разгребание false-positive улучшает и кодовую базу, и самих разработчиков даже в Урюпинске. Понятно, что если вас код плюс-минус устраивает, и он сам по себе не является целью или ценностью, а только средством решения каких-то других проблем, тратить ресурсы на внедрение статического анализа нужно с умом.

Правда же сермяжная состоит в том, что чем раньше ошибку настоящую удалось заметить, тем дешевле ее починить, и в Урюпинске это не чуть ни менее верно, чем в Зеленограде, Берлине, Тель-Авиве, или Сан-Франциско.

Анализатор в существующий CI/CD-пайплайн добавляется без существенных затрат, и сам он стоит далеко не как самолет (а с разработчиками еще и на скидку можно договориться гораздо чаще, чем нет), и при этом он снижает нагрузку на все последующие этапы производства ПО, потому что ловит тупые ошибки программирования вроде копи-пасты неудачной прямо на излете, еще до попадания их в репозиторий или основную ветку.

Если действительно учли сроки и бюджет, изучили количество ложных срабатываний на своем коде, и выяснили, что для вас лично анализатор не нужен или даже вреден — честь вам и хвала, снимаю шляпу.
Уже объяснил, потому что повторная проверка этого условия сама по себе не меняет состояние абстрактной машины, которой оперирует стандарт. Пока ваша func_with_side_effects() трогает a[] — все скорее всего будет работать

А вы можете определиться с тем "скорее всего будет работать" или все-таки "сама по себе не меняет состояние"?

Вы понимаете выражение «сама по себе»? У вас в вопросе дихотомия ложная, потому что это одновременно и «скорее всего будет работать» и «проверка сама по себе не меняет состояние», потому что состояние меняет не проверка, а другая функция.
Вы понимаете выражение «сама по себе»?

Я не понимаю причинно-следственных связей между двумя вашими предложениями, а именно:


Уже объяснил, потому что повторная проверка этого условия сама по себе не меняет состояние абстрактной машины, которой оперирует стандарт.

Звучит как будто вы пишете о том, что вы понимаете. Т.е. здесь вы в себе уверены.


Пока ваша func_with_side_effects() трогает a[] — все скорее всего будет работать

А здесь уже сомнения: "скорее всего будет". Т.е. здесь уже не уверены.


Как из уверенности проистекает неуверенность мне не понятно.


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

Приведенный код корректен и будет работать как задумано пока побочный эффект функции func_with_side_effects() приводит к невозможности применения компилятором sequential read/compare elimination. Вот этот инвариант, и подобные ему, чаще всего существует только в голове у автора кода, и только пока он не забыл его. Дальнейшее сохраниение этого инварианта в условиях постоянной работы над этим кодом людьми, которые не знают о нем ничего — дело исключительно случая, и потому целесообразно с точки зрения безопасника считать что он не выполняется уже сейчас. Поэтому и «скорее всего». Если у вас код пишется одним программистом и не изменяется потом вообще никогда, и все инварианты этот отличный программист поддерживает своими силами — смело считайте срабатывание статического анализатора на этом коде ложным. Я же со своей стороны так сделать не могу и не стану, прошу искреннего пардона.
Приведенный код корректен и будет работать как задумано пока побочный эффект функции func_with_side_effects() приводит к невозможности применения компилятором sequential read/compare elimination.

Т.е. по факту:


  • код корректен;
  • анализатор ошибся.

Вот этот инвариант, и подобные ему, чаще всего существует только в голове у автора кода, и только пока он не забыл его.

Чаще всего такой код приходится писать потому, что лучшего решения силами существующей команды, в условиях существующих сроков и бюджетов, просто не представлялось возможным найти.


Принципиальная проблема этого кода в том, что в нем есть побочные эффекты, которые не видны в локальном контексте. И принципиальное решение было бы в устранении самих этих побочных эффектов. Но никак не то, что вы предложили:


например положите его внутрь no-inline-функции с побочными эффектами, таким же образом, как вы уже сделали с func_with_side_effects()

Т.к. вы предложили способ удовлетворить анализатор, а не способ предолеть изначальную хрупкость кода (хрупкость проистекает из-за использования функций с побочными эффектов).


И тут ж хотелось бы определится: мы решаем проблему кода (что может быть крайне сложно, т.к. есть ограничения команды, сроков, бюджетов) или же мы удовлетворяем анализатор.


С моей точки зрения анализатор здесь себя повел неадекватно. У него была информация о том, что неконстантная ссылка на локальные данные утекает куда-то во внешний контекст, а внутри функции вызваются внешние функции с хз какими побочными эффектами. И в таких условиях говорить разработчику "чувак, у тебя здесь ошибка" вместо "чувак, а ты точно уверен, что здесь нужна проверка, т.к. локальных модификаций я здесь не вижу" — это (как по мне) введение разработчика в заблуждение.


Так что в эту игру можно играть и вдвоем: разработчики анализаторов рассказывают на примерах как анализаторы отлавливают ошибки, а пользователи анализаторов на других примерах рассказывают, как анализатор берет на себя слишком много.

И в таких условиях говорить разработчику «чувак, у тебя здесь ошибка» вместо «чувак, а ты точно уверен, что здесь нужна проверка, т.к. локальных модификаций я здесь не вижу» — это (как по мне) введение разработчика в заблуждение.
Мне кажется, мы с вами принципиально по разному подходим к интерпретации сообщений анализатора. Для меня они всегда указывают не на реальные ошибки, а на потенциальные, и все равно человек в итоге принимает решение, ошибка это или нет. Уже писал, что для меня ложные срабатывания анализатора — это уже сигнал, что с кодом что-то не так, и стоило бы посмотреть на него внимательнее, и возможно улучшить так, чтобы у анализатора не было претензий, но и блокировать релиз для всего этого я тоже не стану.

Кажется вы зря придираетесь. Анализатор кода будет всегда выдавать ложно положительные срабатывания, даже если у него есть вся-вся информация, смотрите теорему Райса, например. Если вам не нравятся false positive — не пользуйтесь анализаторами, они без них не могут. Если не нравиться конкретно этот — лучше напишите баг репорт.


«Анализатор ошибся», «анализатор вводит разработчиков в заблуждение», «анализатор тут неадекватен», «анализатор мог, но не сделал» — не способствует конструктивной дискуссии.

Это верно. В конце концов, лучше false positive в пахнущем коде, чем баг репорт от потребителя с финансовыми и/или репутационными последствиями разной стоимости
Отдельным комментарием отвечу на возможный аргумент про то, что так не бывает, и это все теория без практики. Согласным с этим фактом рекомендую прочитать и осознать вот этот отличный note про memset_s
memset may be optimized away (under the as-if rules) if the object modified by this function is not accessed again for the rest of its lifetime (e.g. gcc bug 8537). For that reason, this function cannot be used to scrub memory (e.g. to fill an array that stored a password with zeroes). This optimization is prohibited for memset_s: it is guaranteed to perform the memory write. Third-party solutions for that include FreeBSD explicit_bzero or Microsoft SecureZeroMemory.
Незнание as-if rule ведет к огромному количеству реальных уязвимостей в коде на С. Если вы все еще пишите критический для безопасность код С, уважайте это правило, пожалуйста.

Не использовать глобальную переменную.

Да, но нет.


class Registry {
    int * ptr_{nullptr};
public:
    void register_pointer(int * ptr) { ptr_ = ptr; }
    void remove_pointer() { ptr_ = nullptr; }
    void increment() { if(ptr_) ++(*ptr_); }
};

void func_with_side_effects(Registry & registry) {
    registry.increment();
}

int demo() {
    Registry registry;
    int a[3]{ 0, 1, 2 };

    struct RegistryModificator {
        Registry & registry_;
        RegistryModificator(Registry & registry, int * ptr)
            : registry_{registry}
        {
            registry_.register_pointer(ptr);
        }
        ~RegistryModificator() {
            registry_.remove_pointer();
        }
    } registry_modificator{registry, a};

    if(0 == a[0]) {
        func_with_side_effects(registry);
        if(0 == a[0]) return -1;
    }

    return 0;
}

int main() {
    return demo();
}

Никаких глобальных переменных. Тоже самое сообщение об ошибке.

А PVS-Studio точно выдаст предупреждение на этом коде?

Перед тем, как его сюда запостить, я проверил его на их on-line демонстраторе. Может, конечно, делал что-то не так, но диагностику от PVS-Studio получил.

А смысл его проверять, это старый код, который уже скорее всего не используется. А если и да, предъявить ошибки всё равно некому (у Андрея может быть другое мнение)

который уже скорее всего не используется

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

Если речь именно о коде ядра, то может там и не сильно всё меняется, но маловероятно, что в коде ядра прямо так уж много багов, т.к. на этом коде основано всё остальное, включая весь прикладной софт, и ошибки бы выявились раньше (если это прямо чистой воды ошибка). Хуже если это не ошибка, а какая-то неоптимальность, но найдет ли ее анализатор?

С МС вполне станется баг не фиксить, потому что другой софт использует его как недокоментированную фичу и не работает без этого бага. Вроде даже они сами это рассказывали

Они рассказывали, что у них есть слой совместимости, который может имитировать поведение (возможно, косячное) определенных системных вызовов для определенных версий программ, если данные версии некорректно работают при исправленном поведении системного вызова.

Если этим косячным было что-то популярное, то подменять придётся для кучи программ.
В принципе, сейчас есть версионное API, и таких проблем быть не должно. Хотя оно так устроено, что реализации всё равно валяются в основных файлах, и соответственно подгружаются для всех программ.

Когда для приложения включен режим совместимости, учитывается встроенная в винду база со списком старых версий программ и какие правки к каждой программе применять.
Немного ссылок по теме:
https://techcommunity.microsoft.com/t5/ask-the-performance-team/demystifying-shims-or-using-the-app-compat-toolkit-to-make-your/ba-p/374947
https://www.itnews.com.au/news/windows-compatibility-mode-resurfaces-old-flaws-473058
https://haacked.com/archive/2014/05/27/backwards-compatibility/

Его сейчас не проверить по очень простой причине — pvs-studio не поддерживает компилятор, который используется для сборки. Разве что поменять тулчейн на более свежий, но в этом случае очень большой кусок кода не соберётся в принципе.
Позвольте уточнить, компилятор может позволить себе заоптимизировать даже если в функции будет явное изменение?
Например:
void SetSynchronizeVar(int * z){(*z)++;}

int main()
{
    int flag = 0;
    SetSynchronizeVar(&flag);
    int X, Y = 1;
    if (flag == 0)
    {
        X = Y;
        if (flag == 0)
            return 1;
    }
    return 2;
}
Но ведь мы говорим о том что компилятор должен НЕ учесть изменения внутри функции, а потому заоптимизировать так как буд-то flag всегда равен 0. На godbolt он приведенный мной код он оптимзирует наоборот увидев что flag не будет 0.
Этого «должен НЕ учесть» в вашем оригинальном комментарии не было. Наоборот, он это учтет, и выкинет все дальнейшие проверки как always false. PVS-Studio, кстати, в compiler explorer'е есть тоже, и она действительно выдает неверное предупреждение
11:1: error: V547 Expression 'flag == 0' is always true на вторую проверку.
Вот хороший пример того, что data-flow-анализ у компилятора оказался лучше, чем у анализатора, и вот это предупреждение — действительно ложное срабатывание.

Таки это предупреждение верно, и в этом месте условие действительно всегда верно. Если бы этот код мог быть выполнен, да.
Другое дело, что компилятор смог убедиться, что условие с самого начала всегда ложно — скорее всего он предподсчитал flag и смог выкинуть весь if.

Зря вы вторую часть написали, щас начнется по новой (ну, вон, уже началось).


Сам я хоть PVS-Studio и не пользуюсь (пишу на C/C++ настолько редко и настолько мало, что в этом нет смысла), но возможностями статических анализаторов, встроенных в IDE семейства IntelliJ IDEA, пользуюсь постоянно, на разных языках. Ложные срабатывания бывают, да. Но в подавляющем большинстве случаев ложное срабатывание заставляет переписать код более по-человечески, так, что не только анализатору, но и человеку код понятнее. А если это не тот случай — ну, что ж, замьютить — это хоткей ткнуть (и багрепорт написать, по-хорошему. Многое правят).

Сугубо субъективно. Заранее извините.


Думать об анализаторах как о средстве поиска ошибок мне кажется не правильно, скорее это средство поиска потенциально проблемных мест.


Когда анализатор работает он говорит: "Чувак, я думаю тут лажа — проверь", но он не говорит "Чувак, тут ошибка — исправь". Поэтому анализаторы должны работать с паре с программистом, который знает о системе больше анализатора и сможет принять решение. Может там вообще умудрились подпаяться к транзисторам внутри проца и в любой момент поменять состояние системы (фантазия), анализатор не может знать об этом — для этого нужен программист.


С DMA пример был вообще из фантастики. Что планируется ожидать при чтении из буфера, с которым в текущий момент работает DMA? Судя по примеру проверчется элемент, в который DMA еще не записал данные. Для подобных проверок существуют спец. флаги в регистрах, которые нужно смотреть, но не значения в буфере.

Регистры эти часто memory mapped, а следовательно спец. флаги проверяются точно так же, как значения в буфере — чтением определённого адреса памяти. Не эту переменную требуется объявить volatile, так другую.
Такой подход контрпродуктивен. Неидеальный инструмент вполне может быть полезен, а его использование экономически целесообразным.

Согласен с этим утверждением. Но и, признаться честно, технический евангелизм как практика у меня тоже особых симпатий не вызывает.
Из комментариев узнал что у PVS-studio есть онлайн-проверка, скормил ей одну из ошибок, допущенную мной когда-то. Он ее не увидел:
#include <stdio.h>
#include <inttypes.h>

#define FIFO_BITS 8
#define FIFO_SIZE (1<<FIFO_BITS)
#define FIFO_MASK (FIFO_SIZE-1)

typedef struct{
  uint8_t st, en;
  uint8_t buf[FIFO_SIZE-1];
  uint8_t some_data;
}fifo_t;

void fifo_reset(fifo_t *f){
  f->st = 0; f->en = 0; f->some_data = 100;
}
unsigned int fifo_size(fifo_t *f){
  return (f->st - f->en) & FIFO_MASK;
}
void fifo_push(fifo_t *f, uint8_t data){
  if(fifo_size(f)==1)return;
  f->buf[f->en] = data;
  f->en++;
  f->en &= FIFO_MASK;
}
int16_t fifo_pop(fifo_t *f){
  if(fifo_size(f)==0)return -1;
  uint8_t tmp;
  tmp = f->buf[f->st];
  f->st++;
  f->st &= FIFO_MASK;
  return (int16_t)tmp;
}

int main(){
  fifo_t fifo;
  fifo_reset(&fifo);
  for(int i=0; i<10; i++)fifo_push(&fifo, i);
  for(int i=0; i<10; i++)fifo_pop(&fifo);
  for(int i=0; i<1000; i++){
    fifo_push(&fifo, i);
  }
  for(int i=0; i<10000; i++){
    int16_t res = fifo_pop(&fifo);
    printf("%i: %i (%i)\n", i, (int)res, fifo.some_data);
    if(res < 0)break;
  }
}

Если что, ошибка не в форматах printf (данный пример я воспроизводил чуть ли не по памяти), а в выходе за пределы массива.

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


Не нужно учить хабаровчан как правильно любить ваш продукт. Пожалуйста, воздержитесь от написания третей статьи.

Про отсутствие гарантий претензии к разработчикам статических анализаторов не по адресу. Претензии следует направлять Алану Тьюрингу, какого, дескать, лешего вы, уважаемый сэр, посмели доказать неразрешимость проблемы останова?

Автор примите непрошенных совет — не нужно писать таких статей. Выглядит как попытка оправдаться или попытка кого-то переубедить, но haters gonna hate. Вместо разбора одного синтетического примера и привязки статьи к треду с критикой я бы рассказал о том, что анализаторы это сложно, что C++ это сложно. Вывод о том, что разбираемый пример это мелочь и время на него тратить не надо оставил бы для читателей. Так вы и критику сможете учесть, уменьшите вероятность повторения такого непродуктивного разговора и дистанцируетесь от контекста, где к вам придираются по мелочам.

Примечание. В очередной версии PVS-Studio 7.10 исправили данный случай:
char get();
int foo(volatile char *p, bool arg)
{
    if (p[1] == 1)
    {
        if (arg)
            p[0] = get();
        if (p[1] == 1)          // OK
            return 1;
    }
    // ....
    return 3;
}
Теперь не ругаемся.
Давно хочу спросить… Объясните логику анализатора (не вашего)
Неопределенный порядок выполнения

#pragma pack(push,1) 
typedef union
  {
  struct eeprom_tag
    {                                                                          
    signed short D0;
    signed short D1;
    unsigned char  CRC; 
    };
  unsigned char EepromAlignment[((sizeof(struct eeprom_tag)+2)/4)*4]; 
  }                                                                            
  eeprom_t;  
#pragma pack(pop)    
...
eeprom_t Ep;
...
Ep.CRC=GetCRC((unsigned char*)&Ep, sizeof(struct eeprom_tag)-sizeof(Ep.CRC) );   


Откуда предупреждение? В аргументах функции побочных эффектов нет, а sizeof вообще вычисляется на этапе компиляции.
Как писать чтобы предупреждения не было?
Иногда анализторы просто ошибаются и выдают ложные срабатывания. :)
Тот анализатор которым я пользуюсь, выдает именно такие предупреждения…
Не разу он мне не выдал такого предупреждения, которое мне удалось исправить.
Хочется понять его логику. Может дело во мне.
Only those users with full accounts are able to leave comments. Log in, please.