Ну вот тут у нас и остается один шаг до собственного SDK, раз уж мы решили так яростно вмешиваться в работу юзерспейса, что запускаемся там init'ом, контролируем сокеты и т.п. «Так долго боролся с драконами, что сам им стал».
Китайские ODMы ничего не могут сделать с проблемами безопасности, потому что у них нет профильных специалистов, поэтому отдать туда разработку — гарантировать себе наличие проблем безопасности и\или бэкдоров.
Еще один пример UINT24 — это UEFI Firmware File System v2, где Intel не хватило одного байта в заголовке размером 24 байта, и чтобы не поплыло все выравнивание его решили откусить от размера файла, потому что 16 мегабайт (0xFFFFFF) должно хватить всем. В результате, понятно, не хватило, и пришлось выдумывать FFSv3, править все заголовки, обновлять все парсеры, и до сих пор еще встречаются прошивки, которые тома с v3 толком прожевать не могут и зависают в случайных местах.
Короче, если у вас там не прямо жесть-жесть, то экономить один байт, чтобы потом иметь геморрой и новый заголовок на восемь — это плохая негодная стратегия, не рекомендую.
Разгребание false-positive улучшает и кодовую базу, и самих разработчиков даже в Урюпинске. Понятно, что если вас код плюс-минус устраивает, и он сам по себе не является целью или ценностью, а только средством решения каких-то других проблем, тратить ресурсы на внедрение статического анализа нужно с умом.
Правда же сермяжная состоит в том, что чем раньше ошибку настоящую удалось заметить, тем дешевле ее починить, и в Урюпинске это не чуть ни менее верно, чем в Зеленограде, Берлине, Тель-Авиве, или Сан-Франциско.
Анализатор в существующий CI/CD-пайплайн добавляется без существенных затрат, и сам он стоит далеко не как самолет (а с разработчиками еще и на скидку можно договориться гораздо чаще, чем нет), и при этом он снижает нагрузку на все последующие этапы производства ПО, потому что ловит тупые ошибки программирования вроде копи-пасты неудачной прямо на излете, еще до попадания их в репозиторий или основную ветку.
Если действительно учли сроки и бюджет, изучили количество ложных срабатываний на своем коде, и выяснили, что для вас лично анализатор не нужен или даже вреден — честь вам и хвала, снимаю шляпу.
И в таких условиях говорить разработчику «чувак, у тебя здесь ошибка» вместо «чувак, а ты точно уверен, что здесь нужна проверка, т.к. локальных модификаций я здесь не вижу» — это (как по мне) введение разработчика в заблуждение.
Мне кажется, мы с вами принципиально по разному подходим к интерпретации сообщений анализатора. Для меня они всегда указывают не на реальные ошибки, а на потенциальные, и все равно человек в итоге принимает решение, ошибка это или нет. Уже писал, что для меня ложные срабатывания анализатора — это уже сигнал, что с кодом что-то не так, и стоило бы посмотреть на него внимательнее, и возможно улучшить так, чтобы у анализатора не было претензий, но и блокировать релиз для всего этого я тоже не стану.
Этого «должен НЕ учесть» в вашем оригинальном комментарии не было. Наоборот, он это учтет, и выкинет все дальнейшие проверки как always false. PVS-Studio, кстати, в compiler explorer'е есть тоже, и она действительно выдает неверное предупреждение 11:1: error: V547 Expression 'flag == 0' is always true на вторую проверку.
Вот хороший пример того, что data-flow-анализ у компилятора оказался лучше, чем у анализатора, и вот это предупреждение — действительно ложное срабатывание.
Не приведи рандом добиваться этого. Хотите считать это «сперва добейся» — ваше право, я делюсь тем опытом, который имею, и теми условиями, в которых приходится работать, если у вас другие условия — отлично, это не значит что вам нужно стремиться к моим, или мне — к вашим. В моем мире не просто «с анализатором лучше», а давно уже «без анализатора полный конец обеда», если у вас не так — я реально рад.
Приведенный код корректен и будет работать как задумано пока побочный эффект функции func_with_side_effects() приводит к невозможности применения компилятором sequential read/compare elimination. Вот этот инвариант, и подобные ему, чаще всего существует только в голове у автора кода, и только пока он не забыл его. Дальнейшее сохраниение этого инварианта в условиях постоянной работы над этим кодом людьми, которые не знают о нем ничего — дело исключительно случая, и потому целесообразно с точки зрения безопасника считать что он не выполняется уже сейчас. Поэтому и «скорее всего». Если у вас код пишется одним программистом и не изменяется потом вообще никогда, и все инварианты этот отличный программист поддерживает своими силами — смело считайте срабатывание статического анализатора на этом коде ложным. Я же со своей стороны так сделать не могу и не стану, прошу искреннего пардона.
Ваша позиция тоже понятная, вопросов больше не имею. Меня спросили почему код воняет — я ответил так, как я сам это понимаю. Меня в детстве безопасники покусали, и теперь я предпочитаю дуть на воду, чем обжигаться на молоке, особенно в проектах, над которыми работает больше сотни людей каждый день, а баги в нем стоят миллионы настоящих долларов.
Вы понимаете выражение «сама по себе»? У вас в вопросе дихотомия ложная, потому что это одновременно и «скорее всего будет работать» и «проверка сама по себе не меняет состояние», потому что состояние меняет не проверка, а другая функция.
Отдельным комментарием отвечу на возможный аргумент про то, что так не бывает, и это все теория без практики. Согласным с этим фактом рекомендую прочитать и осознать вот этот отличный 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 ведет к огромному количеству реальных уязвимостей в коде на С. Если вы все еще пишите критический для безопасность код С, уважайте это правило, пожалуйста.
Уже объяснил, потому что повторная проверка этого условия сама по себе не меняет состояние абстрактной машины, которой оперирует стандарт. Пока ваша func_with_side_effects() трогает a[] — все скорее всего будет работать, как только при очередном рефакторинге перестанет, компилятор праве сразу же выбросить повторную проверку и начать возвращать -1 прямо из первой.
Если это для вас ожидаемое поведение — отлично, только вот для большинства пользователей статического анализатора это большой сюрприз, и сообщение об этой повторной проверке для них — это не ложное срабатывание, а указание на то, что осторожно, здесь в засаде сидит медведь, если вы его туда посадили сами — классно, но если нет — обратите внимание, он там.
Перепишите проверку if (0 == a[0]) так, чтобы компилятор не смог использовать оптимизацию sequential read/compare elimination, например положите его внутрь no-inline-функции с побочными эффектами, таким же образом, как вы уже сделали с func_with_side_effects(). Повторное сравнение одного и того же элемента non-volatile-массива с константой само по себе не меняет состояние абстрактной машины, которой оперирует стандарт, и потому может быть пропущено в полностью конформной программе на С и С++ благодаря as-if rule. Хотите сделать так, чтобы сравнение это туда не попало — скажите компилятору об этом явно.
Проблема в том, что здесь мы играем в «мой data-flow-анализ против твоего data-flow-анализа», т.е. в «сможет статический анализатор использовать всю информацию, которая доступна компилятору, или нет». Понятно, что без встраивания очень глубоко в сам компилятор ответ будет «нет, не сможет», но даже тот факт, что анализатор на этот код указывает как на странный — это уже благо, потому что код этот воняет, и может сломаться от случайного рефакторинга, как уже выше написали до меня. False positive намного лучше true negative, и намного лучше незнания о том, что код может перестать работать в случайного изменения в другом месте, а вы потом потратите две недели в попытках понять, что вообще произошло, и почему у вас результат сборки в дизассемблере так сильно отличается от модели этого результата в голове.
Поддержу. Ответ на вопрос «зачем тебе личные проекты» очень простой — «мне с ними лучше, чем без них». Это творчество чистое практически, что само по себе в кайф, а после этого творчества остается полезный для общества продукт, за который потом тебе другие люди скажут спасибо разными способами. Код при этом сильно выигрывает у разного рода самодельных табуретов и прочего подобного тем, что он тиражируется без затрат, и потому одной хорошей программой можно улучшить немного жизнь очень большого числа даже незнакомых тебе людей. В итоге сплошные плюсы, а то, что на это тратится время — хобби оно на то и хобби, чтобы время на удовольствие разменивать.
Курочка по зернышку, скажем так, и фаззингом уже давно занимаемся, и санитайзерами собираемся сами и других заставляем собираться, и указатели толстые принесли в прошивку, чтобы совсем голышом не сидеть. Это все намного лучше, чем ничего, в любом случае.
Я согласный, я тут больше про то, что по unsafe очень удобно искать в случае серьезных непонятных проблем, чтобы проверить эти блоки первыми. К сожалению, нарушения инвариантов в unsafe нелокально, т.е. способно приводить к поломкам safe-кода хрен знает в какой дали от самого нарушения, но даже с этим всем оно сильно лучше, чем у остальных, особенно у C и C++.
Короче, если у вас там не прямо жесть-жесть, то экономить один байт, чтобы потом иметь геморрой и новый заголовок на восемь — это плохая негодная стратегия, не рекомендую.
Правда же сермяжная состоит в том, что чем раньше ошибку настоящую удалось заметить, тем дешевле ее починить, и в Урюпинске это не чуть ни менее верно, чем в Зеленограде, Берлине, Тель-Авиве, или Сан-Франциско.
Анализатор в существующий CI/CD-пайплайн добавляется без существенных затрат, и сам он стоит далеко не как самолет (а с разработчиками еще и на скидку можно договориться гораздо чаще, чем нет), и при этом он снижает нагрузку на все последующие этапы производства ПО, потому что ловит тупые ошибки программирования вроде копи-пасты неудачной прямо на излете, еще до попадания их в репозиторий или основную ветку.
Если действительно учли сроки и бюджет, изучили количество ложных срабатываний на своем коде, и выяснили, что для вас лично анализатор не нужен или даже вреден — честь вам и хвала, снимаю шляпу.
11:1: error: V547 Expression 'flag == 0' is always trueна вторую проверку.Вот хороший пример того, что data-flow-анализ у компилятора оказался лучше, чем у анализатора, и вот это предупреждение — действительно ложное срабатывание.
Если это для вас ожидаемое поведение — отлично, только вот для большинства пользователей статического анализатора это большой сюрприз, и сообщение об этой повторной проверке для них — это не ложное срабатывание, а указание на то, что осторожно, здесь в засаде сидит медведь, если вы его туда посадили сами — классно, но если нет — обратите внимание, он там.