Pull to refresh

Анализ утилит статического анализа C++ кода

Reading time 6 min
Views 12K
Анализ следующих утилит:Все необходимое можно найти пройдя по ссылкам, а мы сразу перейдем к делу.

Тест 1:

int main()
{
	vector<int> v;
	v.reserve(2);
	assert(v.capacity() == 2);
	v[0];
	v[0] = 1;
	v[1] = 2;
	cout << v[0] << endl;
	v.reserve(100);
	cout << v[0] << endl;
	return 0;
}

Внимательно изучите данный код, сколько проблем можете выявить вы?
Разумеется все нужные заголовки инклудированы(iostream, vector, cassert).
До того как мы обсудим версии профессионалов на счет этого кода, произведем анализ известных утилит для статического анализа исходников. Далее дан результат выше указанных утилит.

RATS: ничего
Cppcheck: ничего
Graudit: ничего
g++(с флагом -Wall): ничего

Может и на первый взгляд все не так уж плохо или вообще нет ничего плохого? Не говорите это при Саттере (между прочим пример взят из его книги «Новые сложные задачи на С++. 40 новых головоломных примеров с решениями»).
На первой строке в теле функции main() мы объявляем вектор v состоящий из целых чисел. Потом резервируем место для 2 элементов. Прежде чем перейдем к следующей строке, взглянем на стандарт С++. Что она говорит нам о методе vector::reserve? (warning: вольный перевод)
"
void reserve(size_type n);
Сообщает о планированном изменении в размерах, возможно соответственное управление распределением памяти. После reserve(), capacity()(емкость) больше или равна аргументу переданному reserve() в случае перераспределения памяти. В противном случае равна прежнему значению capacity(). Перераспределение памяти проиходит тогда и только тогда, когда текущее значение capacity() меньше чем значение аргумента reserve(). Она не меняет размер контейнера и требует в худшем случае линейного времени выполнения. При значении аргумента больше чем max_size(), генерирует исключение long_error. В случае перераспределения делает недействительными все ссылки, указатели и итераторы, ссылающиеся на элементы контейнера."

Пока этого достаточно, пойдем дальше. Строка assert(v.capacity() == 2); в лучшем случае плохая, во первых reserve() гарантирует увелечение емкости и в этом случае assert просто лишняя строка кода (которые так любят вставлять «программисты», которые слепо верят ассерту после каждой операции и считают это профессиональным тоном). Во-вторых, как мы узнали из самого стандарта, reserve() может увеличить емкость контейнера больше, чем заданный аргумент, так что Саттер рекомендует assert(v.capacity() >= 2). Рекомендую вообще убрать ее.
Дальше — хуже. «Оператор vector<T>::operator[] может, но не обязан выполнять проверку диапазона. В стандарте об этом ничего не сказано, так что разработчик стандартной библиотеки имеет полное право как добавить такую проверку, так и обойтись без нее.» Саттер. Всему причина эффективность, проверка занимает некоторое время, многочисленные такие проверки потребуют значительную часть времени работы программы. Но это не значит, что твоя эффективная программа лучше надежной программы Васи. Побробуй замени строчку v[0]; на v.at(0); прога вылетит с исключением. Что и в большинстве случаев нужно надежному коду. vector<T>::at выполняет проверку диапазона значения индекса. Строгая рекомендация использовать этот метод в таких, и не только, ситуациях.

Строки v[0] = 1; v[1] = 2; вполне работают, убедится можно откомпилировав пример. Но надо помнить разницу между size/resize и reserve/capacity. resize изменяет количество элементов в контейнере на указанное значение аргумента путем добавления или удаления их из конца контейнера. О методе reserve мы узнали из стандарта, добавим что он увеличивает размер внутреннего буфера, чтобы он был способен вместить указанное количество элементов (как минимум). "Мы можем безопасно использовать operator[](или метод at) только для изменения элементов, которые реально содержатся в контейнере, т.е. реально учтены в size." Понятно? Так и утилитам статического анализа тоже не понятно. Но ведь проблема значительная. Саттер и многие другие профи пишат в своих книгах о тонких моментах в кодинге на любимом С++. На первый взгляд может показаться, что умение решать проблемы и знание синтаксиса языка более чем достаточно для разработки. Но мы то с тобой знаем, что сами себя обманываем, сколько из вас вставляли cout-ы на каждой строке кода, чтобы выявить проблему? Сколько из вас не мог терпеть отладку, которую выполнял уже третий день? Так вот, есть люди, которым не присущи такие вещи, в их число входят люди, владеющие профессиональными навыками программирования, которые не ленятся идти дальше и подряд изучают книги таких гуру как Саттер и Майерс (и многие другие).

И еще, в примере 1 нас ждет последняя логическая ошибка, как вы думаете что выводит cout << v[0]; после reserve(100)? Правильно, в лучшем случае ноль! Заметьте, что до резервирования мы дали v[0] ненулевое значение. «Проблема» опять в функции резервирования.
Так в чем дело в общем случае? В том, что мне нужна утилита, которая подскажет, что в случае v.reserve(2); v[0] = 1; лучше использовать v.resize(2); v[0] = 1; или v.reserve(2); v.push_back(1)! А рассмотренные нами утилиты мило промолчали. Еще мне нужна утилита, проверяющая неправеренные границы, и в случае v[0]; посоветовать использовать v.at(0). А наши рассмотренные утилиты опять мило промолчали. Отсюда мораль, в тонких ситуациях не полагайтесь на такие утилиты, как RATS, Cppcheck, и тем более на Graudit. Даже на компилятор с включенным высшим уровнем предупреждений иногда следует смотреть глазами кодера-параноика.
Ладно, утилиты ведь не зря занимают место на ваших винтах, и не зря на нем трудились разработчики.

