Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Повторная проверка TortoiseSVN с помощь…

Повторная проверка TortoiseSVN с помощью анализатора кода PVS-Studio

25 Июн 2013

Мы отправили разработчикам TortoiseSVN на некоторое время бесплатный ключ для анализатора PVS-Studio. Пока они не успели им воспользоваться, я решил быстро скачать исходные коды TortoiseSVN и самостоятельно выполнить анализ. Цель понятна. Очередная небольшая статья для рекламы PVS-Studio.

0202_TortoiseSVN2_ru/image1.png

Мы уже проверяли проект TortoiseSVN. Это было давно. Проверка проекта как раз совпала с выпуском PVS-Studio 4.00, где впервые появились диагностические правила общего назначения.

Время от времени мы повторяем проверки, чтобы продемонстрировать пользу от регулярного использования инструмента. Нет смысла один или два раза проверить проект. В изменяемом коде постоянно появляются новые ошибки. А потом медленно и печально исправляются. Соответственно, максимальная польза будет достигнута при ежедневном использовании PVS-Studio. А ещё лучше, при использовании инкрементального анализа.

Итак, посмотрим, что удалось найти интересного в проекте с помощью PVS-Studio версии 5.05. Исходные коды TortoiseSVN были взяты 19.06.2013 из tortoisesvn.googlecode.com/svn/trunk. Кстати, проект TortoiseSVN является очень качественным и имеет огромную базу пользователей-программистов. Так что если найти хоть что-то, это уже большое достижение.

Странные условия

