盘点 C++ 开源项目中的十大 Bug

2020 年 10 月 30 日

盘点 C++ 开源项目中的十大 Bug

又一年即将结束,是时候盘点一下开源项目中的 Bug 了。2020 年的盘点可能还需要点时间,本文我们先来看看 2019 年开源 C/C++ 项目中遇到的一些最有趣的槽点。


No. 10. 我们正运行在什么操作系统上?


V1040 可能拼写错误预定义宏名称。’MINGW32_‘有点儿像’MINGW32__’。winapi.h 4112


#if !defined(__UNICODE_STRING_DEFINED) && defined(__MINGW32_)#define __UNICODE_STRING_DEFINED#endif
复制代码


MINGW32_ 宏的名称拼写有误(MINGW32 实际上被声明为 MINGW32__)。在项目的其它地方,拼写是正确的:



顺便说一句,这个 bug 并不是在文章"CMake: the Case when the Project’s Quality is Unforgivable"中首次被描述,而是在一个开源项目的 V1040 诊断中就真正被第一次发现的 bug(2019 年 8 月 19 日)。


https://www.viva64.com/en/b/0658/?ref=hackernoon.com


No. 9. 哪个先?


V502 可能’?:‘运算符的工作方式与预期不符。’?:‘运算符的优先级比’=='运算符低。mir_parser.cpp 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>(....));  }  ....}
复制代码


我们感兴趣的下面的部分:


if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) {  ....}
复制代码


'=='运算符的优先级比三元运算符 (?:) 高。因此,这个条件表达式的求值顺序错误,等效于如下代码:


if ((o == !isAssigned) ? OP_intrinsiccall : OP_intrinsiccallassigned) {  ....}
复制代码


由于常量 OP_intrinsiccall 和 OP_intrinsiccallassigned 都是非 null 的,这个条件会一直返回 true,这意味着 else 分支是无法访问的代码。


这个 bug 在文章"Checking the Ark Compiler Recently Made Open-Source by Huawei"中被提到。


https://www.viva64.com/en/b/0690/?ref=hackernoon.com


No. 8. 危险的位运算符


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;}
复制代码


代码建议 SetFunctionList 函数遍历一个迭代器列表。如果至少有一个迭代器是无效的,这个函数会返回 false,否则就返回 true。


然而,SetFunctionList 函数对于有效的迭代器也会返回 false。让我们来看看是为什么。AddFunction 函数返回 fFunctions 列表中有效迭代器的数目。也就是说,添加非空迭代器将导致列表的大小递增:1、2、3、4,以此类推。这就是 bug 生效的地方:


ret &= AddFunction(*f);
复制代码


由于这个函数返回一个 int 类型的值而不是 bool 类型,因此对于偶数值’&='运算符也会返回 false,因为偶数的最低有效位始终设置为 0。这就是为什么一个微小的 bug 会打破 SetFunctionsList 的返回值,即使它的参数是有效的。


如果你仔细阅读了代码片段(你是认真的,对吧?),你可能已经发现,它来自 ROOT 项目。是的,我们也发现了这个 bug:“Analyzing the Code of ROOT, Scientific Data Analysis Framework”。


https://www.viva64.com/en/b/0682/?ref=hackernoon.com


No. 7. 变量混淆


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;  };  ....};
复制代码


对函数参数和类成员使用相同的名字是非常危险的,因为你很可能把它们混淆。而这里就是这样的。下面的表达式没有意义:


Mode &= Mask;
复制代码


函数的参数变化之后,这个参数之后不会以任何形式被使用。编程人员很可能想要写的是:


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


这个 bug 在 LLVM 发现。我们有一个传统,不时地检查这个项目。今年我们又 检查了一次这个项目。


No. 6. C++ 有自己的的规则


这个 bug 源于 C++ 规则并不总是遵循数学规则或“常识”。看看下面的代码片段,试着自己找出 bug 吧。


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;    ....  }....}
复制代码