Тест 2:

Пойдем дальше, увеличим дозу ошибок в коде. Рассмотрим тот же код, с новой функцией prettyFormat (пример опять из Саттера). Она получает число и буфер, потом вставляет это число в полученный буфер при помощи sprintf.
void prettyFormat(int i, char* buf)
{
    sprintf(buf, "%4d", i);
}

int main()
{
    vector<int> v;
    v.reserve(2);
    //....
    char buf[5];
    prettyFormat(10, buf);
    return 0;
}

Как видно, добавили prettyFormat, и строку char buf[10]. Хотя код из предыдущего примера нам больше не понадобиться, но пусть остаеться, может одна из утилит что-то увидит? Вот результат:

RATS: на строке char buf[5]; "High: fixed size local buffer. Extra care should be taken to ensure that character arrays that are allocated on the stack are used safely. They are prime targets for buffer overflow attacks." Советует быть внимательным при использовании символьных массивов, типо они беспомощны при атаках на переполнение буфера.
Cppcheck: ничего
Graudit: ничего
g++(с опцией -Wall): ничего

О переполнении буфера сказано очень много, так что будем кратки и только покритикуем. У меня такое впечатление, что утилиты для статического анализа предназначены только для нахождения возможных случаев переполнения буфера. Впрочем, несколько из них молчат даже в таких случаях (как Cppcheck или Graudit). Компилятора не виню, он не обешал мне, что найдет проблемы, а вот утилиты специально предназначенные для таких вещей в большинстве случаев мило молчат. В таких ситуациях нам нужно получить от утилит предупреждения и рекомендации использовать альтернативы sprintf (ну и для других возможных функции). Об альтернативных ситуациях следует прочитать в «Саттере» (речь идет о std::stringstream, std::strstream, boost::lexical_cast, впрочем последняя основана на std::stringstream, и наконец snprintf, позволячщая задать размер буфера, обеспечив тем самым защиту от переполнения буфера).

Тест 3:

Добавим в функцию prettyFormat неинициализированный указатель.
int* prettyFormat(int i, char* buf)
{
    sprintf(buf, "%4d", i);
    int* a;
    return a;
}

int main()
{ 
    ...

В остольном код не изменился по сравнению с предыдущими тестами. Это не то, что может смахивать на серъезный тест, но даст много интересной информации на счет используемых утилит. В частности, про Cppcheck говорилось, что она не дублирует предупреждения компилятора, но как мы вскоре увидим — это блеф. А другие утилиты просто не видят проблему или и вправду не дублируют предупреждения компилятора. Ниже приведен результат.

RATS: тоже, что и в предыдущем тесте(речь идет о строке char buf[5];)
Cppcheck: на строке int* a; выдает (error) Uninitialized variable: a.
Прекрасно, но…
g++(с опцией -Wall): In function ‘int* prettyFormat(int, char*)’:
warning: ‘a’ is used uninitialized in this function.

Что-то знакомое не правда ли? Тоже самое предупреждение, которое наконец дал Cppcheck.
Graudit вообще молчит.

Тест 4:

Добавим юмора, изменим функцию prettyFormat так:
int* prettyFormat(int i, char* buf)
{
    sprintf(buf, "%4d", i);
    int* a;
    fopen("filename", "r");
    char buf2[5];
    strcpy(buf2, buf);
    return a;
}

Добавились fopen, которая открывает не знаю что, добавился локальный буфер buf2, в которую с помощью strcpy копируется buf, получаемый как аргумент. Посмотрим, кто сколько даст.

RATS: Для строк char buf2[5]; char buf[10] то же предупреждение про внимательность использования символьных буферов фиксированного размера.
И наконец: «High: strcpy
Check to be sure that argument 2 passed to this function call will not copy
more data than can be handled, resulting in a buffer overflow.»

Проверьте, не скопирует ли второй аргумент больше данных, чем может содержать buf2.
Неплохо, во многих ситуациях бы помог. Пойдем дальше.
Cppcheck: (error) Unitialized variable: a.
Что-то знакомое, то же что и в предыдущем примере, а жаль.
g++(с опцией -Wall): То же, что и в предыдущем тесте.

С задачей более-менее справился RATS, остальных — расстрелять! Почему все так плохо? Во-первых, «писавшие» эти утилиты ребята сами толком не гуру С++, многие больше всего любят просто публиковать программу и все. Во-вторых, многие тонкие ошибки сложно обнаружить, и с ними справляются в основном коммерческие утилиты, хотя сам еще не пробовал. В-третьих, читай книги, журналы, в частности Хабр и не будешь нуждатся в утилитах подобных этим.

Спасибо за внимание!

P. S. Хотел опубликовать в блог «Ревизия кода», кармы не хватает. Нет, не то, чтобы хочу карму таким способом увеличить, так на всякий случай, чтоб толпа не заорала — «а почему не там-сям-...».

P. P. S. Огромное спасибо Хабрасообществу — так держать!
Tags:
Hubs:
+40
Comments 34
Comments Comments 34

Articles