static void ColouriseA68kDoc (....)
{
  if (((sc.state == SCE_A68K_NUMBER_DEC) && isdigit(sc.ch))
      ....
      || ((sc.state == SCE_A68K_MACRO_ARG) && isdigit(sc.ch))
      || ((sc.state == SCE_A68K_MACRO_ARG) && isdigit(sc.ch))
      ....
}

Диагностическое сообщение: V501 There are identical sub-expressions '((sc.state == 11) && isdigit(sc.ch))' to the left and to the right of the '||' operator. lexa68k.cxx 160

Присутствует два одинаковых сравнения. Возможно, имеет место опечатка.

Опечатка, наверное, присутствует и в следующем коде. Два раза проверяется значение переменной 'rv'.

struct hentry * AffixMgr::compound_check(
  ....
  if (rv && forceucase && (rv) && ....)
  ....
}

Диагностическое сообщение: V501 There are identical sub-expressions to the left and to the right of the '&&' operator: rv && forceucase && (rv):

  • affixmgr.cxx 1784
  • affixmgr.cxx 1879

Следующий фрагмент кода с неправильным сравнением:

int main(int argc, char **argv)
{
  ....
  DWORD ticks;
  ....
  if (run_timers(now, &next)) {
    ticks = next - GETTICKCOUNT();
    if (ticks < 0) ticks = 0;
  } else {
    ticks = INFINITE;
  }
  ....
}

Диагностическое сообщение: V547 Expression 'ticks < 0' is always false. Unsigned type value is never < 0. winplink.c 635

Переменная 'ticks' является беззнаковой. Это значит, что проверка "if (ticks < 0)" не имеет смысла. Ситуация возникновения переполнения не будет обработана.

Рассмотрим ошибку, из-за которой функция 'strncmp' сравнивает строки не полностью.

int  AffixMgr::parse_convtable(...., const char * keyword)
{
  char * piece;
  ....
  if (strncmp(piece, keyword, sizeof(keyword)) != 0) {
  ....
}

Диагностическое сообщение: V579 The strncmp function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. affixmgr.cxx 3654

Оператор 'sizeof' вычисляет размер указателя. Это значение никак не связанно с длиной строки.

Подозрительное формирование строки

Функции с переменным количеством аргументов всегда везде есть, и как всегда опасны.

class CTSVNPath
{
  ....
private:
  mutable CString m_sBackslashPath;
  mutable CString m_sLongBackslashPath;
  mutable CString m_sFwdslashPath;
  ....
};

const FileStatusCacheEntry * SVNFolderStatus::BuildCache(
  const CTSVNPath& filepath, ....)
{
  ....
  CTraceToOutputDebugString::Instance() (_T(__FUNCTION__)
    _T(": building cache for %s\n"), filepath);
  ....
}

Диагностическое сообщение: V510 The 'operator()' function is not expected to receive class-type variable as second actual argument:

  • svnfolderstatus.cpp 150
  • svnfolderstatus.cpp 355
  • svnfolderstatus.cpp 360

Спецификатор "%s" указывает, что в качестве фактического аргумента функция ожидает строку. Однако, переменная 'filepath' вовсе не строка, а сложный объект, состоящий из множества строк. Затрудняюсь сказать, что будет распечатано и не упадёт ли вообще этот код.

Опасно использовать такие функции, как 'printf()' следующим образом: "printf(myStr);". Если внутри 'myStr' будут присутствовать управляющие спецификаторы, то программа может распечатать то, что ей не полагается или аварийно завершиться.

Рассмотрим код из TortoiseSVN:

BOOL CPOFile::ParseFile(....)
{
  ....
  printf(File.getloc().name().c_str());
  ....
}

Диагностическое сообщение: V618 It's dangerous to call the 'printf' function in such a manner, as the line being passed could contain format specification. The example of the safe code: printf("%s", str); pofile.cpp 158

Если имя файла будет "myfile%s%i%s.txt", то результат будет плачевен.

Примечание. У нас есть интересная заметка на тему опасности использования функции printf().

Неправильное обнуление массивов

Я не знаю, насколько для TortoiseSVN опасно оставить содержимое буферов, не обнулив их. Возможно, вообще безопасно. Однако есть код для обнуления буферов. А поскольку он не работает, стоит про это упомянуть. Ошибки выглядят так:

static void sha_mpint(SHA_State * s, Bignum b)
{
  unsigned char lenbuf[4];
  ....
  memset(lenbuf, 0, sizeof(lenbuf));
}

Диагностическое сообщение: V597 The compiler could delete the 'memset' function call, which is used to flush 'lenbuf' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. sshdss.c 23

Перед выходом из функции, массив 'lenbuf' следует очистить. Так как затем массив больше не используется, оптимизатор удалит вызов функции 'memset'. Чтобы этого не происходило, требуется использовать специальные функции.

Другие места, где компилятор удалит вызов 'memset()':

  • sshdss.c 37
  • sshdss.c 587
  • sshdes.c 861
  • sshdes.c 874
  • sshdes.c 890
  • sshdes.c 906
  • sshmd5.c 252
  • sshrsa.c 113
  • sshpubk.c 153
  • sshpubk.c 361
  • sshpubk.c 1121
  • sshsha.c 256

Странное

BOOL InitInstance(HINSTANCE hResource, int nCmdShow)
{
  ....
  app.hwndTT; // handle to the ToolTip control
  ....
}

Диагностическое сообщение: V607 Ownerless expression 'app.hwndTT'. tortoiseblame.cpp 1782

Скорее всего, в функции 'InitInstance()' , член 'hwndTT' должен чем-то инициализироваться. Однако, из-за опечатки, код оказался недописанным.

64-битные ошибки

Поиск ошибок я делаю весьма поверхностно. Я внимателен, ровно на столько, чтобы мне хватило примеров для написания статьи. Нет, я не бяка. Просто авторы проекта, всё равно выполнят анализ качественнее, чем могу сделать я.

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

Приведу только пару опасных мест:

void LoginDialog::CreateModule(void)
{
  ....
  DialogBoxParam(g_hmodThisDll, MAKEINTRESOURCE(IDD_LOGIN),
                 g_hwndMain, (DLGPROC)(LoginDialogProc),
                 (long)this);
  ....
}

Диагностическое сообщение: V220 Suspicious sequence of types castings: memsize -> 32-bit integer -> memsize. The value being casted: 'this'. logindialog.cpp 105

Указатель 'this' явно приводится к типу 'long'. Затем он неявно расширяется до типа LPARAM (LONG_PTR). Важно то, что указатель на какое-то время превращается в 'long'. Это плохо, если программа является 64-битной. Указатель занимает 64-бита. Тип 'long' в Win64 по прежнему является 32-битным типом. В результате старшие биты 64-биной переменной будут потеряны.

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

Правильный код: DialogBoxParam(...., (LPARAM)this);

Рассмотрим другое опасное приведение типа:

static int cmpforsearch(void *av, void *bv)
{
  Actual_Socket b = (Actual_Socket) bv;
  unsigned long as = (unsigned long) av,
                bs = (unsigned long) b->s;
  if (as < bs)
    return -1;
  if (as > bs)
    return +1;
  return 0;
}

Диагностическое сообщение: V205 Explicit conversion of pointer type to 32-bit integer type: (unsigned long) av:

  • winnet.c 139
  • winhandl.c 359
  • winhandl.c 348

Указатели явным образом приводятся к типу 'unsigned long' и помещаются в переменные 'as' и 'bs'. Так как старшие биты адреса при этом могут быть потеряны, сравнение может работать неправильно. Вообще не понятно, зачем здесь указатели приводятся к целочисленным типам. Можно просто сравнить указатели.

Устаревшие проверки нулевых указателей

Оператор 'new', когда не может выделить память, давным-давно не возвращает NULL. Он генерирует исключение std::bad_alloc. Конечно, можно сделать, чтобы оператор 'new' возвращал 0, но это сейчас к делу не относится.

Тем не менее, в программах продолжает жить код следующего вида:

int _tmain(....)
{
  ....
  pBuf = new char[maxlength];
  if (pBuf == NULL)
  {
    _tprintf(_T("Could not allocate enough memory!\n"));
    delete [] wc;
    delete [] dst;
    delete [] src;
    return ERR_ALLOC;
  }
  ....
}

Диагностическое сообщение: V668 There is no sense in testing the 'pBuf' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error.

  • subwcrev.cpp 912
  • repositorybrowser.cpp 2565
  • repositorybrowser.cpp 4225
  • svnstatuslistctrl.cpp 5254
  • svnprogressdlg.cpp 2357
  • bugtraqassociations.cpp 116
  • xmessagebox.cpp 792
  • xmessagebox.cpp 797
  • hyperlink_base.cpp 166
  • affixmgr.cxx 272
  • hashmgr.cxx 363
  • hashmgr.cxx 611

И так сойдёт

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

void CBaseView::OnContextMenu(CPoint point, DiffStates state)
{
  ....
  popup.AppendMenu(MF_STRING | oWhites.HasTrailWhiteChars ?
                   MF_ENABLED : (MF_DISABLED|MF_GRAYED),
                   POPUPCOMMAND_REMOVETRAILWHITES, temp);
  ....
}

Диагностическое сообщение: V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '|' operator. baseview.cpp 2246

В зависимости от значения переменной 'oWhites.HasTrailWhiteChars' требуется получить одно из сочетаний констант:

  • MF_STRING | MF_ENABLED
  • MF_STRING | MF_DISABLED | MF_GRAYED

Но код работает совсем не так. Приоритет операции '|' выше, чем приоритет операции '?:'. Расставим скобки для наглядности:

(MF_STRING | oWhites.HasTrailWhiteChars) ? MF_ENABLED : MF_DISABLED | MF_GRAYED

Код корректно работает, только благодаря тому, что константа 'MF_STRING' равна 0. Она не оказывает никакого влияния на результат. В результате, неправильное выражение работает правильно.

Рассмотрим другой пример везения. В программе TortoiseSVN тип HWND нередко используется как тип 'unsigned'. Для этого приходится выполняться явные преобразования типа. Например, так, как показано в следующих функциях:

HWND m_hWnd;
UINT_PTR uId;
INT_PTR CBaseView::OnToolHitTest(....) const
{
  ....
  pTI->uId = (UINT)m_hWnd;
  ....
}

UINT_PTR  idFrom;
HWND m_hWnd;

BOOL CBaseView::OnToolTipNotify(
  UINT, NMHDR *pNMHDR, LRESULT *pResult)
{
  if (pNMHDR->idFrom != (UINT)m_hWnd)
    return FALSE;
  ....
}

Или, например, значение переменной типа HWND печатается, как если бы это был тип 'long'.

bool CCommonAppUtils::RunTortoiseProc(....)
{
  ....
  CString sCmdLine;
  sCmdLine.Format(L"%s /hwnd:%ld",
    (LPCTSTR)sCommandLine, AfxGetMainWnd()->GetSafeHwnd());
  ....
}

Формально этот код неверен. Дело в том, что тип 'HWND' представляет собой указатель. А значит, его нельзя превращать в 32-битные целочисленные типы. И анализатор PVS-Studio переживает по этому поводу, выдавая предупреждения.

Но интересно то, что этот код будет работать совершенно корректно!

Тип HWND используется для хранения дескрипторов, которые используются в Windows для работы с различными системными объектами. Такими же типами являются HANDLE, HMENU, HPALETTE, HBITMAP и так далее.

Хотя дескрипторы являются 64-битными указателями, для большей совместимости (например, для возможности взаимодействия между 32-битынми и 64-битными процессами) в них используется только младшие 32-бита. Подробнее смотри "Microsoft Interface Definition Language (MIDL): 64-Bit Porting Guide" (USER and GDI handles are sign extended 32b values).

Помещая тип HWND в 32-битные типы, разработчики вряд ли основывались именно на этих допущениях. Скорее всего, это не очень аккуратный код, работающий корректно благодаря везению и стараниям разработчиков Windows API.

Вывод

Используйте статический анализ регулярно при разработке, и вы найдете множество ошибок на самых ранних этапах. Я естественно рекомендую в первую очередь познакомиться с анализатором кода PVS-Studio. Однако, есть много других хороших анализаторов кода: инструменты статического анализа кода.

Ссылки

Дополнительные ссылки, которые могут пояснить некоторые тонкие моменты, описываемые в статье.

Популярные статьи по теме
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

Автор: Андрей Карпов

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Любите статический анализ кода!

Дата: 16 Окт 2017

Автор: Андрей Карпов

Я в шоке от возможностей статического анализа кода, хотя сам участвую в разработке инструмента PVS-Studio. На днях я был искренне удивлён тому, что анализатор оказался умнее и внимательнее меня.
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

Автор: Андрей Карпов

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
PVS-Studio для Java

Дата: 17 Янв 2019

Автор: Андрей Карпов

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

Автор: Андрей Карпов

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Зло живёт в функциях сравнения

Дата: 19 Май 2017

Автор: Андрей Карпов

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

Дата: 31 Май 2014

Автор: Андрей Карпов

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

Автор: Андрей Карпов

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

Автор: Андрей Карпов

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

Автор: Андрей Карпов

Краткое описание технологий, используемых в инструменте PVS-Studio, которые позволяют эффективно обнаруживать большое количество паттернов ошибок и потенциальных уязвимостей. Статья описывает реализа…

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

Следующие комментарии

На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Этот сайт использует куки и другие технологии, чтобы предоставить вам более персонализированный опыт. Продолжая просмотр страниц нашего веб-сайта, вы принимаете условия использования этих файлов. Если вы не хотите, чтобы ваши данные обрабатывались, пожалуйста, покиньте данный сайт. Подробнее →
Принять