Пользователь
0,0
рейтинг
24 октября 2013 в 13:09

Разработка → Пример использования статического анализатора

Когда PVS-Studio сообщили о том, что они наконец-то выпустили standalone версию, не требующую для своей работы Visual Studio, я, конечно же, не мог пройти мимо :) До этого я уже игрался с пробной версией на коде одного из старых проектов. Сейчас же появилась возможность посмотреть на код нашего последнего проекта, собирающегося в среде разработки AVR Studio (которая eclipse-based).

Для работы требуются файлы сразу после препроцессора. Среда AVR Studio это умеет, с одним маленьким исключением — после включения флага «Только препроцессор» на выходе действительно появляются файлы после препроцессора — но по-прежнему с расширением.о вместо ожидаемого .i. Ну что ж, 5-минутный скрипт на Питоне решает это недоразумение, и анализатор отлично запускается!

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

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

Но самой интересной, из-за чего я и пишу этот пост, была одна-единственная строчка «Possible NULL pointer dereferencing»…


Так сложилось, что повсюду в коде использовалась конструкция вида

void fun(error_t * perr)
{
 *perr = SUCCESS;
 ...
 if (something)
 {
    *perr = SOME_ERROR;
 }
}


И буквально в нескольких функциях конструкция была немного другая:

void init(void)
{
  error_t err = SUCCESS;
  ...
  fun(&err);
}


Пока однажды после одного из небольших рефакторингов в одном из мест не появилось следующее:

void some_init(void)
{
  error_t *perr = SUCCESS;
  ...
  some_fun(perr);
}


Собственно на эту строчку и ругнулся анализатор. SUCCESS, конечно же, имел значение 0.

Отмотаем время немного назад, к тому моменту, когда это изменение попало в репозиторий.

После рефакторинга весьма большой набор автоматических тестов продолжал успешно выполняться. Ревью кода эту строчку оставило незамеченным (уж слишком часто в коде мелькали строчки *perr = SUCCESS).

Дней через 30 после того самого коммита ночные тесты упали в первый раз. Воспроизвести падение не получилось.

Потом тесты упали еще раз. И еще. Опытным путем было установлено, что падение происходит в среднем один раз на тридцать запусков набора тестов.

На поиски ошибки командой было потрачено около 50 часов. Безуспешно. Правда, удалось локализовать коммит, после которого все началось — но причина так и не была найдена.

Причина, кстати, была на две ступеньки ниже. Функция some_fun(perr) внутри себя вызывала some_other_fun(perr), а та — some_third_fun(perr). И уже в some_third_fun(perr) был код, проверяющий возникновение ошибки:

for(number_of_loops)
{
  some_action(perr);
  if (*perr != SUCCESS)
    return;
}


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

Резюме: ошибка, на обнаружение которой было безуспешно потрачено около 50 часов, при помощи однократного запуска анализатора была обнаружена и исправлена менее чем за час!

