Эксперимент по проверке библиотеки glibc

    glibc and PVS-Studio
    Мы провели эксперимент по проверке библиотеки glibc с помощью PVS-Studio. Цель эксперимента посмотреть, насколько успешно анализатор может проверять Linux-проекты. Пока плохо может. Возникает огромное количество ложных срабатываний из-за использования нестандартных расширений. Однако, всё равно удалось найти кое что интересное.

    glibc


    glibc — GNU C Library (GNU библиотека). Glibc является библиотекой Си, которая обеспечивает системные вызовы и основные функции, такие как open, malloc, printf и т.д. Библиотека C используется для всех динамически скомпонованных программ. Она написана Free Software Foundation для GNU операционных систем. glibc выпущена под лицензией GNU LGPL.

    Взято из Wikipedia: glibc.

    Не так давно в интернете появились новости, что вышла новая версия библиотеки glibc. Это подвигло нас на проверку этой библиотеки с помощью нашего анализатора PVS-Studio. А именно, была проверена версия glibc-2-19-90. К сожалению, на пару недель я отвлекся, поэтому нашёл время написать статью только сейчас. Я был занят большим сравнением нескольких статических анализаторов. Это очень важная для нас задача так как не перестают спрашивать, чем мы лучше Cppcheck и статического анализатора в Visual Studio 2013. Поэтому glibc пришлось немного подождать.

    Найти что-то ужасное мы не рассчитывали и действительно не нашли. Библиотека glibc очень качественная и проверяемая многими анализаторами. Как минимум, это:
    • Coverity;
    • Clang;
    • Cppcheck.
    Так что найти хоть что-то, это уже большое достижение.

    В чем сложность анализа


    Незнакомым с внутренней кухней инструментов статического анализа, они кажутся очень простыми утилитами. Это не так. Это очень сложные программы.

    С толку могут сбивать такие инструменты, как RATS. Если кто-то смотрел код RATS, то видел, что это просто поиск определённых имён функций в файлах. Этот инструмент тоже называется статическим анализатором кода. Однако, он очень далёк от того, чем занимаются настоящие анализаторы кода. Статический анализ, это вовсе не поиск с помощью регулярных выражений [1].

    Неоднократно мы повторяли, что Linux-версия, это вовсе не тоже самое, что перекомпилированный исполняемый модуль [2]. Между исполняемым модулем и программным продуктом лежит пропасть. Одно из препятствий, поддержка специфичных расширений и тому подобное.

    Что это такое, стороннему человеку совершенно непонятно. Вот от видит, в программе вызов функции strcmp():
    cmpres = strcmp (newp->from_string, root->from_string);

    И он даже не подозревает, в какой ужас может раскрываться эта строка после препроцессирования и какие нестандартные расширения будут использованы. Конкретно в нашем случае, строка превращается в это:
    cmpres = __extension__ ({ size_t __s1_len, __s2_len;
      (__builtin_constant_p (newp->from_string) &&
      __builtin_constant_p (root->from_string) &&
      (__s1_len = strlen (newp->from_string),
      __s2_len = strlen (root->from_string),
      (!((size_t)(const void *)((newp->from_string) + 1) -
      (size_t)(const void *)(newp->from_string) == 1) ||
      __s1_len >= 4) &&
      (!((size_t)(const void *)((root->from_string) + 1) -
      (size_t)(const void *)(root->from_string) == 1) ||
      __s2_len >= 4)) ?
      __builtin_strcmp (newp->from_string, root->from_string) :
      (__builtin_constant_p (newp->from_string) &&
      ((size_t)(const void *)((newp->from_string) + 1) -
      (size_t)(const void *)(newp->from_string) == 1) &&
      (__s1_len = strlen (newp->from_string), __s1_len < 4) ?
      (__builtin_constant_p (root->from_string) &&
      ((size_t)(const void *)((root->from_string) + 1) -
      (size_t)(const void *)(root->from_string) == 1) ?
       __builtin_strcmp (newp->from_string, root->from_string) :
      (__extension__ ({ const unsigned char *__s2 =
      (const unsigned char *) (const char *) (root->from_string);
      int __result = (((const unsigned char *) (const char *)
      (newp->from_string))[0] - __s2[0]);
      if (__s1_len > 0 && __result == 0) {
      __result = (((const unsigned char *) (const char *)
      (newp->from_string))[1] - __s2[1]);
      if (__s1_len > 1 && __result == 0) { __result =
      (((const unsigned char *) (const char *)
      (newp->from_string))[2] - __s2[2]);
      if (__s1_len > 2 && __result == 0)
      __result = (((const unsigned char *)
      (const char *) (newp->from_string))[3] -
      __s2[3]); } } __result; }))) :
      (__builtin_constant_p (root->from_string) &&
      ((size_t)(const void *)((root->from_string) + 1) -
      (size_t)(const void *)(root->from_string) == 1) &&
      (__s2_len = strlen (root->from_string), __s2_len < 4) ?
      (__builtin_constant_p (newp->from_string) &&
      ((size_t)(const void *)((newp->from_string) + 1) -
      (size_t)(const void *)(newp->from_string) == 1) ?
      __builtin_strcmp (newp->from_string, root->from_string) :
      (- (__extension__ ({ const unsigned char *__s2 =
      (const unsigned char *) (const char *) (newp->from_string);
      int __result = (((const unsigned char *) (const char *)
      (root->from_string))[0] - __s2[0]);
      if (__s2_len > 0 && __result == 0) { __result =
      (((const unsigned char *) (const char *)
      (root->from_string))[1] - __s2[1]);
      if (__s2_len > 1 && __result == 0)
      { __result = (((const unsigned char *)
      (const char *) (root->from_string))[2] -
      __s2[2]); if (__s2_len > 2 && __result == 0)
      __result = (((const unsigned char *) (const char *)
      (root->from_string))[3] - __s2[3]); } } __result; })))) :
      __builtin_strcmp (newp->from_string, root->from_string))));
    });

    Анализатор не готов к такому повороту событий и временами выдаёт на подобных конструкциях бессмысленные ложные сообщения.

    Поясню про ложные сообщения на более простом примере. Пусть у нас есть строка кода:
    assert(MAP_FAILED == (void *) -1);

    Макрос assert() раскрывается в следующий код:
    ((((void *) -1) == (void *) -1) ? (void) (0) :
      __assert_fail ("((void *) -1) == (void *) -1",
        "loadmsgcat.c", 840, __PRETTY_FUNCTION__));

    Анализатор PVS-Studio выдаёт ложное предупреждение касательно сравнения (((void *) -1) == (void *) -1):

    V501 There are identical sub-expressions to the left and to the right of the '==' operator: ((void *) — 1) == (void *) — 1 loadmsgcat.c 840

    Мы не удивлены этому. Всё это мы уже проходили, ведя разработку для программ, собираемых с помощью Visual C++. Там тоже много всякого интересного и необычного. Нужно проделать большую работу, чтобы научить анализатор понимать, что есть что. Нужно научить его понять, что он имеет дело с макросом «assert» который безобиден и просто проверяет что макрос MAP_FAILED равен "(void *) -1". Всё это уже сделано для Visual C++. А для Linux нет.

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

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

    Найденные подозрительные участки кода


    Хотя проект glibc проверяется многими инструментами, кое что интересного всё-таки удалось найти. Давайте посмотрим на эти участки кода.

    Странное выражение


    char *DCIGETTEXT (....)
    {
      ....
      /* Make CATEGORYVALUE point to the next element of the list. */
      while (categoryvalue[0] != '\0' && categoryvalue[0] == ':')
        ++categoryvalue;
      ....
    }

    V590 Consider inspecting this expression. The expression is excessive or contains a misprint. dcigettext.c 582

    Условие можно упросить до:
    while (categoryvalue[0] == ':')

    Возможно, это не ошибка и первая часть условия (categoryvalue[0] != '\0') просто лишняя. Однако, вдруг должно быть написано так:
    while (categoryvalue[0] != '\0' && categoryvalue[0] != ':')

    Разыменование указателя до проверки


    Не обязательно, это место опасно. Возможно, указатель никогда не может быть равен нулю. Но тем не менее:
    static enum clnt_stat
    clntraw_call (h, proc, xargs, argsp, xresults, resultsp, timeout)
         CLIENT *h;
         u_long proc;
         xdrproc_t xargs;
         caddr_t argsp;
         xdrproc_t xresults;
         caddr_t resultsp;
         struct timeval timeout;
    {
      struct clntraw_private_s *clp = clntraw_private;
      XDR *xdrs = &clp->xdr_stream;
      ....
      if (clp == NULL)
        return RPC_FAILED;
      ....
    }

    V595 The 'clp' pointer was utilized before it was verified against nullptr. Check lines: 145, 150. clnt_raw.c 145

    Рядом в этом файле можно увидеть аналогичный дефект: V595 The 'clp' pointer was utilized before it was verified against nullptr. Check lines: 232, 235. clnt_raw.c 232

    Другой пример опасного кода:
    int
    __nss_getent_r (....)
    {
      ....
      if (res && __res_maybe_init (&_res, 0) == -1)
      {
        *h_errnop = NETDB_INTERNAL;
        *result = NULL;
        return errno;
      }
      ....
      if (status == NSS_STATUS_TRYAGAIN
          && (h_errnop == NULL || *h_errnop == NETDB_INTERNAL)
          && errno == ERANGE)
    }

    V595 The 'h_errnop' pointer was utilized before it was verified against nullptr. Check lines: 146, 172. getnssent_r.c 146

    Если выполнится условие if (res && __res_maybe_init (&_res, 0) == -1), то функция возвращает информацию об ошибке. При этом она разыменовывает указатели 'h_errnop' и 'result'. Однако, эти указатели вполне могут быть равны NULL. Этот вывод можно делать, исследуя код ниже.

    Опасная оптимизация (уязвимость)


    char *
    __sha256_crypt_r (key, salt, buffer, buflen)
         const char *key;
         const char *salt;
         char *buffer;
         int buflen;
    {
      ....
      unsigned char temp_result[32]
      ....
      memset (temp_result, '\0', sizeof (temp_result));
      ....
      .... // temp_result далее не используется
    }

    V597 The compiler could delete the 'memset' function call, which is used to flush 'temp_result' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. sha256-crypt.c 385

    Компилятор вправе удалить вызов функции memset() при компиляции Release версии. Точнее он не только в праве, но и обязан это сделать с целью оптимизации. Буфер 'temp_result' после вызова функции memset() нигде не используется, следовательно и сам вызов функции тоже лишний.

    Мы имеем дело с уязвимостью, так как не будут очищены приватные данные. Следует заменить функцию memset() на более подходящую. Анализатор предлагает RtlSecureZeroMemory(), которой конечно нет в Linux. Но есть аналоги.

    Аналогичная ситуация: V597 The compiler could delete the 'memset' function call, which is used to flush 'temp_result' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. sha512-crypt.c 396

    Undefined behavior


    Казалось бы, библиотека glibc должна быть написана максимально переносимо. Однако мы встречаем в ней немало конструкций сдвига, которые нельзя назвать безопасными с точки зрения переносимости.

    Вот что нам говорит стандарт языка Си о сдвигах:

    The integer promotions are performed on each of the operands. The type of the result is that of the promoted left operand. If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined.

    The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits are filled with zeros. If E1 has an unsigned type, the value of the result is E1 * 2 pow E2, reduced modulo one more than the maximum value representable in the result type. If E1 has a signed type and nonnegative value, and E1 * 2 pow E2 is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined.

    5 The result of E1 >> E2 is E1 right-shifted E2 bit positions. If E1 has an unsigned type or if E1 has a signed type and a nonnegative value, the value of the result is the integral part of the quotient of E1 / 2 pow E2. If E1 has a signed type and a negative value, the resulting value is implementation-defined.

    Из стандарта следует, что неправильно сдвигать отрицательные числа. Однако, это очень распространённая операция в библиотеке glibc.

    Пример сдвига влево:
    static void init_cacheinfo (void)
    {
      ....
      count_mask = ~(-1 << (count_mask + 1));
      ....
    }

    V610 Undefined behavior. Check the shift operator '<<. The left operand '-1' is negative. cacheinfo.c 645

    Пример сдвига вправо:
    utf8_encode (char *buf, int val)
    {
      ....
      *buf = (unsigned char) (~0xff >> step);
      ....
    }

    Выражение "~0xff" имеет тип 'int' и равно -256.

    Вот список всех мест, где можно наблюдать некорректные сдвиги:
    • strxfrm_l.c 68
    • clock_nanosleep.c 38
    • ifaddrs.c 786
    • xdr_intXX_t.c 35
    • xdr_intXX_t.c 41
    • private.h 327
    • private.h 331
    • zic.c 696
    • zdump.c 212
    • zdump.c 216
    • timer_create.c 47
    • timer_create.c 49
    • loop.c 331
    • loop.c 437
    • mktime.c 207
    • mktime.c 208
    • mktime.c 211
    • mktime.c 212
    • mktime.c 230
    • mktime.c 298
    • mktime.c 298
    • ld-collate.c 298


    Использование неинициализированной переменной


    static int send_vc(....)
    {
      ....
      int truncating, connreset, resplen, n;
      ....
      #ifdef _STRING_ARCH_unaligned
        *anssizp2 = orig_anssizp - resplen;
        *ansp2 = *ansp + resplen;
      #else
      ....
    }
    V614 Uninitialized variable 'resplen' used. res_send.c 790

    Некорректное форматирование строк


    В некоторых местах для распечатки знаковых переменных используется '%u'. Ещё в некоторых местах для распечатки беззнаковых переменных используется '%d'. Это конечно мелочи, но их тоже стоит упомянуть.

    Пример:
    typedef unsigned int __uid_t;
    typedef __uid_t uid_t;
    
    int
    user2netname (...., const uid_t uid, ....)
    {
      ....
      sprintf (netname, "%s.%d@%s", OPSYS, uid, dfltdom);
      ....
    }

    V576 Incorrect format. Consider checking the fourth actual argument of the 'sprintf' function. The SIGNED integer type argument is expected. netname.c 51

    Другие соответствующие сообщения:
    • Consider checking the second actual argument of the 'printf' function. The SIGNED integer type argument is expected. locarchive.c 1741
    • Consider checking the fourth actual argument of the 'printf' function. The SIGNED integer type argument is expected. locarchive.c 1741
    • Consider checking the fifth actual argument of the 'fprintf' function. The SIGNED integer type argument is expected. res_debug.c 236
    • Consider checking the third actual argument of the 'sprintf' function. The UNSIGNED integer type argument is expected. inet_net_ntop.c 134
    • Consider checking the fourth actual argument of the 'sprintf' function. The SIGNED integer type argument is expected. ns_print.c 500
    • Consider checking the fifth actual argument of the 'sprintf' function. The SIGNED integer type argument is expected. ns_print.c 500
    • Consider checking the third actual argument of the 'sprintf' function. The SIGNED integer type argument is expected. ns_print.c 572
    • Consider checking the fourth actual argument of the 'sprintf' function. The SIGNED integer type argument is expected. ns_print.c 572
    • Consider checking the fifth actual argument of the 'sprintf' function. The SIGNED integer type argument is expected. ns_print.c 572
    • Consider checking the third actual argument of the 'sprintf' function. The UNSIGNED integer type argument is expected. ns_print.c 628
    • Consider checking the fourth actual argument of the 'sprintf' function. The UNSIGNED integer type argument is expected. ns_print.c 628
    • Consider checking the fifth actual argument of the 'sprintf' function. The UNSIGNED integer type argument is expected. ns_print.c 628
    • Consider checking the third actual argument of the 'sprintf' function. The SIGNED integer type argument is expected. ns_print.c 645
    • Consider checking the third actual argument of the 'sprintf' function. The UNSIGNED integer type argument is expected. ns_print.c 685
    • Consider checking the second actual argument of the 'printf' function. The SIGNED integer type argument is expected. nis_print.c 209
    • Consider checking the second actual argument of the 'printf' function. The SIGNED integer type argument is expected. sprof.c 480


    Заключение


    Выбран не удачный проект для начала проверки кода из мира Linux. Он слишком качественный. :) Сложно написать интересную статью про ошибки. Но не беда. нас ждёт немало других известных и интересных проектов в Linux, которые мы попроверяем для демонстрации возможностей анализатора PVS-Studio.

    Дополнительные ссылки


    1. Андрей Карпов. Статический анализ и регулярные выражения.
    2. Дмитрий Ткаченко. Беседа с Андреем Карповым, техническим директором проектов PVS-Studio и CppCat.


    Ответы на вопросы
    Часто к нашим статьям задают одни и те же вопросы. Ответы на них мы собрали здесь: Ответы на вопросы читателей статей про PVS-Studio и CppCat, версия 2014.
    PVS-Studio
    766.91
    Static Code Analysis for C, C++, C# and Java
    Share post

    Similar posts

    Comments 63

      +5
      Картинка зачетная!
        +2
        Интересно, у кого рог больше?
        +28
        Ваш проект плох тем, что делает лучше виндовые программы, а не линуксовые =)
        Я шучу и желаю проекту успеха и коммерческой успешности. Приятно читать ваши статьи, даже когда уже умер как программист давным давно.
          0
          Спасибо.
            +3
            Для linux недавно facebook открыл flint
              +1
              Спасибо. Не знал про такое. Но по всей видимости какая-то фитюлька. Объем исходного кода проекта 340 килобайт. Для сравнения, объем исходного кода консольной версии PVS-Studio — около 6 мегабайт.
                +1
                Там в зависимостях google test на 4.5 мб, возможно поэтому
                  +4
                  Нет конечно :-). Это библиотека для юнит-тестов, а не мозг для статического анализатора.
                    0
                    Как мне кажется, это просто для тестирования себя.
                  +1
                  Это больше style-checker, а не статический анализатор кода. Просто посмотрите на заданные правила и сообщения об «ошибках»:

                            lintWarning(*it, "Protected inheritance is sometimes not a good "
                                "idea. Read "
                                "http://stackoverflow.com/questions/6484306/effective-c-discouraging-protected-inheritance "
                                "for more information.\n");
                  

                        lintError(*it, "Open Source Software may not include files from "
                                       "other fbcode projects (except folly). "
                                       "If this is not an fbcode include, please use "
                                       "'#include <...>' instead of '#include \"...\"'. "
                                       "You may suppress this warning by including the "
                                       "comment 'nolint' after the #include \"...\".\n");
                  
                  +1
                  Вообще, если без шуток порассуждать о данном моменте то получается, что PVS действительно плох отсутствием кроссплатформенности. Однако с учётом размера команды проект развивается очень даже замечательно. А дальше тоже не понятно что делать, ведь если начать сильно увеличивать штат, то можно легко потонуть в бюрократии, и испортить проект. В общем ребята правда молодцы.

                  Мне в PVS нравится то, что у него довольно мало воды в предупреждениях, и это очень ценно поскольку многие, даже опытные разработчики, обязательно следуют рекомендациям анализатора, не пытаясь понять зачем они это делают, и не понимая всех последствий. Для примера могу привести свой опыт, когда в одном, довольно большом и старом проекте, человек, позиционирующий себя мантейнером и руководителем проекта засрал все конструкторы инициализациями цифровых членов в 0 или какими то другими «безопасными» значениями, только потому что так ему рекомендовал CppCheck, в результате данной «подушки безопасности» провалилась производительность во многих местах поскольку была поломана отложенная инициализация. которую вносили в проект годами. Помимо этого данный руководитель не понимает и не хочет понимать как работает move-semantics и старательно выпиливает её из классов, которые уже были переписана на C++11. Вот такой маразм, уж простите за «крик души», просто наболело :)
                    +3
                    Спасибо за комментарий, хочу обратить внимание (прежде всего других людей), что «мало воды в предупреждениях» — это результат очень долгой и сложной доводки анализатора под конкретный компилятор (VisualC++) и конкретную платформу. Как только анализатор запускается без такой доводки, скажем в гипотетическом линуксе — из него вода льется как из дырявого ведра.
                      +1
                      Именно так, и за это вам всем огромная благодарность!
                      0
                      * Доп про конструкторы: теперь вообще не возможно узнать отработал ли какой либо функционал или нет, поскольку раньше программа сразу же падала на ошибках в новом коде, а теперь из-за 0 (nullptr) в конструкторах на указателях, и проверок на nullptr везде где только можно программа не падает, но и не пойми что делает. Этот момент заметен по жалобам пользователей у которых постоянно что то начинает глючить, и появляется всё больше и больше гейзенбагов в новых версиях. Вот такая печаль, а проект был хорошим :(
                        +1
                        Свершилось: PVS-Studio для Linux.
                          0
                          Здорово, благодарю!
                      +7
                      С другой стороны, на glibc можно неплохо тренироваться по устранению ложноположительных срабатываний из гвинпинного мира.
                        +17
                        Я бы лучше попробовал поковырять gcc, про который пишут, что кроме пары бородатых программистов никто не понимает как он работает. Ну и заодно что-нибудь из кодовой базы X11, вот там точно код тот еще. Ну и напоследок наверное что-нибудь типа Mesa явно не лишним было бы проверить.
                          +12
                          Мы ещё год назад собирали проверяли gcc. Подтверждаю, что разобраться в проекте очень сложно. Там сплошные макросы, которые раскрываются в макросы, которые раскрываются в макросы, которые раскрываются в макросы…
                          Я сдался и бросил анализ проекта, хотя очень хотелось найти ошибки.
                            +1
                            можна еще попробовать boost

                            упс, уже посмотрел в faq`e
                            +1
                            К сожалению не помню, делали ли вы этоо уже или нет, но могу посоветовать проверить LLVM. Код вполне читаемый и атавизмов в нем практически нет. А вот багов может быть много, поскольку многие модули, судя по всему, писались в спешке и очень разными людьми.
                        +17
                        If E1 has a signed type and a negative value, the resulting value is implementation-defined.

                        implementation-defined это не совсем то же самое, что и undefined. В данном случае «все понимают» (серьезно) что на 2's complement machine будет «правильная» реализация, и это так во всех разумных компиляторах.

                        Еще есть такой момент, что glibc — это стандартная библиотека языка C, ее интерфейсы описаны в стандарте C. По сути, она частично реализует язык C. Соответственно, она написана не на C, а на чем-то слегка его напоминающем, с кучей нестандартных расширений, платформо-зависимой магии и т.д.
                          +5
                          Вообще между undefined behavior и implementation-defined behavior лежит пропасть. Если вы используете implementation-defined behavior — то вы, всё-таки всё ещё в рамках C и во многих случаях всё ещё в безопасности (скажем если вы использете какой-нибудь int8_t, то вы сразу попадаете в рамки 2's complement machine и можете с лёгостью делать подобные сдвиги).

                          А вот разъменование указателя, который позже проверяется на NULL — это не просто ошибка, это катастрофа. Дело в том, что компилятор может после этого проверку на NULL просто выкинуть. Именно потому что undefined behavior с точки зрения компилятора «не бывает» и уж если ты указатель разименовал, то ты тем самым «пообещал», что соответствующий указатель — не NULL.

                          А если после этого он ещё и немного код «пооптимизирует», то всё может кончится чем-нибудь ешё похлеще. Например переменная, которая инициализировалась через этот указатель может оказаться и вообще ненужной и будет выкинута. Получится просто программа, которая игнорирует тот факт, что какой-то указатель может быть равен NULLу.
                            +1
                            XDR *xdrs = &clp->xdr_stream;

                            Это не совсем разыменование указателя, а взятие адреса, по этому код имеет потенциальное право на жизнь. Существуют реализации макроса offsetof точно таким-же образом:
                            en.wikipedia.org/wiki/Offsetof
                            habrahabr.ru/company/pvs-studio/blog/198060/#comment_6871226

                            У меня нет уверенности на счет того, что это точно не UB, но известные мне компиляторы понимают этот код правильно, так как сами используют такое.
                              +2
                              Согласен. Я собственно и говорил, что особенных вещей не нашлось. Но надо попридираться то. А то вроде как зря мучился. :)
                                0
                                Согласно стандарту — это UB.

                                При этом компилятор вправе реализовать offsetof таким образом, потому что реализация стандартной библиотеки привязана к конкретному компилятору, и может спокойно полагаться на его особенности вроде вот этой.
                                  +2
                                  С точки зрения компилятора код
                                  XDR *xdrs = &clp->xdr_stream;
                                  

                                  эквивалентен коду
                                  XDR *xdrs = &((*clp).xdr_stream);
                                  


                                  Т.е. в этом коде разыменование — это вот это:
                                  clp->
                                  

                                  А взятие адреса там уже после разыменования.
                              0
                              По поводу «memset (temp_result, '\0', sizeof (temp_result));»

                              Можно ли заменить это на конструкцию типа?
                              void * (*volatile vol_memset)(void *, int, size_t) = memset;
                              void safe_memclear(void *mem, size_t len) { vol_memset(mem, 0, len); }

                              И еще вот нашел ссылочку по этой теме
                                –1
                                Я не знаю. Возможно. В winnt.h сделано так:

                                FORCEINLINE
                                PVOID
                                RtlSecureZeroMemory(
                                    _Out_writes_bytes_all_(cnt) PVOID ptr,
                                    _In_ SIZE_T cnt
                                    )
                                {
                                    volatile char *vptr = (volatile char *)ptr;
                                
                                #if defined(_M_AMD64)
                                
                                    __stosb((PBYTE )((DWORD64)vptr), 0, cnt);
                                
                                #else
                                
                                    while (cnt) {
                                
                                #if !defined(_M_CEE) && defined(_M_ARM)
                                
                                        __iso_volatile_store8(vptr, 0);
                                
                                #else
                                
                                        *vptr = 0;
                                
                                #endif
                                
                                        vptr++;
                                        cnt--;
                                    }
                                
                                #endif // _M_AMD64
                                
                                    return ptr;
                                }
                                
                                  +2
                                  Кстати в новом C11 [ISO/IEC 9899:2011] должна быть функция memset_s() как раз для таких слачаев. Только не нашел какие компиляторы ее поддерживают.
                                +1
                                __builtin_strcmp

                                Иногда гнать весь код через препроцессор компилятора не самое удачное решение. Именно как раз потому, что это implementation-defined, зависит от используемого компилятора/ОС и не позволяет вычислять ошибки совместимости. Но есть хорошие новости: можно задать параметр оптимизации -O0, когда никаких «левых» inline-подстановок компилятора не возникнет.

                                Для меня стало открытием наличие такого бреда в препроцессоре. В ассемблерном варианте окажется что-то вроде этого:

                                	movq	%rdi, %rax
                                	movq	%rsi, %rdi
                                	movq	%rax, %rsi
                                	jmp	strcmp
                                
                                  0
                                  Удачное/не удачное — выбора нет. Препроцессинг нужен. И нужно уметь работать, в независимости, от ключей. Как проект собирается, с теми ключами и надо его проверять, подменив собой компилятор. Спасибо за информацию про -O0. Но к сожалению, жизнь разработчикам анализаторов это не упрощает. :)
                                    +2
                                    А если учесть что GLibC с -O0 вообще собрать нельзя…
                                      –1
                                      Насколько я знаю, этот анализатор проект не собирает — надо вручную прогнать через препроцессор и потом уже скормить ему полученные i-файлы.
                                        +2
                                        «надо вручную прогнать через препроцессор» — Не надо сбивать с толку людей. А то останется в голове, что это велосипед с ручным приводом. :)

                                        Да, есть и вариант с *.i файлами. А также масса других методов, с которыми можно познакомиться в документации. Например, это интеграция в makefile. Ну а если у Вас Visual C++, так вообще просто надо кнопку нажать и всё проверится.
                                          +1
                                          Ну а если у Вас Visual C++

                                          Особенно легко и непринуждённо VS запускается в RHEL.

                                          Если статья об анализе ключевой библиотеки Linux, вы привлекли внимание всех читателей хаба Linux — извольте приоткрыть тайну анализа линуксовых библиотек. Сейчас из поста получается «раз! — и результат» — это-то и сбивает с толку людей. А процесс копания в настройках компилятора/препроцессора, а также ковыряние километрового лога остался за кадром. Было бы интересно читать как вы решали эти проблемы, а не как тяжело писать код под Linux.
                                            0
                                            Эти проблемы не решены. Сейчас есть только демонстрация возможностей анализатора на проекте из мира Linux.
                                  –1
                                  Как на счёт проверить Transmission?
                                    0
                                    По поводу
                                    while (categoryvalue[0] != '\0' && categoryvalue[0] == ':')

                                    Это не могло быть странной попыткой оптимизации? Например, такая логика автора. Проверка на '\0', если не проходит, сразу вываливаемся из цикла, без проверки на ':'.
                                      0
                                      И где оптимизация? :)
                                        0
                                        Вообще, судя по коду ниже (ветке else после while) похоже действительно просто лишняя проверка, а никакая не ошибка…
                                        0
                                        в коде все четко и однозначно. сей кусок для людей предназначен, а не анализаторов — опенсорс же. ну да, в статье это представлено в духе «скандалы-интриги-расследования», видимо чтобы читать веселее было. :)
                                          +4
                                          > 581 /* Make CATEGORYVALUE point to the next element of the list. */
                                          > 582 while (categoryvalue[0] != '\0' && categoryvalue[0] == ':')
                                          > 583 ++categoryvalue;
                                          И что же такого этот код поясняет, что не пояснил бы
                                          > 582 while (categoryvalue[0] == ':')
                                          > 583 ++categoryvalue;
                                          ? Как по мне, наоборот, запутывает.
                                        0
                                        А не планируете ли Вы проверить опенсорсный проект Tesseract-OCR?
                                        code.google.com/p/tesseract-ocr/
                                          +5
                                          Я проверял этот проект. Как и на любом другом проекте, первая проверка PVS Studio вываливает кучу предупреждений. Как и на любом другом проекте с такой долгой историей, начатой еще на заре С++, почти все предупреждения в дело. Как пишут в своих статься авторы PVS Studio, ошибки действительно везде одни и те же:
                                          • Проверка на NULL после new
                                          • Копипаст условий и then/else блоков у оператора if
                                          • Сначала разыменование, потом проверки
                                          • Очень много 64 битных предупреждений (Лучше не пользоваться 64 битным тессерактом).
                                          • И т.д.


                                          За время своей проверки послал четыре патча. Основная проблема тут во времени их принятия, видимо настоящая разработка тессеракта идет где то в другом месте, а на гугл код просто вываливают релизы. За это время ваши правки могут стать неактуальны, потому что файл к примеру уже удалили из репозитория. Сейчас там кстати появился парень, который заспамил весь Issue List своими патчами, созданными скорее всего тоже под влиянием какого-то статического анализатора.

                                          А вообще не надо просить других сделать то, что вы можете сами. Скачайте и проверьте.
                                          +1
                                          sunrpc/clnt_raw.c это пропиетарное наследие от уже вымерших мамонтов. по умолчанию не компилируется. тут анализатор занимается археологией — в самом деле, поучительно.

                                          там история коммитов выглядит интереснее, чем сам код:
                                          2012-05-10 Andreas Jaeger Make sunrpc code usable again
                                          2011-04-17 Ulrich Drepper Obsolete RPC implementation in libc.
                                          2010-08-19 Ulrich Drepper Once again change RPC copyright notices.
                                          2010-06-28 Ulrich Drepper Revert «Sun agreed to a change of the license for the…
                                          2009-05-21 Ulrich Drepper Sun agreed to a change of the license for the RPC code…

                                          1995-02-18 Roland McGrath initial import

                                          кстати, в текущей ревизии в копирайтах значится Oracle.
                                            0
                                            Как насчёт проверки KPHP HHVM?
                                              +1
                                              для nss/getnssent_r.c анализатор написал ерунду, и у вас код отквочен не правильно:

                                              смотрим
                                               127 int
                                               128 __nss_getent_r (const char *getent_func_name,
                                               129                 const char *setent_func_name,
                                               130                 db_lookup_function lookup_fct,
                                               131                 service_user **nip, service_user **startp,
                                               132                 service_user **last_nip, int *stayopen_tmp, int res,
                                               133                 void *resbuf, char *buffer, size_t buflen,
                                               134                 void **result, int *h_errnop)
                                              ....
                                               144   if (res && __res_maybe_init (&_res, 0) == -1)
                                               145     {
                                               146       *h_errnop = NETDB_INTERNAL;
                                               147       *result = NULL;
                                               148       return errno;
                                               149     }
                                               150 
                                              ...
                                               159   while (! no_more)
                                               160     {
                                               161       int is_last_nip = *nip == *last_nip;
                                               162 
                                               163       status = DL_CALL_FCT (fct.f,
                                               164                             (resbuf, buffer, buflen, &errno, &h_errno));
                                               165 
                                               166       /* The status is NSS_STATUS_TRYAGAIN and errno is ERANGE the
                                               167          provided buffer is too small.  In this case we should give
                                               168          the user the possibility to enlarge the buffer and we should
                                               169          not simply go on with the next service (even if the TRYAGAIN
                                               170          action tells us so).  */
                                               171       if (status == NSS_STATUS_TRYAGAIN
                                               172           && (h_errnop == NULL || *h_errnop == NETDB_INTERNAL)
                                               173           && errno == ERANGE)
                                               174         break;
                                              

                                              тут видно, что в первом случае (145) присваивается значение, в указатель, пришедший в функцию в качестве параметра. он не может быть NULL по соглашению. в 163 этот указатель передается по ссылке функции DL_CALL_FCT, и в 172 проверяется уже новое значение, возвращаемое этой функцией.
                                                +1
                                                Вот оно как. Ну и хорошо, что ошибки нет.
                                                +1
                                                XDR *xdrs = &clp->xdr_stream;

                                                if (clp == NULL)


                                                Ложное срабатывание. Разыменовывание не происходит, просто один указатель (xdrs) получает значение другого указателя (cpl) плюс смещение поля xdr_stream.

                                                UPD. А. уже написали выше… habrahabr.ru/company/pvs-studio/blog/213969/#comment_7356737
                                                  0
                                                  Есть такой крутой открытый графический движок — Ogre3D. У него очень хороший код, было бы интересно посмотреть на результаты проверки. Ну, вдруг, будет время…
                                                    +7
                                                    К сожалению поделиться будет нечем. Команда Ogre3D давно и успешно применяет PVS-Studio, так что извините, «срыва покровов» не будет.
                                                      0
                                                      Круто!

                                                      А bitcoin не проверяли?
                                                  0
                                                  А можно у вас заказать проверку других опенсорсных библиотек? В частности, интересует ffmpeg
                                                    +3
                                                    А это интересный способ монетизации для такого продукта )
                                                    0
                                                    Вы еще линукс-ядро не смотрели… Там иногда такое встречается-сомневаешься на С ли это написано
                                                      0
                                                      Мне кажется, там всё тоже будет очень хорошо, как и в glibc. Быть может, когда-то проверим.
                                                      0
                                                      А если github.com/zinnschlag/openmw/ проверить?
                                                        0
                                                        Взяли на заметку.

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