15 December 2011

cppcheck

I.

В большом материале про новый стандарт C++ я написал, что самый главный источник всех undefined behaviour в нашем проекте -- непроинициализированное в конструкторе поле класса (обычно типа bool). Суть этой проблемы ясна -- человеку свойственно ошибаться, а разрыв объявления переменной и ее инициализации (зачастую, эти точки вообще находятся в разных файлах) сильно способствует выявлению этого свойства бренного человеческого тела.

Компиляторы, с которыми мы в данный момент работаем, по поводу этой ситуации, тривиально выявляемой машиной, молчат как рыба об лед. Стали копать в сторону статических анализаторов. Сначала я решил попробовать нереально распиаренную на хабре (ваши волосы на ногах станут на 50% шелковистее!) PVS-Studio


Блевотина и гей атрибутика как бэ намекают нам 

Тут все не заладилось с самого начала. При установке триальной версии, софтина каким-то волшебным образом обнаружила фантомную инсталляцию MSVS 2008 (которая была давно полностью деинсталлирована) и попыталась в нее "интегрироваться". Закончилось это все банальным зависоном, а сам инсталлятор пришлось просто прибить.

В MSVS 2005 анализатор вроде как интегрировался, правда IDE стала ругаться на краш какого-то компонента при старте... Компиляция проекта с включенным анализатором выявила следующие замечательные особенности PVS-Studio. 
Первое -- дикие тормоза. Компиляция замедлилась минимум раза в два, если не больше. Очевидно, что при таких раскладах постоянно использовать анализатор не будешь, потому что хороший плюсовых проект и без этих паразитов учит программистов дзену самопознания и смирения. Кто понял жизнь -- тот не спешит. Это про "плюсовиков", инфа 100%. 
Второе -- неинициализированные поля класса анализатор не находит. А ведь для этих целей он и ставился. 
Третье -- анализатор упорно лезет в содержимое stdafx.h и рассказывает мне о страшных проблемах Qt и boost, которые, конечно, интересны, но проблемы моего лично проекта мне как-то поважнее. 

Если добавить к этому еще какие-то совершенно дурацкие ограничения на рапорт по ошибкам, вроде урезания текста сообщений, то вы поймете, что желание снести к такой-то матери эту чудесную программу у меня возникло буквально через две минуты ее использования... Но и тут оказалась засада! Процесс инсталляции толком то не прошел, поэтому при удалении я тоже получил кучу страшных сообщений об ошибках, которые, в итоге, заканчивались крашем программы.
Ну и завершилась вся эта история сломанной IDE -- в MSVS 2005 после деинсталляции стало падать окно со свойствами проекта, ругалось, что не находит компонент DataGrid'а, переставьте мол .NET 2.0. Танцы с бубнами вокруг этой ошибки, в итоге, не к чему не привели, поэтому я попал еще и на полную переустановку IDE.
Такая вот замечательная программа PVS-Studio, за которую добрые люди просят каких-то жалких 3'500 евро. 

II.

Но подделка под названием PVS-Studio была тут к ночи помянута, ибо, на самом деле, это добрый и светлый пост об совсем другом анализаторе -- cppcheck, который мне (в комментах к той самой записи про новый стандарт) посоветовал добрый человек, которому я, пользуясь случаем, еще раз говорю "дякую"!


Сначала -- про среду вокруг cppcheck, а не про сам анализ.

В IDE я это дело не интегрировал, хотя, по идее, сделать это при большой желании можно. 
Есть возможность запуска анализа с командной строки либо работать через простенький GUI (кросс-платформенный, написанный на Qt).
Сам GUI довольно бестолковый и нормально организовать работу с его помощью вряд ли получится по причине его stateless'ности. Две сущности, которые можно сохранить на диске, это отчет и проект. 
Отчет -- результат анализа. Главная проблема при сохранении -- нет возможности запомнить что ты уже просмотрел и обработал, а что -- нет. 
Проект -- грубо говоря, это набор путей и настроек для анализа. И все. 

Если не брать какие-то относительно мелкие шероховатости, вроде отсутствия игнорирования предупреждений конкретного типа, то самая серьезное проблема среды вокруг cppcheck -- невозможность работать в инкрементальном режиме, когда ты создал проект, просмотрел все проблемы, а дальше уже реагируешь только на новые предупреждения, которые возникают только для изменившихся частей исходника.

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

