To get a trial key
fill out the form below
Team License (a basic version)
Enterprise License (an extended version)
* By clicking this button you agree to our Privacy Policy statement

Request our prices
New License
License Renewal
--Select currency--
USD
EUR
GBP
RUB
* By clicking this button you agree to our Privacy Policy statement

Free PVS-Studio license for Microsoft MVP specialists
* By clicking this button you agree to our Privacy Policy statement

To get the licence for your open-source project, please fill out this form
* By clicking this button you agree to our Privacy Policy statement

I am interested to try it on the platforms:
* By clicking this button you agree to our Privacy Policy statement

Message submitted.

Your message has been sent. We will email you at


If you haven't received our response, please do the following:
check your Spam/Junk folder and click the "Not Spam" button for our message.
This way, you won't miss messages from our team in the future.

>
>
>
Checking Intel IPP Samples for Windows …

Checking Intel IPP Samples for Windows - Continuation

Oct 12 2011
Author:

The progress keeps on going. My favorite static code analyzer PVS-Studio is developing too. It has occurred to me recently that those projects we already checked, we can well check again. It would be strange if we wrote articles on this topic, and they would hardly be interesting. But I think we can write one: it will become one more argument for the idea that you can get real benefit from static analysis only using it regularly and not from time to time. So, let's see what new interesting things we have managed to find in the Intel IPP Samples project.

The previous post "Intel IPP Samples for Windows - error correction" [1] was published on 27th January, 2011. About 9 months have passed since then. During this time, developers have fixed many errors in IPP Samples described in the article. Since Intel developers had showed no interest in PVS-Studio, the check was performed only once. Now we can see clear what new interesting errors the analyzer has found after 9 months of development.

Let's not fall into idle talk, for we are developers at last. So let's go over to code samples. Analysis was performed by PVS-Studio 4.37 (the previous post refers to version 4.10). Of course, we will cite not all the defects but only interesting and not recurrent ones. Those who want to see all the issues, may use PVS-Studio and study the report. But our purpose is different this time.

Classic undefined behavior

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's diagnostic messages:

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

It's just a canonical example cited in articles to demonstrate undefined behavior [2]. You cannot tell whether or not variables pTemp and pDst will be incremented because they are changed twice within one sequence point. The result depends on the compiler and optimization settings.

There is another similar code fragment:

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

Undefined behavior and prefix increment

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's diagnostic message:

V567 Undefined behavior. The 'm_curIndex' variable is modified while being used twice between sequence points. vc1_enc umc_vc1_enc_planes.h 630

Here you are another example of undefined behavior. Although prefix increment is being used here, it doesn't make the difference actually. There is still one sequence point, while the m_curIndex variable changes twice. Theoretically, the compiler might well create the following pseudocode:

A = m_curIndex + 1;

B = A % m_maxN;

m_curIndex = B;

m_curIndex = A;

It will hardly happen in practice and the variable will be incremented at once, but you should not rely on that.

Object name misprinted

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's diagnostic message:

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

If you look attentively at the code, you will notice a misprint creeping into the code. The check "(srcA->depth == IPL_DEPTH_16U)" is missing.

Incomplete buffer clearing

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

PVS-Studio's diagnostic message:

V512 A call of the 'memset' function will lead to underflow of the buffer 'MEParams'. vc1_enc umc_vc1_enc_adv.cpp 1767

Only part of the buffer is cleared since "sizeof(MEParams)" returns the pointer's size and not structure's size. To calculate the correct size, the pointer must be dereferenced: "sizeof(*MEParams)".

Copy-Paste error

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

PVS-Studio's diagnostic message:

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

Most likely, the programmer wanted to simplify the task and copied the string but forgot to fix it. I think there should be the following code here:

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

Array overrun

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's diagnostic message:

V557 Array overrun is possible. The value of 'j' index could reach 2. mp3_enc mp3enc_psychoacoustic_fp.c 361

This code causes a segmentation fault. The 'j' variable can take value 2, but acceptable indexes are only 0 and 1. The loop most likely should look this way:

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

We have found some other loops which seem to cause array overruns.

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's diagnostic message:

V557 Array overrun is possible. The value of 't * 12 + j' index could reach 35. mp3_enc mp3enc_quantization_12_fp.c 275

If it is possible that t == 2, while j == 11, an array overrun will occur. I cannot say what the correct version of this code is.

