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

我们的文章读者有时会注意到,PVS-Studio静态代码分析器会检测到大量无关紧要且不会影响应用程序的错误。 真的是这样。 在大多数情况下,由于手动测试,用户反馈和其他昂贵的方法,重要的错误已得到修复。 同时,可以在代码编写阶段发现许多这些错误,并以最少的时间,声誉和金钱损失进行纠正。 本文将提供一些实际错误的示例,如果项目作者使用了静态代码分析,这些错误可能会立即得到解决。


这个想法很简单。 我们将在GitHub上搜索提取请求的示例,这些请求指定问题是错误修正。 然后,我们将尝试使用PVS-Studio静态代码分析器查找这些错误。 如果分析程序可以发现错误,则说明它是在代码编写阶段发现的错误。 错误越早得到纠正,成本越便宜。

不幸的是,GitHub让我们失望了,我们没有设法发表有关该主题的豪华文章。 GitHub本身有一个故障(或功能),它不允许您在仅使用某些编程语言编写的项目中搜索请求请求的注释。 或者我不知道怎么做。 尽管我指定要在C,C ++,C#项目中搜索注释,但结果适用于所有语言,包括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; 

在旧的代码版本中,没有检查空指针。 如果发生这种情况,以便将空指针作为第二个参数传递给mc6809dis_direct函数,则其取消引用将发生在该函数的主体中。 结果是令人遗憾的和不可预测的

空指针取消引用是我们被告知的最常见的模式之一:“这不是一个严重的错误。 谁在乎它在代码中蓬勃发展? 如果发生取消引用,程序将安静地崩溃,仅此而已。” 很奇怪和悲伤从C ++程序员在听到这一点,但生活中发生。

无论如何,在这个项目中,这种取消引用已经变成了一个错误,正如提交的主题告诉我们的那样:“ 错误修复--- 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指针所引用的变量的值,但将32添加到指针本身。 指针的更改值不再使用,预期的变量值保持不变。

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位变量的值大于0xFFFFF ,则12个最高位将至少具有一个非空值。 向左移动此变量后,这些有效位将丢失,从而导致用x写入错误的信息

为了消除高位丢失,我们首先需要将a 强制转换为无符号long long类型,并且仅在此后将变量移位:

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

这样,正确的值将始终以pa写入

没关系,但是这个错误(与本文的第一个示例相同)也很关键。 它是作者在评论中写的。 而且,此错误已被大量项目发现。 为了充分理解悲剧的严重性,我建议在GitHub上搜索此错误修正时查看结果数量。 吓人,不是吗?



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

Source: https://habr.com/ru/post/zh-CN460121/


All Articles