edas bug 2019_2019年在C ++项目中发现的十大bug

edas bug 2019

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.

又一年即将结束，这是一个完美的时机，让自己喝杯咖啡，重新阅读今年跨开放源代码项目收集的bug的评论。 当然，这将花费相当长的时间，因此我们准备了本文以使您更轻松。 今天，我们将回想起2019年开源C / C ++项目中遇到的最有趣的黑点。

否。10.我们正在运行什么操作系统？ (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 V1040预定义的宏名称的拼写可能有错字。 “ __MINGW32_”宏类似于“ __MINGW32__”。 winapi.h 4112

#if !defined(__UNICODE_STRING_DEFINED) && defined(__MINGW32_)
#define __UNICODE_STRING_DEFINED
#endif

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:

__MINGW32 _宏的名称中有一个错字(MINGW32实际上是由__MINGW32__声明的)。 在项目的其他地方，检查是否正确编写：

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).

顺便说一句，该错误不仅是文章“ CMake：项目质量不可原谅的情况 ”中首次描述，而且还是V1040诊断程序在一个真正的开源项目中发现的第一个真正错误(8月19日) ，2019)。

9号。谁先？ (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 V502也许'？：'运算符的工作方式与预期的不同。 '？：'运算符的优先级低于'=='运算符。 第884章

enum Opcode : uint8 {
  kOpUndef,
  ....
  OP_intrinsiccall,
  OP_intrinsiccallassigned,
  ....
  kOpLast,
};

bool MIRParser::ParseStmtIntrinsiccall(StmtNodePtr &stmt, bool isAssigned) {
  Opcode o = !isAssigned ? (....)
                         : (....);
  auto *intrnCallNode = mod.CurFuncCodeMemPool()->New<IntrinsiccallNode>(....);
  lexer.NextToken();
  if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) {
    intrnCallNode->SetIntrinsic(GetIntrinsicID(lexer.GetTokenKind()));
  } else {
    intrnCallNode->SetIntrinsic(static_cast<MIRIntrinsicID>(....));
  }
  ....
}

We are interested in the following part:

我们对以下部分感兴趣：

if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) {
  ....
}

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:

'=='运算符的优先级高于三元运算符(？:)的优先级。 因此，条件表达式的计算顺序错误，并且等效于以下代码：

if ((o == !isAssigned) ? OP_intrinsiccall : OP_intrinsiccallassigned) {
  ....
}

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.

由于常量OP_intrinsiccall和OP_intrinsiccallassigned不为null，因此条件将一直返回true ，这意味着else分支的主体是不可访问的代码。

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

该错误在“ 检查华为最近使开源的方舟编译器 ”中有描述。

No. 8.危险的按位运算 (No. 8. Dangerous bitwise operations)

V1046 Unsafe usage of the bool' and 'int' types together in the operation '&='. GSLMultiRootFinder.h 175 V1046在操作“＆=”中不安全地同时使用bool和int类型。 GSLMultiRootFinder.h 175

int AddFunction(const ROOT::Math::IMultiGenFunction & func) {
  ROOT::Math::IMultiGenFunction * f = func.Clone();
  if (!f) return 0;
  fFunctions.push_back(f);
  return fFunctions.size();
}

template<class FuncIterator>
bool SetFunctionList( FuncIterator begin, FuncIterator end) {
  bool ret = true;
  for (FuncIterator itr = begin; itr != end; ++itr) {
    const ROOT::Math::IMultiGenFunction * f = *itr;
    ret &= AddFunction(*f);
  }
  return ret;
}

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.

该代码建议SetFunctionList函数遍历迭代器列表。 如果至少一个迭代器无效，则该函数返回false ，否则返回true 。

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:

但是，即使对于有效的迭代器， SetFunctionList函数也可以返回false 。 让我们来找出why.The 调用addFunction函数返回fFunctions名单上的有效迭代器的数量。 也就是说，添加非null迭代器将导致列表的大小逐渐增加：1、2、3、4，依此类推。 这是该错误起作用的地方：

ret &= AddFunction(*f);

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.

由于函数返回的是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".

如果您正在仔细阅读该代码段(并且确实是，不是吗？)，您可能已经注意到它来自项目ROOT。 是的，我们也进行了检查：“ 分析ROOT代码，科学数据分析框架 ”。

