Проверяем Wine с помощью PVS-Studio и Clang Static Analyzer

    PVS-Studio, Clang, Wine
    В статье хочу рассказать о проверке проекта Wine такими статическими анализаторами C/C++ кода, как PVS-Studio и Clang Static Analyzer.

    Wine


    Wine (Wine Is Not Emulator — Wine — не эмулятор) — это набор программ, позволяющий пользователям Linux, Mac, FreeBSD, и Solaris запускать Windows-приложения без необходимости установки на компьютер самой Microsoft Windows. Wine является активно развивающимся кросс-платформенным свободным ПО, распространяемым под лицензией GNU Lesser General Public License.

    Исходный код проекта можно получить командой git clone git://source.winehq.org/git/wine.git

    Об анализаторах


    • PVS-Studio — статический анализатор, выявляющий ошибки в исходном коде приложений на языке C/C++/C++11. Для проверки проекта использовалась релиз-версия PVS-Studio 5.18.
    • Clang Static Analyzer — статический анализатор, который находит ошибки в C, C++ и Objective-C программах. Для проверки проекта использовалась релиз-версия Clang 3.4.2 для openSUSE 13.1.

    Результаты проверки в PVS-Studio



    Сдвиг отрицательного числа


    V610 Undefined behavior. Check the shift operator '<<. The left operand '(LONGLONG) — 1' is negative. propvar.c 127
    ...
    if (*res >= ((LONGLONG)1 << (dest_bits-1)) ||
      *res < ((LONGLONG)-1 << (dest_bits-1)))
      return HRESULT_FROM_WIN32(ERROR_ARITHMETIC_OVERFLOW);
    ...

    Тип LONGLONG объявлен как 'typedef signed __int64 LONGLONG;', т.е. является знаковым типом. Сдвиги отрицательных чисел по новому стандарту приводят к неопределённому или неуточненному поведению. Почему такой код может работать и как его лучше исправить, можно прочитать в статье: Не зная брода, не лезь в воду. Часть третья.

    Опечатки


    V501 There are identical sub-expressions '!lpScaleWindowExtEx->xNum' to the left and to the right of the '||' operator. enhmetafile.c 1418
    ...
    if (!lpScaleWindowExtEx->xNum || !lpScaleWindowExtEx->xDenom ||
        !lpScaleWindowExtEx->xNum || !lpScaleWindowExtEx->yDenom)
      break;
    ...

    В условии два раза проверяется lpScaleWindowExtEx->xNum, скорее всего в одном месте должно быть lpScaleWindowExtEx->yNum. В объявлении используемой структуры такое поле есть:
    typedef struct {
        EMR  emr;
        LONG xNum;   //<==
        LONG xDenom;
        LONG yNum;   //<==
        LONG yDenom;
    } EMRSCALEVIEWPORTEXTEX, *PEMRSCALEVIEWPORTEXTEX,
      EMRSCALEWINDOWEXTEX,   *PEMRSCALEWINDOWEXTEX;

    V501 There are identical sub-expressions '!(types[i + 1] & PathPointTypeBezier)' to the left and to the right of the '||' operator. graphics.c 1751
    ....
    for(i = 1; i < count; i++){
      ....
      if((i + 2 >= count) ||
        !(types[i + 1] & PathPointTypeBezier) ||
        !(types[i + 1] & PathPointTypeBezier)){
        ....
      }
      i += 2;
    }
    ....

    Это место обнаружено аналогичной диагностикой V501, но причина одинаковых условий тут не так очевидна. Скорее всего, в одном условии должно быть types[i + 2], потому что выше проверили возможность обратиться к элементу массива с индексом на 2 больше, чем 'i'.

    V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. request.c 3354
    if ((hr = SafeArrayAccessData( sa, (void **)&ptr )) != S_OK)
      return hr;
    if ((hr = SafeArrayGetUBound( sa, 1, &size ) != S_OK)) //<==
    {
        SafeArrayUnaccessData( sa );
        return hr;
    }

    Приоритет оператора != выше, чем оператора присваивания =. Причём по условию выше хорошо видно, что здесь тоже необходимо обернуть присваивание ещё в одни скобки, иначе hr получает значение 0 или 1.

    Ещё похожее место:

    V501 There are identical sub-expressions to the left and to the right of the '|' operator: VT_ARRAY | VT_ARRAY vartest.c 2161

    Каскадирование условных операторов


    V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 1754, 1765. msi.c 1754
    if (!strcmpW( szProperty, INSTALLPROPERTY_LOCALPACKAGEW )) //<==
    {
      ...
    }
    else if (!strcmpW( szProperty, INSTALLPROPERTY_INSTALLDATEW ))
    {
      ...
    }
    else
    if (!strcmpW( szProperty, INSTALLPROPERTY_LOCALPACKAGEW )) //<==
    {
      ...
    }
    else if (!strcmpW( szProperty, INSTALLPROPERTY_UNINSTALLABLEW ) ||
             !strcmpW( szProperty, INSTALLPROPERTY_PATCHSTATEW ) ||
             !strcmpW( szProperty, INSTALLPROPERTY_DISPLAYNAMEW ) ||
             !strcmpW( szProperty, INSTALLPROPERTY_MOREINFOURLW ))
    {
      ...
    }
    else
    {
      ...
    }

    Если в каскаде условных операторов проверяются одинаковые условия, то последние не получают управление. Возможно здесь опечатка в переданной константе INSTALLPROPERTY_*.

    Эквивалентные ветви оператора if


    V523 The 'then' statement is equivalent to the 'else' statement. filedlg.c 3302
    if(pDIStruct->itemID == liInfos->uSelectedItem)
    {
      ilItemImage = (HIMAGELIST) SHGetFileInfoW (
        (LPCWSTR) tmpFolder->pidlItem, 0, &sfi, sizeof (sfi),
        shgfi_flags );
    }
    else
    {
      ilItemImage = (HIMAGELIST) SHGetFileInfoW (
        (LPCWSTR) tmpFolder->pidlItem, 0, &sfi, sizeof (sfi),
        shgfi_flags );
    }

    Подобный код либо избыточен, либо имеет место опечатка.

    V523 The 'then' statement is equivalent to the 'else' statement. genres.c 1130
    ...
    if(win32)
    {
      put_word(res, 0);  /* Reserved */
      /* FIXME: The ResType in the NEWHEADER structure should
       * contain 14 according to the MS win32 doc. This is
       * not the case with the BRC compiler and I really doubt
       * the latter. Putting one here is compliant to win16 spec,
       * but who knows the true value?
       */
      put_word(res, 1);  /* ResType */
      put_word(res, icog->nicon);
      for(ico = icog->iconlist; ico; ico = ico->next)
      {
        ...
      }
    }
    else /* win16 */
    {
      put_word(res, 0);  /* Reserved */
      put_word(res, 1);  /* ResType */
      put_word(res, icog->nicon);
      for(ico = icog->iconlist; ico; ico = ico->next)
      {
        ...
      }
    }
    ...

    Здесь одна из повторяющихся ветвей прокомментирована. Возможно анализатор нашёл место, которое недоделали. Не ошибка, но решил отметить.

    Изменение длины строки


    V692 An inappropriate attempt to append a null character to a string. To determine the length of a string by 'strlen' function correctly, a string ending with a null terminator should be used in the first place. appdefaults.c 390
    ...
    section[strlen(section)] = '\0'; /* remove last backslash  */
    ...

    На самом деле ноль запишется в ноль и ничего не изменится. Для работы функции strlen() строка уже должна заканчиваться терминальным нулём. Комментарий намекает нам, что, наверное, хотели отрезать косую черту в конце. Тогда, видимо код должен быть такой:
    section[strlen(section) - 1] = '\0';

    Один счётчик на два цикла


    V535 The variable 'i' is being used for this loop and for the outer loop. Check lines: 980, 1003. iphlpapi_main.c 1003
    ...
    for (i = 0; i < num_v6addrs; i++)    //<==
    {
        ...
        for (i = 0; i < 8 && !done; i++) //<==
        {
            ...
        }
        ...
        if (i < num_v6addrs - 1)
        {
            prefix->Next = (IP_ADAPTER_PREFIX *)ptr;
            prefix = prefix->Next;
        }
    }
    ...

    Подозрительное место: вложенный цикл организован с использованием переменной i, которая также используется и во внешнем цикле.

    Двойное приведение типов


    В следующих двух примерах к указателю *void применяется два приведения к типу: сначала к char*, потом к DWORD*, после чего прибавляется смещение. В выражении не хватает скобок, либо код избыточен. Есть ли тут ошибка, зависит от того, на сколько хотели увеличить значение указателя.

    V650 Type casting operation is utilized 2 times in succession. Next, the '+' operation is executed. Probably meant: (T1)((T2)a + b). typelib.c 9147
    ...
    struct WMSFT_SegContents arraydesc_seg;
    typedef struct tagWMSFT_SegContents {
        DWORD len;
        void *data;
    } WMSFT_SegContents;
    ...
    DWORD offs = file->arraydesc_seg.len;
    DWORD *encoded;
    encoded = (DWORD*)((char*)file->arraydesc_seg.data) + offs;//<==

    Ещё одна схожая ситуация:

    V650 Type casting operation is utilized 2 times in succession. Next, the '+' operation is executed. Probably meant: (T1)((T2)a + b). protocol.c 194
    INT WINAPI
    EnumProtocolsW(LPINT protocols, LPVOID buffer, LPDWORD buflen)
    {
      ...
      unsigned int string_offset;
      ...
      pi[i].lpProtocol = (WCHAR*)(char*)buffer + string_offset;//<==
      ...
    }

    Разность беззнаковых чисел


    V555 The expression 'This->nStreams — nr > 0' will work as 'This->nStreams != nr'. editstream.c 172
    static HRESULT
    AVIFILE_RemoveStream(IAVIEditStreamImpl* const This, DWORD nr)
    {
      ...
      This->nStreams--;
      if (This->nStreams - nr > 0) { //<==
        memmove(This->pStreams + nr, This->pStreams + nr + 1,
                (This->nStreams - nr) * sizeof(EditStreamTable));
      }
      ...
    }

    Переменная nr имеет баззнаковый тип DWORD. Вычитая её, результат разности также будет иметь беззнаковый тип. Если nr будет больше This->nStreams — условие всё равно будет истинным.

    Аналогичное место:

    V555 The expression 'This->fInfo.dwStreams — nStream > 0' will work as 'This->fInfo.dwStreams != nStream'. avifile.c 469

    Сначала казнь, потом обед


    V595 The 'decl' pointer was utilized before it was verified against nullptr. Check lines: 1411, 1417. parser.y 1411
    ...
    var_t *v = decl->var; //<==
    expr_list_t *sizes = get_attrp(attrs, ATTR_SIZEIS);
    expr_list_t *lengs = get_attrp(attrs, ATTR_LENGTHIS);
    int sizeless;
    expr_t *dim;
    type_t **ptype;
    array_dims_t *arr = decl ? decl->array : NULL;     //<==
    type_t *func_type = decl ? decl->func_type : NULL; //<==
    ...

    Сначала брали значение по указателю, потом решили проверять.

    Аналогичные места:
    • V595 The 'pcbData' pointer was utilized before it was verified against nullptr. Check lines: 1859, 1862. registry.c 1859
    • V595 The 'token_user' pointer was utilized before it was verified against nullptr. Check lines: 206, 213. lsa.c 206
    • V595 The 'psp' pointer was utilized before it was verified against nullptr. Check lines: 2680, 2689. propsheet.c 2680
    • V595 The 'lpFindInfo' pointer was utilized before it was verified against nullptr. Check lines: 6285, 6289. listview.c 6285
    • V595 The 'compiland' pointer was utilized before it was verified against nullptr. Check lines: 287, 294. symbol.c 287
    • V595 The 'graphics' pointer was utilized before it was verified against nullptr. Check lines: 2096, 2112. graphics.c 2096
    • V595 The 'current' pointer was utilized before it was verified against nullptr. Check lines: 240, 251. request.c 240

    Печать результата одинаковых функций


    V681 The language standard does not define an order in which the 'tlb_read_byte' functions will be called during evaluation of arguments. tlb.c 650
    ...
    printf("\\%2.2x \\%2.2x\n", tlb_read_byte(), tlb_read_byte());
    ...

    Согласно стандарту языка Си++, не определена последовательность вычисления фактических аргументов функции. Какая функция будет вызвана первой, зависит от компилятора, параметров компиляции и так далее.

    Сомнительные тесты


    В директориях некоторых модулей имеется каталог test с исходными файлами для тестов. Для вывода отладочной информации используется макрос 'ok'. Вот несколько подозрительных мест:

    V501 There are identical sub-expressions to the left and to the right of the '==' operator: ddsd3.lpSurface == ddsd3.lpSurface dsurface.c 272
    ...
    ok(ddsd3.lpSurface == ddsd3.lpSurface,    //<==
      "lpSurface from GetSurfaceDesc(%p) differs\
        from the one returned by Lock(%p)\n",
      ddsd3.lpSurface, ddsd2.lpSurface);      //<==
    ...

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

    V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '==' operator. url.c 767
    ...
    ok(size == no_callback ? 512 : 13, "size=%d\n", size);
    ...

    Приоритет оператора "==" выше '?:', поэтому здесь переменная size не сравнивается со значениями 512 и 13. Выражение всегда истинно, так как будет равно 512 или 13 и значит проверка ничего не проверят.

    Аналогичные предупреждения:
    • V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '==' operator. string.c 1086
    • V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '==' operator. string.c 1111
    • V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '==' operator. reader.c 761
    • V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '==' operator. protocol.c 2928
    • V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '==' operator. dde.c 1594
    • V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '==' operator. reader.c 761

    Результаты проверки в Clang Static Analyzer


    Поиск потенциальных ошибок этим анализатором осуществляется путём обхода возможных сценариев выполнения программы. Если подозрительное место найдено, то анализатор создаст отчёт для этого файла в формате HTML (по умолчанию) или PLIST, в котором будут прокомментированы от одного до нескольких десятков шагов, приводящих к подозрительному участку кода.

    Большинство сообщений, полученных в ходе проверки проекта Wine, имеют один и тот же характер: объявляется переменная без инициализации; в функции, в которую передаётся адрес переменной, пропущена инициализация в некоторых ветвях оператора switch, либо выполнен 'return' до инициализации. Подобные ситуации в дальнейшем не обрабатываются и неинициализированная переменная продолжает использоваться. Счёт таких мест в проекте идёт на сотни, поэтому в данном обзоре рассмотрены не будут. Что-то из них, думаю, может представлять настоящие серьезные ошибки. Оставим это на совести разработчиков.

    Неинициализированная переменная в условии


    File: dlls/atl110/../atl/atl_ax.c

    Location: line 1092, column 10

    Description: Branch condition evaluates to a garbage value
    HRESULT
    WINAPI AtlAxCreateControlEx(LPCOLESTR lpszName, HWND hWnd,
      IStream *pStream, IUnknown **ppUnkContainer,
      IUnknown **ppUnkControl, REFIID iidSink, IUnknown *punkSink)
    {
      ...
      IUnknown *pContainer;
      ...
      hRes = AtlAxAttachControl( pUnkControl, hWnd, &pContainer );
      if ( FAILED( hRes ) ) 
        WARN("cannot attach control to window\n");
      ...
      if ( pContainer ) //<==
      //Clang: Branch condition evaluates to a garbage value
            IUnknown_Release( pContainer );
      return S_OK;
    }

    Неинициализированная переменная pContainer используется в условии после вызова AtlAxAttachControl. Описание этой функции представлено ниже.
    HRESULT
    WINAPI AtlAxAttachControl(IUnknown *control, HWND hWnd,
                              IUnknown **container)
    {
      HRESULT hr;
      ...
      if (!control)
        return E_INVALIDARG;//<==
      hr = IOCS_Create( hWnd, control, container );
      return hWnd ? hr : S_FALSE;
    }

    Здесь может быть выполнен возврат значения E_INVALIDARG до инициализации переменной container. В результате, функция AtlAxCreateControlEx выдаст предупреждение и продолжит работать с неинициализированной переменной.

    Возможное переполнение буфера


    File: tools/widl/typegen.c

    Location: line 1158, column 28

    Description: String copy function overflows destination buffer
    static unsigned int write_new_procformatstring_type(...)
    {
      char buffer[64];
      ...
      strcpy( buffer, "/* flags:" );
      if (flags & MustSize) strcat( buffer, " must size," );
      if (flags & MustFree) strcat( buffer, " must free," );
      if (flags & IsPipe) strcat( buffer, " pipe," );
      if (flags & IsIn) strcat( buffer, " in," );
      if (flags & IsOut) strcat( buffer, " out," );
      if (flags & IsReturn) strcat( buffer, " return," );
      if (flags & IsBasetype) strcat( buffer, " base type," );
      if (flags & IsByValue) strcat( buffer, " by value," );
      if (flags & IsSimpleRef) strcat( buffer, " simple ref," );
      ...
    }

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

    Потенциальная утечка памяти


    File: libs/wpp/ppl.yy.c

    Location: line 4475, column 1

    Description: Potential memory leak
    static void macro_add_arg(int last)
    {
      ..
      if(last || mep->args[mep->nargs-1][0])
      {
        yy_push_state(pp_macexp);
        push_buffer(NULL, NULL, NULL, last ? 2 : 1);
        ppy__scan_string(mep->args[mep->nargs-1]);
        //Clang: Calling 'ppy__scan_string'
        //Clang: Returned allocated memory
      }
        //Clang: Potential memory leak
    }

    Функция pyy__scan_string имеет возвращаемое значение, которое не используется. Вызов этой функции так или иначе приводит к возврату значения функцией malloc(), после использования которой необходимо освобождать память.

    Давайте рассмотрим, как вызов функции pyy__scan_string приводит к вызову malloc.
    YY_BUFFER_STATE ppy__scan_string (yyconst char * yystr )
    {
      return ppy__scan_bytes(yystr,strlen(yystr) );
    }
    
    YY_BUFFER_STATE ppy__scan_bytes  (yyconst char * yybytes,
                                      yy_size_t  _yybytes_len )
    {
      YY_BUFFER_STATE b;
      char *buf;
      ...
      buf = (char *) ppy_alloc(n  );
      ...
      b = ppy__scan_buffer(buf,n );
      ...
      return b;
    }
    
    YY_BUFFER_STATE ppy__scan_buffer  (char * base, yy_size_t size )
    {
      YY_BUFFER_STATE b;
        ...
      b=(YY_BUFFER_STATE) ppy_alloc(sizeof(struct yy_buffer_state));
      ...
      return b;
    }
    
    void *ppy_alloc (yy_size_t  size )
    {
      return (void *) malloc( size );
    }

    Деление на ноль


    File: dlls/winex11.drv/palette.c

    Location: line 601, column 43

    Description: Division by zero
    #define NB_RESERVED_COLORS 20
    ...
    static void X11DRV_PALETTE_FillDefaultColors(....)
    {
      ...
      int i = 0, idx = 0;
      int red, no_r, inc_r;
      ...
      if (palette_size <= NB_RESERVED_COLORS)
        return;
      while (i*i*i < (palette_size - NB_RESERVED_COLORS)) i++;
      no_r = no_g = no_b = --i;
      ...
      inc_r = (255 - NB_COLORCUBE_START_INDEX)/no_r;
      //Clang: Division by zero
      ...
    }

    Код продолжит выполняться если значение переменной palette_size равно 21 и более. При значении 21, переменная 'i' в начале будет увеличена на единицу, а затем уменьшена на единицу. В результате 'i' останется равна нулю, что и приведёт к делению на ноль.

    Неинициализированный элемент массива


    File: dlls/avifil32/api.c

    Location: line 1753, column 10

    Description: Assigned value is garbage or undefined
    #define MAX_AVISTREAMS 8
    ...
    HRESULT WINAPI AVISaveVW(....int nStreams ....)
    {
      ...
      //Объявление массива из 8 элементов, [0..7]
      PAVISTREAM     pInStreams[MAX_AVISTREAMS];
      ...
      if (nStreams >= MAX_AVISTREAMS) {
        WARN(...);
        return AVIERR_INTERNAL;
      }
      ...
      //Инициализация первых семи элементов, [0..6].
      for (curStream = 0; curStream < nStreams; curStream++) {
        pInStreams[curStream]  = NULL;
        pOutStreams[curStream] = NULL;
      }
      ...
      for (curStream = 0; curStream < nStreams; curStream++) {
      ...
      if (curStream + 1 >= nStreams) {
        /* move the others one up */
        PAVISTREAM *ppas = &pInStreams[curStream];
        int            n = nStreams - (curStream + 1);
    
        do {
          *ppas = pInStreams[curStream + 1];
          //Clang: Assigned value is garbage or undefined
        } while (--n);
      }
      ...
      }
    ...
    }

    Здесь объявляется массив из 8 элементов. Код продолжит выполняться если значение переменной nStreams меньше 8, т.е. максимум 7. Все циклы в этой функции, с условным выражением (curStream < nStreams) не досчитываются последнего элемента, как при инициализации, так и при использовании. В месте, где выдал сообщение Clang, как раз берётся восьмой элемент с индексом 7, так как выражение (curStream + 1 >= nStreams) будет истинно при значениях curStream==6 и nStreams==7.Обращение к массиву pInStreams[curStream + 1] даст последний неинициализированный ранее элемент.

    Нулевой путь


    File: dlls/crypt32/rootstore.c
    Location: line 413, column 10
    Description: Null pointer passed as an argument to a 'nonnull' parameter
    static BOOL import_certs_from_path(LPCSTR path,
      HCERTSTORE store, BOOL allow_dir)
    {
      ...
      fd = open(path, O_RDONLY);
      //Clang: Null pointer passed as
      //an argument to a 'nonnull' parameter
      ...
    }

    Чтобы понять, почему Clang считает, что сюда может прийти NULL, рассмотрим место, где вызывается эта функция:
    static BOOL import_certs_from_dir(LPCSTR path, HCERTSTORE store)
    {
      ...
      char *filebuf = NULL;
      //Clang: 'filebuf' initialized to a null pointer value
      struct dirent *entry;
      while ((entry = readdir(dir)))
      {
        ...
        size_t name_len = strlen(entry->d_name);
    
        //Вызов функции для изменения filebuf
    
        if (!check_buffer_resize(&filebuf, &bufsize,
                                  path_len + 1 + name_len + 1))
        {
          ERR(...);
          break;
        }
        snprintf(filebuf, bufsize, "%s/%s", path, entry->d_name);
        if (import_certs_from_path(filebuf, store, FALSE) && !ret)
          //Clang: Passing null pointer value via 1st parameter 'path'
          //Clang: Calling 'import_certs_from_path'
          ret = TRUE;
        ...
      }
    }

    Здесь зовётся функция check_buffer_resize, в которой должно изменяться значение переменной filebuf или возвращаться FALSE, но функция может не изменить filebuf и вернуть TRUE, рассмотрим код функции ниже:
    static BOOL check_buffer_resize(char **ptr_buf,
      size_t *buf_size, size_t check_size)
    {
      if (check_size > *buf_size)
      {
        ...
        *ptr_buf = CryptMemAlloc(*buf_size);
        ...
      }
      return TRUE;
    }

    Функция содержит только одно условие, в котором изменяется переменная ptr_buf и в случае невыполнения условия, истинный результат возврата позволяет в дальнейшем использовать эту переменную.

    Похожая ситуация с функцией memcpy():

    File: server/directory.c

    Location: line 548, column 21

    Description: Null pointer passed as an argument to a 'nonnull' parameter

    Ненадёжная проверка


    File: dlls/advapi32/registry.c

    Location: line 1209, column 13

    Description: Array access (from variable 'str') results in a null pointer dereference
    LSTATUS WINAPI RegSetValueExW(...., const BYTE *data, .... )
    {
      ...
      if (data && ((ULONG_PTR)data >> 16) == 0)
        //Assuming pointer value is null
        return ERROR_NOACCESS;
    
      if (count && is_string(type))
      {
        LPCWSTR str = (LPCWSTR)data;
        //Clang: 'str' initialized to a null pointer value
        if (str[count / sizeof(WCHAR) - 1] &&
            !str[count / sizeof(WCHAR)])
        //Clang: Array access (from variable 'str') results in
        //a null pointer dereference
            count += sizeof(WCHAR);
      }
      ...
    }

    Если придёт нулевой указатель data, то программа продолжит выполняться до обращения к переменной str.

    Похожее место:

    File: dlls/comctl32/comctl32undoc.c

    Location: line 964, column 12

    Description: Array access (from variable 'lpDest') results in a null pointer dereference

    Заключение


    Сравниваемые анализаторы PVS-Studio и Clang Static Analyzer применяют разную методологию анализа кода, следовательно, были получены различные, но неплохие результаты для обоих анализаторов.

    Стоит отметить, что Clang Static Analyzer отличился отсутствием разнообразия в диагностиках. По сути он предупреждает везде о том, что переменная имеет некорректное значение (нулевой указатель, ноль, неинициализированная переменная и так далее). В зависимости от значения переменной и как она используется, и формируется диагностическое сообщение. PVS-Studio более разнообразен в типах диагностик и хорошо умеет выявлять различные опечатки.

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

    Эта статья на английском


    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Svyatoslav Razmyslov. Checking Wine with PVS-Studio and Clang Static Analyzer.

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

    Similar posts

    Comments 28

      –28
      А в этот раз спросят о том, отправили ли Вы разработчикам багрепорты?
        +14
        Ну очередной юморист в лице вас уже нашелся, поэтому вряд ли.
          +5
          Не юморист, а суицидник ))
        +66
        Все ошибки Wine призываю считать фичами для совместимости с WinAPI!
          0
          А проверьте в следующий раз универсальный драйвер UniATA

          alter.org.ua/ru/soft/win/uni_ata/
            +4
            Симметричный ответ — это проверить cygwin :)
              +3
              Jeditobe, Вы весьма рискуете и ставите под возможный удар Ваш любимый проект РеактОС, появляясь в тредах PVS-Studio. Будет стыдно! Покайтесь, пока не поздно!
                +3
                А можно по подробней? Чего именно мне должно быть стыдно? То, что найдут баги?

                Так пусть будет, как говорил известный герой ютуба: «Ломай меня полностью, полностью ломай. Я спортсмен...»
                Чем больше ошибок найдется, тем лучше, мы их исправим.
                  +1
                  Уж сколько раз твердили миру, что смысл статического анализатора в регулярных проверках. Тем более, что есть программа поддержки проектов с открытым исходным кодом.
                +2
                В этом проекте всего два десятка исходных файлов. Он слишком маленький, но так как проект собирается с помощью nmake, то вы можете самостоятельно проверить его с помощью этой утилиты: PVS-Studio теперь поддерживает любую сборочную систему на Windows и любой компилятор. Легко и «из коробки». Триального режима более чем достаточно. С точки зрения анализа, в этом режиме возможности программы ничем не ограничены.
                +1
                Всегда можно сказать, что в Wine не баги, а слой совместимости.
                  +2
                  Даешь проверку cocos2d-x!
                  0
                  Ребята, Вы офигенны :) Технологичные проекты из РФ — рулез!

                  Вы в оснвоном тестируете прикладной софт, а может попробуете пройтись по серьезному системному софту? Например, ploop git.openvz.org/?p=ploop;a=tree и git.openvz.org/?p=vzctl;a=tree у этих проектов сотни тысяч кастомеров, а ошибка в их коде намного более опасна и деструктивна чем в прикладном софте, так как тут речь идет уже об управлении целыми окружениям.
                    +1
                    На первый взгляд это довольно маленькие проекты, правильно?
                      0
                      Да, я думаю там не более 10 тысяч строк кода в каждом. Но они реализуют очень опасные вещи — управляют ядром Linux (vzctl) и кастомной файловой системой для VPS (ploop). Цена ошибки тут — потерянные данные.
                        0
                        Как правило в маленьких проектах ошибок бывает очень мало, поскольку их легко просмотреть глазами. Выгода от статического анализа видна на средних и больших проектах.
                      +1
                      Ждем обновлений windows, ведь как говорят: «wine настолько суров, что Microsoft сам качает у него библиотеки.»
                        0
                        Автор, каково ваше впечатление от кода wine в целом?
                          0
                          На мой взгляд в проект безусловно внесли вклад малоопытные люди. Формат вывода сообщений Clang'ом заставил перечитать много кода, и там хватает мест, где без анализаторов понятно, что код написан крайне ненаглядно и запутано, но если подумать, то можно упростить и где-то даже оптимизировать. Но в первую очередь необходимо смотреть полные отчёты проверки, потому что проект очень крупный, чтобы разобрать его даже в нескольких статьях.
                          +2
                          А с Objective-C эта штука не заработает совсем? Все таки это надмножество C.
                            +1
                            Был бы подмножеством — заработало бы.
                            0
                            И всё же мне непонятно, чем undefined behavior -1 << N отличается от ~0u << N (описанного в Не зная брода, не лезь в воду. Часть третья.), который по сути получает ту же -1 и сдвигает её. Для чего совершать на одну операцию больше? Только потому, что определён беззнаковый ноль? Может можно воспользоваться конструкцией (unsigned) -1.
                              +1
                              -1 — имеет знаковый тип
                              ~0u — беззнаковый тип

                              (unsigned) -1. — да, можно и так.
                              +1
                              Читая тематику PVS и эту статью, раздел «Каскадирование условных операторов», прихожу к логическому умозаключнию, что копипаста блоков — это зло и лушче потратить дополнительную минуту, но переписать код руками.

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