2017年6月26日

开发流程中的静态分析

作者 Andrey Karpov, PVS-Studio

PVS-Studio 团队的 Ilya Ivanov 和 Sergei Vasilyev 提供了一些代码段。

虚幻引擎持续开发,从而不断地添加新代码,更改先前编写的代码。随着项目的持续开发,会有怎样不可避免的后果呢?代码中出现新的错误,而程序员希望能够尽可能早发现这些错误。减少错误的一种方法是使用 PVS-Studio 这样的静态分析工具。此外,该分析工具不仅在不断演变,还持续地学习寻找新错误模式,我们将在本文中进行相关探讨。如果您注重代码质量,本文正合你意。

静态代码分析,理论参考

静态代码分析是一种检测程序源代码中的错误和缺陷的过程。可以将其视为一种自动化代码复查过程。

代码复查是一种最原始最有用的缺陷检测方法。包括联读源代码和提供改进建议。该过程帮助检测错误或将来可能错误的代码片段。此外,还有一条不成文的规定,即对于某部分程序的运行原理,代码作者不应做任何解释。通过查看程序文本和代码注释,算法应该清晰明了。如果不是这样,应修改代码。

代码复查通常效果不错,因为程序员会更容易察觉到别人代码中的错误,而面对自己的代码则不然。您可以在 Steve McConnell 的经典著作《Code Complete》中查找有关代码复查方法的更多细节。

代码复查方法有两个缺点:

  1. 成本极高。需要抽调几个程序员搁置自己的主要任务,复查新编写的代码,或者执行建议修改后重新编写的代码。与此同时,程序员在工作期间还应定期休息。如果某人尝试查看大量代码段,很容易会失去注意力,而达不到复查的目的。

  2. 很难检测到与新代码/修改后代码有直接关系的错误。在查看全新的代码段时,很难假设 malloc 函数不能正常工作,因为没有包含头文件 stdlib.h。您可以在“A nice 64-bit error in C”一文中查找有关该情境的更多详情。再举一个例子:更改头文件中的函数类型或变量。理想情况下,程序员应该在进行这些更改后,复查使用该函数或变量的所有代码。实际上,这样太过耗时,复查仅限于程序员有改动的这些代码段。

一方面希望能够定期执行代码复查。另一方面成本又太过高昂。权衡之下,静态分析成为折中之选。静态分析工具检查程序的源文本,并建议程序员复查特定代码段。分析工具不会疲累,可以检查头文件更改所影响的所有代码。当然,程序并不能取代由开发者团队执行的彻底的代码复查。但是,性价比让静态分析成为一种十分有用的方法,被许多公司所采用。

与任何其他错误检测方法相比,静态分析也有其长处和短板。没有任何一种理想的程序测试方法,结合使用多种方法可以实现最佳结果,例如:好的编码风格、静态代码分析、动态代码分析、单元测试、回归测试等等。

静态分析的一个重要优势是能够在代码中出现大量错误后立刻检测到,因此修复这些错误的成本也不会过高。越早检测到错误,更正错误的成本越低。因此,根据 McConnell 所写的《Code Complete》一书,在测试代码阶段更正错误的成本要超过编写代码阶段的十倍:

更正缺陷的平均成本取决于检测到缺陷的时间(表中显示的数据取自 S. McConnell 所写的《Code Complete》一书)

静态分析工具能够检测到大量错误,通常是在最初编写任何给定代码期间,这样能够显著降低整个项目的开发成本。

静态分析工具的流程程度随着时间的推移会逐渐提高,因为现代应用程序的代码基在不断增长。程序会变得越来越大,越来越复杂。同时,错误的密度与代码大小呈非线性相关。

项目越大,其所含的每 1000 行代码的错误数越多。让我们来看一下这个图表:

项目大小和典型错误密度。资料来源:“Program Quality and Programmer Productivity”(Jones,1977);“Estimating Software Costs”(Jones,1998)。

下图更直观地显示了该数据。

项目中的典型错误密度。蓝色 - 最大数量。红色 - 平均数量。绿色 - 最小错误数。

该图显示了随着项目的增长,程序员被迫使用更多工具来保持所要求的代码质量。现在就无法用八年前的方式创建高质量的代码。这对于团队来说可不是一个喜闻乐见的发现:似乎他们还是正常地编写代码,但代码状况却变差了。

需要探索新方法和工具,因为旧方法可能已经不适合于不断发展和变化的技术。最有用且值得使用的一种方法是静态分析。

