Tremble, peace! We have released PVS-Studio 4.00 with a free general-purpose analyzer.

We bring to the attention of programmers a new tool for finding errors in the source code of C / C ++ applications. Within the PVS-Studio analyzer, a new set of general-purpose rules has been implemented. This functionality is currently free. You can download PVS-Studio at http://www.viva64.com/en/pvs-studio-download/ .

The article briefly describes the new features of PVS-Studio. Using the static analysis of the source code of the TortoiseSVN project, the use of new diagnostic capabilities will be demonstrated.

PVS-Studio is a modern source code analyzer that integrates into Microsoft Visual Studio 2005/2008/2010. The analyzer allows you to conveniently work with the list of warnings and use several processor cores for your work. PVS-Studio is focused on developers of modern Windows applications in C / C ++ / C ++ 0x.

So far, PVS-Studio has included two sets of rules. The first to detect defects in 64-bit programs. The second to detect defects in parallel programs built on the basis of OpenMP technology.
')
Now the third universal set of checks appeared in the analyzer, revealing various errors in the code. This set of rules is free and can be used without any restrictions. Whether over time this set will be paid or not is difficult to say now. In the field of general purpose analysis, we are just beginning our journey.

Now you can get acquainted with the new set of rules by downloading PVS-Studio 4.00 BETA. We will be happy to receive comments from you about the errors we have noticed and suggestions for improvement. Just want to note that for a start, we implemented only 50 general-purpose rules. That's not a lot. Therefore, if you download and try PVS-Studio right away, you will not find something interesting in your code, do not rush to conclusions. We offer you to try PVS-Studio again in the future, when the set of checks will be significantly increased. We plan soon to actively work on expanding the base of diagnostic rules (if there is enough health and luck).

Let us demonstrate the use of the new rule set PVS-Studio using the example of TortoiseSVN . TortoiseSVN is a client for the Subversion version control system, executed as a Windows shell extension. TortoiseSVN is well known to many developers, and I think it makes no sense to describe this application in more detail. I will only note that in 2007, on SourceForge.net, TortoiseSVN was recognized as the best project in the category “tools and utilities for developers”.

Step 1

We are downloading PVS-Studio from the site of the company OOO Program Verification Systems (this is us). I hope you will be pleased that you do not need to fill out any forms or solve captcha. Just download .

Step 2

Install PVS-Studio. You can safely click the Next button. No need to configure anything. The PVS-Studio distribution kit is digitally signed. However, some antiviruses can still be alerted to actions related to integration into Visual Studio. Everything is allowed.

Step 3

Download the set of source texts of the TortoiseSVN project. We worked with source code version 1.6.11.

Step 4

Open the TortoiseSVN project and launch the analysis. To do this, PVS-Studio select the Check Solution command in me.

Step 5

The analyzer will think for a while (the TortoiseSVN project is not so simple and contains a bunch of files). Therefore, we are not in a hurry to do something at this moment and wait a bit. After a while, a progress dialog will appear and analysis will begin. The speed of analysis depends on the number of cores in the processor. If PVS-Studio eats up too many resources, you can limit its desires in the settings.

Messages The analyzer issues in its own window, where there are controls for turning on / off various types of messages. And we will take advantage of these opportunities. Now it is not at all interesting to look at the many errors associated with 64 bits. Moreover, 64-bit analysis is paid, and therefore limited (for more details about the trial mode, see here ).

In the window you can see a set of three buttons, responsible for displaying messages from the three sets of rules.


1) 64 - diagnostic messages about 64-bit defects (Viva64);
2) MP - diagnostic messages about parallel defects (VivaMP);
3) GA - general type diagnostic messages (General Analysis).

We are interested only in a set of general-purpose rules (GA). We press the other buttons, and the extra messages in the list will be immediately hidden.


We are waiting for the completion of the analysis.

Step 6

The analysis is completed, and we see a list of all the places in the program for which the analyzer recommends reviewing the code (code review).

All warnings are divided into 3 levels of importance (this is new in PVS-Studio 4.00). Usually, it makes sense to watch only the first and second level. PVS-Studio 4.00 BETA issued 33 first-level warnings, 14 second-level warnings and 8 third-level warnings.

It is worth starting an acquaintance with the warnings of the first level. Therefore, it is possible to disable the button indicating the output of second level messages. The third level is already disabled by default.

Step 7

Consider the interesting places in the code detected by the analyzer.

Situation 1

At the very beginning, two messages belong to the same function. Hopefully, this feature is not used anywhere else.

V530 contextmenu.cpp 434