Допилить инфраструктуру вокруг анализатора было бы относительно не сложно. И, скорее всего, будь я лет на десять моложе, может я бы и сделал это доброе дело. В данный момент, на это нет времени, и неизвестно когда оно может появиться в принципе. Дык, если кому хочется попрактиваться, по ходу пьесы еще и сделав доброе дело для других людей -- welcome, готов помочь с консультациями что и как надо сделать, плюс курировать такого рода работу. 

III.

Теперь про самое интересное, про анализ. 

Все обнаруженные проблемы делятся на на пять больших групп -- ошибки, предупреждения, стилистические предупреждения, переносимость и производительность. Как я писал выше, внутри самих групп, увы, фильтрации по типам нету. 

Самое главное -- непроинициализированные в конструкторе поля в классах находит анализатор четко. У нас их нашлось порядком, хотя реально мест, которые могли привести к каким-то проблемам, оказалось совсем чуть-чуть. И тем не менее.
Тут же отмечу, что при настройке анализатора надо помнить один важный момент, связанный с заголовочными файлами. Если никаких дополнительных путей не указывать, то заголовочные файлы будут браться только из текущей директории. Гору стандартных хидеров, конечно, особого смысла подключать для анализа нету, но есть смысл каким-то образом подключить хидер, который объяснит анализатору, что используемый в проекте Platform::byte, на самом деле есть unsinged char, потому что иначе вы никакого предупреждения про проблемное поле этого типа в классе, разумеется, не получите.

Еще одну реальную ошибку анализатор нашел в коде, использующем итераторы. Исходная версия писалась для цикла while() и в ней был инкремент итератора. Потом цикл переделали на for() и забыли убрать "++i". Вот этот баг анализатор, к нашему удивлению и восхищению, обнаружил (эта ветка кода не использовалась на практике, иначе проблема была бы быстро обнаружена). 

Далее -- утечки памяти. Нет, у нас в коде их, я надеюсь, нет -- сплошь и рядом умные указатели и практически никаких ручных delete (в крайнем случае пулы, которые в деструкторах обязательно проверяют, что все блоки вернули на место). Проблему анализатор обнаружил в коде, который на нашей embedded железке провоцирует исчерпание памяти и реагирование программы на kernel panic по этому поводу.
Насчет утечек -- я бы не сильно обольщался по этому поводу. Конечно, если вы пишите в стиле Кармака, и у вас на один alloc для указателя на стеке идет десять free (просто потому что у вас столько точек выхода из функции), то на забытое удаление вам точно укажут. Только вот как это поможет людям с руками, растущими не из жопы, для меня остается загадкой.

Далее -- разыменовывание нулевого указателя. В 99.9% речь идет о том, что анализатор не понимает, что мы кидаем исключение и выходим из функции (у нас для выбрасывания исключений используются специальные макросы, но это тема для отдельного большого разговора).
Кстати, авторы анализатора предлагают делать тюнинг, связанный с управлением ресурсами -- вы можете зарегистрировать собственные парные функции типа alloc/free для того, чтобы при анализе можно было обнаруживать утечки и по ним. Как я уже писал, это вряд ли будет полезно плюсовикам, а вот возможность указывать на свой выход через исключение -- пожалуй, было бы полезно.

Теперь про всякие стилистические проблемы и прочую мелочевку.

Обнаружение методов, к которым можно приписать const. Работает замечательно, весьма полезная штука.

Обнаружение мест, где отсутствие элементов в контейнере проверяется через size(), а не через empty(). Почему-то считается, что это влияет на производительность.

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

Постинкремент для сложных типов (типа iter++). Тут да, если вы ваша цель -- максимальная производительность, то такая запись скорости не прибавляет.

На этом, пожалуй, все. Упомяну только вот еще о каком нюансе -- про условную компиляцию и дефайны. В нашем коде условная компиляция используется, по-моему, от силы в паре мест, а вот для кода, где этим балуются постоянно, нужно держать ухо востро. У анализатора есть опция быстрого анализа таких мест, и опция для полного анализа. Так вот, много раз подумайте, перед тем, как ее включать, потому что это фактически приводит к тому, что файл анализируется многократно, со всеми возможными комбинациями частей кода, т.е. сложность анализа возрастает в 2^N раз, где N это число мест, где у вас используется условная компиляция.

Вот теперь все, любите друг друга!

No comments:

Post a Comment