# Проверка Intel IPP Samples for Windows \- продолжение

Прогресс не стоит на месте\. Развивается и мой любимый статический анализатор кода PVS\-Studio\. И недавно я понял, что те проекты, которые мы уже проверяли, можно вполне проверять заново\. Писать про это новые статьи как\-то странно, и вряд ли они получатся интересными\. Однако, одну такую статью я всё\-таки думаю надо сделать\. Она станет еще одним аргументом в пользу утверждения, что настоящую пользу от статического анализа можно получить только при его регулярном использовании, а не при проверках от случая к случаю\. Итак, посмотрим, что же удалось найти нового интересного в проекте Intel IPP Samples\.

Предыдущая заметка "Intel IPP Samples for Windows \- работа над ошибками" \[[1](https://pvs-studio.ru/ru/blog/posts/cpp/a0069/)\] была опубликована 27 января 2011 года\. Прошло около 9 месяцев\. За это время в IPP Samples были поправлены многие ошибки, описанные в статье\. Вышло несколько обновлений библиотеки\. Поскольку разработчики из Intel не заинтересовались PVS\-Studio, то проверка получилась разовая\. И теперь можно хорошо увидеть, какие новые интересные ошибки найдет анализатор через 9 месяцев своего развития\.

Не будем предаваться пустословию, мы ведь программисты и перейдем побыстрее к примерам кода\. Анализ выполнялся [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/) 4\.37 \(предыдущая заметка относится к версии 4\.10\)\. Конечно, приведены будут не все дефекты, а только интересные и не сильно повторяющиеся\. Тот, кто хочет увидеть все подводные камни, может [воспользоваться](https://pvs-studio.ru/ru/blog/posts/0092/) PVS\-Studio и изучить отчёт\. Но мы здесь не для этого\.

## Классическое неопределенное поведение

```cpp
template<typename T, Ipp32s size>
void HadamardFwdFast(..., Ipp16s* pDst)
{
  Ipp32s *pTemp;
  ...
  for(j=0;j<4;j++) {
    a[0] = pTemp[0*4] + pTemp[1*4];
    a[1] = pTemp[0*4] - pTemp[1*4];
    a[2] = pTemp[2*4] + pTemp[3*4];
    a[3] = pTemp[2*4] - pTemp[3*4];
    pTemp = pTemp++;

    pDst[0*4] = (Ipp16s)(a[0] + a[2]);
    pDst[1*4] = (Ipp16s)(a[1] + a[3]);
    pDst[2*4] = (Ipp16s)(a[0] - a[2]);
    pDst[3*4] = (Ipp16s)(a[1] - a[3]);
    pDst = pDst++;
  }
  ...
}
```

Диагностические сообщения PVS\-Studio:

V567 Undefined behavior\. The 'pTemp' variable is modified while being used twice between sequence points\. me umc\_me\_cost\_func\.h 168

V567 Undefined behavior\. The 'pDst' variable is modified while being used twice between sequence points\. me umc\_me\_cost\_func\.h 174

Просто канонический пример, который приводится в статьях для демонстрации неопределенного поведения [\[2\]](http://alenacpp.blogspot.com/2005/11/sequence-points.html)\. Невозможно сказать увеличатся переменные pTemp и pDst или нет, так как они два раза изменяются в рамках одной точки следования\. Результат зависит от компилятора и настроек оптимизации\.

Есть и другой аналогичный фрагмент кода:

```cpp
void VC1BRC_I::CompleteFrame(ePType picType)
{
  ...
  m_Quant.LimIQuant = m_Quant.LimIQuant--;
  ...
  m_Quant.IQuant = m_Quant.IQuant--;
  ...
}
```

## Неопределенное поведение и префиксный инкремент

```cpp
bool MoveOnNextFrame()
{
  if (m_nFrames>0)
  {
    m_pFrame[m_curIndex] = 0;
    m_curIndex = (++m_curIndex)%m_maxN;
    m_nFrames--;
    return true;
  }
  return false;
}
```

Диагностическое сообщение PVS\-Studio:

V567 Undefined behavior\. The 'm\_curIndex' variable is modified while being used twice between sequence points\. vc1\_enc umc\_vc1\_enc\_planes\.h 630

А вот еще один пример неопределенного поведения\. Хотя здесь используется префиксный инкремент, по сути, это ничего не меняет\. Все равно здесь одна точка следования, а переменная m\_curIndex изменяется 2 раза\. Теоретически компилятор вправе создать следующий псевдокод:

A \= m\_curIndex \+ 1;

B \= A % m\_maxN;

m\_curIndex \= B;

m\_curIndex \= A;

На практике это вряд ли произойдет, и переменная сразу будет увеличена, но полагаться на это никак нельзя\.

## Опечатка в имени объекта

```cpp
IPLFUN(void, iplMpyRCPack2D,
  (IplImage* srcA, IplImage* srcB, IplImage* dst))
{
  ...
  if( (srcA->depth == IPL_DEPTH_8U ) ||
      (srcB->depth == IPL_DEPTH_8U ) ||
      (srcB->depth == IPL_DEPTH_16U) ||
      (srcB->depth == IPL_DEPTH_16U) ||
      (srcA->depth == IPL_DEPTH_1U ) ||
      (srcB->depth == IPL_DEPTH_1U ) )
  ...
}
```

Диагностическое сообщение PVS\-Studio:

V501 There are identical sub\-expressions '\(srcB\-\>depth \=\= 16\)' to the left and to the right of the '\|\|' operator\. ipl iplmpy2d\.c 457

Если присмотреться к коду, то можно заметить вкравшуюся опечатку\. Потерялась проверка: "\(srcA\-\>depth \=\= IPL\_DEPTH\_16U\)"\.

## Неполная очистка буфера

```cpp
UMC::Status
VC1EncoderADV::SetMEParams_I_Field(UMC::MeParams* MEParams)
{
  UMC::Status umcSts    UMC::UMC_OK;
  memset(MEParams,0,sizeof(MEParams));
  ...
}
```

Диагностическое сообщение PVS\-Studio:

V512 A call of the 'memset' function will lead to underflow of the buffer 'MEParams'\. vc1\_enc umc\_vc1\_enc\_adv\.cpp 1767

Очищается только часть буфера, так как "sizeof\(MEParams\)" возвращает размер указателя, а не структуры\. Для вычисления корректного размера указатель необходимо разыменовать: "sizeof\(\*MEParams\)"\.

## Ошибка Copy\-Paste

```cpp
Status VC1VideoDecoder::ResizeBuffer()
{
  ...
  if(m_pContext && m_pContext->m_seqLayerHeader &&
     m_pContext->m_seqLayerHeader->heightMB &&
     m_pContext->m_seqLayerHeader->heightMB)
  ...
}
```

Диагностическое сообщение PVS\-Studio:

V501 There are identical sub\-expressions 'm\_pContext\-\>m\_seqLayerHeader\-\>heightMB' to the left and to the right of the '&&' operator\. vc1\_dec umc\_vc1\_video\_decoder\.cpp 1351

Скорее всего, для упрощения строчку скопировали, но забыли поправить\. Как мне кажется, здесь должно было быть так:

```cpp
if(m_pContext && m_pContext->m_seqLayerHeader &&
   m_pContext->m_seqLayerHeader->heightMB &&
   m_pContext->m_seqLayerHeader->widthMB)
```

## Выход за границы массива

```cpp
Ipp32f pa_nb_long[NUM_CHANNELS][2][MAX_PPT_LONG];
MP3Status mp3enc_psychoacousticInit(...)
{
  ...
  for (ch = 0; ch < NUM_CHANNELS; ch++)
    for (i = 0; i < MAX_PPT_LONG; i++) {
      for (j = 0; j < 3; j++)
        state->pa_nb_long[ch][j][i] = (Ipp32f)1.0e30;
    }
  ...
}
```

Диагностическое сообщение PVS\-Studio:

V557 Array overrun is possible\. The value of 'j' index could reach 2\. mp3\_enc mp3enc\_psychoacoustic\_fp\.c 361

Этот код приводит к ошибке сегментации\. Переменная 'j' может принимать значение 2\. Однако допустимыми индексами являются только числа 0 и 1\. Скорее всего, цикл должен был выглядеть так:

```cpp
for (j = 0; j < 2; j++)
```



Были найдены и другие циклы, которые, скорее всего, приведут к выходу за рамки массивов\.

```cpp
typedef Ipp32f samplefbout[2][18][32];
samplefbout fbout_data[NUM_CHANNELS];

static void mp3enc_scale_factor_calc_l2(MP3Enc *state)
{
  ...
  for (ch = 0; ch < stereo + state->com.mc_channel; ch++) {
    for (t = 0; t < 3; t++) {
      for (sb = 0; sb < sblimit_real; sb++){
        for (j = 0; j < 12; j++)
          fbout[j] = state->fbout_data[ch][0][t * 12 + j][sb];
  ...
}
```

Диагностическое сообщение PVS\-Studio:

V557 Array overrun is possible\. The value of 't \* 12 \+ j' index could reach 35\. mp3\_enc mp3enc\_quantization\_12\_fp\.c 275

Если возможна ситуация, что t \=\= 2, а j \=\= 11, то произойдет выход за границы массива\. Как должен был выглядеть этот код, я затрудняюсь ответить\.



С использованием массива 'samplefbout' вообще не всё гладко\. Ещё один фрагмент кода:

```cpp
typedef Ipp32f samplefbout[2][18][32];
samplefbout fbout_data[NUM_CHANNELS];

static void mp3enc_join_LR_l2(MP3Enc *state)
{
  Ipp32s sb, j;
  Ipp32s sblimit_real = state->com.sblimit_real;

  for (sb = 0; sb < sblimit_real; sb++)
    for (j = 0; j < 36; j++)
      state->fbout_data[2][0][j][sb] =
        0.5f * (state->fbout_data[0][0][j][sb] +
        state->fbout_data[1][0][j][sb]);
}
```

Диагностические сообщения PVS\-Studio:

V557 Array overrun is possible\. The value of 'j' index could reach 35\. mp3\_enc mp3enc\_quantization\_12\_fp\.c 639

V557 Array overrun is possible\. The value of 'j' index could reach 35\. mp3\_enc mp3enc\_quantization\_12\_fp\.c 640

## Использование одной переменной для двух циклов

```cpp
JERRCODE CJPEGDecoder::DecodeScanBaselineNI(void)
{
  ...
  for(c = 0; c < m_scan_ncomps; c++)
  {
    block = m_block_buffer +
     (DCTSIZE2*m_nblock*(j+(i*m_numxMCU)));

    // skip any relevant components
    for(c = 0; c < m_ccomp[m_curr_comp_no].m_comp_no; c++)
    {
      block += (DCTSIZE2*m_ccomp[c].m_nblocks);
    }
    ...
  }
  ...
}
```

Диагностическое сообщение PVS\-Studio:

V535 The variable 'c' is being used for this loop and for the outer loop\. jpegcodec jpegdec\.cpp 4652

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



## Неисправленные ошибки

Многие ошибки, описанные в первой статье, были поправлены\. Но на некоторые разработчики IPP Samples или не обратили внимания, или посчитали, что это не ошибки\. Например, подобная ситуация вот с этим фрагментом:

```cpp
vm_file* vm_file_fopen(const vm_char* fname, const vm_char* mode)
{
  ...
  mds[3] = FILE_ATTRIBUTE_NORMAL |
    (islog == 0) ? 0 : FILE_FLAG_NO_BUFFERING;
  ...
}
```

Диагностическое сообщение PVS\-Studio:

V502 Perhaps the '?:' operator works in a different way than it was expected\. The '?:' operator has a lower priority than the '\|' operator\. vm vm\_file\_win\.c 393

## Странный код

Есть немало участков кода, про которые сложно сказать, настоящая ошибка это или просто избыточный код\. Приведу несколько примеров\.

```cpp
int ec_fb_GetSubbandNum(void* stat)
{
    _fbECState* state = (_fbECState*)stat;
    return (state->freq - state->freq);
}
```

Диагностическое сообщение PVS\-Studio:

V501 There are identical sub\-expressions to the left and to the right of the '\-' operator: state\-\>freq \- state\-\>freq speech ec\_fb\.c 253

Очень странная функция\. Здесь или странным образом боролись с неиспользуемыми переменными, или оператор 'return' должен возвращать результат другого выражения\.



```cpp
AACStatus alsdecGetFrame(...)
{
  ...
  if (state->msbFirst == 0) {
    for (i = 0; i < num; i++) {
      *tmpPtr = *srcPrt;
      tmpPtr += state->numChannels;
      srcPrt++;
    }
  } else {
    for (i = 0; i < num; i++) {
      *tmpPtr = *srcPrt;
      tmpPtr += state->numChannels;
      srcPrt++;
    }
  }
  ...
}
```

Диагностическое сообщение PVS\-Studio:

V523 The 'then' statement is equivalent to the 'else' statement\. aac\_dec als\_dec\_api\.c 923

Ну что тут сказать? Подозрительно\! Зачем нужны два идентичных цикла при разных условиях?



```cpp
void rrGetNextBunch_Spiral(...)
{
  int x,y;
  ...
  if(x < 0)
    if(x < 0)  goto _begine;
  ...
  if(y < 0)
    if(y < 0)  goto _begine;
  ...
}
```

Диагностические сообщения PVS\-Studio:

V571 Recurring check\. The 'if \(x < 0\)' condition was already verified in line 1025\. 3d\-viewer rrdemosupport\.cpp 1026

V571 Recurring check\. The 'if \(y < 0\)' condition was already verified in line 1028\. 3d\-viewer rrdemosupport\.cpp 1029

Странные дублирующиеся проверки\.



```cpp
Status H264ENC_MAKE_NAME(H264CoreEncoder_UpdateRefPicMarking)
  (void* state)
{
  ...
  // set frame_num to zero for this picture, for correct
  // FrameNumWrap
  core_enc->m_pCurrentFrame->m_FrameNum = 0;
  core_enc->m_pCurrentFrame->m_FrameNum = 0;
  ...
}
```

Диагностическое сообщение PVS\-Studio:

V519 The 'core\_enc\-\>m\_pCurrentFrame\-\>m\_FrameNum' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 1804, 1805\. h264\_enc umc\_h264\_video\_encoder\_tmpl\.cpp\.h 1805

Случайно скопировали строку? Или хотели обнулить другую переменную? Непонятно\.

## Заключение

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

## Библиографический список

1. Андрей Карпов\. [Intel IPP Samples for Windows \- работа над ошибками](https://pvs-studio.ru/ru/blog/posts/cpp/a0069/)\.
1. Елена Сагалаева\. [Точки следования \(sequence points\)](http://alenacpp.blogspot.com/2005/11/sequence-points.html)\.