如果读者对静态分析方法还不熟悉,希望诸位阅读本文后能够对此提起兴趣。以下是一些建议阅读的链接,可以为读者提供更多详情:

  1. John Carmack 编写的 Static Code Analysis

  2. Wikipedia 上的 Static Code Analysis

  3. Wikipedia 上的 List of tools for static code analysis

  4. A Few Billion Lines of Code Later:Using Static Analysis to Find Bugs in the Real World,联合作者 Al Bessey、Ken Block、Ben Chelf、Andy Chou、Bryan Fulton、Seth Hallem、Charles Henri-Gros、Asya Kamsky、Scott McPeak 和 Dawson Engler。

  5. Ekaterina Milovidova 提供的 Videos about static code analysis

  6. PVS-Studio 博客

现在该把理论运用到实践中了,看看静态分析如何帮助虚幻引擎 4 这样的项目。

虚幻引擎

我们的团队再一次荣幸地使用虚幻引擎的代码!虽然两年前曾实践过,但那时有很多代码编辑和改进工作要做。两年后再审视项目的代码基总是非常有用又有趣。主要有以下几个原因。

首先,我们很期待看到分析工具出现的误报。这样有助于我们改进工具,减少不必要的消息数量。抵制误报对于任何一个代码分析工具的开发者而言都是一项长远工作。对于愿意阅读更多细节的读者,建议阅读这篇文章“The way static analyzers fight against false positives, and why they do it”。

虚幻引擎 4 的代码基这两年以来发生了明显的变化。添加了一些代码段,也删除了一些代码段,有时甚至整个文件夹都消失了。这也是为什么并不是所有代码部分都得到了足够多的关注,意味着留给 PVS-Studio 的工作量是巨大的。

我在此想要称赞一下 Epic Games 如此细心地维护代码和使用 PVS-Studio 这样的工具。读者可能会笑道:“你们团队当然应该称赞 Epic Games,因为他们是你们的客户”。坦白说,我们当然有动机对 Epic Games 的开发人员持有正面的反馈。但是,我的称赞也绝对是发自肺腑。这家公司使用静态工具,不仅反映出了项目开发周期的成熟,还给予了足够多的关注,确保代码的可靠性和安全。

我为什么敢肯定,使用 PVS-Studio 能够极大地改善代码质量呢?因为这是最强大的静态分析工具,即使在以下类型的项目中,也最容易检测到错误:

使用 PVS-Studio 能够让代码质量更上一层楼。Epic Games 通过这种方法,反映出了他们也很关心所有使用虚幻引擎 4 开发自己项目的用户。每检测出一个错误,都能减少某一个人的麻烦。

有趣的错误

我并不会讨论所发现和解决的所有错误。只会重点介绍一些我印象深刻的值得关注的错误。感兴趣的用户可以在 Github 上的 pull request 上查看其他错误。要访问源代码和指定的拉取请求,必须能够访问 GitHub 上的虚幻引擎存储库。该博客概述了如何获取访问权,适用于尚未获得访问权的读者。

PVS-Studio 分析工具的开发不仅仅是创建新诊断,还会对现有诊断进行改进。例如,评估变量可能值的算法始终在改进。因此,该分析工具一年前多以前就能够检测这种类型的错误:

uint8* Data = (uint8*)PointerVal; if (Data != nullptr || DataLen == 0) { NUTDebug::LogHexDump(Data, DataLen); } else if (Data == nullptr) { Ar.Logf(TEXT("Invalid Data parameter.")); } else // if (DataLen == 0) { Ar.Logf(TEXT("Invalid DataLen parameter.")); }

PVS-Studio 警告:V547 Expression 'Data == nullptr' is always true. unittestmanager.cpp 1924

如果条件 (Data != nullptr || DataLen == 0) 不是 true,意味着指针 Data 肯定等于 nullptr。因此,进一步检查 (Data == nullptr) 没有任何意义。

该代码的正确变体:

if (Data != nullptr && DataLen > 0)

诊断 V547 是在 2010 年编写的。但是评估变量值的机制并不完美,它不允许查找此错误。分析工具对检查变量值 DataLen 感到迷惑。它无法确定变量值在各种条件下是否相等。对于分析此类代码的人来说,这可能并不是问题,但编写算法来查找此类错误就不那么简单了。

这是对 PVS-Studio 内部机制改进的演示,帮助检测新错误。这些是内部改进,通过这些改进,分析工具开始能够更准确地工作了。

我们也进行了一些“外部”改进,支持以新版本 C++ 语言编写的新构造。但是只是学习解析 C++11、C++14 等等总是不够的。优化旧诊断和落实新诊断,查找新语言构造中的错误,同样很重要。例如,诊断 V714 查找基于错误范围的循环。在虚幻引擎中,V714 诊断指向以下循环:

