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

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
Request our prices
New License
License Renewal
--Select currency--
USD
EUR
GBP
RUB
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
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

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
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

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
I am interested to try it on the platforms:
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
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.

>
>
Comparing the general static analysis i…

Comparing the general static analysis in Visual Studio 2010 and PVS-Studio by examples of errors detected in five open source projects

Apr 20 2011

The article demonstrates errors detected with the static code analyzer integrated into Visual Studio 2010. The research was performed on five open source projects. The same projects were also checked with PVS-Studio. Results of comparing these two tools are presented at the end of the article.

Introduction

The article "Difficulties of comparing code analyzers, or don't forget about usability" [1] tells that it is not so easy to compare two tools as it may seem because the parameter of usability is also highly significant besides the technical characteristics proper.

Still we cannot do without comparing tools by errors they can detect. Of course, there is no sense in just calculating the number of errors. So we decided to carry out a practical experiment of error detection in real projects.

We checked five random open source projects with the static analyzer integrated into Visual Studio 2010 Premium. We looked through the whole message list and chose explicit errors. Then we made the same steps with PVS-Studio.

Here is a list of projects which participated in the research:

Let's go!

eMule Plus

The total number of messages generated by the Visual Studio static analyzer is 237, 4 of them being real errors.

The total number of messages generated by PVS-Studio is 68, 3 of them being real errors.

Visual Studio: warning C6054: String 'szwThemeFile' might not be zero-terminated. c:\emuleplus\dialogmintraybtn.hpp 445

WCHAR szwThemeFile[MAX_PATH];
WCHAR szwThemeColor[256];
if (m_themeHelper.GetCurrentThemeName(szwThemeFile,
    ARRSIZE(szwThemeFile), szwThemeColor, 
    ARRSIZE(szwThemeColor), NULL, 0) != S_OK)
  return NULL;
WCHAR *p;
if ((p = wcsrchr(szwThemeFile, L'\\')) == NULL)

Indeed, a line may not end with 0, which will cause potential problems. But in this particular case, this error is not likely to reveal itself.

Visual Studio: warning C6269: Possibly incorrect order of operations: dereference ignored. c:\emuleplus\customautocomplete.cpp 277

PVS-Studio: V532 Consider inspecting the statement of '*pointer++' pattern. Probably meant: '(*pointer)++'. customautocomplete.cpp 277

if (pceltFetched != NULL)
  *pceltFetched++;

The programmer here "is not good at" using the (*ptr)++ expression. Although this construct seems to be rather safe, still this error is widespread.

Visual Studio: warning C6298: Argument '6': using a read-only string as a writable string argument. This will attempt to write into static read-only memory and cause random crashes. c:\emuleplus\firewallopener.cpp 183

HRESULT hr = pNSC->AddPortMapping(
  riPortRule.m_strRuleName.AllocSysString(), riPortRule.m_byProtocol,
  riPortRule.m_nPortNumber, riPortRule.m_nPortNumber, 0, L"127.0.0.1",
  ICSTT_IPADDRESS, &pNSPM);

Although it is not an error, the analyzer-generated message is fair. In general, this is a problem of all the static analyzers. They produce absolutely correct messages but they are far not always real errors. Does it mean that such tools and messages are useless? No, it does not, because fixing even such warnings helps to increase an overall quality of code.

Visual Studio: warning C6314: Incorrect order of operations: bitwise- or has higher precedence than the conditional-expression operator. Add parentheses to clarify intent. c:\emuleplus\searchlistctrl.cpp 659

PVS-Studio: V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '|' operator. searchlistctrl.cpp 659

menuSearchFile.AppendMenu( MF_STRING |
  ((iSelectionMark != -1) && (dwSelectedCount > 0) &&
  g_App.m_pServerConnect->IsConnected() &&
  ((pCurServer = g_App.m_pServerConnect->GetCurrentServer())!= NULL)&&
  (pCurServer->GetTCPFlags() & SRV_TCPFLG_RELATEDSEARCH)) ? 
    MF_ENABLED : MF_GRAYED, MP_SEARCHRELATED,
      GetResString(IDS_SEARCHRELATED));