There are some problems about using the 'samplefbout' array. Here you are another code fragment:

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's diagnostic messages:

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

Using one variable for two loops

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's diagnostic message:

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

The problem with this code is that the 'c' variable is being used in an outer and a nested loop at the same time. This code might process only part of the data or result in an eternal loop depending on the loop's boundary values.

Uncorrected errors

Many errors described in the first article have been fixed. But regarding some others, IPP Samples developers either paid no attention to them, or didn't take them as errors. For example, one of them is in the following fragment:

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's diagnostic message:

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

Strange code

There are many code fragments about which I just cannot say exactly whether there is a real error or just redundant code. Here you are some examples.

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

PVS-Studio's diagnostic message:

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

This is a very strange function. Either the developers were fighting unused variables in a strange way, or the 'return' operator must return a result of some other expression.


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's diagnostic message:

V523 The 'then' statement is equivalent to the 'else' statement. aac_dec als_dec_api.c 923

What can you say here? It's suspicious! What for do you need two identical loops at different conditions?

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

PVS-Studio's diagnostic messages:

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

Strange duplicating checks.

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's diagnostic message:

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

A string was accidentally copied? Or it was intended to zero another variable? I don't know.

Conclusion

Try to integrate static analysis into the development process of your projects. It seems to be just a waste of efforts at first. But then you will feel it clear how unusual and pleasant it is to get an error message before you start testing a freshly written code fragment. And you will also watch with interest if something else can be found in your code after the new version of the analyzer is released.

References

Popular related articles
The Last Line Effect

Date: May 31 2014

Author: Andrey Karpov

I have studied many errors caused by the use of the Copy-Paste method, and can assure you that programmers most often tend to make mistakes in the last fragment of a homogeneous code block. I have ne…
The way static analyzers fight against false positives, and why they do it

Date: Mar 20 2017

Author: Andrey Karpov

In my previous article I wrote that I don't like the approach of evaluating the efficiency of static analyzers with the help of synthetic tests. In that article, I give the example of a code fragment…
PVS-Studio for Java

Date: Jan 17 2019

Author: Andrey Karpov

In the seventh version of the PVS-Studio static analyzer, we added support of the Java language. It's time for a brief story of how we've started making support of the Java language, how far we've co…
The Evil within the Comparison Functions

Date: May 19 2017

Author: Andrey Karpov

Perhaps, readers remember my article titled "Last line effect". It describes a pattern I've once noticed: in most cases programmers make an error in the last line of similar text blocks. Now I want t…
Appreciate Static Code Analysis!

Date: Oct 16 2017

Author: Andrey Karpov

I am really astonished by the capabilities of static code analysis even though I am one of the developers of PVS-Studio analyzer myself. The tool surprised me the other day as it turned out to be sma…
Free PVS-Studio for those who develops open source projects

Date: Dec 22 2018

Author: Andrey Karpov

On the New 2019 year's eve, a PVS-Studio team decided to make a nice gift for all contributors of open-source projects hosted on GitHub, GitLab or Bitbucket. They are given free usage of PVS-Studio s…
PVS-Studio ROI

Date: Jan 30 2019

Author: Andrey Karpov

Occasionally, we're asked a question, what monetary value the company will receive from using PVS-Studio. We decided to draw up a response in the form of an article and provide tables, which will sho…
Static analysis as part of the development process in Unreal Engine

Date: Jun 27 2017

Author: Andrey Karpov

Unreal Engine continues to develop as new code is added and previously written code is changed. What is the inevitable consequence of ongoing development in a project? The emergence of new bugs in th…
How PVS-Studio Proved to Be More Attentive Than Three and a Half Programmers

Date: Oct 22 2018

Author: Andrey Karpov

Just like other static analyzers, PVS-Studio often produces false positives. What you are about to read is a short story where I'll tell you how PVS-Studio proved, just one more time, to be more atte…
Technologies used in the PVS-Studio code analyzer for finding bugs and potential vulnerabilities

Date: Nov 21 2018

Author: Andrey Karpov

A brief description of technologies used in the PVS-Studio tool, which let us effectively detect a large number of error patterns and potential vulnerabilities. The article describes the implementati…

Comments (0)

Next comments
This website uses cookies and other technology to provide you a more personalized experience. By continuing the view of our web-pages you accept the terms of using these files. If you don't want your personal data to be processed, please, leave this site.
Learn More →
Accept