Лев Толстой и статический анализ кода

Apache HTTP Server vs PVS-Studio

В этот раз с помощью PVS-Studio мы проверили Apache HTTP Server. Как и ожидалось, нашли в нём ошибки. Ошибок мало. Это тоже ожидаемо.

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

Война и Мир

Представим следующую ситуацию. У нас имеется произведение Льва Толстого, например, "Война и Мир". Мы хотим испытать на этой книге механизм проверки орфографии, встроенной в Microsoft Word. Мы запускаем анализ, обнаруживаем всего несколько настоящих ошибок и большое количество ложных срабатываний. Вдобавок, Лев Толстой любил писать длинные предложения. Microsoft Word, не сумев разобраться в тексте, будет подчеркивать много зелёным цветом, предполагая наличие пунктуационных ошибок. Мы смотрим на всё это и воротим нос. Нам не нравится, что мы почти не нашли ошибок. Нам не нравится, что многие названия считаются неверными. Нам не нравится анализ пунктуации. Мы приходим к выводу, что проверка орфографии в Word абсолютно бесполезная в работе вещь.

Разберем в чем некорректность такого изучения инструмента.

Мы забываем, что над ошибками в книге УЖЕ ПОРАБОТАЛИ люди. И затратили на это массу времени и энергии. Лев Толстой переписывал свои черновики и истреблял ошибки. Работал редактор, превращая текст в книгу. Исправлялось что-то в последующих изданиях.

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

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

Думаю глупо спорить о пользе механизмов проверки текстов, встроенных в такие программы как Word, Thunderbird, TortoiseSVN. Мы их используем и не переживаем, что они найдут мало ошибок в книге "Война и Мир".

Идентичная ситуация и со статическим анализом исходного кода программ. Бессмысленно проверять такой проект как Apache HTTP Server и пытаться оценить возможную пользу от анализа. Код Apache HTTP Server стар по программистским меркам. Как проект Apache HTTP Server существует уже более 15 лет. Качество кода подтверждается огромным количеством проектов, построенных на его основе. Естественно, что огромное количество ошибок было исправлено благодаря широкому использованию этого кода множеством программистов и длительному периоду развитию.

Логично предположить, что многие из найденных ранее ошибок, можно было бы найти гораздо проще, если бы использовался статический анализ. Аналогия с Word здесь полная. Полезнее постоянно искать ошибки, чем делать это разово.

Как и в редакторе Word, ложные ошибки PVS-Studio легко устранить. Достаточно нажать на ложное сообщение и скрыть его (в код автоматически будет добавлен специальный комментарий).

Сделаем последнее сопоставление по скорости нахождения ошибок. Редактор Word умеет подчеркивать ошибки сразу. Анализатор PVS-Studio делает это после компиляции файлов. Раньше нельзя, синтаксис слишком сложный, чтобы анализировать недописанный код. Впрочем, этого достаточно. Можно рассматривать PVS-Studio как средство увеличения предупреждений, выдаваемых компилятором. Кстати, теперь PVS-Studio умеет это делать в разных версиях Visual Studio: 2005, 2008, 2010. Так что приглашаю пробовать инкрементальный анализ.

В общем, аналогия с Word, на мой взгляд, полная. Если кто-то не согласен, то готов продолжить обсуждение в комментариях или в почте (karpov [@] viva64.com).

Я закончил основную мысль, которой неудержимо хотелось поделиться. Однако, я разочарую читателей, если ничего не напишу про ошибки, найденные в Apache HTTP Server. Так что теперь немного про найденные ошибки.

Вот пример лишнего sizeof().

PSECURITY_ATTRIBUTES GetNullACL(void)

{

  PSECURITY_ATTRIBUTES sa;


  sa  = (PSECURITY_ATTRIBUTES) LocalAlloc(LPTR,

           sizeof(SECURITY_ATTRIBUTES));

  sa->nLength = sizeof(sizeof(SECURITY_ATTRIBUTES));

  ...

}


Диагностическое сообщение PVS-Studio: V568 It's odd that the argument of sizeof() operator is the 'sizeof (SECURITY_ATTRIBUTES)' expression. libhttpd util_win32.c 115

