Главная » Хабрахабр » Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Как PVS-Studio оказался внимательнее, чем три с половиной программиста

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

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

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

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

И поясняющая картинка, которая была прикреплена к письму. Вот текст письма, публикуемый с разрешения автора.

Running with most recent version of PVS-Studio for personal use. V560 warnings here are all false. Outer one is done for speed — inner ones are still needed and non are always true or false. Basically, «IF» statement is correct.

Письмо

Видите ошибку? Теперь, уважаемый читатель, ваше время проверить себя!

А единорог пока немного подождёт. Ваше время проявить внимательность.

Единорог-ждун

Когда настроен найти ошибку, она находится. После вводной части статьи, скорее всего многие нашли ошибку. Намного сложнее заметить ляп после прочтения письма, где это называется «ложными срабатываниями» :).

Ещё раз рассмотрим условие: Теперь пояснение для тех, кто поленился искать баг.

if (!((ch >= 0x0FF10) && (ch <= 0x0FF19)) || ((ch >= 0x0FF21) && (ch <= 0x0FF3A)) || ((ch >= 0x0FF41) && (ch <= 0x0FF5A)))

Автор кода планировал проверить, что символ не входит ни в один из трёх диапазонов.

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

Если выполнилось условие:

!((ch >= 0x0FF10) && (ch <= 0x0FF19))

то вычисление выражения прерывается согласно short-circuit evaluation. Если условие не выполняется, то значение переменной ch лежит в диапазоне [0xFF10..0xFF19]. Соответственно, четыре дальнейших сравнения не имеют смысла. Все они будут ложными или истинными.

Смотрите, если ch лежит в диапазоне [0xFF10.. Ещё раз. 0xFF19] и продолжается вычисление выражения, то:

  • ch >= 0x0FF21 — всегда false
  • ch <= 0x0FF3A — всегда true
  • ch >= 0x0FF41 — всегда false
  • ch <= 0x0FF5A — всегда true

Об этом и предупреждает анализатор PVS-Studio.

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

Чтобы исправить ситуацию, надо добавить дополнительные скобки:

if (!(((ch >= 0x0FF10) && (ch <= 0x0FF19)) || ((ch >= 0x0FF21) && (ch <= 0x0FF3A)) || ((ch >= 0x0FF41) && (ch <= 0x0FF5A))))

Или переписать условие:

if (((ch < 0x0FF10) || (ch > 0x0FF19)) && ((ch < 0x0FF21) || (ch > 0x0FF3A)) && ((ch < 0x0FF41) || (ch > 0x0FF5A)))

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

const bool isLetterOrDigit = (ch >= 0x0FF10 && ch <= 0x0FF19) // 0..9 || (ch >= 0x0FF21 && ch <= 0x0FF3A) // A..Z || (ch >= 0x0FF41 && ch <= 0x0FF5A); // a..z
if (!isLetterOrDigit)

Обратите внимание, что я убрал часть скобок. Как только что мы видели, большое количество скобок вовсе не помогло избежать ошибки. Скобки должны облегчать чтение кода, а не усложнять. Программисты хорошо помнят, что приоритет сравнений = выше, чем у оператора &&. Поэтому здесь скобки не нужны. А вот если спросить, что приоритетней — && или ||, многие растеряются. Поэтому для задания последовательности вычислений &&, || скобки лучше поставить.

главу: Выравнивайте однотипный код «таблицей»). Почему лучше писать || в начале я описывал в статье "Главный вопрос программирования, рефакторинга и всего такого" (см.

Скачивайте и начинайте использовать PVS-Studio. Всем спасибо за внимание. Он поможет выявлять многие ошибки и потенциальные уязвимости на самых ранних этапах.

How PVS-Studio Proved to Be More Attentive Than Three and a Half Programmers. Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov.


Оставить комментарий

Ваш email нигде не будет показан
Обязательные для заполнения поля помечены *

*

x

Ещё Hi-Tech Интересное!

[Перевод] Цена персональной безопасности для директоров крупнейших IT-компаний в год

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

[Перевод] Создание игры для Game Boy

Несколько недель назад я решила поработать над игрой для Game Boy, создание которой доставило мне большое удовольствие. Её рабочее название «Aqua and Ashes». Игра имеет открытые исходники и выложена на GitHub. Как мне пришла в голову эта идея Недавно я ...