Before you go, check out these stories!

0
Hackernoon logoTop 10 C++ Open Source Project Bugs Found in 2019 by@pvs-studio

Top 10 C++ Open Source Project Bugs Found in 2019

Author profile picture

@pvs-studioPVS-Studio

Search for bugs in C, C++, C#, Java on Windows, Linux, macOS. https://www.viva64.com/en/pvs-studio/

Another year is drawing to an end, and it's a perfect time to make yourself a cup of coffee and reread the reviews of bugs collected across open-source projects over this year. This would take quite a while, of course, so we prepared this article to make it easier for you. Today we'll be recalling the most interesting dark spots that we came across in open-source C/C++ projects in 2019.

No. 10. What Operating System Are We running On?

V1040Ā Possible typo in the spelling of a pre-defined macro name. The '__MINGW32_' macro is similar to '__MINGW32__'. winapi.h 4112

There is a typo in the name of theĀ __MINGW32_ macro (MINGW32 is actually declared by __MINGW32__). Elsewhere in the project, the check is written correctly:

By the way, this bug was not only the first to be described in the article "CMake: the Case when the Project's Quality is Unforgivable" but the very first genuine bug found by the V1040 diagnostic in a real open-source project (August 19, 2019).

No. 9. Who's First?

V502Ā Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '==' operator. mir_parser.cpp 884

We are interested in the following part:

The precedence of the '==' operator is higher than that of the ternary operator (?:). Therefore, the conditional expression is evaluated in the wrong order and is equivalent to the following code:

Since the constantsĀ OP_intrinsiccallĀ andĀ OP_intrinsiccallassignedĀ are non-null, the condition will be returningĀ trueĀ all the time, which means the body of theĀ elseĀ branch is unreachable code.

This bug was described in the article "Checking the Ark Compiler Recently Made Open-Source by Huawei".

No. 8. Dangerous Bitwise Operations

V1046Ā Unsafe usage of the bool' and 'int' types together in the operation '&='. GSLMultiRootFinder.h 175

The code suggests that theĀ SetFunctionListĀ function traverses an iterator list. If at least one iterator is invalid, the function returnsĀ false, orĀ true otherwise.

However, theĀ SetFunctionListĀ function can returnĀ falseĀ even for valid iterators. Let's figure out why.Ā TheĀ AddFunctionĀ function returns the number of valid iterators on theĀ fFunctionsĀ list. That is, adding non-null iterators will cause the list to incrementally grow in size: 1, 2, 3, 4, and so on. This is where the bug comes into play:

Since the function returns a value of typeĀ intĀ rather thanĀ bool, the '&=' operation will returnĀ falseĀ for even values because the least significant bit of an even number is always set to zero. This is how one subtle bug can break the return value ofĀ SetFunctionsListĀ even when its arguments are valid.

