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

静态分析错误定位

The idea is very simple. We'll search for examples of pull requests on GitHub that specify that an issue is a bugfix. Then we'll try to find these bugs using the PVS-Studio static code analyzer. If an error could be found by the analyzer, then it is a bug which could have been found at the code writing stage. The earlier the bug is corrected, the cheaper it costs.

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

Unfortunately, GitHub let us down and we didn't manage to make a big posh article on the subject. GitHub itself has a glitch (or a feature) that doesn't allow you to search for comments of pull requests in projects written only in certain programming languages. Or I don't know how to cook it. Despite that I specify to search for comments in C, C++, C# projects, the results are given for all languages, including PHP, Python, JavaScript, and others. As a result, looking for suitable cases has proved to be extremely tedious, and I'll go for just a few examples. However, they are enough to demonstrate the usefulness of static code analysis tools when used regularly.

不幸的是，GitHub让我们失望了，我们没有设法发表有关该主题的豪华文章。 GitHub本身有一个故障(或功能)，不允许您在仅以某些编程语言编写的项目中搜索请求请求的注释。 或者我不知道怎么做。 尽管我指定要在C，C ++，C＃项目中搜索注释，但是针对所有语言(包括PHP，Python，JavaScript和其他语言)都给出了结果。 结果，寻找合适的案例非常繁琐，我仅举几个例子。 但是，它们足以证明定期使用静态代码分析工具的有用性。

What if the bug had been caught at the earliest stage? The answer is simple: programmers wouldn't have to wait for it to show itself, then search and correct the defective code.

如果该漏洞最早被发现怎么办？ 答案很简单：程序员不必等待它显示出来，然后搜索并纠正有缺陷的代码。

Let's look at the errors that PVS-Studio could have immediately detected:

让我们看一下PVS-Studio可能立即检测到的错误：

The first example is taken from the SatisfactoryModLoader project. Before fixing the error, the code looked as follows:

第一个示例来自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();
  }
}

This code contained an error, that PVS-Studio would immediately issue a warning to:

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

V591 Non-void function should return a value. ModFunctions.cpp 44 V591非无效函数应返回一个值。 ModFunctions.cpp 44

The above function has no return statement, so it will return a formally undefined value. The programmer didn't use the code analyzer, so he had to look for the bug on his own. The function after editing:

上面的函数没有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;
}

Curiously, in the commit, the author marked the bug as critical: "fixed

奇怪的是，在提交，笔者标志着错误的关键：“ 固定

严重错误 (critical bug)

where API functions were not returned". 没有返回API函数的位置 。”。

In the second commit from the mc6809 project history, edits were introduced in the following code:

在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)
  {
    ...
  }
}

The author corrected only one line. He replaced the expression

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

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

for the following one

对于以下之一

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

In the old code version there was not any check for a null pointer. If it happens so that a null pointer is passed to the mc6809dis_direct function as the second argument, its dereference will occur in the body of the function. The result is deplorable and unpredictable.

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

Null pointer dereference is one of the most common patterns we are told about: «It's not a critical bug. Who cares that it is thriving in code? If dereference occurs, the program will quietly crash and that's it.» It's strange and sad to hear this from C++ programmers, but life happens.

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

Anyway, in this project such dereference has turned into a bug, as the commit's subject tells us: "Bug fix---NULL dereference".

无论如何，在这个项目中，这种取消引用已经变成了一个错误，正如提交的主题告诉我们的那样：“ 错误修复--- NULL取消引用 ”。

If the project developer had used PVS-Studio, he could have checked and found the warning two and a half months ago. This is when the bug was introduced. Here is the warning:

如果项目开发人员使用过PVS-Studio，则可以在两个半月前检查并找到警告。 这是引入错误的时间。 这是警告：

V595 The 'cpu' pointer was utilized before it was verified against nullptr. Check lines: 1814, 1821. mc6809dis.c 1814 V595在针对nullptr对其进行验证之前，已使用了“ cpu”指针。 检查线：1814、1821。mc6809dis.c 1814

Thus, the bug would have been fixed at the time of its appearance, which would have saved the developer's time and nerves :).