V530 contextmenu.cpp 442

  STDMETHODIMP CShellExt :: Initialize (....)
 {
   ...
   ignoredprops.empty ();
   for (int p = 0; p <props.GetCount (); ++ p)
   {
     if (props.GetItemName (p).
           compare (SVN_PROP_IGNORE) == 0)
     {
       std :: string st = props.GetItemValue (p);
       ignoredprops = UTF8ToWide (st.c_str ());
       // remove all escape chars ('\\')
       std :: remove (ignoredprops.begin (),
                   ignoredprops.end (), '\\');
       break;
     }
   }
   ...
 } 

Hereinafter, only a brief commentary on code fragments will be given. In more detail, you can find out why the code is considered suspicious in the online documentation for PVS-Studio (it is available in Russian and English ). Also complete with PVS-Studio is the documentation (the same one) in the format of a PDF file. Further links to the corresponding descriptions of diagnostic messages will be given.

The V530 warning tells us that “ignoredprops.empty ()” doesn’t clear the string at all, and “std :: remove ()” doesn’t actually remove characters.

Situation 2

Here it checks that a variable of type 'char' is greater than or equal to the value 0x80.

V547 Expression 'c> = 0x80' is always false. The value range of signed char type: [-128, 127]. pathutils.cpp 559

  CString CPathUtils :: PathUnescape (const char * path)
 {
   // try quick path
   size_t i = 0;
   for (; char c = path [i]; ++ i)
     if ((c> = 0x80) || (c == '%'))
     {
       // quick path does not work for
       // non-latin or escaped chars
       std :: string utf8Path (path);
       CPathUtils :: Unescape (& utf8Path [0]);
       return CUnicodeUtils :: UTF8ToUTF16 (utf8Path);
     }
   ...
 } 

Message V547 notifies that such a check is meaningless. A value of type 'char' will always be less than 0x80, which means that the condition is always false. And perhaps it is because of this error that the comment "quick path does not work for non-latin or escaped chars" is written in the code. Of course it does not work, but not because it is impossible to convert the string. When a non-latin symbol is encountered, we simply do not go into the body of the 'if' operator.

Situation 3

Many threads are created and killed using the CreateThread / ExitThread functions. And, therefore, we risk quickly exhausting the thread stack or not freeing some of the resources when the thread ends. Read more in the description of the warning V513 . It is much safer to use the _beginthreadex () and _endthreadex () functions.

There is no sense to give an example of code here, and the text of all messages is the same:

V513 Use _beginthreadex / _endthreadex functions instead of CreateThread / ExitThread functions. crashhandler.cpp 379

Situation 4

This applies to related TortoiseSVN utilities. It is very likely that when working with CrashLog, everything will end with another Crash.

V510 The 'printf_s' function is not expected to receive class-type variable as the fourth actual argument. excprpt.cpp 199

  string CExceptionReport :: getCrashLog ()
 {
   ...
   _tprintf_s (buf, _T ("% s \\% s.xml"),
   getenv ("TEMP"), CUtility :: getAppName ());
   ...
 } 

The V510 message warns that it is very bad to pass a parameter of type std :: string to the printf_s function. And the CUtility :: getAppName () function returns exactly std :: string. The mistake is that you forgot to write ".c_str ()". The consequence can be either just incorrect output of data, or a program crash.

Situation 5

They wanted to clear the array, but could not.

V530 mailmsg.cpp 40

  CMailMsg & CMailMsg :: SetFrom (string sAddress,
                             string sName)
 {
    if (initIfNeeded ())
    {
       // only one sender allowed
       if (m_from.size ())
          m_from.empty ();
       m_from.push_back (TStrStrPair (sAddress, sName));
    }
    return * this;
 } 

Again, the V530 message suggests that instead of “clear ()” it is accidentally written “empty ()”.

Situation 6

And in the SetTo () function also failed to clear the array.

V530 mailmsg.cpp 54

  CMailMsg & CMailMsg :: SetTo (string sAddress,
                           string sName)
 {
    if (initIfNeeded ())
    {
       // only one recipient allowed
       if (m_to.size ())
          m_to.empty ();

       m_to.push_back (TStrStrPair (sAddress, sName));
    }
    return * this;
 } 

Situation 7

There are, of course, false positives. Here, for example, is a snippet of code from the zlib library included in the TortoiseSVN project. There is no error here, but it’s very useful to mark such a place with a V501 message.

V501 operator - the operator: size - size zutil.c 213

  voidpf zcalloc (opaque, items, size)
     voidpf opaque;
     unsigned items;
     unsigned size;
 {
   / * make compiler happy * /
   if (opaque) items + = size size size;
   return (voidpf) calloc (items, size);
 } 

The compiler is certainly happy here, but the subtraction looks suspicious.

Situation 8

There are other dark places with the encodings. Here is another condition that is always false.

