Андрей Карпов

19 Апр 2012

Я хочу продать в компанию Intel лицензию на PVS-Studio

19 Апр 2012

Автор: Андрей Карпов

Меня не покидает желание продать команде разработчиков Intel Performance Primitives Library лицензию на PVS-Studio.

Конечно, в другие подразделения Intel я тоже хочу сделать продажи. Просто разработчики IPP кажутся ближе. Во-первых, часть из них находится в России. Во-вторых, они уже применяют в работе статические анализаторы. В-третьих, у нас уже есть задел в данном направлении. Я имею в виду, что мы уже проверяли два раза проект IPP Samples (первая проверка, вторая проверка). Конечно, намного было бы интереснее проверить не примеры, а саму библиотеку IPP, но к её исходным кодам у нас нет доступа.

Итак, пытаться прорекламировать себя я буду стандартной методикой. Мы будем продолжать время от времени перепроверять IPP Samples и находить новые ошибки. Это говорит о том, что анализатор кода PVS-Studio активно развивается. Хотя конечно основная ценность его применения не в разовых запусках, а в регулярном использовании. Не стоит забывать про возможности ночных запусков и фоновый анализ после компиляции.

Привожу некоторые новые ошибки, выявленные с помощью PVS-Studio v4.60. Конечно, я привожу не все подозрительные места. Их гораздо больше, но я затрудняюсь понять, являются они ошибочными или нет. Заодно это повод разработчикам IPP установить и попробовать наш инструмент. Мы изменили триальную модель, и теперь инструмент обладает полной функциональностью. Попробуйте.

Итак, рассмотрим подозрительные места в коде IPP Samples.

Ситуация N1. Бессмысленное высказывание

AACStatus bsacdecSetNumChannels(Ipp32s channelConfiguration,
                                AACDec *state)
{
  state->com.m_channel_number = channelConfiguration;
  if (channelConfiguration == 7) {
    state->com.m_channel_number;
  }

  return AAC_OK;
}

Диагностическое сообщение PVS-Studio: V607 Ownerless expression 'state->com.m_channel_number'. aac_dec aac_dec_api_fp.c 1404

Высказывание "state->com.m_channel_number;" очень странное. Возможно, здесь забыто присваивание или что-то ещё.

Ситуация N2. Заполнение таблицы виртуальных методов

В IPP Samples встречается достаточно много мест, где память для классов выделяется с помощью функции malloc() и инициализируется с помощью функции memset(). Может в этом и нет ничего страшного, но смущает то, что у этих классов есть виртуальные методы. Таким образом, есть опасения, что таблица виртуальных методов может быть испорчена, и что-то будет работать не так.

Например, класс _MediaDataEx содержит виртуальные функции:

virtual bool TryStrongCasting(....) const;
virtual bool TryWeakCasting(....) const;

А вот как создаётся объект этого класса:

Status VC1Splitter::Init(SplitterParams& rInit)
{
  MediaDataEx::_MediaDataEx *m_stCodes;
  ...
  m_stCodes = (MediaDataEx::_MediaDataEx *)
    ippsMalloc_8u(
      START_CODE_NUMBER*2*sizeof(Ipp32s)+
      sizeof(MediaDataEx::_MediaDataEx));
  ...
  memset(m_stCodes, 0,
   (START_CODE_NUMBER*2*sizeof(Ipp32s)+
   sizeof(MediaDataEx::_MediaDataEx)));
  ...
}

Диагностическое сообщение PVS-Studio: V598 The 'memset' function is used to nullify the fields of '_MediaDataEx' class. Virtual method table will be damaged by this. vc1_spl umc_vc1_spl.cpp 131

Не знаю, есть здесь беда или нет, но предупредить стоит.

Аналогичную картину можно наблюдать в следующих местах:

V598 The 'memset' function is used to nullify the fields of '_MediaDataEx' class. Virtual method table will be damaged by this. vc1_dec umc_vc1_video_decoder.cpp 641 False

V598 The 'memset' function is used to nullify the fields of 'AVS_DISASSEMBLING_CONTEXT' class. Virtual method table will be damaged by this. avs_enc umc_avs_enc_slice.cpp 45

V598 The 'memset' function is used to nullify the fields of 'AVS_DISASSEMBLING_CONTEXT' class. Virtual method table will be damaged by this. avs_enc umc_avs_enc_slice.cpp 29

V598 The 'memset' function is used to nullify the fields of 'AVS_DISASSEMBLING_CONTEXT' class. Virtual method table will be damaged by this. avs_enc umc_avs_enc_slice.cpp 22

V598 The 'memcpy' function is used to copy the fields of 'AVSVideoEncoderParams' class. Virtual method table will be damaged by this. avs_enc umc_avs_enc.cpp 115

V598 The 'memset' function is used to nullify the fields of 'AVS_DECODING_CONTEXT' class. Virtual method table will be damaged by this. avs_dec umc_avs_dec_slice_init.cpp 65

V598 The 'memset' function is used to nullify the fields of 'AVS_DEBLOCKING_CONTEXT' class. Virtual method table will be damaged by this. avs_common umc_avs_slice.cpp 153

V598 The 'memset' function is used to nullify the fields of 'AVS_RECONSTRUCTING_CONTEXT' class. Virtual method table will be damaged by this. avs_common umc_avs_slice.cpp 133

V598 The 'memset' function is used to nullify the fields of 'AVS_DEBLOCKING_CONTEXT' class. Virtual method table will be damaged by this. avs_common umc_avs_slice.cpp 43

V598 The 'memset' function is used to nullify the fields of 'AVS_RECONSTRUCTING_CONTEXT' class. Virtual method table will be damaged by this. avs_common umc_avs_slice.cpp 42