Here (because of the construct's complexity) we have wrong priorities of operators. For how long it has been said... Who prevented the programmer from arranging this code in several separate expressions instead of writing everything in one line (as it was done in the original program)? No, programmers would always want to "write it smart".

PVS-Studio: V519 The 'm_clrSample' object is assigned values twice successively. Perhaps this is a mistake. fontpreviewcombo.cpp 61

CFontPreviewCombo::CFontPreviewCombo()
{
  ...
  m_clrSample = GetSysColor(COLOR_WINDOWTEXT);
  m_clrSample = RGB(60,0,0);
  ...
}

Perhaps the developers wanted to see how the RGB(60,0,0) color would look and forgot to remove it.

Pixie

The total number of messages generated by the Visual Studio static analyzer is 18, 0 of them being real errors.

The total number of messages generated by PVS-Studio is 65, 5 of them being real errors.

PVS-Studio: V519 The 'numRays' object is assigned values twice successively. Perhaps this is a mistake. bundles.cpp 579

void CGatherBundle::post() {
 numRays = last;
 numRays = 0;
 last = 0;
 depth++;
}

Oh, how suspicious it is when numRays is first initialized by one value and then by another. Is it an error or not? Only the code's author knows exactly. But it alerts me!

PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '|' operator: PARAMETER_DPDU | PARAMETER_DPDU quadrics.cpp 880

if (up & (PARAMETER_DPDU | PARAMETER_DPDU)) {

There must be something else here. By the way, here you a general note on fixing errors detected by a static analyzer. While in some cases correction is obvious and anyone can fix an error, in some other cases only the author of the code can make out what exactly was intended there. It is on the question if one could create a tool that can correct errors "like in Word".

PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '|' operator: SLC_VECTOR | SLC_VECTOR expression.cpp 2604

lock(N, getConversion(SLC_VECTOR | SLC_VECTOR,parameters[2]));

SLC_VECTOR mentioned twice in such a context certainly signals an error.

PVS-Studio: V505 The 'alloca' function is used inside the loop.

This can quickly overflow stack. polygons.cpp 1120

inline  void  triangulatePolygon(...) {
  ...
  for (i=1;i<nloops;i++) {
    ...
    do {
      ...
      do {
        ...
        CTriVertex  *snVertex  =  (CTriVertex *)
          alloca(2*sizeof(CTriVertex));
        ...
      } while(dVertex != loops[0]);
      ...
    } while(sVertex != loops[i]);
    ...
  }
  ...
}

Having deep nesting, the alloca call might cause stack overflow.

VirtualDub

The total number of messages generated by the Visual Studio static analyzer is 24 messages, 0 of them being real errors.

The total number of messages generated by PVS-Studio is 41, 2 of them being real errors.

PVS-Studio: V547 Expression 'c < 0' is always false. Unsigned type value is never < 0. lexer.cpp 279

typedef unsigned short wint_t;

wint_t lexgetescape() {
  wint_t c = lexgetc();
  if (c < 0)
    fatal("Newline found in escape sequence");
  ...
}

The code will never be called because the expression is always false.

PVS-Studio: V557 Array overrun is possible. The '9' index is pointing beyond array bound. f_convolute.cpp 73

struct ConvoluteFilterData {
 long m[9];
 long bias;
 void *dyna_func;
 DWORD dyna_size;
 DWORD dyna_old_protect;
 BOOL fClip;
};

static unsigned long __fastcall do_conv(
  unsigned long *data,
  const ConvoluteFilterData *cfd,
  long sflags, long pit)
{
  long rt0=cfd->m[9], gt0=cfd->m[9], bt0=cfd->m[9];
  ...
}

A trivial array overflows.

WinMerge

The total number of messages generated by the Visual Studio static analyzer is 343, 3 of them being real errors.

The total number of messages generated by PVS-Studio is 69, 12 of them being real errors.

Visual Studio: warning C6313: Incorrect operator: zero-valued flag cannot be tested with bitwise-and. Use an equality test to check for zero-valued flags. c:\winmerge\src\bcmenu.cpp 1489

else if (nFlags&MF_STRING){
 ASSERT(!(nFlags&MF_OWNERDRAW));
 ModifyMenu(pos,nFlags,nID,mdata->GetString());
}

Not very lucky condition. Of course, the programmer wanted to write something different, but it happened that way.

Visual Studio: warning C6287: Redundant code: the left and right sub-expressions are identical. c:\winmerge\src\editlib\ccrystaleditview.cpp 1560

PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '||' operator: c == '}' || c == '}' ccrystaleditview.cpp 1560

bool
isclosebrace (TCHAR c)
{
  return c == _T ('}') || c == _T ('}') || c == _T (']') 
      || c == _T ('>');
}

Not all the parentheses types are checked. Why? It is usual that "Copy-paste-technology" leads to errors.

Visual Studio: warning C6287: Redundant code: the left and right sub-expressions are identical. c:\winmerge\src\mergedoc.cpp 1165

PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '||' operator. mergedoc.cpp 1165

if ((m_nBufferType[nBuffer] == BUFFER_UNNAMED) ||
 (m_nBufferType[nBuffer] == BUFFER_UNNAMED))
    nSaveErrorCode = SAVE_NO_FILENAME;

Another unlucky condition and again it seems to be the copy-paste's fault.

PVS-Studio: V551 The code under this 'case' label is unreachable. The value range of signed char type: [-128, 127]. ccrystaltextview.cpp 1646

TCHAR ch = strText[i];
switch (ch)
{
  case 0xB7:
  case 0xBB:
    strHTML += ch;
    strHTML += _T("<wbr>");
    bLastCharSpace = FALSE;
    nNonbreakChars = 0;
    break;

And here we have a sample of code which will never be used. Everything seems alright, case is written and all, but it will never get control because the value range is too narrow. TCHAR in this case is the char type.

PVS-Studio: V524 It is odd that the body of 'IsValidTextPosX' function is fully equivalent to the body of 'IsValidTextPos' function (ccrystaltextview.cpp, line 3700). ccrystaltextview.cpp 3707

bool CCrystalTextView::IsValidTextPos (const CPoint &point)
{
  return GetLineCount () > 0 && m_nTopLine >= 0 && 
         m_nOffsetChar >= 0 && point.y >= 0 && 
         point.y < GetLineCount () && point.x >= 0 &&
         point.x <= GetLineLength (point.y);
}

bool CCrystalTextView::IsValidTextPosX (const CPoint &point)
{
  return GetLineCount () > 0 && m_nTopLine >= 0 && 
         m_nOffsetChar >= 0 && point.y >= 0 && 
         point.y < GetLineCount () && point.x >= 0 && 
         point.x <= GetLineLength (point.y);
}

bool CCrystalTextView::IsValidTextPosY (const CPoint &point)
{
  return GetLineCount () > 0 && m_nTopLine >= 0 && 
         m_nOffsetChar >= 0 && point.y >= 0 && 
         point.y < GetLineCount ();
}

These are very similar functions... The developers copy-pasted again and again and forgot to fix the result. The IsValidTextPosX() function performs an excess check.

PVS-Studio: V563 It is possible that this 'else' branch must apply to the previous 'if' statement. bcmenu.cpp 1852

if(IsLunaMenuStyle())
  if(!xp_space_accelerators)return;
else
  if(!original_space_accelerators)return;

Who can look at the code and say exactly to what if else refers? Was it the thing the programmer wanted to do? Are you sure?

PVS-Studio: V556 The values of different enum types are compared: switch(ENUM_TYPE_A) { case ENUM_TYPE_B: ... }. diffwrapper.cpp 956

enum output_style {}
...
enum DiffOutputType m_outputStyle;

switch (m_options.m_outputStyle)
{
  case OUTPUT_CONTEXT:

enum and switch types were a bit mixed up. But it's alright, isn't it?

PVS-Studio: V530 The return value of function 'empty' is required to be utilized. diractions.cpp 1307

void CDirView::GetItemFileNames(int sel, String& strLeft,
                                String& strRight) const
{
  UINT_PTR diffpos = GetItemKey(sel);
  if (diffpos == (UINT_PTR)SPECIAL_ITEM_POS)
  {
    strLeft.empty();
    strRight.empty();

The case when empty() does not empty. This is an example of an extremely wrong name for a method.

PVS-Studio: V524 It is odd that the body of 'OnUpdateLastdiff' function is fully equivalent to the body of 'OnUpdateFirstdiff' function (DirView.cpp, line 2189). dirview.cpp 2220

void CDirView::OnUpdateLastdiff(CCmdUI* pCmdUI)
{
  int firstDiff = GetFirstDifferentItem();
  if (firstDiff > -1)
    pCmdUI->Enable(TRUE);
  else
    pCmdUI->Enable(FALSE);
}

void CDirView::OnUpdateFirstdiff(CCmdUI* pCmdUI)
{
  int firstDiff = GetFirstDifferentItem();
  if (firstDiff > -1)
    pCmdUI->Enable(TRUE);
  else
    pCmdUI->Enable(FALSE);
}

Two more very similar functions...

PVS-Studio: V501 There are identical sub-expressions 'pView1->GetTextBufferEol (line)' to the left and to the right of the '!=' operator. mergedoclinediffs.cpp 216

if (pView1->GetTextBufferEol(line) != 
    pView1->GetTextBufferEol(line))
{

Either this or that... Or not? Perhaps there must be pView2 here.

PVS-Studio: V530 The return value of function 'empty' is required to be utilized. varprop.cpp 133

void VariantValue::Clear()
{
  m_vtype = VT_NULL;
  m_bvalue = false;
  m_ivalue = 0;
  m_fvalue = 0;
  m_svalue.empty();
  m_tvalue = 0;
}

Again empty() does not empty the string at all.

PVS-Studio: V510 The 'Format' function is not expected to receive class-type variable as 'N' actual argument". PropShel 105

String GetSysError(int nerr);
...
CString msg;
msg.Format(
  _T("Failed to open registry key HKCU/%s:\n\t%d : %s"),
  f_RegDir, retVal, GetSysError(retVal)
  );

When various emergencies occur, WinMerge will try to inform the user about errors but in some cases it will fail. At first sight everything looks OK but actually the "String" type is just "std::wstring". Therefore we will print rubbish at best or get an Access Violation error at worst. The correct code must have a call of c_str().

PVS-Studio: V534 It is likely that a wrong variable is being compared inside the 'for' operator. Consider reviewing 'i'." BinTrans.cpp 357

// Get length of translated array of bytes from text.
int Text2BinTranslator::iLengthOfTransToBin(
  char* src, int srclen )
{
  ...
  for (k=i; i<srclen; k++)
  {
    if (src[k]=='>')
      break;
  }
  ...
}

The analyzer found a suspicious loop. This code is prone to Access Violation. The loop must go on until it finds the '>' character or a string with the length of 'srclen' characters comes to an end. But the programmer by accident used the 'k' variable instead of 'i' for comparison. If the '>' character is not found, everything will be sad.

XUIFramework

The total number of messages generated by the Visual Studio static analyzer is 93, 2 of them being real errors.

The total number of messages generated by PVS-Studio is 30, 5 of them being real errors.

Visual Studio: warning C6269: Possibly incorrect order of operations: dereference ignored c:\xui-gui framework\widgets\cstatichtml\ppdrawmanager.cpp 298

PVS-Studio: V532 Consider inspecting the statement of '*pointer++' pattern. Probably meant: '(*pointer)++'. ppdrawmanager.cpp 298

for (DWORD pixel = 0; pixel < dwWidth * dwHeight; pixel++, *pBits++)

Again the programmer is not good at using *ptr++. As I have said above, this is a widespread error.

Visual Studio: warning C6283: 'pBuffer' is allocated with array new[], but deleted with scalar delete. c:\xui-gui framework\widgets\cxstatic\cxstatic.cpp 544

BYTE* pBuffer = new BYTE [ nBufferLen ];
...
delete pBuffer;

The programmer confuses delete and delete[]. This causes issues which may occur and may not. But you should not do so anyway.

PVS-Studio: V519 The 'm_xSt' object is assigned values twice successively. Perhaps this is a mistake. resizedlg.cpp 244

m_xSt = CST_RESIZE;
m_xSt = CST_RESIZE;

Judging by the code, there must be m_ySt here. But we cannot keep from using copy-paste again and again, right?

V531 It is odd that a sizeof() operator is multiplied by sizeof(). pphtmldrawer.cpp 258

DWORD dwLen = ::LoadString(hInstDll, dwID, szTemp, 
              (sizeof(szTemp) * sizeof(TCHAR)));

There must be sizeof(szTemp) / sizeof(TCHAR) .

PVS-Studio: V556 The values of different enum types are compared: enuHAlign == Center. cxstatic.cpp 151

if (enuHAlign == Center)

There must be enuHAlign == Midle. There is also if in the code nearby (enuVAlign == Middle) though it must be Center. Confusion with enum, in short.

PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '||' operator. resizedlg.cpp 157

HDWP CItemCtrl::OnSize(...)
{
  ...
  if (m_styTop == CST_ZOOM ||
      m_styTop == CST_ZOOM ||
      m_styBottom == CST_DELTA_ZOOM ||
      m_styBottom == CST_DELTA_ZOOM)
  ...
}

Perhaps the code must look this way:

HDWP CItemCtrl::OnSize(...)
{
  ...
  if (m_styTop == CST_ZOOM ||
      m_styTop == CST_DELTA_ZOOM ||
      m_styBottom == CST_ZOOM ||
      m_styBottom == CST_DELTA_ZOOM)
  ...
}

Comparison results

a0073_VS_vs_PVS-Studio/image1.png

We do not draw any certain conclusions. One of the tools was better in some projects and the other tool was better in others.

References

Popular related articles
Characteristics of PVS-Studio Analyzer by the Example of EFL Core Libraries, 10-15% of False Positives

Date: Jul 31 2017

Author: Andrey Karpov

After I wrote quite a big article about the analysis of the Tizen OS code, I received a large number of questions concerning the percentage of false positives and the density of errors (how many erro…
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…
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…
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…
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…
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…
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…
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 site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
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