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? Possible typo in the spelling of a pre-defined macro name. The '__MINGW32_' macro is similar to '__MINGW32__'. winapi.h 4112 V1040 There is a typo in the name of the _ macro (MINGW32 is actually declared by __MINGW32__). Elsewhere in the project, the check is written correctly: __MINGW32 By the way, this bug was not only the first to be described in the article " " but the very first genuine bug found by the V1040 diagnostic in a real open-source project (August 19, 2019). CMake: the Case when the Project's Quality is Unforgivable No. 9. Who's First? 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 V502 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 and are non-null, the condition will be returning all the time, which means the body of the branch is unreachable code. OP_intrinsiccall OP_intrinsiccallassigned true else This bug was described in the article " ". Checking the Ark Compiler Recently Made Open-Source by Huawei No. 8. Dangerous Bitwise Operations Unsafe usage of the bool' and 'int' types together in the operation '&='. GSLMultiRootFinder.h 175 V1046 The code suggests that the function traverses an iterator list. If at least one iterator is invalid, the function returns , or otherwise. SetFunctionList false true However, the function can return even for valid iterators. Let's figure out why. The function returns the number of valid iterators on the 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: SetFunctionList false AddFunction fFunctions Since the function returns a value of type rather than , the '&=' operation will return 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 even when its arguments are valid. int bool false SetFunctionsList 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 [CWE-563] The 'Mode' variable is assigned but is not used by the end of the function. SIModeRegister.cpp 48 V1001 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 . We have a tradition to check this project every now and then. This year we it one more time. LLVM checked 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. Suspicious comparison found: 'f0 == f1 == m_fractureBodies.size()'. Remember that 'a == b == c' is not equal to 'a == b && b == c'. btFractureDynamicsWorld.cpp 483 V709 The condition seems to be checking that is equal to and is equal to the number of elements in . It was probably meant to check if and are located at the end of the array since they contain an object position found by the method. But in reality, this conditional expression checks if is equal to and then if is equal to the result of the expression . That is, the third operand here is checked against 0 or 1. f0 f1 m_fractureBodies f0 f1 m_fractureBodies findLinearSearch() f0 f1 m_fractureBodies.size() f0 == f1 That's a nice bug! And, fortunately, a pretty rare one. So far we have 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 " ". seen 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. EOF should not be compared with a value of the 'char' type. The 'ch' should be of the 'int' type. json.cpp 762 V739 This is one of those bugs that you can't easily spot if you don't know that is defined as -1. So, if you try to compare it with a variable of type , the condition will almost always be . The only exception is the character encoded as 0xFF (255). When compared with , this character will turn into -1, thus making the condition true. EOF signed char false EOF 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 There is probably a misprint in '3.141592538' constant. Consider using the M_PI constant from <math.h>. PhysicsClientC_API.cpp 4109 V624 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 " ", where it was placed sixth. If you haven't read it yet, this is your last chance. PVS-Studio Looked into the Red Dead Redemption's Bullet Engine 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 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 V702 The analyzer has detected a class derived from the class using the modifier (which is used by default if not specified otherwise). The problem with this code is that an attempt to catch a generic will cause the program to miss an exception of type . This behavior stems from the fact that private inheritance forbids implicit type conversion. std::exception private std::exception CalcException You definitely wouldn't like to see your program crash because of a missed modifier. By the way, I bet you have used this application at least once in your life because it's the good old , which we also earlier this year. public Windows Calculator checked No. 2. Unclosed HTML Tags Possibly an incorrect HTML. The "</body>" closing tag was encountered, while the "</div>" tag was expected. book.cpp 127 V735 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 tag. There are many html-code fragments here, so the authors need to revise it. <div> 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: A part of conditional expression is always true: ('\n' != c). params.c 136. V560 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 variable is equal to then the seemingly harmless function will return , thus preventing the second part of the check from executing due to short-circuit evaluation. And if executes, the variable will be equal either to or which is obviously not being equal to . c '\n', isspace(c) false isspace(c) c ' ' '\t', '\n' You could argue that this macro is similar to 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. #define true false 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 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. learning 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 and try on your own projects. It detects bugs in programs written in C, C++, C#, and Java. download 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: and . C# Java Previously published at https://www.viva64.com/en/b/0700/