for (TSharedPtr<SWidget> SlateWidget :SlateWidgets) { SlateWidget = nullptr; }

PVS-Studio 警告:V714 Variable is not passed into for-each loop by a reference, but its value is changed inside of the loop. vreditorradialfloatingui.cpp 170

程序员想要将值 nullptr 指定给容器 SlateWidgets 中的所有元素。错误是 SlateWidget 是在循环的每个新迭代中都会创建的常用局部变量。为该变量指定值并不会导致容器中的元素发生更改。我们应使用一个引用,以便代码正确工作:

for (TSharedPtr<SWidget> &SlateWidget :SlateWidgets) { SlateWidget = nullptr; }

当然,我们也会添加与语言无关的诊断。例如,2015 年我们团队编写上一篇关于虚幻引擎 4 的文章时还没有诊断 V767。该诊断出现于 PVS-Studio V6.07(2016 年 8 月 8 日)。正因为有了这个诊断,我们才能检测到这样的错误:

for(int i = 0; i < SelectedObjects.Num(); ++i) { UObject* Obj = SelectedObjects[0].Get(); EdObj = Cast<UEditorSkeletonNotifyObj>(Obj); if(EdObj) { break; } }

PVS-Studio 警告:V767 Suspicious access to element of 'SelectedObjects' array by a constant index inside a loop. skeletonnotifydetails.cpp 38

该循环不应包含对 UEditorSkeletonNotifyObj 类型的元素的搜索。由于输入错误,在元素选择期间写入了数字文字 0 代替 i 变量。

该代码的正确变体:

UObject* Obj = SelectedObjects[i].Get();

让我们来看下另外一个诊断 V763,该诊断也是在 PVS-Studio 6.07 中提供的。该错误十分有意思,但必须要引用一段非常长的 RunTest 函数:

bool FCreateBPTemplateProjectAutomationTests::RunTest( const FString& Parameters) { TSharedPtr<SNewProjectWizard> NewProjectWizard; NewProjectWizard = SNew(SNewProjectWizard); TMap<FName, TArray<TSharedPtr<FTemplateItem>> >& Templates = NewProjectWizard->FindTemplateProjects(); int32 OutMatchedProjectsDesk = 0; int32 OutCreatedProjectsDesk = 0; GameProjectAutomationUtils::CreateProjectSet(Templates, EHardwareClass::Desktop, EGraphicsPreset::Maximum, EContentSourceCategory::BlueprintFeature, false, OutMatchedProjectsDesk, OutCreatedProjectsDesk); int32 OutMatchedProjectsMob = 0; int32 OutCreatedProjectsMob = 0; GameProjectAutomationUtils::CreateProjectSet(Templates, EHardwareClass::Mobile, EGraphicsPreset::Maximum, EContentSourceCategory::BlueprintFeature, false, OutMatchedProjectsMob, OutCreatedProjectsMob); return ( OutMatchedProjectsDesk == OutCreatedProjectsDesk ) && ( OutMatchedProjectsMob == OutCreatedProjectsMob ); }

下面是最重要的部分:

  • 程序员尝试通过第一次调用 CreateProjectSet 函数来初始化变量 OutMatchedProjectsDesk 和 OutCreatedProjectsDesk。

  • 通过第二次调用 CreateProjectSet 函数,尝试初始化变量 OutMatchedProjectsMob 和 OutCreatedProjectsMob。

然后检查这些变量的值是否符合条件:

return ( OutMatchedProjectsDesk == OutCreatedProjectsDesk ) && ( OutMatchedProjectsMob == OutCreatedProjectsMob );

请不要在审查后的函数主体中查找错误;这里并不存在错误。我提供这段代码是为了显示函数 CreateProjectSet 应该向作为最后两个实际参数传递的两个变量写入值

错误潜藏在函数 CreateProjectSet 内部:

static void CreateProjectSet(.... int32 OutCreatedProjects, int32 OutMatchedProjects) { ....OutCreatedProjects = 0; OutMatchedProjects = 0; ....OutMatchedProjects++; ....OutCreatedProjects++; ....}

PVS-Studio 将在这里发出两个警告:

  • V763 Parameter 'OutCreatedProjects' is always rewritten in function body before being used. gameprojectautomationtests.cpp 88

  • V763 Parameter 'OutMatchedProjects' is always rewritten in function body before being used. gameprojectautomationtests.cpp 89

该分析工具确实应该提醒,无论怎样都不会使用参数 OutCreatedProjects 和 OutMatchedProjects 的值,而是立即写入 0。

错误很简单:程序员忘记通过引用传递参数。该代码的正确变体:

static void CreateProjectSet(.... int32 &OutCreatedProjects, int32 &OutMatchedProjects)

