Главная
Блог разработчиков phpBB
 
+ 17 предустановленных модов
+ SEO-оптимизация форума
+ авторизация через соц. сети
+ защита от спама

Анализируем начальный код с поддержкой cppcheck

Anna | 24.06.2014 | нет комментариев

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

Вовсе незадолго cppcheck помог обнаружить уязвимость в плане Xorg, которая существовала там примерно 23 года! Он помог теснее тысячам программистов по каждому миру, на официальном сайте дозволено обнаружить информацию о обнаруженных с поддержкой cppcheck уязвимостях в программах, и данный список непрерывно растёт. Выходит, если вы хотите знать, отчего необходимо применять cppcheck неизменно и всюду — умоляю под кат.

CppCat и cppcheck

Начну со сопоставления этих утилит, от того что данная просьба озвучивалась многократно в комментариях. Разработчики CppCat теснее независимо провели такое сопоставление (с PVS-Studio), но с тех пор утекло много воды, а сопоставление не дюже непредвзято, так как PVS-Studio (насколько я понимаю всю замысловатость фразы «Please write us to get a price for PVS-Studio. Please specify interesting license type.») не предуготовлена для программистов-одиночек. CppCat же, как и cppcheck, доступен всякому (с оговоркой на привязку к VisualStudio определённых версий и лицензию на год).

Сравнить эти анализаторы будет сложно: у меня не имеется под руками ни одной версии Visual Studio под Linux. Следственно вначале я ограничусь обзором теснее проанализированного кода в недавнем обзоре CppCat: натравлю cppcheck на Notepad , приведу статистику ошибок/предупреждений, которые дозволено будет сравнить с теснее готовым обзором CppCat.

Параллельно пробую поставить в виртуальной машине CppCat. Нужно сказать, что позже того как Visual Studio 2010 был установлен, инсталлятор недолго думая выдал следующее:

image

Из-за чего тестирование усложнилось квестом обнаружь-поставь-Visual-Studio-2013-переустанови-IE-11-перезагрузись-обновись, что на виртуалке, не обременённой обновлениями, заняло ровно полдня.

Что в результате? При открытии плана Notepad Visual Studio горделиво завис. Попытка сделать новейший план привела к тому, что в обзоре CppCat посыпались ошибки о не обнаруженных заголовочных файлах. По сему сопоставлять придётся с тем, что имеем в предыдущей статье. Правда Visual Studio я ставлю фактически в 1-й раз, но результат «юзабилити» налицо.

Подготовка

Так как cppcheck — план с открытым начальным кодом, никто не мешает загрузить самую свежую версию изгита и скомпилировать её независимо. Cppcheck предуготовлен для разработчиков, следственно компиляция программы из начального кода не должна вызывать каких-либо задач:

unzip cppcheck-master.zip
cd cppcheck-master
make

Готово. Я намеренно взял свежую версию из гита и положил её в отдельную папку — с ней будет проще трудиться, так как в последующем cppcheck дозволено крепко усовершенствовать и настроить «под себя».

Тестовая конфигурация: RHEL 6.1, процессор i5-2400 @ 3.10GHz (для оценки времени работы анализатора).

Комфорта ради действия будут производится в командной строке — при желании их дозволено будет повторить (в Linux:). Безусловно, cppcheck имеет несколько плагинов для знаменитых IDE, но сегодня не об этом.

Обзор Notepad

cppcheck устроен так, что все предупреждения отсортированы по категориям. По умолчанию включён только один вид обзора — error (ошибки). Ошибки невозможно игнорировать, так как если cppcheck выдал error — это место придётся переписывать в 99% случаев. Стержневой улов cppcheck — утраты памяти и переполнение буфера, а это теснее чего-то стоит.

Может появиться резонный вопрос — как исследовать notepad в операционной системе Linux, если notepad использует экстраординарно WInAPI? Результат примитивен — cppcheck на моей памяти исключительный анализатор, тот, что не привязан к сборочной среде либо операционной системе. Он использует свой лексический анализатор, не требует непременного присутствия всех заголовочных файлов, лояльно относится к хитросплетениям классов и т. п. Это восхитительное качество разрешает применять cppcheck где желательно и для чего желательно, чего не скажешь о CppCat (см. квест по установке выше).

