由于未使用静态代码分析而无法找到的错误

有时，我们检查开放源代码项目的文章的读者会注意到，PVS-Studio静态代码分析器检测到很大比例的微不足道的错误或根本不影响应用程序的错误。 真的是 由于手动测试，用户审查和其他昂贵的方法，大多数重要的错误已得到修复。 同时，即使在编写代码的阶段，也可以发现许多这样的错误，并且可以在不浪费时间，声誉和金钱的情况下进行纠正。 本文将提供一些实际错误的示例，如果项目的作者使用静态代码分析，这些错误将立即得到解决。

这个想法很简单。 让我们看一下GitHub中的请求示例，这些请求的注释表明这是一个错误修复。 并尝试使用PVS-Studio静态代码分析器查找这些错误。 如果分析仪找到了纠正的错误，则意味着即使在编写代码的阶段也可以纠正该错误。 错误越早得到修复，价格越便宜。

不幸的是，GitHub让我们失望了，并且不允许在这个话题上写出色的文章。 GitHub本身也有一个故障（或功能），它不允许您在仅以某些编程语言编写的项目中搜索请求请求中的注释。 好吧，否则我不知道如何烹饪。 无论我告诉您如何在C ++，C＃或Java项目中查找注释，结果都将以所有语言显示，包括PHP，Python，JavaScript等。 结果，寻找合适的案例是一个非常繁琐的任务，我将仅举几个例子。 但是，它们足以证明定期使用静态代码分析工具的有用性。

如果错误被早期发现怎么办？ 答案很简单：程序员不必等待其表现，然后寻找并修复有缺陷的代码。
让我们看一下PVS-Studio可以立即检测到的错误：

第一个示例来自SatisfactoryModLoader项目。 在更正错误之前，代码如下所示：

// gets an API function from the mod handler SML_API PVOID getAPIFunction(std::string name) { bool found = false; for (Registry reg : modHandler.APIRegistry) { if (reg.name == name) { found = true; } } if (!found) { std::string msg = ...; MessageBoxA(NULL, msg.c_str(), "SatisfactoryModLoader Fatal Error", MB_ICONERROR); abort(); } } 

此代码包含一个错误，PVS-Studio将立即发出警告：

V591非无效函数应返回一个值。 ModFunctions.cpp 44

上面的函数没有return ，因此它将返回一个形式上不确定的值。 程序员没有使用代码分析器，因此他必须自己寻找错误。 编辑后的功能：

 // gets an API function from the mod handler SML_API PVOID getAPIFunction(std::string name) { bool found = false; PVOID func = NULL; for (Registry reg : modHandler.APIRegistry) { if (reg.name == name) { func = reg.func; found = true; } } if (!found) { std::string msg = ...; MessageBoxA(NULL, msg.c_str(), "SatisfactoryModLoader Fatal Error", MB_ICONERROR); abort(); } return func; } 

奇怪的是，在提交中，作者指出该错误为严重错误：“ 修复了 不返回API函数的 严重错误 ”。

在mc6809项目历史记录的第二次提交中，对以下代码进行了更正：

 void mc6809dis_direct( mc6809dis__t *const dis, mc6809__t *const cpu, const char *const op, const bool b16 ) { assert(dis != NULL); assert(op != NULL); addr.b[MSB] = cpu->dp; addr.b[LSB] = (*dis->read)(dis, dis->next++); ... if (cpu != NULL) { ... } } 

作者只纠正了一行。 他取代了表达

 addr.b[MSB] = cpu->dp; 

表达

 addr.b[MSB] = cpu != NULL ? cpu->dp : 0; 

在旧版本的代码中，没有执行cpu检查是否等于空指针。 如果突然发现将空指针作为第二个参数传递给函数mc6809dis_direct ，则它将在函数主体中取消引用。 结果是令人遗憾的和不可预测的 。

空指针解引用是我们被告知的最常见的模式之一：“这不是严重错误。 好吧，她生活在代码中并活在当下，如果发生取消引用，程序将安静地崩溃，仅此而已。” 奇怪和悲伤而从C ++程序员听到这一点，但生活就是生活。

无论如何，在这个项目中，这种取消引用仍然变成了错误，因为commit标头告诉我们：“ 错误修复--- NULL取消引用 ”。

如果项目开发人员使用PVS-Studio，那么他将能够在两个半月前检查他的代码并检测到一条警告（这是从发生错误以来经过了多少时间）：