我提供了以下至少需要注意检测的错误。但是,还有很多简单平常的错误。例如,缺少 break 语句:

{ case EWidgetBlendMode::Opaque:ActualBackgroundColor.A = 1.0f; case EWidgetBlendMode::Masked:ActualBackgroundColor.A = 0.0f; }

或者等式几个变量的比较不正确:

checkf(GPixelFormats[PixelFormat].BlockSizeX    == GPixelFormats[PixelFormat].BlockSizeY    == GPixelFormats[PixelFormat].BlockSizeZ    == 1, TEXT("Tried to use compressed format?"));

如果有读者是刚开始接触 C++,不明白这个比较为什么是错误的,我建议查看 V709 诊断描述。

这些错误是 PVS-Studio 检测到最多次的错误。但是,它们看起来这么简单,为什么仍然不会被发现呢?如果在本文中加以强调,又似乎很微不足道。但是,在实际应用程序中确实很难发现。即使在代码复查期间,某程序员看到下面的代码块也可能看不到任何错误。

{ case EWidgetBlendMode::Opaque:ActualBackgroundColor.A = 1.0f; case EWidgetBlendMode::Masked:ActualBackgroundColor.A = 0.0f; }

该代码看起来很简单,程序员甚至都不会试着认真阅读,认为是完全正确的。

现在我们来讨论一个问题:有办法减少错误吗?

建议

使用 PVS-Studio 可以找到本文描述的错误,读者很可能想到,我会建议使用静态分析工具。是的,我确实建议在开发过程中集成 PVS-Studio 静态分析工具。没有理由拒绝在编写代码后立即发现一些错误的可能性。但是,我希望讨论一个非常重要的事实,一个在代码质量相关的文章中通常不会提及的事实。

如果一个程序员团队都不肯承认自己出错,甚至有时是非常简单的错误,那么是不可能成就高质量项目的。这句话似乎很微不足道,但非常重要。如果程序员没有意识到这句话指的并不是一个抽象的程序员,而是与他们个人相关,那么任何工具和方法都没有用。换言之,程序员通常太要面子而不肯承认自己需要其他工具和方法来编写高质量的代码。

所有程序员都知道,所有程序中都会存在错误。但他们仍认为那些规则、建议和工具与自己无关,因为他们是编写无错误代码的伟大的专业开发者。这是过高估计自身水平的问题。“The Problem With ‘Above Average Programmers’”一文很好地说明了这种效应。我摘录了一句话:

“你如何评价自己的编程技能?(低于平均水平、平均水平或高于平均水平)?”

根据不同群体的心里研究,约有 90% 的程序员都会回答“高于平均水平”。显然这不可能是事实。在 100 人的组中,50 人高于平均水平,50 人低于平均水平。这种效应称为虚幻优越感。通过多个角度描述了这种效应,但如果你都没听过这种效应,那你也很有可能会回答“高于平均水平”。

这个问题会阻碍程序员学习新的技术和方法。我的主要建议是,试着重新思量对团队和个人成果的态度。“我/我们编写的代码最好”的态度只会适得其反。人非圣贤孰能无过,程序员也同样如此。想通了这一点,就朝着高质量软件的方向迈进了一大步。同事建议项目经理阅读此文

我还想要提醒大家不要误解。静态和动态分析工具主要检测简单的错误和输入错误。它们并不能检测高级逻辑错误,因为尚未发明出人工智能。但是,简单错误也可能造成巨大的损害,需要投入大量的时间/金钱/精力来修补。您可以阅读下文了解更多相关信息:“If the coding bug is banal, it doesn’t mean it’s not crucial”。

还有:不要试图寻找万能的解决方案。结合使用多种要素,例如:

  • 忘记“我们团队高于平均水平”。

  • 制定编码标准,分享给所有团队开发者。

  • 执行代码复查(至少复查最重要的代码段和初级程序员编写的代码)。

  • 静态代码分析。

  • 动态代码分析。

  • 回归测试、基本测试。

  • 使用单元测试,TDD。

  • 等等。

不建议马上就开始使用以上所列的所有方法。在不同的项目中,某些方法可能更有用,某些方法作用较小。重点是不要寄希望于单一工具或方法,而是合理地结合使用不同方法。只有这样才能改善代码的质量和可靠性。

总结

虚幻引擎 4 开发者注重代码的质量,PVS-Studio 团队尽最大努力帮助他们达到目的。

PVS-Studio 团队也准备好处理诸位的项目代码。除了提供工具许可和进一步支持之外,我们还执行代码审计、代码迁移等等

祝愿诸位的程序错误越来越少。