Обзор с поддержкой cppcheck примитивен до безобразия:

./cppcheck-master/cppcheck -q -j4 npp.6.5.3.src/

Пока проводится примитивный обзор «из коробки». Команда определяет два параметра -q («тихий» режим — не выводить прогресс выполнения на экран) и -j4 — многопоточный обзор в 4 потока по количесву ядер процессора.

Итог выполнения предыдущей команды:

[npp.6.5.3.src/PowerEditor/src/tools/ChangeIcon/ChangeIcon.cpp:214]: (error) Mismatching allocation and deallocation: resData
[npp.6.5.3.src/PowerEditor/src/tools/ChangeIcon/ChangeIcon.cpp:216]: (error) Mismatching allocation and deallocation: resData
[npp.6.5.3.src/scintilla/lexers/LexBash.cxx] -> [npp.6.5.3.src/scintilla/lexers/LexBash.cxx:194]: (error) Internal error. Token::Match called with varid 0. Please report this to Cppcheck developers

Время работы — 5 минут. В глаза сразу кидается оплошность “(error) Internal error. Token::Match called with varid 0. Please report this to Cppcheck developers”. Это обозначает, что взамен бага в анализируемой программе нашёлся баг в самом анализаторе:) Сделаем скидку на то, что план заточен под Win, и cppcheck не подозревает, что обозначают DWORD, LPTR и т. п. Допустимо, в Win он покажет себя напротив.

Реально нашлась каждого одна оплошность (с разницей в 2 строчки). Хорошо, допустимо, что автор notepad сам пользуется cppcheck. Участок кода, тот, что вызвал у cppcheck сомнение:

	BYTE* resData = new BYTE[cbRes];
	LPBYTE writePtr = resData;
...
	if(!UpdateResource(hUpdate, RT_GROUP_ICON, lpResName, resLangId, resData, cbRes)) { _tprintf(_T("Unable to update icon group\n")); delete resData; return false; }
	IFDEBUG( _tprintf(_T("Updated group %d (lang %d)\n"), lpResName, resLangId); )
	delete resData;
	}

Это выглядит как ложное срабатывание, правда исходники, Добросовестно говоря, шокируют. UPD: это всё-таки undefined behaviour, массив необходимо освобождать с поддержкой оператора delete [].

Но это ещё не всё. Дело в том, что лозунгом cppcheck является неимение неверных срабатываний, то есть по умолчанию сканер ищет только дюже скептические ошибки — переполнение буфера, утраты памяти. Позже того как все ошибки обнаружены, дозволено просканировать вторично, включив флаги предупреждений:

./cppcheck-master/cppcheck -q -j4 --enable=performance,portability,warning,style npp.6.5.3.src/ 2> npp.out

Применяется параметр –enable, тот, что включает категории проверок:

— performance — задачи продуктивности;
— portability — задачи совместимости;
— warning — предупреждения — подозрительные места программы;
— style — ошибки жанра программирования.

В таком режиме вылавливается львиная доля стилистических/логических ошибок и возможных багов (т. е. ошибки, в которых cppcheck «не уверен»). Время сканирования — 5 минут. Итог я сразу отправил в файл, Дабы собрать статистику.

Каждого сообщений:

wc -l < npp.out
379

Маленькая статистика по типу обнаруженных ошибок:

tr '()' '*' < npp.out | cut -d* -f2 | sort | uniq -c
      3 error
     39 performance
     14 portability
    211 style
    112 warning

Статистика по сообщениям