If you were reading the snippet carefully (and you were, weren't you?), you could have noticed that it came from the project ROOT. Yes, we checked it too: "Analyzing the Code of ROOT, Scientific Data Analysis Framework".

No. 7. Variables Mixed Up

V1001Ā [CWE-563] The 'Mode' variable is assigned but is not used by the end of the function. SIModeRegister.cpp 48

It's very dangerous to use the same names for function arguments as for class members because you risk mixing them up. And that's exactly what happened here. The following expression doesn't make sense:

The function's argument changes, and that's it. This argument is not used in any way after that. What the programmer really wanted to write was probably the following:

This bug was found inĀ LLVM. We have a tradition to check this project every now and then. This year weĀ checkedĀ it one more time.

No. 6. C++ Has Its Own Laws

This bug stems from the fact that C++ rules don't always follow mathematical rules or "common sense". Look at the small snippet below and try to find the bug yourself.

V709Ā Suspicious comparison found: 'f0 == f1 == m_fractureBodies.size()'. Remember that 'a == b == c' is not equal to 'a == b && b == c'. btFractureDynamicsWorld.cpp 483

The condition seems to be checking thatĀ f0Ā is equal toĀ f1Ā and is equal to the number of elements inĀ m_fractureBodies. It was probably meant to check ifĀ f0Ā andĀ f1Ā are located at the end of theĀ m_fractureBodiesĀ array since they contain an object position found by theĀ findLinearSearch() method. But in reality, this conditional expression checks ifĀ f0Ā is equal toĀ f1 and then ifĀ m_fractureBodies.size()Ā is equal to the result of the expressionĀ f0 == f1. That is, the third operand here is checked against 0 or 1.

That's a nice bug! And, fortunately, a pretty rare one. So far we haveĀ seenĀ it only in three open-source projects, and, interestingly, all the three were game engines. This is not the only bug found in Bullet; the most interesting ones were described in the article "PVS-Studio Looked into the Red Dead Redemption's Bullet Engine".

No. 5. What's At The End Of The Line?

This one is easy if you know one tricky detail.

V739Ā EOF should not be compared with a value of the 'char' type. The 'ch' should be of the 'int' type. json.cpp 762

This is one of those bugs that you can't easily spot if you don't know that EOF is defined as -1. So, if you try to compare it with a variable of type signed char, the condition will almost always beĀ false. The only exception is the character encoded as 0xFF (255). When compared withĀ EOF, this character will turn into -1, thus making the condition true.

A lot of bugs in this years Top 10 were found in computer gaming software: engines or open-source games. As you already guessed, this one came from that area too. More errors are described in the article "Cataclysm Dark Days Ahead: Static Analysis and Roguelike Games".

No. 4. The Magic Constant Pi

V624Ā There is probably a misprint in '3.141592538' constant. Consider using the M_PI constant from <math.h>. PhysicsClientC_API.cpp 4109

There's a tiny typo in the Pi number (3,141592653...): the number "6" is missing at the 7th decimal place.

An incorrect one-millionth decimal digit would hardly cause any noticeable harm, but it's still better to use existing constants from libraries, whose correctness is guaranteed. The Pi number, for instance, is represented by the constant M_PI from the header math.h.

You already read about this bug in the article "PVS-Studio Looked into the Red Dead Redemption's Bullet Engine", where it was placed sixth. If you haven't read it yet, this is your last chance.

A Small Diversion

We are approaching the Top 3 most interesting bugs. As you have probably noticed, I'm sorting the bugs not by their impact but by the effort it takes a human reviewer to find them. After all, the advantage of static analysis over code reviews is basically the inability of software tools to get tired or forget things. :)

Now, let's see what we have in our Top 3.

No. 3. An Elusive Exception

V702Ā Classes should always be derived from std::exception (and alike) as 'public' (no keyword was specified, so compiler defaults it to 'private'). CalcManager CalcException.h 4

The analyzer has detected a class derived from theĀ std::exceptionĀ class using theĀ privateĀ modifier (which is used by default if not specified otherwise). The problem with this code is that an attempt to catch a genericĀ std::exceptionĀ will cause the program to miss an exception of typeĀ CalcException. This behavior stems from the fact that private inheritance forbids implicit type conversion.

You definitely wouldn't like to see your program crash because of a missed publicĀ modifier. By the way, I bet you have used this application at least once in your life because it's the good oldĀ Windows Calculator, which we also checkedĀ earlier this year.

No. 2. Unclosed HTML Tags

V735Ā Possibly an incorrect HTML. The "</body>" closing tag was encountered, while the "</div>" tag was expected. book.cpp 127

As it often happens, C/C++ source code doesn't say much by itself, so let's take a look at the preprocessed code generated from the snippet above:

The analyzer has found an unclosedĀ <div>Ā tag. There are many html-code fragments here, so the authors need to revise it.

Surprised we can diagnose this kind of bugs? I was impressed too when I saw that for the first time. So, yes, we do know something about analyzing html code. Well, only if it's within C++ code. :)

Not only is this bug placed second but it's a second calculator on our Top 10 list. To learn what other bugs we found in this project, see the article "Following in the Footsteps of Calculators: SpeedCrunch".

No. 1. Elusive Standard Functions

Here's the bug that placed first. This one is an impressively weird bug, which managed to make it through the code review.

Try to find it yourself:

Now let's see what the analyzer has to say:

V560Ā A part of conditional expression is always true: ('\n' != c). params.c 136.

Weird, isn't it? Let's take a look at some other curious spot but in a different file (charset.h):

Hm, this is strange indeed... So, if theĀ cĀ variableĀ is equal toĀ '\n',Ā then the seemingly harmless functionĀ isspace(c)Ā willĀ returnĀ false,Ā thus preventing the second part of the check from executing due to short-circuit evaluation. And ifĀ isspace(c)Ā executes, theĀ cĀ variableĀ will be equal either toĀ ' 'Ā orĀ '\t',Ā which is obviously not being equal toĀ '\n'.

You could argue that this macro is similar toĀ #define true falseĀ and code like that would never make it through a code review. But this particular snippet did ā€“ and was sitting in the repository waiting to be discovered.

For more detailed commentary on this bug, see the article "Wanna Play a Detective? Find the Bug in a Function from Midnight Commander".

Conclusion

We found tons of bugs in 2019. Those were common copy-paste mistakes, inaccurate constants, unclosed tags, and lots of other defects. But our analyzer is evolving andĀ learningĀ to diagnose more and more types of issues, so we are certainly not going to slow down and will be publishing new articles about bugs found in projects just as regularly as before.

Just in case you haven't read our articles before, all these bugs were found using our PVS-Studio static analyzer, which you are welcome toĀ download and try on your own projects. It detects bugs in programs written in C, C++, C#, and Java.

You've finally got to the finish line! If you missed the first two levels, I suggest that you seize the opportunity and complete these levels with us: C# andĀ Java.

Previously published at https://www.viva64.com/en/b/0700/

Author profile picture

@pvs-studioPVS-Studio

Read my stories

Search for bugs in C, C++, C#, Java on Windows, Linux, macOS. https://www.viva64.com/en/pvs-studio/

Tags

Join Hacker Noon

Create your free account to unlock your custom reading experience.