Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо
to the top
Главная
>
Публикации
>
C++
>
Проверка Intel IPP Samples for Windows …
Andrey Karpov
Андрей Карпов
12 Окт 2011
Теги:
#Cpp

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

12 Окт 2011

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

Предыдущая заметка "Intel IPP Samples for Windows - работа над ошибками" [1] была опубликована 27 января 2011 года. Прошло около 9 месяцев. За это время в IPP Samples были поправлены многие ошибки, описанные в статье. Вышло несколько обновлений библиотеки. Поскольку разработчики из Intel не заинтересовались PVS-Studio, то проверка получилась разовая. И теперь можно хорошо увидеть, какие новые интересные ошибки найдет анализатор через 9 месяцев своего развития.

Не будем предаваться пустословию, мы ведь программисты и перейдем побыстрее к примерам кода. Анализ выполнялся PVS-Studio 4.37 (предыдущая заметка относится к версии 4.10). Конечно, приведены будут не все дефекты, а только интересные и не сильно повторяющиеся. Тот, кто хочет увидеть все подводные камни, может воспользоваться PVS-Studio и изучить отчёт. Но мы здесь не для этого.

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

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]. Невозможно сказать увеличатся переменные pTemp и pDst или нет, так как они два раза изменяются в рамках одной точки следования. Результат зависит от компилятора и настроек оптимизации.

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

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

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

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;

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

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

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)".

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

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

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

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

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

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

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. Скорее всего, цикл должен был выглядеть так:

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

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

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' вообще не всё гладко. Ещё один фрагмент кода:

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

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

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 или не обратили внимания, или посчитали, что это не ошибки. Например, подобная ситуация вот с этим фрагментом:

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

Странный код

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

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' должен возвращать результат другого выражения.

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

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

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

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

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

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

Заключение

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

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

#Cpp
blog-subscribe blog-subscribe
blog-favorite blog-favorite

0

0

0

0

vk share
ПОДЕЛИТЬСЯ

Теги:

#Cpp

Присылаем лучшие статьи раз в месяц

Популярные статьи по теме

Комментарии (0)

Следующие комментарии next comments
close comment form
close form

Заполните форму в два простых шага ниже:

Ваши контактные данные:

Шаг 1
Поздравляем! У вас есть промокод!

Тип желаемой лицензии:

Шаг 2
Team license
Enterprise license
* Разница между Team и Enterprise
Не получили письмо?
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности
close form
Запросите информацию о ценах
Почему мы просим корпоративную почту?
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
RUB
Не получили письмо?
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Бесплатная лицензия PVS‑Studio для специалистов Microsoft MVP
Почему мы просим корпоративную почту?
Не получили письмо?
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
Не получили письмо?
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Мне интересно попробовать плагин на:
Почему мы просим корпоративную почту?
Не получили письмо?
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
check circle
Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте, отфильтровано ли письмо в одну из следующих стандартных папок:

  • Промоакции
  • Оповещения
  • Спам