Меня не покидает желание продать команде разработчиков 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.