这个条件表达式似乎是在检查 f0 等于 f1,并且等于 m_fractureBodies 中元素的数目。这可能意味着,检查 f0 和 f1 是否位于 m_fractureBodies 数组的末尾,因为它们都包含被 findLinearSearch() 方法发现的一个对象。但实际上,这个条件表达式检查 f0 是否等于 f1,然后检查 m_fractureBodies.size() 是否等于 f0 == f1 表达式的结果。也就是说,这里第三个运算数是 0 或 1。


这是一个好 bug!而且,幸运的是,这个 bug 非常罕见。到目前为止,我们只在 3 个开源项目中看到过这个 bug,而且有趣的是,这 3 个项目都是游戏引擎。这不是 Bullet 中发现的唯一 bug;最有趣的一些 bug 在文章"PVS-Studio Looked into the Red Dead Redemption’s Bullet Engine"有描述。


https://www.viva64.com/en/b/0647/?ref=hackernoon.com


No. 5. 行末尾是什么?


这个 bug 很容易发现,如果你知道其中细节的话。


V739 EOF 不应该与一个’char’类型的值进行比较。'ch’应该是’int’类型。json.cpp 762


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


这是你很难发现的一些 bugs 之一,如果你不知道 EOF 是被定义为 -1 的话。因此,如果你试图将它与一个带标志的字符类型变量比较时,条件表达式的结果几乎总会是 false。唯一的例外是编码为 0xFF(255) 的字符。当与 EOF 比较时,这个字符会变成 -1,因此会让这个条件表达式的结果为 true。


在这几年的众多 bugs 中,前 10 名都是在计算机游戏软件中发现的:引擎或开源游戏。你可能已经猜到了,这个 bug 也是来自游戏领域。更多错误在"Cataclysm Dark Days Ahead: Static Analysis and Roguelike Games"一文中有描述。


https://www.viva64.com/en/b/0628/?ref=hackernoon.com


No. 4. 常量 Pi


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);  ....}
复制代码


在 Pi 数字 (3,141592653…) 中有一个微小的打印错误:第 7 个小数位的数字“6”丢失了。



一个不正确的百万分之一的小数位很难造成任何明显的损害,但是最好使用库里已有的常量,其正确性有所保障。例如,Pi 数字由头文件 math.h 中的 M_PI 常量表示。


你可能在文章"PVS-Studio Looked into the Red Dead Redemption’s Bullet Engine"中已经读到过这个 bug,它在其中排第 6 位。如果你还没读到过,这次可别错过。


https://www.viva64.com/en/b/0647/?ref=hackernoon.com


No. 3. 难以捉摸的异常


V702std::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;};
复制代码


分析器检测到来自 std::exception 的一个类使用了 private 修饰符(如果没有指定的话默认使用 private)。这段代码的问题是,试图捕获一个通用的 std::exception 将会导致这个程序错过 CalcException 类型的异常。这个行为源于 private 继承禁止隐式类型转换。


你肯定不希望看到你的程序因为一个漏掉的 public 修饰符而崩溃。顺便说一句,我打赌你在一生中肯定至少用过这个应用程序一次,因为它就是老的 Windows Calculator,我们早几年也检查过这个应用程序。


No. 2. 未闭合的 HTML 标签


V735 可能是一个不正确的 HTML。碰到"“闭合标签时,预期的是”" 标签。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;}
复制代码


由于它经常发生,C/C++ 源代码本身没有太多说明,因此让我们看看上面的代码片段生成的预处理代码:



分析器发现了一个未闭合的 div 标签。这里有很多 html 代码片段,因此作者需要修改代码。


很惊讶我们能诊断出这种类型的 bugs 吗?我第一次看到这一点时,印象也非常深刻。因此,是的,我们确实知道一些关于分析 html 代码的知识。不过,只在 C++ 代码中才行。:)


不仅这个 bug 被排在第二位,这也是我们的前 10 榜单中的第二个计算器。可以阅读"Following in the Footsteps of Calculators: SpeedCrunch"这篇文章,来看看我们在这个项目发现的其它 bugs。


https://www.viva64.com/en/b/0618/?ref=hackernoon.com


No. 1. 难以捉摸的标准函数