V547 Expression '* utf8CheckBuf> = 0xF5' is always false. The value range of signed char type: [-128, 127]. tortoiseblame.cpp 312

  BOOL TortoiseBlame :: OpenFile (const TCHAR * fileName)
 {
   ...
   // check each line for illegal utf8 sequences.
   // If one is found, we treat
   // the file as ASCII, otherwise we assume
   // an UTF8 file.
   char * utf8CheckBuf = lineptr;
   while ((bUTF8) && (* utf8CheckBuf))
   {
     if ((* utf8CheckBuf == 0xC0) ||
         (* utf8CheckBuf == 0xC1) ||
         (* utf8CheckBuf> = 0xF5))
     {
       bUTF8 = false;
       break;
     }

    ...
   } 

By the way, the terms "* utf8CheckBuf == 0xC0", "* utf8CheckBuf == 0xC1" are also always false. Therefore, the code "bUTF8 = false;" will never get control. The fact that the PVS-Studio analyzer did not scold the expression "* utf8CheckBuf == 0xC0" is a flaw. Recorded in the list of improvements. In the next version we will swear on this.

Situation 9

With the next message is not so simple. Theoretically, there is an error in the code. Practically - this code works.

V507 Pointer to local array 'stringbuf' is stored outside of this array. Such a pointer will become invalid. mainwindow.cpp 277

  LRESULT CALLBACK CMainWindow :: WinMsgHandler (....)
 {
   ...
   if (pNMHDR-> code == TTN_GETDISPINFO)
   {
     LPTOOLTIPTEXT lpttt;

     lpttt = (LPTOOLTIPTEXT) lParam;
     lpttt-> hinst = hResource;

     // Specify the resource identifier of the
     // descriptive text for the given button.
     TCHAR stringbuf [MAX_PATH] = {0};
     ...
     lpttt-> lpszText = stringbuf;
   }
   ...
 } 

Diagnostic message V507 warns that the object is used after the end of its existence. The 'stringbuf' buffer will be used after exiting the 'if' operator from the body.

If 'stringbuf' were a class object, for example std :: string, then this code would behave incorrectly. We would use the already destroyed object. But here 'stringbuf' is an array created on the stack. The Visual C ++ compiler does not reuse this section of the stack, and the buffer will exist until the end of the 'CMainWindow :: WinMsgHandler' function. Thus, no error will occur, although the code is potentially dangerous.

Situation 10

And one more same place as in the example above. Again, this is code that works, but is fragile.

V507 Pointer to local array 'stringbuf' is stored outside of this array. Such a pointer will become invalid. picwindow.cpp 443

  if ((HWND) wParam == m_AlphaSlider.GetWindow ())
 {
   LPTOOLTIPTEXT lpttt;

   lpttt = (LPTOOLTIPTEXT) lParam;
   lpttt-> hinst = hResource;
   TCHAR stringbuf [MAX_PATH] = {0};
   _stprintf_s (stringbuf, .....);
   lpttt-> lpszText = stringbuf;
 } 

Situation 11

A bad idea to throw and not to handle exceptions in the destructor.

V509 The 'throw' operator inside the destructor should be placed within the try..catch block. Raising exception inside the destructor is illegal. cachefileoutbuffer.cpp 52

 CCacheFileOutBuffer :: ~ CCacheFileOutBuffer () {if (IsOpen ()) {streamOffsets.push_back (GetFileSize ());  size_t lastOffset = streamOffsets [0];  for (size_t i = 1, count = streamOffsets.size (); i <count; ++ i) {size_t offset = streamOffsets [i];  size_t size = offset - lastOffset;  if (size> = (DWORD) (- 1)) throw CStreamException ("stream too large");  Add ((DWORD) size);  lastOffset = offset;  } Add ((DWORD) (streamOffsets.size () - 1));  }} 

The V509 message warns that if a CCacheFileOutBuffer object is destroyed during exception handling, a new exception will immediately crash the program.

Situation 12

Another comparison that does not make sense.

V547 Expression 'endRevision <0' is always false. Unsigned type value is never <0. cachelogquery.cpp 999

  typedef index_t revision_t;
 ...
 void CCacheLogQuery :: InternalLog (
    revision_t startRevision
  , revision_t endRevision
  , const CDictionaryBasedTempPath & startPath
  , int limit
  , const CLogOptions & options)
 {
   ...
   // we cannot receive logs for rev <0
   if (endRevision <0)
     endRevision = 0;

   ...
 } 

Negative values ​​here simply can not be. The endRevision variable is unsigned, and therefore endRevision is always> = 0. The potential trouble is that if somewhere before a negative value turned into an unsigned type, then we will start working with a very large number. There is no verification for this.

Situation 13

There are no more useful messages of the first level. Not yet. After all, this is only the first attempt at writing out new features of PVS-Studio. We have recorded at least 150 examples in the development plan that we should learn to identify. Thanks again to readers who responded to previous articles and sent examples in which you can identify errors with static analysis at the stage of writing code.

Let's look at the second level. We will find one interesting error related to the use of the Copy-Paste method. By the way, at the third level there was nothing interesting at all, so for good reason we disable it by default.

V524 It is the odd that the function GetDbgHelpVersion function is fully equivalent to the GetImageHlpVersion function (SymbolEngine.h, line 98). symbolengine.h 105

  BOOL GetImageHlpVersion (DWORD & dwMS, DWORD & dwLS)
 {
   return (GetInMemoryFileVersion (("DBGHELP.DLL")),
                                 dwMS,
                                 dwLS));
 }

 BOOL GetDbgHelpVersion (DWORD & dwMS, DWORD & dwLS)
 {
   return (GetInMemoryFileVersion (("DBGHELP.DLL")),
                                 dwMS,
                                 dwLS));
 } 