Убедительный довод для использования анализатора? Увы, не всегда. В частности, у нас как раз тот самый случай: проект с оплатой по схеме time and material. Поскольку потраченные на поиск 50 часов оплачиваются заказчиком, для руководства внедрение анализатора означает прямые убытки :(((

Еще, к слову: в проекте используется FreeRTOS. Так вот, в ее коде не было ни одного предупреждения!

И да, пост этот исключительно из любви к искусству анализаторам.
Александр Лотохов @olekl
карма
45,0
рейтинг 0,0
Реклама помогает поддерживать и развивать наши сервисы

Подробнее
Реклама

Самое читаемое Разработка

Комментарии (14)

  • +8
    Ну, говорить что, это «не ваш случай я бы не стал». Я абсолютно уверен, что вы и ваши коллеги с удовольствием бы потратили вот эти вот 50 часов на что-нибудь другое. Что-то, что повысило бы желание продолжать платить у вашего заказчика. Да и вам самим, наверняка, интереснее делать что-то новое, чем искать иголку в стоге сена днями напролёт.
    • +2
      Мы бы, конечно, потратили. Но к сожалению, мы не принимаем решений о приобретении инструментов.
  • +14
    Большое спасибо за заметку. Мы были приятно удивлены. Во-первых, тому, что написали про положительный опыт (обычно пишут по негативный :). Во-вторых, что удалось прожевать проект от AVR Studio, хотя мы никак к этому не адаптировали анализатор.

    Кстати, эта статья демонстрирует, как не надо использовать статический анализатор. Если использовать анализ регулярно (инкрементальный анализ), то этой и возможно многих более простых ошибок просто бы не возникло. Понятно, что это не тот случай (не существует плагина для AVR Studio). Однако это намек тем, что использует Visual Studio или Embarcadero RAD Studio.

    Ещё раз спасибо за статью.

    Дополнительные ссылки:
    1. Статья о standalone версии. PVS-Studio теперь работает и без среды Visual Studio или C++Builder – проверяем препроцессированные файлы от чего угодно.
    2. Скачать и попробовать.
    3. Поговорить с нами о процессе приобретении и обсудить цену.

    • 0
      Кстати, если бы можно было запустить анализатор из командной строки и получить отчет в читаемом виде, то анализатор бы легко встраивался в процесс непрерывной интеграции и запускался при каждой сборке.

      В AVR же используется слегка модифицированный avr-gcc, а для gcc вроде поддержка заявлена. Да и С — это С, нет?
      • +4
        Ужас в том (для нас), что все это давно поддерживается инструментом, но никто этим не пользуется.
  • 0
    >>Поскольку потраченные на поиск 50 часов оплачиваются заказчиком, для руководства внедрение анализатора означает прямые убытки :(((

    Скорее не убытки, а недополученный по данному проекту доход. Но, конечно, если новые проекты находить сложнее чем их реализовывать, то использование инструментов, повышающих скорость разработки экономически нецелесообразно. Пока удается находить проекты, для которых скорость разработки не решающее преимущество.
    • 0
      Находить новые, конечно же, сложнее, чем реализовывать имеющиеся. Но это, на мой взгляд, лишь повод реализовывать их более качественно, чтоб у заказчика возникало желание продолжать сотрудничать. Но этот фактор, увы, в финансовых отчетах не отражается.
  • –6
    в остатке я так и не понял, как повторить конфигурацию в своем проекте. зато узнал про некий чудо-софт, который как-то связан с тратами заказчика. досада, плюс знакомые лозунги в резюме наводят на мысль о том, что «любовь к искусству анализаторам», вполне может быть продажной. %)
    • +5
      Что именно вы не поняли? Как в вашем проекте сказать компилятору, чтобы он остановился после препроцессора? Или как в чудо-софте заполнить целых три поля, значение которых ясно из названий этих полей (и про заполнение которых была недавно статья авторов чудо-софта)? А продажность увидеть можно везде, было бы желание.
    • +1
      Дружище funca, будешь работать на нас? Писать статьи за деньги? Предложение актуально!
    • +2
      Представьте себе, любовь бывает непродажной. Мы даже не знаем, кто этот таинственный Александр. Он давно интересуется нашим продуктом (что видно из предыдущей статьи). И мы общались с ним, но к сожалению так и не смогли убедить приобрести PVS-Studio. :)

      Более того, есть ещё более таинственный поклонник. Выглядит, как замаскированные под другого человека, наши статьи. Но нет. Действительно, кто-то написал пару статей про PVS-Studio. :) Но никому не докажешь, что кто-то сам может такое просто так написать. Везде видят заговор. :)
      • –6
        любой процесс с определенной целью — конечен. истинная любовь — вечна. поэтому она не может иметь определенной цели.
        маркетиноговые цели данного поста, даже если это всего лишь случайное совпадение, в ней прописаны практически прямым текстом. значит речь тут не об истинной любви, как минимум.
        Мы даже не знаем, кто этот таинственный Александр. Он давно интересуется нашим продуктом (что видно из предыдущей статьи). И мы общались с ним, но к сожалению так и не смогли убедить приобрести PVS-Studio. :)

        ну вот и вы туда же — смотрите статьи, общаетесь, продаете и публично отрицаете знакомство, ага? как вы так умудряетесь строить отношения со своими клиентами и почему стесняетесь знакомиться с ними, если основной метод, как следует из ваших предыдущих постов, это прямые продажи? :)
  • +3
    Вы все еще не используете assert? Тогда мы идем к вам.

    void fun(error_t * perr)
    {
    assert(perr);
    *perr = SUCCESS;

    if (something)
    {
    *perr = SOME_ERROR;
    }
    }
    • 0
      Это тот самый assert, который разворачивается в ((void)0) в release конфигурации? Если да, то он поможет исключительно в случае, когда управление пройдет через строку с ним и непременно в отладочной конфигурации, это обычно бывает с часто вызваемым кодом или при наличии очень хорошего тестового покрытия. А статический анализ найдет такой фрагмент, даже если код с ним никогда не вызывается.

      Так что assert, конечно, очень полезен, но далеко не панацея.

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