这个 bug 位于第一位,是一种非常奇怪的 bug,能够成功通过代码评审。


你自己试试发现这个 bug:


static intEatWhitespace (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 */
复制代码


现在让我们看看分析器怎么说:


V560 部分条件表达式总是 true:(’\n’ != c)。params.c 136.


很奇怪,对不对?让我们看看在另一个文件(charset.h)中其它奇怪的点:


#ifdef isspace#undef isspace#endif....#define isspace(c) ((c)==' ' || (c) == '\t')
复制代码


嗯,这确实很奇怪… 因此,如果变量 c 等于’\n’,那么看起来无害的函数 isspace© 会返回 false,从而因为短路逻辑而不执行第二部分的检查。而且如果 isspace© 执行,变量 c 将会等于 ‘’ 或’\t’,明显不会等于’\n’。


你可能会说,这个宏与 #define true false 类似,像这样的代码永远不能通过一个代码评审。但是这个特殊的代码片段确实通过了代码评审——而且还在代码库中等待被发现。


有关这个 bug 的更详细的评论,请查看文章"Wanna Play a Detective? Find the Bug in a Function from Midnight Commander"。


https://www.viva64.com/en/b/0610/


结 论



我们发现的这些 Bug 都是一些常见的复制 - 粘贴错误、不准确的常量、未闭合的标签以及许多其它缺陷。但是我们的分析器正在不断演进和 学习 来诊断越来越多类型的问题,因此我们肯定不会放慢脚步,并且会像以前一样定期发布关于项目中发现的 bugs 的新文章。


作者介绍:


PVS-Studio 致力于寻找 C、C++、C#、Java 在 Windows、Linux 和 macOS 上的 bugs。


原文链接:


https://www.viva64.com/en/b/0700/?ref=hackernoon.com


2020 年 10 月 30 日 17:321309

评论

发布
暂无评论
发现更多内容

第六周总结

fmouse

极客大学架构师训练营

第六周课后练习

天天向上

极客大学架构师训练营

week2 框架设计 作业和学习总结

杨斌

盘点 Mac 上好用的七款软件

彭宏豪95

效率 效率工具 软件 Mac

架构师系列之1:UML 系统设计用例图

桃花原记

训练营第六周作业 2

仲夏

极客大学架构师训练营

架构师训练营 - 第六周总结

一个节点

极客大学架构师训练营

架构师训练营第六周命题作业

成长者

极客大学架构师训练营

如何抽取实体关系?——基于依存句法分析的事实三元组抽取

Guanngxu

自然语言处理

第六周作业

fmouse

极客大学架构师训练营

架构师训练营第二周作业1

韩儿

架构师系列之2:依赖倒置设计原则

桃花原记

Week 6 作业01

Croesus

极客大学架构师课程作业-第二周

井中人

极客大学架构师训练营

CAP原理

A p7+

架构师入门学习感悟二

莫问

怎么样让自己的博客被谷歌和百度收录!

java金融

百度 SEO 博客收录 谷歌收录

架构师训练营第 1 期 - 第 6 周课后练习

Anyou Liu

极客大学架构师训练营

Week 6 作业02

Croesus

分布式数据库,NOSQL,搜索引擎

garlic

极客大学架构师训练营

CAP原理, Doris 临时失效的处理过程

garlic

极客大学架构师训练营

2.8 第二周课后练习

lithium

极客时间 架构师训练

第二周作业

CraspLion

软件设计原则

猴子胖胖

软件设计原则

架构师训练营第 1 期 - 第 6 周学习总结

Anyou Liu

极客大学架构师训练营

第二周学习总结

lithium

极客大学 架构师训练

架构师训练营第二周作业2

韩儿

第二周设计原则

Geek_9527

java安全编码指南之:序列化Serialization

程序那些事

java安全编码 java安全 java安全编码指南 java代码规范

训练营第六周作业 1

仲夏

极客大学架构师训练营

Redis Cluster你弄明白了吗?

Man

分布式 中间件 redis cluster

盘点 C++ 开源项目中的十大 Bug-InfoQ