Pull to refresh

Comments 27

… статический анализатор может рассмотреть, как будет работать код при всех возможных вариантах входных данных...

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


Это распространенное заблуждение. Проблема останова актуальна только для машины Тьюринга, поскольку она бесконечна. Для любой конечной системы возможно решение этой проблемы с использованием более «мощной» системы.
Если программа работает с сетью, вам потребуется более мощная система, чем весь Интернет.
Если под «анализируемой программой» подразумевается только та часть, которая исполняется непосредственно на устройстве, а не все что можно «выполнить удаленно» на другой машине в сети, то вовсе нет.
Т.к. такая связь с сетью ничем по сути не отличается от обычного ввода/вывода и принципиально анализ программы не усложняет.

Другое дело что сложность анализа может расти сильно нелинейно от сложности системы. Но чисто теоретически, к проблеме останова это отношения ни имеет.
Другими словами, статический анализатор может рассмотреть, как будет работать код при всех возможных вариантах входных данных. Но делать он это будет 150 лет на кластере, где вдобавок установлен невероятный объём памяти.

А кто будет делить данные на фактор-группы? Скажем, если ошибка будет только в случае определенного совпадения байтов или чего-то еще, на что вы не будете расчитывать?

Я не обещал, что такой анализатор не будет давать ложно позитивных и ложно негативных срабатываний. :) Просто он будет работать чуть тщательнее, чем обычно.
А вы проверяли PVS-Studio Valgrind'ом? Было ли найдено что-нибудь интересное?
Пока не пробовали. Там ожидаемо будет 100500 утечек памяти, так как узлы дерева не удаляется в процессе работы, так как построенное дерево используется до конца работы программы. А в конце просто завершить программу, без предварительного удаления дерева. Этакая оптимизация скорости работы.
Ну, такие структуры часто заворачиваются в какой нить shared_ptr, так что как такового мемори лика нет — указатель на память не теряется, просто живет всю жизнь процесса. При выходе из main счетчик станет равен 0 и память освободится. Если же там сырой указатель, то да valgrind скажет что течет.
Освобождение памяти отсутствует сознательно. Удаление сотен тысяч отдельных объектов это медленно. Впрочем, их можно почистить быстрее, так как для их выделения используется свой собственный менеджер памяти. Но даже он отключен, так как всё равно это пустая потеря времени.

Механизм освобождения в менеджере памяти используется только при прогоне юнит-тестов, чтобы не сжирать слишком много памяти при проверке различных тестовых *.i файлов. В случае же работы в пользовательском режиме память освобождает операционная система, завершая процесс. Такой подход экономит порядка 0.1 секунду (значение приблизительное, давно не замерял время) на запуск. Проверили 1000 файлов и вот уже экономия в 100 секунд: приятно.
А как же ваше предыдущее мнение, что качество кода важнее всего? :-) Теория — это хорошо, а в реальности на рекламу на хабре у вас ресурсы есть, а вот на повышение качества кода — ресурсов нет. Даже на бесплатный Valgrind ресурсов не нашлось.

Неужели вы поменяли своё мнение, и теперь считаете, что лишний тестировщик выгоднее автоматической проверки? Да ещё и бесплатной?

Видите насколько ваши слова расходятся с делом? :-) Хотя в реальности вы правы — реклама действительно важнее качества кода.

Увидев КПДВ, я подумал было, что вы проверили код статического анализатора PVS-Studio статическим анализатором PVS-Studio :)

UFO just landed and posted this here
Член off является переменной беззнакового типа, а значит он всегда больше или равен нулю. Таким образом, условие (!(ce->off >= 0)) всегда ложно.

На это автор кода может возразить, что так задумано на случай, если тип поменяется в будущем на знаковый. Учитывая, что тип поля объявлен не напрямую, а через typedef, шансы на это вполне себе возрастают. Если дальше пойдёт какая-то магия с указателями, то лучше уж при потенциальной замене типа в будущем на goto fail пойти, чем на UB напороться.


Кстати, вопрос вам. Такой код:


int cmp = compare(a, b);
if(cmp < 0) { ... }
else if(cmp == 0) { ... }
else if(cmp > 0) { ... }

Будет ли PVS-Studio ругаться в последней строчке, что условие всегда истинно?

Дополнительный вопрос — а если там в конце ещё один else?
Анализтор промолчал в обоих случаях. Во втором случае стоит что-то сказать, взял на карандашь. Спасибо.

Тогда ещё вопрос:


int *x = ...
if(x == NULL) { ... }
else if(x != NULL) { ... }

Тут тоже молчите, что x != NULL всегда истинно?

Нет-нет, я не с целью поддеть вас, у меня исключительно профессиональный интерес ;-)

Нам пришлось убрать подобный код из-за предупреждений какого-то из компиляторов.

Сложно сказать, стоит ли так писать. Иногда (из-за паранойи) хочется туда ещё и ветку else добавить

if(x == NULL) { ... }
else if(x != NULL) { ... }
else {assert(false);}


Если у нас операторы == и != переопределены, и в переопределении есть тонкая ошибка (ну или баг в компиляторе, или полуработающий процессор) срабатывание ветки else возможно.

Другой момент, что я бы, пожалуй, предпочел тут ложное срабатывание статанализатора и пометку такого места руками. Потому что ляп программиста — все таки вероятнее. Ну скажем вторая проверка должна использовать хх, а не х. То есть такой код — он немного пахнет.

Альтернативный вариант:


if(x == NULL) { ... }
else {
  assert(x != NULL);
  ...
}

Так вы и документировали вторую ветку (на случай если первая разрастётся), и добавили отладочную проверку. Можно бы было ещё так:


if(x == NULL) { ... }
else {
  assert(x != NULL); // PVS-Studio: static assert
  ...
}

Комментарий специального вида после ассерта должен указывать, что PVS-Studio в состоянии статически вывести, что этот ассерт всегда истинный. Если со временем окажется, что это не так (например, условие в верхнем if поменяется, а про else все забудут), то PVS-Studio начнёт ругаться, что больше не может вывести истинность утверждения, помеченного комментарием.

Хорошая идея, но первый вариант легче читается.

PVS-Studio начнёт ругаться, что больше не может вывести истинность утверждения, помеченного комментарием.

Ну изменение х из прерывания или гонку тредов он вряд ли верно поймет. Думаю, что volatile просто отключит эту проверку.
На это автор кода может возразить

Может. Это его право. Задача статического анализтора указать на подозрительный код. Задача программиста оценить это сообщение и предпринять меры, если они нужны или подавить предупреждение.
В отличие от статанализа динамический анализатор, как правило, указывает на реальные ошибки. Причем ещё и те, что встречаются у большинства пользователей.
Кто-то из компиляторов ругается.
Скорее всего окажется, что это действительно ошибка, и правильный код должен быть таким:

char *str = foo();
if (*str == '\0')


Мне кажется в правильном коде должна быть еще проверка на NULL, потому что указатель мы получаем из вызова foo и он вполне может оказаться нулевым. Поэтому я бы предложил такой вариант:
char *str = foo();
if (str && *str == '\0')
Sign up to leave a comment.