No. 7.变量混在一起 (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 V1001 [CWE-563]分配了'Mode'变量，但在功能结束时未使用。 SIModeRegister.cpp 48

struct Status {
  unsigned Mask;
  unsigned Mode;

  Status() : Mask(0), Mode(0){};

  Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
    Mode &= Mask;
  };
  ....
};

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:

对于函数参数使用与类成员相同的名称是非常危险的，因为您可能会混淆它们。 这就是这里发生的事情。 以下表达式没有意义：

Mode &= Mask;

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:

函数的参数改变了，仅此而已。 此后将不再使用该参数。 程序员真正想要写的可能是以下内容：

Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
  this->Mode &= Mask;
};

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.

在LLVM中发现此错误。 我们有不时检查此项目的传统。 今年，我们又检查了一次。

No.6。C++有自己的规律 (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.

此错误源于以下事实：C ++规则并不总是遵循数学规则或“常识”。 查看下面的小片段，然后尝试自己查找错误。

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 V709发现可疑比较：“ f0 == f1 == m_fractureBodies.size()”。 请记住，“ a == b == c”不等于“ a == b && b == c”。 btFractureDynamicsWorld.cpp 483

btAlignedObjectArray<btFractureBody*> m_fractureBodies;

void btFractureDynamicsWorld::fractureCallback()
{
  for (int i = 0; i < numManifolds; i++)
  {
    ....
    int f0 = m_fractureBodies.findLinearSearch(....);
    int f1 = m_fractureBodies.findLinearSearch(....);

    if (f0 == f1 == m_fractureBodies.size())
      continue;
    ....
  }
....
}

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.

该条件似乎正在检查f0等于f1并等于m_fractureBodies中的元素数 。 可能是要检查f0和f1是否位于m_fractureBodies数组的末尾，因为它们包含由findLinearSearch()方法找到的对象位置。 但实际上，此条件表达式检查f0是否等于f1 ，然后检查m_fractureBodies.size()是否等于表达式f0 == f1的结果 。 也就是说，此处的第三个操作数将根据0或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".

那是一个不错的错误！ 而且，幸运的是，这是非常罕见的。 到目前为止，我们仅在三个开源项目中看到了它，有趣的是，这三个都是游戏引擎。 这不是Bullet中发现的唯一bug； 文章“ PVS-Studio看着Red Dead Redemption的子弹引擎 ”中描述了最有趣的引擎 。

No. 5.行尾是什么？ (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 V739 EOF不应与“ char”类型的值进行比较。 “ ch”应为“ int”类型。 json.cpp 762

void JsonIn::skip_separator()
{
  signed char ch;
  ....
  if (ch == ',') {
    if( ate_separator ) {
      ....
    }
    ....
  } else if (ch == EOF) {
  ....
}

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.

如果您不知道EOF定义为-1，则这是您不容易发现的错误之一。 因此，如果尝试将其与具有符号char类型的变量进行比较，则该条件几乎总是false 。 唯一的例外是编码为0xFF(255)的字符。 与EOF比较时，此字符将变为-1，从而使条件成立。

A lot of bugs in this year's 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".

在今年的前10名中，有很多错误是在计算机游戏软件中发现的：引擎或开源游戏。 正如您已经猜到的那样，这一个人也来自该地区。 更多的错误描述在文章“大灾变：黑暗的未来：静态分析和Roguelike游戏 ”中。

4号。魔术常数Pi (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 V624'3.141592538 '常量中可能存在打印错误。 考虑使用<math.h>中的M_PI常量。 PhysicsClientC_API.cpp 4109

B3_SHARED_API void b3ComputeProjectionMatrixFOV(float fov, ....)
{
  float yScale = 1.0 / tan((3.141592538 / 180.0) * fov / 2);
  ....
}

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

Pi编号(3,141592653 ...)中有一个很小的错字：小数点后7位缺少数字«6»。

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.

您已经在文章“ PVS-Studio深入研究Red Dead Redemption的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. :)

我们正在处理最有趣的3个错误。 正如您可能已经注意到的那样，我对bug的分类不是根据它们的影响，而是要由人工审核者才能找到它们。 毕竟，静态分析优于代码审查的优势基本上是软件工具无法累或忘了事情。 :)

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

现在，让我们看看我们的前三名。

否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 V702类应始终从std :: exception(类似)作为“ public”派生(未指定关键字，因此编译器默认将其设置为“ private”)。 CalcManager CalcException.h 4

class CalcException : std::exception
{
public:
  CalcException(HRESULT hr)
  {
    m_hr = hr;
  }
  HRESULT GetException()
  {
    return m_hr;
  }
private:
  HRESULT m_hr;
};

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.

分析器已使用private修饰符检测到从std :: exception类派生的类(如果未另行指定，则默认使用该类)。 此代码的问题在于，尝试捕获通用std :: exception将导致程序错过CalcException类型的异常。 此行为源于以下事实：私有继承禁止隐式类型转换。

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.

您肯定不希望由于缺少公共修饰符而导致程序崩溃。 顺便说一句，我敢打赌您一生中至少使用过此应用程序一次，因为它是旧的Windows计算器 ，我们也在今年早些时候对其进行了检查 。

No. 2.未封闭HTML标签 (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 V735可能是不正确HTML。 遇到“ </ body>”结束标记，而应使用“ </ div>”标记。 book.cpp 127

static QString makeAlgebraLogBaseConversionPage() {
  return
    BEGIN
    INDEX_LINK
    TITLE(Book::tr("Logarithmic Base Conversion"))
    FORMULA(y = log(x) / log(a), log<sub>a</sub>x = log(x) / log(a))
    END;
}

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:

通常，C / C ++源代码本身并不能说太多，所以让我们看一下从上面的代码片段生成的预处理代码：

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

分析器发现一个未关闭的<div>标签。 这里有许多html代码片段，因此作者需要对其进行修改。

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. :)

我们可以诊断出此类错误感到惊讶吗？ 当我第一次看到它时，我也印象深刻。 因此，是的，我们确实了解有关分析html代码的知识。 好吧，只有它在C ++代码内。 :)

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".

该错误不仅排在第二位，而且还是我们的“十佳”列表中的第二个计算器。 要了解我们在该项目中发现了哪些其他错误，请参阅文章“ 跟随计算器的脚步：SpeedCrunch ”。

No. 1.难以捉摸的标准功能 (No. 1. Elusive standard functions)

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

这是首先放置的错误。 这是一个令人印象深刻的怪异错误，该错误通过代码审查得以实现。

Try to find it yourself:

尝试自己找到它：

static int
EatWhitespace (FILE * InFile)
  /* ----------------------------------------------------------------------- **
   * Scan past whitespace (see ctype(3C)) and return the first non-whitespace
   * character, or newline, or EOF.
   *
   *  Input:  InFile  - Input source.
   *
   *  Output: The next non-whitespace character in the input stream.
   *
   *  Notes:  Because the config files use a line-oriented grammar, we
   *          explicitly exclude the newline character from the list of
   *          whitespace characters.
   *        - Note that both EOF (-1) and the nul character ('\0') are
   *          considered end-of-file markers.
   *
   * ----------------------------------------------------------------------- **
   */
{
    int c;

    for (c = getc (InFile); isspace (c) && ('\n' != c); c = getc (InFile))
        ;
    return (c);
}                               /* EatWhitespace */

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

现在，让我们看看分析器要说的是：

V560 A part of conditional expression is always true: ('\n' != c). params.c 136. V560条件表达式的一部分始终为true：('\ n'！= c)。 c。第136章

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

很奇怪，不是吗？ 让我们看一下其他一些有趣的地方，但是在另一个文件(charset.h)中：

#ifdef isspace
#undef isspace
#endif
....
#define isspace(c) ((c)==' ' || (c) == '\t')

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

嗯，这确实很奇怪……因此，如果c变量等于'\ n'，那么看似无害的函数isspace(c)将返回false ，从而由于短路评估而导致无法执行检查的第二部分。 如果执行isspace(c) ，则c变量将等于''或'\ t'，这显然不等于'\ 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.

您可能会争辩说此宏类似于#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".

有关此错误的更多详细评论，请参阅文章“ 想扮演侦探？从Midnight Commander中查找功能中的错误 ”。

结论 (Conclusion)

We have found tons of bugs over this year. 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.

以防万一您以前没有阅读过我们的文章，所有这些错误都是使用我们的PVS-Studio静态分析器发现的，欢迎您下载并尝试使用自己的项目。 它检测用C，C ++，C＃和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.

您终于到了终点线！ 如果您错过了前两个级别，建议您抓住机会并与我们一起完成这些级别： C＃和Java 。

翻译自: https://habr.com/en/company/pvs-studio/blog/481188/

edas bug 2019