因此，该错误将在其出现时得到修复，这将节省开发人员的时间和精力：)。

An example of another interesting fix was found in the libmorton project.

libmorton项目中找到了另一个有趣的修复程序的示例。

Code to be fixed:

固定代码：

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
}

In his edit, a programmer replaces the expression "firstbit_location += 32" with "

在他的编辑中，程序员将表达式“ firstbit_location + = 32 ”替换为“

* (*)

firstbit_location += 32". The programmer expected that 32 will be added to the value of the variable referred to by the firstbit_location + = 32 “。程序员期望将32添加到 firstbit_location pointer, but 32 was added to the pointer itself. The changed value of the pointer wasn't used anywhere any more and the expected variable value remained unchanged. firstbit_location指针所引用的变量的值，但将32添加到指针本身。指针的更改值不再在任何地方使用，并且预期变量值保持不变。

PVS-Studio would issue a warning to this code:

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

V1001 The 'firstbit_location' variable is assigned but is not used by the end of the function. morton_common.h 22 V1001分配了“ firstbit_location”变量，但未在函数末尾使用。 morton_common.h 22

Well, what is so bad about the modified but further unused expression? The V1001 diagnostic doesn't look like it's meant for detecting particularly dangerous bugs. Despite this, it found an important error that influenced the program logic.

那么，修改后又未使用的表达式到底有什么不好呢？ V1001诊断程序看起来并不像用于检测特别危险的错误。 尽管如此，它还是发现了影响程序逻辑的重要错误。

Moreover, it turned out that that error wasn't so easy to find! Not only has it been in the program since the file was created, but it has also experienced many edits in neighboring lines and existed in the project for as many as 3 (!) years! All this time the logic of the program was broken, and it didn't work in the way developers expected. If they had used PVS-Studio, the bug would have been detected much earlier.

而且，事实证明，发现该错误并非易事！ 自文件创建以来，它不仅存在于程序中，而且还经历了许多相邻行的编辑，并且在项目中存在长达3(！)年！ 一直以来，程序的逻辑都被破坏了，而且它并没有像开发人员所期望的那样起作用。 如果他们使用过PVS-Studio，则可能会更早发现该错误。

In the end, let's look at another nice example. While I was collecting bug fixes on GitHub, I came across a fix with the following content several times. The fixed error was here:

最后，让我们看另一个好例子。 在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 issued a warning for this code snippet:

PVS-Studio针对此代码段发出了警告：

V629 Consider inspecting the 'vma->vm_pgoff << 12' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. mmu.c 1795 V629考虑检查'vma-> vm_pgoff << 12'表达式。 32位值的位移，随后扩展为64位类型。 1795年

I checked out declarations of variables, used in the expression "phys_addr_t pa = (vma->vm_pgoff << PAGE_SHIFT) + vm_start — vma->vm_start;" and found out that the code given above is equal to the following synthetic example:

我检查了在表达式“ 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;
}

If the value of the a 32-bit variable is greater than 0xFFFFF, 12 highest bits will have at least one nonnull value. After shifting this variable left, these significant bits will be lost, resulting in incorrect information written in x.

如果一个 32位的变量的值大于较大的0xFFFFF，12最高位将具有至少一个非空值。 向左移动此变量后，这些有效位将丢失，从而导致用x写入错误的信息。

To eliminate loss of high bits, we need first to cast a to the unsigned long long type and only after this shift the variable:

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

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

This way, a correct value will always be written in pa.

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

That'd be okay but this bug, the same as the first example from the article, also turned out to be critical. It's author wrote about it in the comment. Moreover, this error found its way to an enormous number of projects. To fully appreciate the scale of the tragedy, I suggest looking at the number of results when searching for this bugfix on GitHub. Scary, isn't it?

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

So I've taken a new approach to demonstrate the benefits of a regular static code analyzer usage. I hope you enjoyed it. Download and try the PVS-Studio static code analyzer to check your own projects. At the time of writing, it has about 700 implemented diagnostic rules to detect a variety of error patterns. Supports C, C++, C# and Java.

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

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

静态分析错误定位