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

Когда 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. Так вот, в ее коде не было ни одного предупреждения!

И да, пост этот исключительно из любви к искусству анализаторам.