How PVS-Studio turned out to be more attentive than three and a half programmers

PVS-Studio, like other static code analyzers, often produces false positives. But do not hurry to consider strange triggers false. This is a short story about how PVS-Studio again turned out to be more attentive than several people.

We wrote in support of the user, claiming that the analyzer generates four false alarms for one line of code at once. The letter, written in support, initially came to Yevgeny Ryzhkov, who, after fluently reading it and not noticing the anomalous feedback feed, immediately sent it to the lead developer, Svyatoslav Razmyslov. Eugene did not peer into the code, so it would be honest to consider it only for half of the programmer :).


Svyatoslav read the letter and doubted that the analyzer could be so blatantly wrong. Therefore, he came to me for a consultation. Svyatoslav had a hope that my eye was a master and I would notice something that would tell you why the analyzer issued all these strange messages. Unfortunately, I just confirmed that the messages are really very strange and should not be. However, what caused their occurrence, I could not notice. It was decided to open the task in the bugtracker and start to figure out what is wrong.

And only when Svyatoslav began to make synthetic examples in order to describe the problem in detail in the bugtracker, an insight descended on him. Let's now see if you can quickly find the reason why the analyzer generates 4 messages.

Here is the text of the letter, published with the permission of the author. And an explanatory picture that was attached to the letter.

V560 warnings here are all false. Running with most recent version of PVS-Studio for personal use. Basically, the “IF” statement is correct. It is true or false.

Now, dear reader, your time to check yourself! See a mistake?

Your time to be attentive. And the unicorn will wait a little while.

After the introductory part of the article, most likely many have found a mistake. When configured to find the error, it is located. It is much more difficult to notice a blunder after reading a letter, where it is called “false positives” :).

Now an explanation for those who are too lazy to look for a bug. Consider the condition again:

if (!((ch >= 0x0FF10) && (ch <= 0x0FF19)) ||
     ((ch >= 0x0FF21) && (ch <= 0x0FF3A)) ||
     ((ch >= 0x0FF41) && (ch <= 0x0FF5A)))

The author of the code planned to check that the symbol does not belong to any of the three ranges.

The error is that the logical NOT (!) Operator applies only to the first subexpression.

If the condition is met:

!((ch >= 0x0FF10) && (ch <= 0x0FF19))

then the evaluation of the expression is interrupted according to the short-circuit evaluation . If the condition is not met, then the value of the variable ch lies in the range [0xFF10..0xFF19]. Accordingly, four further comparisons are meaningless. They will all be false or true.

Again. See, if ch is in the range [0xFF10..0xFF19] and the expression evaluation continues, then:

• ch> = 0x0FF21 - always false
• ch <= 0x0FF3A - always true
• ch> = 0x0FF41 - always false
• ch <= 0x0FF5A - always true

The PVS-Studio analyzer warns about this.

So, the static analyzer turned out to be more attentive than the user and two and a half programmers from our team.

To fix the situation, add additional brackets:

if (!(((ch >= 0x0FF10) && (ch <= 0x0FF19)) ||
      ((ch >= 0x0FF21) && (ch <= 0x0FF3A)) ||
      ((ch >= 0x0FF41) && (ch <= 0x0FF5A))))

Or rewrite the condition:

if (((ch < 0x0FF10) || (ch > 0x0FF19)) &&
    ((ch < 0x0FF21) || (ch > 0x0FF3A)) &&
    ((ch < 0x0FF41) || (ch > 0x0FF5A)))

However, I can not recommend to use any of these options. I would write to simplify the reading of the code:

constbool isLetterOrDigit =    (ch >= 0x0FF10 && ch <= 0x0FF19)  // 0..9
                             || (ch >= 0x0FF21 && ch <= 0x0FF3A)  // A..Z
                             || (ch >= 0x0FF41 && ch <= 0x0FF5A); // a..zif (!isLetterOrDigit)

Note that I removed some of the brackets. As we have just seen, a large number of brackets did not at all help to avoid an error. The brackets should make it easier to read the code, and not complicate things. Programmers remember well that the priority of comparisons = <, => is higher than that of the && operator. Therefore, brackets are not needed here. But if you ask what is more priority - && or ||, many will be confused. Therefore, to assign a sequence of calculations &&, || it is better to put brackets.

Why is it better to write || At the beginning I described in the article " The main issue of programming, refactoring and all that " (see chapter: Align the same type code with a "table").

Thank you all for your attention. Download and start using PVS-Studio. It will help identify many errors and potential vulnerabilities at the earliest stages.

If you want to share this article with an English-speaking audience, then please use the link to the translation: Andrey Karpov. How PVS-Studio Prods to Be More Attentive Than Three and a Half Programmers .