Неправильно устанавливается размер структуры SECURITY_ATTRIBUTES. В результате корректная работа с такой структурой невозможна.

Впрочем, я подозреваю, что большинство примеров, которые я приведу, находятся в коде, крайне редко получающем управление. Поэтому сильно переживать не стоит. Таков и следующий пример. Ошибка находится в функции связанной с обработкой ошибок.

apr_size_t ap_regerror(int errcode, const ap_regex_t *preg,

  char *errbuf, apr_size_t errbuf_size)

{

  ...

  apr_snprintf(errbuf, sizeof errbuf,

               "%s%s%-6d", message, addmessage, (int)preg->re_erroffset);

  ...

}


Диагностическое сообщение PVS-Studio: V579 The apr_snprintf function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the second argument. libhttpd util_pcre.c 85

Смысл таков. Вместо размера буфера в функцию передаётся размер указателя. В результате сообщение об ошибке не может быть сформировано правильно. Практически, сообщение будет длиной в 3 или 7 символов.

Есть и классические ошибки Copy-Paste.

static int magic_rsl_to_request(request_rec *r)

{

  ...

  if (state == rsl_subtype || state == rsl_encoding ||

      state == rsl_encoding) {

  ...

}


Диагностическое сообщение PVS-Studio: V501 There are identical sub-expressions 'state == rsl_encoding' to the left and to the right of the '||' operator. mod_mime_magic mod_mime_magic.c 787

На самом деле, сравнение должно быть таким:

if (state == rsl_subtype || state == rsl_separator ||

    state == rsl_encoding) {


Есть ошибки, которые проявят себя только в операционной системе Windows.

typedef UINT_PTR SOCKET;

static unsigned int __stdcall win9x_accept(void * dummy)

{

  SOCKET csd;

  ...

  do {

      clen = sizeof(sa_client);

      csd = accept(nsd, (struct sockaddr *) &sa_client, &clen);

  } while (csd < 0 && APR_STATUS_IS_EINTR(apr_get_netos_error()));

  ...

}


Диагностическое сообщение PVS-Studio: V547 Expression 'csd < 0' is always false. Unsigned type value is never < 0. libhttpd child.c 404

Функция accept в заголовочных файлах Visual Studio возвращает значение, имеющее беззнаковый тип SOCKET. Поэтому проверка 'csd < 0' недопустима, ведь её результат всегда ложь (false). Возвращенные значения надо явно сравнивать с различными константами, например, с SOCKET_ERROR.

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

typedef  size_t apr_size_t;

APU_DECLARE(apr_status_t) apr_memcache_getp(...)

{

  ...

  apr_size_t len = 0;

  ...

  len = atoi(length);

  ...

  if (len < 0) {

    ...

  }

  else {

    ...

  }

}


Диагностическое сообщение PVS-Studio: V547 Expression 'len < 0' is always false. Unsigned type value is never < 0. aprutil apr_memcache.c 814

Условие "len < 0" в этом коде всегда ложно, так как переменная 'len' имеет беззнаковый тип. Соответственно, если подать на вход строку с отрицательным числом, то видимо произойдет сбой.

Ещё в Apache много кода, где обнуляются различные буферы с данными. Скорее всего, это связано с безопасностью. Вот только само обнуление часто не происходит из-за ошибки следующего вида:

#define MEMSET_BZERO(p,l)       memset((p), 0, (l))


void apr__SHA256_Final(sha2_byte digest[], SHA256_CTX* context) {

  ...

  MEMSET_BZERO(context, sizeof(context));

  ...

}


Диагностическое сообщение PVS-Studio: V512 A call of the 'memset' function will lead to underflow of the buffer '(context)'. apr sha2.c 560

Обнуляются только первые байты в буфере, поскольку оператор sizeof() возвращает размер указателя, а не буфера с данными. Таких ошибок я насчитал как минимум 6 штук.

Остальные ошибки более скучные, так что не буду про них писать. Спасибо за внимание. Приходите пробовать PVS-Studio. И мы по-прежнему предоставляем ключи для разработчиков бесплатных open-source проектов и тем, кто очень хочет попробовать анализатор в полную силу. Пишите нам.

Categories:
For more complete information about compiler optimizations, see our Optimization Notice.