sort -t] -k2 npp.out | grep -v '(error)' | cut -d\) -f2- | sed "s/'[^']*'/%{VAR}/g" | sort | uniq -c | sort -n
      1  Function parameter %{VAR} should be passed by reference.
      1  memset() called to fill 0 bytes of %{VAR}.
      1  scanf without field width limits can crash with huge input data.
      1  The class %{VAR} does not have a constructor.
      1  Unused variable: ent
      1  Unused variable: loc
      2  Array index %{VAR} is used before limits check.
      2  Checking if unsigned variable %{VAR} is less than zero.
      2  Found duplicate branches for %{VAR} and %{VAR}.
      2  The class %{VAR} defines member variable with name %{VAR} also defined in its parent class %{VAR}.
      2  Unsigned variable %{VAR} can't be negative so it is unnecessary to test it.
      2  %{VAR} should return %{VAR}.
      3  scanf without field width limits can crash with huge input data on some versions of libc.
      4  Same expression on both sides of %{VAR}.
      4  %{VAR} does not have a copy constructor which is recommended since the class contains a pointer to allocated memory.
      5  Assignment of function parameter has no effect outside the function.
      5  Ineffective call of function %{VAR}. Did you intend to call %{VAR} instead?
      7  Consecutive return, break, continue, goto or throw statements are unnecessary.
     11  Exception should be caught by reference.
     11  The extra qualification %{VAR} is unnecessary and is considered an error by many compilers.
     12  Variable %{VAR} is reassigned a value before the old one has been used.
     22  Variable %{VAR} is assigned a value that is never used.
     26  Variable %{VAR} is assigned in constructor body. Consider performing initialization in initialization list.
     42  C-style pointer casting
     98  Member variable %{VAR} is not initialized in the constructor.
    108  The scope of the variable %{VAR} can be reduced.

Каждого 28 уникальных сообщений.

Сообщения «The scope of the variable %{VAR} can be reduced», «C-style pointer casting», «Variable %{VAR} is assigned in constructor body.» дозволено не рассматривать — это стилистические рекомендации, дюже характерные для многих планов, написанных на ветхих плюсах.

Море переменных не инициализировано в конструкторе: (warning) Member variable %{VAR} is not initialized in the constructor. Это ошибку cppcheck считает предупреждением. Допустимо, поведение такого кода зависит от компилятора, потому что npp каким-то чудом работает.

Пример

//[npp.6.5.3.src/PowerEditor/src/WinControls/AnsiCharPanel/ansiCharPanel.h:46]: (warning) Member variable 'AnsiCharPanel::_ppEditView' is not initialized in the constructor.
class AnsiCharPanel : public DockingDlgInterface {
public:
	AnsiCharPanel(): DockingDlgInterface(IDD_ANSIASCII_PANEL) {};

	void init(HINSTANCE hInst, HWND hPere, ScintillaEditView **ppEditView) {
		DockingDlgInterface::init(hInst, hPere);
		_ppEditView = ppEditView;
	};

    virtual void display(bool toShow = true) const {
        DockingDlgInterface::display(toShow);
    };

    void setParent(HWND parent2set){
        _hParent = parent2set;
    };

	void switchEncoding();
	void insertChar(unsigned char char2insert) const;

protected:
	virtual BOOL CALLBACK AnsiCharPanel::run_dlgProc(UINT message, WPARAM wParam, LPARAM lParam);

private:
	ScintillaEditView **_ppEditView;
	ListView _listView;
};

Инициализация переменной не в конструкторе, а в функции init, так что на мой взор ворнинги по делу.

(style) Same expression on both sides of ‘||’. Проверка одного и того же данные. Эту же ошибку выдавал CppCat, впрочем то ли в той статье версия npp ветхая, то ли ошибку теснее пофиксили, но теперь тот самый код выглядит так:

while (closeFound.success && (styleAt == SCE_H_DOUBLESTRING || styleAt == SCE_H_SINGLESTRING) && searchStartPoint <= caret);

А это — новая добыча cppcheck:

if (!(!commentLineSybol || !commentLineSybol[0] || commentLineSybol == NULL))

(performance) Variable ‘lineIndent’ is reassigned a value before the old one has been used. По сути — двойное присваивание. Традиционно это следствие копипасты, но cppckeck характеризует такую ошибку как ошибку продуктивности. Данный код стоит проверить, так как неведомо, что подразумевал автор программы. Таких двойных присваиваний, а также неиспользуемых значений переменных по коду дюже много:

        int lineIndent = lineStart;
...
        lineIndent = _pEditView->execute(SCI_GETLINEINDENTPOSITION, i);
		_pEditView->getGenericText(linebuf, linebufferSize, lineIndent, lineEnd);

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

(portability) The extra qualification ‘FunctionListPanel::’ is unnecessary and is considered an error by many compilers. Пригодное предупреждение, которого физически нет и не планируется в CppCat: ошибки переносимости между различными платформами (portability). Данный кусок кода будет трудиться не во всех компиляторах:

