Отдельным комментарием отвечу на возможный аргумент про то, что так не бывает, и это все теория без практики. Согласным с этим фактом рекомендую прочитать и осознать вот этот отличный 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++.
Вот еще примеры. По сути, это такой способ пояснить анализатору, что вот эта твоя функция — это memcpy на самом деле, или malloc, а то, что она выглядит странно и вызывается через указатель на указатель — не повод пропускать ее на taint-анализе, и не повод ругаться на нее за то, что она повторяет якобы системную библиотеку, которой у нас все равно нет.
Спасибо вам за ваш труд, серьезно. Проблема только в том, что гарантировать и не нужно, нужно улучшить статут кво. Бизнесу не интересно переписывать все их миллионы строк кода на языках, которые непонятно когда появятся, потому что продукт нужно выпускать уже в этом году, а код сам по себе — это не продукт, это технические детали реализации. Нельзя отмахиваться от инструментов улучшения существующей кодовой базы только потому, что инструменты эти не универсальные, или пропускают ошибки, или не находят ошибки, потому что нет вообще ничего совершенного и даже идеальные программы все равно на реальном железе исполняются.
Ну там в Rust пришлось внести очень серьезные ограничения на инварианты для ссылочных типов, чтобы анализ этот стал дешевле, но по сравнению с C и C++ компилятор медленный все равно. Совет не то, чтобы вредный, но вот это «прокрустово ложе до горизонта» нужно принять (кому-то это дается легче, кому-то — тяжелее). Есть огромное множество языков без него на выбор, и появление новых языков с ним я приветствую, потому что оно расширяет этот самый выбор. «Пусть расцветают сто цветов, и процветают сто школ».
Тут уже volatile все вспомнили, да и линтер хорош даже для системного программиста. Более того, для системного он особенно хорош, потому что цена ошибки там намного выше, и потому любые инструменты по их раннему обнаружению — это безусловное благо.
Все вот эти хитрые трюки, которые необходимы на системном уровне — их тоже можно и нужно пояснить и компилятору, и анализатору. «Я знаю, что делаю, не мешай», как с unsafe в Rust. И это тоже замечательно, посколько позволяет потом обычным поиском по тексту найти все места с практической магией, и обратить на них внимание в первую очередь при отладке действительно сложных проблем.
Принуждение можно организовать по-разному, в Rust вот реализовали прямо в компиляторе (и это отлично), а на С никто не мешает внутри отдела RnD потребовать использования анализатора, либо поставить его на pre-commit hook и выдавать предупреждения сразу же там, плюс еще раз снова на ревью. Понятно, что это решение технических проблем административными мерами, но это тем не менее решение, и оно вполне подходит тем, кто свой код на Rust или SPARK не может просто так переписать — его там до этого писали 15 лет, и там его 500к строк, например.
Даже ложные срабатывания это полезно, и время на их начальное разгребание (т.к. когда у вас уже 10к+ строк кода, а анализатор до этого не использовался никогда) окупится сторицей и найденными багами (не видел я вот ни разу еще, чтобы в большом проекте на С или С++, в котором до этого анализаторы не использовались, ничего бы не нашлось), и повышением вашего профессионализма как разработчика на этих языках (потому что расследования ответов на вопросы вроде «да какого хрена ему тут то не нравится?» сильно расширяют кругозор).
Уже приводил этот пример в осуждениях PVS-Studio на opennet.ru, процитирую сам себя оттуда:
боролся я однажды с интересным багом при инициализации дополнительных ядер процессора AMD Merlin Falcon в прошивке. Нулевое ядро там назвают BSP (BootStrap Processor), а остальные — AP (Application Processor), так вот, инициализация очередного AP иногда зависала на ровном месте в ~0.1% случаев (а т.к. процесс работы прошивки до этого времени детерминирован, то в таких зависаниях чаще всего виновато оборудование или код, который его неправильно использует).
Зависающая прошивка — это show stopper, т.е. пока это не починим, дальше разрабатывать нет смысла. Я просидел две недели в попытках понять, что не так, и пошел пробовать статические анализаторы, которые давали попробовать (AMI использует CppCheck для своего кода, но он совсем слабый по сравнению с любыми коммерческими анализаторами).
В итоге PVS-Studio нашла переменную в структуре, у которой был пропущен аттрибут volatile, а в коде было что-то вроде такого:
// Prepare CPU context structure
…
apStructPtr->InSync = 0;
…
// Send context to AP
…
// Wait for AP sync
while(apStructPtr->InSync) {
// Still waiting
}
Т.к. ни apStructPtr, ни InSync не были помечены как volatile, то компилятор просто выбрасывал цикл, т.к. он не влияет на наблюдаемое поведение и аналогичен while(0), в результате получалась гонка между внутренними процессами в CPU и продолжением исполнения, и выигрывали её чаще всего внутренние процессы, т.е. все работало почти всегда. Но потом стало больше ядер, вышли новые процессоры и более долгой синхронизацией, и все сломалось.
А PVS-Studio сразу сообщила, что вот тут место подозрительное, проверьте. Проверил, добавил volatile, баг исчез. Вот так и помогает, вполне реально.
According to the C99 Rationale [C99 Rationale 2003], other than calling a limited, prescribed set of library functions, «the C89 Committee concluded that about the only thing a strictly conforming program can do in a signal handler is to assign a value to a volatile static variable which can be written uninterruptedly and promptly return.»
However, this issue was discussed at the April 2008 meeting of ISO/IEC WG14, and it was agreed that there are no known implementations in which it would be an error to read a value from a volatile sig_atomic_t variable, and the original intent of the committee was that both reading and writing variables of volatile sig_atomic_t would be strictly conforming.
Более того, компилятору тоже придется указать на то, что данные в этом A[] могут меняться «внезапно» вне текущего контекста исполнения, и анализатор эти указания скорее всего тоже примет к сведению. И в #3 весь буфер будет volatile, в #2 — тоже, плюс там еще синхронизацией обмазываться в любом случае.
Если кусок памяти не помечен специальным образом, то оптимизирующий компилятор по стандарту вправе считать, что данные в нем «внезапно» не изменяются, и потому удалить вот эту повторную проверку. Анализатор, кстати, сообщает именно об этом, т.е. «ребята, data flow-анализ есть не только у меня, но и у вашего компилятора, вы действительно имели в виду то, что написали?»
Не расстраивайтесь, Андрей, люди по мере набора опыта придут к пониманию, что статический анализатор — это благо. Поначалу всегда кажется, что это все не нужно, и все ошибки на поверхности, код почитай и все увидишь. На деле же любые инструменты, помогающие в разработке и отладке на ранних этапах, снижают в итоге и количество ошибок, и цену этих ошибок.
Писал много раз, и еще напишу: изучайте ваши инструменты, анализаторы, санитайзеры, фаззеры, SAT-солверы, профилировщики, отладчики, средства символьного исполнения и т.п., потому что все они и сильно повышают качество вашего ПО, и сильно ускоряют его разработку, и снижают накал рутины, потому что действия по монотонной проверке инвариантов теперь исполняет машина, которая не скучает, не отвлекается, и не устает.
Тут еще очень сильная культурная разница, потому что в западных крупных компаниях нужно непрерывно сохранять лицо, и никогда не упоминать в разговоре с кем либо, кроме начальника 1:1 то, что тебе активно не нравится, потому что это «негативно» и «демотивирует окружающих». Люди совершенно не хотят слушать от своих лидеров, что «вокруг огонь, все в огне, и мы в аду», и потому сказать про это по сути можно только своему терапевту, своему начальнику на 1:1, и своим друзьям из России, потому что у нас тут принято делиться проблемами с близкими, т.к. это и позволяет легче переносить их коллективно, и сплачивает саму группу во время их решения. «Совместный труд, он объединяет!» как говорил известный кот из советского мультфильма. Я пробовал так вести себя на западе, и это очень сильно помешало продвижению по карьерной лестнице. Не повторяйте моих ошибок, ребята.
Зависит от загруженности менеджера, в основном, и от уровня позиций. Чем выше и уровень позиций, и квалификация участников, тем реже 1:1 проводятся, но когда их нет по несколько месяцев, стоит напомнить, что неплохо было поговорить про накопившееся.
Если это для вас ожидаемое поведение — отлично, только вот для большинства пользователей статического анализатора это большой сюрприз, и сообщение об этой повторной проверке для них — это не ложное срабатывание, а указание на то, что осторожно, здесь в засаде сидит медведь, если вы его туда посадили сами — классно, но если нет — обратите внимание, он там.
Все вот эти хитрые трюки, которые необходимы на системном уровне — их тоже можно и нужно пояснить и компилятору, и анализатору. «Я знаю, что делаю, не мешай», как с unsafe в Rust. И это тоже замечательно, посколько позволяет потом обычным поиском по тексту найти все места с практической магией, и обратить на них внимание в первую очередь при отладке действительно сложных проблем.
Уже приводил этот пример в осуждениях PVS-Studio на opennet.ru, процитирую сам себя оттуда:
Если кусок памяти не помечен специальным образом, то оптимизирующий компилятор по стандарту вправе считать, что данные в нем «внезапно» не изменяются, и потому удалить вот эту повторную проверку. Анализатор, кстати, сообщает именно об этом, т.е. «ребята, data flow-анализ есть не только у меня, но и у вашего компилятора, вы действительно имели в виду то, что написали?»
Писал много раз, и еще напишу: изучайте ваши инструменты, анализаторы, санитайзеры, фаззеры, SAT-солверы, профилировщики, отладчики, средства символьного исполнения и т.п., потому что все они и сильно повышают качество вашего ПО, и сильно ускоряют его разработку, и снижают накал рутины, потому что действия по монотонной проверке инвариантов теперь исполняет машина, которая не скучает, не отвлекается, и не устает.