V598 The 'memset' function is used to nullify the fields of 'AVS_DECODING_CONTEXT' class. Virtual method table will be damaged by this. avs_common umc_avs_slice.cpp 41

V598 The 'memset' function is used to nullify the fields of 'AVS_DEBLOCKING_CONTEXT' class. Virtual method table will be damaged by this. avs_common umc_avs_slice.cpp 32

V598 The 'memset' function is used to nullify the fields of 'AVS_RECONSTRUCTING_CONTEXT' class. Virtual method table will be damaged by this. avs_common umc_avs_slice.cpp 31

V598 The 'memset' function is used to nullify the fields of 'AVS_DECODING_CONTEXT' class. Virtual method table will be damaged by this. avs_common umc_avs_slice.cpp 30

Ситуация N3. Проверка указателей после использования

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

Пример:

VIDEO_DRV_CREATE_BUFFERS_FUNC(....)
{
  ...
  VideoDrvVideoMemInfo* drv_vm = &(driver->m_VideoMemInfo);
  ...
  if ((NULL == driver) || (NULL == bufs))
  {
    ERR_SET(VM_NULL_PTR, "null ptr");
  }
  ...
}

Диагностическое сообщение PVS-Studio: V595 The 'driver' pointer was utilized before it was verified against nullptr. Check lines: 40, 46. video_renders drv.c 40

Здесь проверку указателя нужно переместить выше или совсем удалить, если ситуация "driver==NULL" невозможна.

Вот другие аналогичные примеры кода:

Ipp16s *pNewSpeech = encoderObj->stEncState.pSpeechPtrNew;
if (NULL==encoderObj || NULL==src || NULL ==dst )
  return APIGSMAMR_StsBadArgErr;

Диагностическое сообщение PVS-Studio: V595 The 'encoderObj' pointer was utilized before it was verified against nullptr. Check lines: 296, 298. speech encgsmamr.c 296

m_pAVSCompressorParams = DynamicCast<AVSVideoEncoderParams> (pParams);
...  
m_qp = m_pAVSCompressorParams->m_iConstQuant;
// check error(s)
if (NULL == m_pAVSCompressorParams)
  return UMC_ERR_NULL_PTR;

Диагностическое сообщение PVS-Studio: V595 The 'm_pAVSCompressorParams' pointer was utilized before it was verified against nullptr. Check lines: 88, 91. avs_enc umc_avs_enc_fusion_core.cpp 88

Ситуация N4. Подозрительные выражения с запятыми

Есть пара ситуаций с подозрительными запятыми ','. Первый пример:

void GetIntraDCPredictors(VC1Context* pContext)
{
  DCPred.DC[13] = pC->DCBlkPred[5].DC,QurrQuant;
  ...
}

Диагностическое сообщение PVS-Studio: V521 Such expressions using the ',' operator are dangerous. Make sure the expression is correct. vc1_dec umc_vc1_dec_mb_com.cpp 370

Быть может просто опечатка, а возможно здесь что-то не хватает.

Второй пример:

V521 Such expressions using the ',' operator are dangerous. Make sure the expression is correct. speech usc_dtmf.c 309

static int DTMF_16s(....)
{
  ...
  for (i = pIppTDParams->dtmf_fs, j = 0;
       i < dtmf_frame_size+pIppTDParams->dtmf_fs, j < nbytes;
       i++, j++)
}

Диагностическое сообщение PVS-Studio: V521 Such expressions using the ',' operator are dangerous. Make sure the expression is correct. speech usc_dtmf.c 309

Это более интересный пример, чем предыдущий. По всей видимости, логическое условие составлено некорректно. Наверное, условие должно было выглядеть так:

i < dtmf_frame_size+pIppTDParams->dtmf_fs && j < nbytes

Ситуация N5. Подозрительное неявное приведение типа

class MeMV
{
public:
  MeMV(){};
  MeMV(int a0){x = (Ipp16s)a0; y=(Ipp16s)a0;};
  MeMV(int a0, int a1){x = (Ipp16s)a0; y=(Ipp16s)a1;};
  ...
}

MeMV MePredictCalculatorVC1::GetPrediction8x8()
{
  ...
  default:
    return false;
  ...
}

Диагностическое сообщение PVS-Studio: V601 The 'false' value becomes a class object. me umc_vec_prediction.cpp 754

Функция GetPrediction8x8() возвращает тип MeMV. Однако в одной из ветвей, она возвращает значение 'false'. Это значение неявно приводится int и вызывается конструктор MeMV(int a0). Я не знаю, но возможно здесь необходимо вернуть что-то ещё или сгенерировать исключение.

Аналогичное неявное приведение типа присутствует здесь:

V601 The 'false' value becomes a class object. me umc_vec_prediction.cpp 717

Ситуация N6. Неопределенное поведение

В очень многих местах IPP Samples встречаются конструкции, которые приводят к неопределенному или неуточненному поведению. О некоторых я писал в предыдущей заметке. Теперь обнаружилось огромное количество сдвигов отрицательных значений. Я не берусь утверждать, что это может создать где-то проблемы, но предлагаю на всякий случай обратить внимание на эту статью: "Не зная брода, не лезь в воду. Часть третья".

Где располагается потенциально опасный код можно посмотреть в этом файле: ipp-samples-ub.txt.

Заключение

Уважаемые разработчики IPP и IPP Samples, мы ждем ваших писем. Мы готовы обсудить и реализовать недостающий функционал в инструменте PVS-Studio, который препятствует его использованию. Мы также готовы реализовать критичные для вашего проекта диагностические правила.

Всем остальным желаю безглючного кода и приглашаю в твиттер @Code_Analysis. В нём мы размещаем ссылки на интересные статьи про Си++, программирование, статический анализ кода и инструмент PVS-Studio.