A V524 message is issued if two suspicious identical functions are detected. Most likely, the first function should receive the version of the file “imagehlp.dll”, and not at all “dbghelp.dll”.

Step 8

Now found errors should be corrected. This stage is clear and we will skip it.

As for the errors found, we will report them to the developers of TortoiseSVN.

Step 9

Now let's talk a little bit about false positives. Let us give some examples to clarify what a false positive is and how you can deal with them.

The first false positive. PVS-Studio did not understand games with memory copying.

V512 A call of the memcpy function will lead to a buffer overflow or underflow. resmodule.cpp 838

  const word *
 CResModule :: CountMemReplaceMenuExResource (....)
 {
   ...
   if (newMenu! = NULL) {
     CopyMemory (& newMenu [* wordcount], p0, 7 * sizeof (WORD));
   }
    ...
 } 

The V512 warning should indicate that the buffer is not fully processed or that, on the contrary, we have gone beyond it. Now the analyzer was mistaken in deciding that we will work with only one WORD object, and we want to copy 7 objects.

The second false positive. Here the analyzer considers that only part of the array was processed.

V512 A call of the memcmp function will lead to a buffer overflow or underflow. sshsha.c 317

  static int sha1_96_verify (....)
 {
   unsigned char correct [20];
   sha1_do_hmac (handle, blk, len, seq, correct);
   return! memcmp (correct, blk + len, 12);
 } 

Indeed, only part of the 'correct' array is compared, but it was intended to be so.

The third example is a false positive.

V517 The use of if (A) {...} else if (A) {...} 'pattern was detected. There is a possibility of logical error presence. tree234.c 195

  static void * add234_internal (....)
 {
   ...
   if ((c = t-> cmp (e, n-> elems [0])) <0)
     childnum = 0;
   else if (c == 0)
     return n-> elems [0];  / * already exists * /
   else if (n-> elems [1] == NULL
        ||  (c = t-> cmp (e, n-> elems [1])) <0)
          childnum = 1;
   else if (c == 0)
     return n-> elems [1];  / * already exists * /
   else if (n-> elems [2] == NULL
        ||  (c = t-> cmp (e, n-> elems [2])) <0)
          childnum = 2;
   else if (c == 0)
     return n-> elems [2];  / * already exists * /
   else
     childnum = 3;
   ... 

The analyzer does not like that the 'c == 0' check is present several times. The code is correct, since the variable 'c' changes inside other conditions “c = t-> cmp (e, n-> elems [2])”. However, this is quite a rare situation. More often, the V517 message indicates actual defects in the code.

The remaining false positives will not be considered, since they are not interesting. It is easy enough for a programmer to understand that they are false and should not be given much attention.

You can deal with false positives in different ways:

1) You can rewrite the code. Sometimes it is quite reasonable. Refactoring doesn’t hurt the last example with false positives (we are talking about the add234_internal function and V517 warning).

2) You can disable in the settings some checks that in your projects always give a false positive. After disconnection, all these messages will disappear from the table immediately. Read more here: " Settings: Detectable Errors ".

3) If the false positives refer to the code that you should not check, you can exclude individual files or folders. You can use masks. Read more here " Settings: Don't Check Files ". This is convenient to exclude from the analysis of third-party libraries.

4) You can use a mechanism to suppress messages containing specific text. Read more here: " Settings: Message Suppression ".

5) There are situations where you should suppress a specific warning. In this case, you can use the function “Mark as False Alarm” (“Mark as a false alarm”). In this case, a small comment of the form is inserted into the code: "// - V code error". Marked-up sections of the code may be hidden in the list of errors. To do this, use the FA button to turn on / off the display of marked messages. Read more here: " Suppress false warnings ."

Thank you all for your attention. Try PVS-Studio . Send your feedback. Ask questions. Show examples if you have something interesting. Offer new diagnostic rules for implementation.

Sincerely, Andrei Karpov, one of the developers of PVS-Studio.
You can contact us via the Feedback page.
Or by E-mail: support [@] viva64.com, karpov [@] viva64.com.

Email us, we have great support. I personally, the author of the article, will take part in communication, and not an abstract robot person, as happens in most companies.