virtual BOOL CALLBACK FunctionListPanel::run_dlgProc(UINT message, WPARAM wParam, LPARAM lParam);

Это «политкорректное» сообщение на самом деле обозначает, что код не соберётся без костылей в компиляторе gcc. Если вы планируете запускать приложение огромнее чем на одной платформе — cppcheck станет отличным подспорьем.

(style) Exception should be caught by reference. Увлекательное предупреждение — ловить исключение следует по ссылке, а не по значению:

        catch(std::exception e) {
		::MessageBoxA(NULL, e.what(), "Exception", MB_OK);
		return -1;
	}

(style) Consecutive return, break, continue, goto or throw statements are unnecessary. Мёртвый код: break позже return:

		switch(lpnm->wID) {
			case REBAR_BAR_TOOLBAR: {
...
				return TRUE;
				break; }
		}

(warning) Assignment of function parameter has no effect outside the function. Традиционно это пригодное предупреждение, сигнализирующее об опечатке — значение, присвоенное внутри функции, никуда не передаётся. Впрочем тут это видимо ложное срабатывание, так как value — переменная класса:

void SetValue( const TCHAR* _value )	{ value = _value; }

(portability) scanf without field width limits can crash with huge input data on some versions of libc. Сходственная повадка применения scanf может привести к опасному переполнению буфера. В случае числовых переменных это тривиальный undefined behaviour:

if ( sscanf( value.c_str(), "%d", ival ) == 1 )

Для реформирования чисел отменнее применять неопасный strtol.

Ещё один поверенный:

sscanf( wordBuffer, "%[^.<>|&=\\/]", sKeywordBuffer );

Тут нет переполнения буфера только потому, что wordBuffer и sKeywordBuffer идентичного размера.

(style) ‘TiXmlStringA::operator=’ should return ‘TiXmlStringA &’. Оператор = возвращает void:

void operator = (const TiXmlStringA & copy);

С таким оператором невозможно применять стандартную для C цепочку:

a = b = c;

(warning) The class ‘ControlsTab’ defines member variable with name ‘_isVertical’ also defined in its parent class ‘TabBar’. Оплошность двойного определения переменной в классе:

class ControlsTab : public TabBar
{
public :
...
private :
...
    bool _isVertical;
};

которая теснее определена в родительском классе:

class TabBar : public Window
{
...
protected:
...
	bool _isVertical;
};

Не являясь специалистом в плюсах, не могу сразу ответить, дозволено ли так делать (protected/private).

(style) Found duplicate branches for ‘if’ and ‘else’. Схожие ошибки нашёл и CppCat. Лишнее условие:

		if(eol_mode == SC_EOL_CRLF)
			extraEOLLength = 2;
		else if(eol_mode == SC_EOL_LF)
			extraEOLLength = 1;
		else // SC_EOL_CR
			extraEOLLength = 1;

(style) Checking if unsigned variable ‘lenFile’ is less than zero. Схожее сообщение выдавал CppCat за исключением того, что cppcheck, не найдя файла windows.h не стал строить догадки касательно типов как бы WPARAM. Недочёт неориентированности экстраординарно на Windows всё же есть.

		size_t lenFile = 0;
...
			if (lenFile <= 0) break;

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

(style) Array index ‘j’ is used before limits check. Невзирая на то что предупреждение низкоприоритетное, обнаруженная оплошность представляет угроза выхода за границы массива:

     int j;
     int ReturnValue;
     j=startcol;
     if(direction == 1){j  ;}
     if(direction != 1){j--;}

     while((BGHS[SI].columnwidths[j] == 0)&&(j<=BGHS[SI].cols)&&(j>0))

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

Мелкие недочёты

(style) Unused variable: ent. Это предупреждение умеют выдавать и компиляторы, ничего увлекательного.

(warning) %d in format string (no. 2) requires ‘int’ but the argument type is ‘DWORD {aka unsigned long}’ — дюже знаменитая оплошность, программисты в printf пишут тип, не соответствующий типу переменной. Это тоже отстреливается множеством компиляторов.

Источник: programmingmaster.ru

Оставить комментарий
Форум phpBB, русская поддержка форума phpBB
Рейтинг@Mail.ru 2008 - 2017 © BB3x.ru - русская поддержка форума phpBB