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

    PVS-Studio vs Apache HTTP Server
    В этот раз с помощью 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 проектов и тем, кто очень хочет попробовать анализатор в полную силу. Пишите нам.
    PVS-Studio
    399.68
    Static Code Analysis for C, C++, C# and Java
    Share post

    Similar posts

    Comments 45

      +15
      Совсем офтопик — О графах и графоманах или Почему я не люблю Льва Толстого. Прошу прощения за цитату: "В этих сорока шести строчках, словно свет в дивно ограненном алмазе собраны, кажется, все возможные языковые и сюжетные ляпы" — Андрею впору ее использовать как эпиграф к статьям о PVS Studio.
        +2
        ох и разложили в Вашем оффтопике Льва Толстого! Где бы комментарии к статье почитать?
        +1
        После прочтения статьи по вашей ссылке появилось непреодолимое желание воткнуть вилку в глаз Логинову. Вы нарушили мое душевное спокойствие! :)
          +4
          Да, мне всегда казалось что Лев Толстой писал в формате XML.
            +1
            Начало первого предложения опуса: «Прежде чем начать рассуждение о творчестве Льва Толстого необходимо сформулировать несколько до идиотизма элементарных истин».
            Дальше не читал.
            –18
            А вы хоть отправляете авторам Apache и прочих исследованных программ багрепорты?
            Или вас только продажи интересуют?
              +16
              Автор во всех предыдущих статьях отвечал на этот вопрос, отправляет по мере возможности.
                +40
                Знаете, Вы подняли очень важную и ещё неосвященную тему. Именно как Вы, вопрос ещё никто не формулировал. :)
                Были разные варианты, но именно такого вопроса пока ещё не было:
                1. А можно узнать, были ли разработчики Qt уведомлены о найденных ошибках? [1]
                2. А вы в хромиум багрепорты\патчи отправили? [2]
                3. Разработчики в курсе? ) [3]
                4. Поворчу: Вы сделали багрепорты в проекты, указанные в Ваших примерах? [4]
                5. Сформировали письмо с результатами тестирования для команды WinMerge? :) [5]

                6. В общем я затрудняюсь сейчас ответить на вновь поставленный вопрос. Сил нету… :)
                  +9
                  >Знаете, Вы подняли очень важную и ещё неосвященную тему.

                  Хорошая иллюстрация к тому, что проверка орфографии (как и статический анализ ;)) может давать сбои: слово «освященный» является вполне корректным прилагательным, которое вполне может использоваться в данном предложении (если бы речь шла о совершении религиозного ритуала над данной темой), однако в данном контексте должно было быть использовано прилагательное «освещенный» (то, на что «пролили свет»), а «святость» тут никаким боком :)
                    +2
                    Надо перед каждый постом преамбулу типа при подготовке данного материала не пострадала ни одного бага или все собранные во время акции баги перечислены в фонд борьбы с энтропией
                      0
                      Вопрос вполне уместен — у него есть подтекст — человек хочет знать, писать об этих багах разработчикам или уже не нужно. Т.е. все были бы вам благодарны, если вы в каждой статье сообщали какова судьба этих багов — отправлялись ли они разработчикам, есть ли перевод статьи, сам отчёт.
                      Многие готовы сами написать об этих багах разработчикам
                        0
                        Да ладно, никто не хочет сам писать разработчикам…
                          0
                          Если бы в статье было написано, что мол у нас нет времени писать разработчикам, вот баги решайте сами, я бы написал разработчикам сам.
                            0
                            Сделайте доброе дело — напишите отзыв про использование PVS-Studio. Поможем чем сможем.
                          0
                          1) Сегодня появился перевод статьи и я отправил разработчикам багрепорт: issues.apache.org/bugzilla/show_bug.cgi?id=51542
                          2) Всё равно другой никто не сделает это за меня. Это только на словах все молодцы, правильные и т.п. Легко, проходя мимо, дать совет, что надо делать мир лучше, помогать открытым проектам и что надо отписать про ошибки. И идти дальше читать Хабр. Хоть бы раз кто взял и отписал баги. Или попросил ключ для PVS-Studio, чтобы проверить сторонний открытый проект и отписать авторам про ошибки. Я пока только одного такого человека знаю, который проверял Tortoise SVN свежей версией PVS-Studio и сообщал им.
                            0
                            1. Благодарю вас за помощь открытым проектам
                            2. > Хоть бы раз кто взял и отписал баги.
                            так и спрашивают чтобы знать писать или нет, если вы скажите, что вам религия не позволяет писать баги разработчикам, напишу (только пните, не всегда читаю хабр), хотя понятно что так как вы проанализировать не смогу

                            > Или попросил ключ для PVS-Studio
                            это нужно не только ms windows иметь, да ещё и VS, это далеко не у всех есть — у меня не винда, но идея не плохая, подумаю
                            С Visual Studio Express работает?
                              0
                              Visual Studio Express Edition не поддерживает подключаемые модули расширений. Это сделано специально разработчиками из MS, чтобы был смысл пользоваться платными редакциями Visual Studio.
                                0
                                Тогда мне не доступен ваш продукт, я не использую крякнутый софт и у меня только один, из не сильно удалённых, знакомый пользует VS, но его не интересует анализ свободных проектов, он продался MS.
                                  +1
                                  У VS Professional есть триал-период в N дней (не помню, сколько), который потом можно продлить через сайт. Поставьте в виртуалке и прогоните pvs-studio. Я так и сделал у себя.
                                    0
                                    Благодарю за подсказку, а то от этих рабов майкрософта доброго слова не дождёшься, только обещают помощь, а на деле только оскорблять умеют
                                    0
                                    На словах он Лев Толстой… а как дошло до дела — нашлась «уважительная» причина.
                                      –1
                                      Т.е. сначала вы предлагает помощь, а потом оскорбляете, отличный подход к пользователям, при таком подходе коммерческий успех обеспечен.
                                      Хорошо хоть пользоваетель VladX подсказал вариант, так что дорогой мой знаток тюремных стишков — высылай ключ, через пару недель может будет время
                        +1
                        А на книжке Донцовой пробовали запускать?
                          +3
                          Если проводить аналогию между книгами Донцовой и программами, то пожалуй в таких проектах статический анализ не нужен и даже вреден. Нет никакого смысла анализировать код iPhone приложения, жизненный цикл которого составляет пару месяцев. Народ поскачивал, потыкал и пошел за новой програмулькой. А то что баги, так и так сойдёт. Там никто качеством или поддержкой не заворачивается. Анализ только сделает разработку более дорогой.
                            0
                            Тут надо работать по честному: раз она их пишет на автомате — значит автоматом и проверять
                          +16
                          Ждем анализ кода PVS-Studio при помощи PVS-Studio
                            +6
                            Тык, это мы каждую ночь делаем. Впрочем, идея интересная. Как накопится интересное, пройдусь по истории и составлю подбородку.
                              +5
                              Большое спасибо за посты, а можно узнать, как сделана PVS студия в общих чертах. Какая математическая основа, какую часть работы она делает сама, а какую перекладывает на стандртные компоненты? Например, содержит ли она парсер C++ или кто-то парсит за нее? Если бы вам захотелось сделать ее для дргого языка, какие части остались бы неизменными, а какие поменяли. Сама студия, судя по всему, написана на С++ — а почему не на, каком-нибудб более строгом языке типа ML?

                              Конечно, если на эти вопросы можно ответить, не выболтав ноухау.
                                +9
                                Вначале хотел написать ответ, но понял, что он будет весьма объёмен. Думаю, его вполне можно оформить в виде заметки “PVS-Studio изнутри”. Обещаю написать ёё в обозримом времени и ответить на все прозвучавшие вопросы. Кстати, желающие могут заранее задать других вопросов, на которые я обстоятельно отвечу.
                                  0
                                  так планируется ли поддержка других языков? например PHP
                                    0
                                    По-моему очевидно, что не планируется. PVS-Studio сильно завязан на Visual Studio, максимум что можно ожидать, это C#. Вот если бы разработчики сделали stand-alone версию PVS-Studio, было бы круто. Не все используют Visual Studio как основную среду для разработки, а статический анализ нужен всем.
                              +1
                              А еще с инкрементальным анализом нередко ошибки даже не попадают в SVN, а правятся сразу же.
                                +1
                                Кстати да, не учёл это. Так что фиг я такую заметку смогу сделать. Если только хватит того, что раньше успело накопиться.
                              +2
                              мы по-прежнему предоставляем ключи для разработчиков бесплатных open-source проектов

                              Я разрабатываю открытый проект (goo.gl/4AGTN), очень интересно попробовать PVS-Studio. На сайте ничего про это не нашёл. Нужно вам на почту писать, или как получить ключ? Заранее извиняюсь, если чего-то недосмотрел/не нашёл.
                              0
                              Ошибки это часть любого кода и даже человека. Совершать ошибки это часть человеческой природы, не нужно бороться с самим фактом ошибочности нашей природы нужно бороться с последствиями ошибок. Не имеет значения сколько ошибок содержится в коде если код работает исправно. Важен результат труда, и если результат труда не содержит ошибок, то лучше не исправлять ошибки в инструменте труда. Наше восприятие ошибки во многом субъективно. То что является ошибкой в одном коде, не будет таковым в другом коде, и то что является правильным в одном случае, может привести к ошибке в другом случае. В оценке ошибок готового продукта важно не качество конкретного участка кода, а его вклад в интегральное значение для системы целиком. И таким критерием должен являться результат работы системы. И рискуя быть обвиненным в ралятивизме я все таки хочу постулировать главное правило поиска ошибок — «работает — не трогай».

                              Конечно это не отменяет необходимости модульного тестирования на этапе разработки, когда еще система не может быть оценена с токи зрения внешнего наблюдателя, по результатам её работы.
                                0
                                «Работает — не трогай» — спорный постулат, когда мы говорим о программном коде. Подобный подход справедлив, если мы говорим о некой замороженной, никак не развивающейся системе. Однако мир программ не таков. Обновляются операционные системы и как следствие изменяется среда обитания программы. Да и сама программа постепенно модифицируется, собирается новыми компиляторами. И странно считать, что программу не надо исправлять, если она жёстко создает свои временные файлы в «C:\temp». Если раньше, сложно было представить компьютер без диска «C:\», то теперь запросто.
                                Другим примером может быть неопределённое поведение программы. Прошли разные тесты, всё работает, значит и трогать ничего не надо. А потом мы меняем компилятор, изменяется поведение гадкого кода и мы получаем увлекательные часы в отладчике. В общем, спорно, спорно…
                                  0
                                  Динамические системы обладают тем свойством, что результат труда и работы системы перестает удовлетворять потребителя. А следовательно можно сказать, что система перестала работать, хотя внутри ничего не поменялось. А значит она стоит того, что бы её трогать. Можно я опять впаду релятивизм? :): — Есть правильные вещи, есть вещи которые были правильными вчера, но являются неправильными сегодня, есть вещи, которые еще не являются, но вскоре станут правильными. А есть вещи которые не были и не будут правильными никогда. Оценить что есть правильно, а что нет, и есть главная головная боль специалиста по тестированию.
                                    0
                                    Задача специалиста по программированию/тестированию оценить что есть правильно, а что нет. А также то, что со временем станет неправильно! Ну хотя бы попытаться это сделать. Если они это не делают, то их надо уволить. Подход «сейчас работает, а потом видно будет» колоссально убыточен, так как вместо развития продукта придётся заниматься вечными правками. Лучше сразу писать "%windir%" вместо «C:\Windows». Может пример не самый удачный, но смысл, думаю понятен.
                                      0
                                      Да я вами не спорю. Я философствую. Оценка ошибочности кода вещь зачастую очень субъективная. Когда-то Николас Вирт пытался реализовать систему автоматического доказательства разрешимости алгоритма в паскале. Но это наложило такие огромные ограничения на язык, что все производители компиляторов на базе виртовского паскаля, просто игнорировали эту часть его идеи.

                                      И хотя возможность писать код работоспособность, которого на всех наборах данных будет гарантироваться компилятором вещь безусловно сногсшибательная, но для программистов лучше иметь возможность забить в переменную «C:\Windows» потому что, даже использование "%windir%", не спасет, когда в программе понадобится "/etc/bin" :) Поэтому программирование все еще остается искусством, а тестирование это искусство в квадрате. Потому что оценивать произведения искусства может только тот кто умеет создавать более совершенные произведение искусства. Инструменты помогают или мешают это делать ровно нас только на сколько человек сумел их освоить и извлечь пользу. Компиляторы иногда могут находить логические ошибки, но бывает так что программист намерено сделал такой код, который был воспринят как ошибочный. Тут нужно либо жестко ограничивать программиста в свободе кодирования, либо смириться с тем, что человеческий фактор в программировании будет чрезвычайно огромен.
                                0
                                было бы клево увидеть ошибки у nginx
                                +1
                                Оказывается, не один я провожу какие-то параллели между книгой «Война и Мир», статическим анализом и Apache. Надо же, как бывает! Случайно наткнулся вот на этот текст: Hackers bite the (static analysis) dust: Part 2.

                                Цитата оттуда:

                                According to Wikipedia, Microsoft Windows source base consists of about 50,000,000 lines of source code. That sounds like a huge number, but what does it actually mean? Consider a huge novel " «War and Peace» comes to mind. Leo Tolstoy's masterpiece depicting 19th century Russia is a mere 1,500 pages " that is about 100,000 lines of text, or my new unit: one WAP («War And Peace»).
                                That makes Windows source base about 500 WAPs. Imagine reading and intimately understanding five hundred books that rival «War and Peace» in size and complexity. No offense to Tolstoy, but that sounds like a literature-class nightmare.
                                A smaller application, like the Apache web server, will come in at about 1.5 WAPs. Among popular operating systems, Linux Debian source base takes the top prize at 2,000 WAPs.

                                  –1
                                  Небольшое продолжение: Вновь проверяем Apache HTTP Server.

                                  Only users with full accounts can post comments. Log in, please.