V595在对nullptr进行验证之前，已使用了'cpu'指针。 检查线：1814、1821。mc6809dis.c 1814

因此，即使在错误出现时也可以消除该错误，这将节省时间，并可能节省显影剂的时间：)。

libmorton项目中发现了另一个有趣的编辑示例。

更正的代码：

 template<typename morton> inline bool findFirstSetBitZeroIdx(const morton x, unsigned long* firstbit_location) { #if _MSC_VER && !_WIN64 // 32 BIT on 32 BIT if (sizeof(morton) <= 4) { return _BitScanReverse(firstbit_location, x) != 0; } // 64 BIT on 32 BIT else { *firstbit_location = 0; if (_BitScanReverse(firstbit_location, (x >> 32))) { // check first part firstbit_location += 32; return true; } return _BitScanReverse(firstbit_location, (x & 0xFFFFFFFF)) != 0; } #elif _MSC_VER && _WIN64 .... #elif __GNUC__ .... #endif } 

在编辑过程中，程序员将表达式“ firstbit_location + = 32 ”替换为“ * firstbit_location + = 32 ”。 程序员希望将数字32添加到firstbit_location指针绑定到的变量的值上，但是将其直接添加到该指针上。 指针的更改值未在其他任何地方使用，并且变量的期望值保持不变。

PVS-Studio会对此代码发出警告：

V1001分配了“ firstbit_location”变量，但未在函数末尾使用。 morton_common.h 22

在经过修改但又未使用的表达式中，似乎有什么可怕的呢？ V1001的诊断显然似乎并不旨在识别特别危险的错误。 尽管如此，她还是发现了影响程序逻辑的重要错误。

而且，事实证明，发现此错误并非易事！ 从创建文件的那一刻起，她不仅在程序中，而且还在相邻行中的许多编辑中幸存下来，并且在项目中存在了长达3（ ！ ）年！ 一直以来，程序逻辑都被破坏了，并且不能完全按照开发人员的预期工作。 如果他们使用PVS-Studio，则错误会更早被发现。

最后，考虑另一个美丽的例子。 在GitHub上收集错误修复程序时，我多次遇到了有关此内容的修复程序。 该错误已在此处修复：

 int kvm_arch_prepare_memory_region(...) { ... do { struct vm_area_struct *vma = find_vma(current->mm, hva); hva_t vm_start, vm_end; ... if (vma->vm_flags & VM_PFNMAP) { ... phys_addr_t pa = (vma->vm_pgoff << PAGE_SHIFT) + vm_start - vma->vm_start; ... } ... } while (hva < reg_end); ... } 

PVS-Studio对此代码部分发出警告：

V629考虑检查'vma-> vm_pgoff << 12'表达式。 32位值的位移，随后扩展为64位类型。 1795年

我查看了表达式“ phys_addr_t pa =（vma-> vm_pgoff << PAGE_SHIFT）+ vm_start-vma-> vm_start; ”中使用的变量声明，发现上面的代码类似于以下合成示例：

 void foo(unsigned long a, unsigned long b) { unsigned long long x = (a << 12) + b; } 

如果32位变量a的值大于0xFFFFF ，则12个最高有效位将至少具有一个非零值。 将左移操作应用于此变量后，这些有效位将丢失，其结果是将错误的信息写入x 。

要消除高位丢失，必须首先将a 强制转换为unsigned long long类型，然后再执行shift操作：

 pa = (phys_addr_t)vma->vm_pgoff << PAGE_SHIFT; pa += vm_start - vma->vm_start; 

这样， pa将始终写入正确的值。

一切都会好起来的，但是这个错误，就像文章中的第一个示例一样，也变得至关重要，因为该提交的作者在评论中单独写道。 而且，此错误仅涉及大量项目。 为了充分理解悲剧的严重性，我建议在GitHub上搜索此错误修正时查看结果数量。 吓人，不是吗？

因此，我采用了一种新方法来演示定期使用静态代码分析器的有用性。 希望您喜欢。 下载并尝试使用PVS-Studio静态代码分析器来测试您自己的项目。 在撰写本文时，它已经实施了大约700条诊断规则，用于检测各种错误模式。 支持C，C ++，C＃和Java。

如果您想与讲英语的读者分享这篇文章，请使用翻译链接：George Gribkov。 由于未使用静态代码分析而无法找到的错误