Geekly articles weekly

CMake：项目质量不可原谅的情况

图片1

CMake是用于自动化项目构建的跨平台系统。 该系统比PVS-Studio静态代码分析器要早得多，但是没有人试图将分析器应用于其代码并检查错误。 事实证明，其中有很多。 CMake的观众很多。 新项目开始于此，旧项目已移植。 我不禁想到有多少开发人员会遇到任何给定的错误。

引言


CMake是一个跨平台的系统，用于自动从源代码构建软件。 CMake并非直接用于构建，它仅生成文件以从CMakeLists.txt文件控制构建。 该程序的第一版发布于2000年。为了进行比较，PVS-Studio分析仪仅在2008年才出现。当时，它的目的是寻找将32位系统移植到64位系统后所产生的错误。 在2010年，出现了第一套通用诊断程序（V501- V545 ）。 顺便说一句，CMake代码从这第一组开始有一些警告。

无法原谅的错误


V1040预定义的宏名称的拼写可能有错字。 “ __MINGW32_”宏类似于“ __MINGW32__”。 winapi.h 4112

/* from winternl.h */ #if !defined(__UNICODE_STRING_DEFINED) && defined(__MINGW32_) #define __UNICODE_STRING_DEFINED #endif

V1040诊断是在不久前实施的。 最有可能的是，在发布该文章时，它仍不会发布，但是，我们已经在其帮助下发现了一个很酷的错误。

__MINGW32_名称有错字。 最后，缺少一个下划线字符。 如果使用此名称搜索代码，则可以看到在项目中使用了两侧带有两个下划线字符的版本：

图片3

V531将sizeof（）运算符乘以sizeof（）很奇怪。 cmGlobalVisualStudioGenerator.cxx 558

 bool IsVisualStudioMacrosFileRegistered(const std::string& macrosFile, const std::string& regKeyBase, std::string& nextAvailableSubKeyName) { .... if (ERROR_SUCCESS == result) { wchar_t subkeyname[256]; // <= DWORD cch_subkeyname = sizeof(subkeyname) * sizeof(subkeyname[0]); // <= wchar_t keyclass[256]; DWORD cch_keyclass = sizeof(keyclass) * sizeof(keyclass[0]); FILETIME lastWriteTime; lastWriteTime.dwHighDateTime = 0; lastWriteTime.dwLowDateTime = 0; while (ERROR_SUCCESS == RegEnumKeyExW(hkey, index, subkeyname, &cch_subkeyname, 0, keyclass, &cch_keyclass, &lastWriteTime)) { .... } .... }

对于静态声明的数组，考虑到元素数量及其大小， sizeof运算符将以字节为单位计算大小。 在评估cch_subkeyname变量的值时，开发人员没有考虑到它，并且得到的值比预期大4倍。 让我们解释一下“四次”来自何处。

数组及其错误大小将传递给函数RegEnumKeyExW ：

 LSTATUS RegEnumKeyExW( HKEY hKey, DWORD dwIndex, LPWSTR lpName, // <= subkeyname LPDWORD lpcchName, // <= cch_subkeyname LPDWORD lpReserved, LPWSTR lpClass, LPDWORD lpcchClass, PFILETIME lpftLastWriteTime );

lpcchName指针必须指向变量，包含以字符为单位的缓冲区大小：“指向变量的指针，该变量以字符为单位指定lpClass参数指定的缓冲区的大小”。 子项名称数组的大小为512个字节，可以存储256个wchar_t类型的字符（在Windows中，wchar_t为2个字节）。 应该将256传递给该函数。 取而代之的是512乘以2，我们得到1024。

我认为，现在很清楚如何纠正此错误。 您需要使用除法而不是乘法：

 DWORD cch_subkeyname = sizeof(subkeyname) / sizeof(subkeyname[0]);

顺便说一句，当评估cch_keyclass变量的值时，会发生相同的错误。

所描述的错误有可能导致缓冲区溢出。 所有这些片段肯定都必须纠正：

  • V531将sizeof（）运算符乘以sizeof（）很奇怪。 cmGlobalVisualStudioGenerator.cxx 556
  • V531将sizeof（）运算符乘以sizeof（）很奇怪。 cmGlobalVisualStudioGenerator.cxx 572
  • V531将sizeof（）运算符乘以sizeof（）很奇怪。 cmGlobalVisualStudioGenerator.cxx 621
  • V531将sizeof（）运算符乘以sizeof（）很奇怪。 cmGlobalVisualStudioGenerator.cxx 622
  • V531将sizeof（）运算符乘以sizeof（）很奇怪。 cmGlobalVisualStudioGenerator.cxx 649

V595在针对nullptr进行验证之前，已使用“ this-> BuildFileStream”指针。 检查行：133，134。cmMakefileTargetGenerator.cxx 133

 void cmMakefileTargetGenerator::CreateRuleFile() { .... this->BuildFileStream->SetCopyIfDifferent(true); if (!this->BuildFileStream) { return; } .... }

在检查其有效性之前，已先取消引用this-> BuildFileStream指针。 这不是给任何人造成任何问题吗？ 下面是此类摘要的另一个示例。 它就像复本一样制作。 但是实际上，有很多V595警告，而且大多数警告并不那么明显。 根据我的经验，我可以说纠正此诊断的警告花费的时间最长。

  • V595在针对nullptr对其进行验证之前，已使用“ this-> FlagFileStream”指针。 检查行：303、304。cmMakefileTargetGenerator.cxx 303

V614使用了未初始化的指针'str'。 cmVSSetupHelper.h 80

 class SmartBSTR { public: SmartBSTR() { str = NULL; } SmartBSTR(const SmartBSTR& src) { if (src.str != NULL) { str = ::SysAllocStringByteLen((char*)str, ::SysStringByteLen(str)); } else { str = ::SysAllocStringByteLen(NULL, 0); } } .... private: BSTR str; };

分析器检测到未初始化的str指针的使用。 它的出现是由于普通的错字。 调用SysAllocStringByteLen函数时，应该使用src.str指针。

V557阵列可能超限。 “ lensymbol”索引的值可能达到28。archive_read_support_format_rar.c 2749

 static int64_t expand(struct archive_read *a, int64_t end) { .... if ((lensymbol = read_next_symbol(a, &rar->lengthcode)) < 0) goto bad_data; if (lensymbol > (int)(sizeof(lengthbases)/sizeof(lengthbases[0]))) goto bad_data; if (lensymbol > (int)(sizeof(lengthbits)/sizeof(lengthbits[0]))) goto bad_data; len = lengthbases[lensymbol] + 2; if (lengthbits[lensymbol] > 0) { if (!rar_br_read_ahead(a, br, lengthbits[lensymbol])) goto truncated_data; len += rar_br_bits(br, lengthbits[lensymbol]); rar_br_consume(br, lengthbits[lensymbol]); } .... }

这段代码一次隐藏了几个问题。 当访问lengthbases和lengthbits数组时，数组索引可能会超出范围，因为开发人员在上面编写了'>'运算符而不是上面的'> ='。 这张支票开始漏掉一个不可接受的值。 在这里，我们只有一个经典的错误模式，称为Off-by-one Error 。

这是无效索引的数组访问操作的完整列表：

  • V557阵列可能超限。 “ lensymbol”索引的值可能达到28。archive_read_support_format_rar.c 2750
  • V557阵列可能超限。 “ lensymbol”索引的值可能达到28。archive_read_support_format_rar.c 2751
  • V557阵列可能超限。 “ lensymbol”索引的值可能达到28。archive_read_support_format_rar.c 2753
  • V557阵列可能超限。 “ lensymbol”索引的值可能达到28。archive_read_support_format_rar.c 2754
  • V557阵列可能超限。 “ offssymbol”索引的值可能达到60。archive_read_support_format_rar.c 2797

内存泄漏


V773在不释放'testRun'指针的情况下退出了该函数。 可能发生内存泄漏。 cmCTestMultiProcessHandler.cxx 193

 void cmCTestMultiProcessHandler::FinishTestProcess(cmCTestRunTest* runner, bool started) { .... delete runner; if (started) { this->StartNextTests(); } } bool cmCTestMultiProcessHandler::StartTestProcess(int test) { .... cmCTestRunTest* testRun = new cmCTestRunTest(*this); // <= .... if (testRun->StartTest(this->Completed, this->Total)) { return true; // <= } } this->FinishTestProcess(testRun, false); // <= return false; }

分析仪检测到内存泄漏。 如果函数testRun-> StartTest返回true ，则不会释放testRun指针的内存。 当执行另一个代码分支时，此内存将在功能this-> FinishTestProcess中释放。

资源泄漏


V773在没有关闭'fd'句柄引用的文件的情况下退出了该函数。 资源泄漏是可能的。 rhash.c 450

 RHASH_API int rhash_file(....) { FILE* fd; rhash ctx; int res; hash_id &= RHASH_ALL_HASHES; if (hash_id == 0) { errno = EINVAL; return -1; } if ((fd = fopen(filepath, "rb")) == NULL) return -1; if ((ctx = rhash_init(hash_id)) == NULL) return -1; // <= fclose(fd); ??? res = rhash_file_update(ctx, fd); fclose(fd); rhash_final(ctx, result); rhash_free(ctx); return res; }

条件中的奇怪逻辑


V590考虑检查'* s！='\ 0'&& * s =='''表达式。 表达式过多或打印错误。 archive_cmdline.c 76

 static ssize_t get_argument(struct archive_string *as, const char *p) { const char *s = p; archive_string_empty(as); /* Skip beginning space characters. */ while (*s != '\0' && *s == ' ') s++; .... }

* s与null 的字符比较是多余的。 while循环的条件仅取决于字符是否等于空格。 这不是错误，而是代码的不必要的复杂化。

V592该表达式用括号括起来两次：（（expression））。 不需要一对括号，否则会出现打印错误。 cmCTestTestHandler.cxx 899

 void cmCTestTestHandler::ComputeTestListForRerunFailed() { this->ExpandTestsToRunInformationForRerunFailed(); ListOfTests finalList; int cnt = 0; for (cmCTestTestProperties& tp : this->TestList) { cnt++; // if this test is not in our list of tests to run, then skip it. if ((!this->TestsToRun.empty() && std::find(this->TestsToRun.begin(), this->TestsToRun.end(), cnt) == this->TestsToRun.end())) { continue; } tp.Index = cnt; finalList.push_back(tp); } .... }

分析仪警告说，取反操作可能应该放在括号之外。 似乎这里没有这样的错误-只是不必要的双括号。 但是最有可能的是，代码中存在逻辑错误。

仅当this-> TestsToRun的测试列表不为空并且其中没有cnt时，才执行continue运算符。 可以合理地假设，如果测试列表为空，则需要执行相同的操作。 最可能的情况是，条件如下：

 if (this->TestsToRun.empty() || std::find(this->TestsToRun.begin(), this->TestsToRun.end(), cnt) == this->TestsToRun.end()) { continue; }

V592该表达式用括号括起来两次：（（expression））。 不需要一对括号，否则会出现打印错误。 cmMessageCommand.cxx 73

 bool cmMessageCommand::InitialPass(std::vector<std::string> const& args, cmExecutionStatus&) { .... } else if (*i == "DEPRECATION") { if (this->Makefile->IsOn("CMAKE_ERROR_DEPRECATED")) { fatal = true; type = MessageType::DEPRECATION_ERROR; level = cmake::LogLevel::LOG_ERROR; } else if ((!this->Makefile->IsSet("CMAKE_WARN_DEPRECATED") || this->Makefile->IsOn("CMAKE_WARN_DEPRECATED"))) { type = MessageType::DEPRECATION_WARNING; level = cmake::LogLevel::LOG_WARNING; } else { return true; } ++i; } .... }

这是一个类似的示例，但是这次我更确信会发生错误。 函数IsSet（“ CMAKE_WARN_DEPRECATED”）检查是否已全局设置值CMAKE_WARN_DEPRECATED ，函数IsOn（“ CMAKE_WARN_DEPRECATED）检查该值是否已在项目配置中设置。 补码运算符很可能是多余的，因为在两种情况下，设置相同的type和level值都是正确的。

V728过度检查可以简化。 '（（A &&！B）|| （！A && B）'表达式等同于'bool（A）！= Bool（B）'表达式。 cmCTestRunTest.cxx 151

 bool cmCTestRunTest::EndTest(size_t completed, size_t total, bool started) { .... } else if ((success && !this->TestProperties->WillFail) || (!success && this->TestProperties->WillFail)) { this->TestResult.Status = cmCTestTestHandler::COMPLETED; outputStream << " Passed "; } .... }

此代码可以更简单。 可以通过以下方式重写条件表达式：

 } else if (success != this->TestProperties->WillFail) { this->TestResult.Status = cmCTestTestHandler::COMPLETED; outputStream << " Passed "; }

还有几个地方可以简化：

  • V728过度检查可以简化。 '（A && B）|| （！A &&！B）'表达式等同于'bool（A）== bool（B）'表达式。 cmCTestTestHandler.cxx 702
  • V728过度检查可以简化。 '（（A &&！B）|| （！A && B）'表达式等同于'bool（A）！= Bool（B）'表达式。 摘要_sspi.c 443
  • V728过度检查可以简化。 '（（A &&！B）|| （！A && B）'表达式等同于'bool（A）！= Bool（B）'表达式。 tcp.c 1295
  • V728过度检查可以简化。 '（（A &&！B）|| （！A && B）'表达式等同于'bool（A）！= Bool（B）'表达式。 testDynamicLoader.cxx 58
  • V728过度检查可以简化。 '（（A &&！B）|| （！A && B）'表达式等同于'bool（A）！= Bool（B）'表达式。 testDynamicLoader.cxx 65
  • V728过度检查可以简化。 '（（A &&！B）|| （！A && B）'表达式等同于'bool（A）！= Bool（B）'表达式。 testDynamicLoader.cxx 72

各种警告


V523'then '语句等效于后续代码片段。 archive_read_support_format_ar.c 415

 static int _ar_read_header(struct archive_read *a, struct archive_entry *entry, struct ar *ar, const char *h, size_t *unconsumed) { .... /* * "__.SYMDEF" is a BSD archive symbol table. */ if (strcmp(filename, "__.SYMDEF") == 0) { archive_entry_copy_pathname(entry, filename); /* Parse the time, owner, mode, size fields. */ return (ar_parse_common_header(ar, entry, h)); } /* * Otherwise, this is a standard entry. The filename * has already been trimmed as much as possible, based * on our current knowledge of the format. */ archive_entry_copy_pathname(entry, filename); return (ar_parse_common_header(ar, entry, h)); }

最后条件中的表达式类似于函数的后两行。 开发人员可以通过消除条件来简化此代码，或者代码中有错误，应予以修复。

V535变量“ i”用于该循环和外部循环。 检查线：2220、2241。multi.c 2241

 static CURLMcode singlesocket(struct Curl_multi *multi, struct Curl_easy *data) { .... for(i = 0; (i< MAX_SOCKSPEREASYHANDLE) && // <= (curraction & (GETSOCK_READSOCK(i) | GETSOCK_WRITESOCK(i))); i++) { unsigned int action = CURL_POLL_NONE; unsigned int prevaction = 0; unsigned int comboaction; bool sincebefore = FALSE; s = socks[i]; /* get it from the hash */ entry = sh_getentry(&multi->sockhash, s); if(curraction & GETSOCK_READSOCK(i)) action |= CURL_POLL_IN; if(curraction & GETSOCK_WRITESOCK(i)) action |= CURL_POLL_OUT; actions[i] = action; if(entry) { /* check if new for this transfer */ for(i = 0; i< data->numsocks; i++) { // <= if(s == data->sockets[i]) { prevaction = data->actions[i]; sincebefore = TRUE; break; } } } .... }

i变量用作外部和内部循环中的循环计数器。 同时，计数器的值在内部循环中再次从零开始。 此处可能不是错误，但代码可疑。

V519'tagString '变量被连续两次赋值。 也许这是一个错误。 检查行：84，86。cmCPackLog.cxx 86

 void cmCPackLog::Log(int tag, const char* file, int line, const char* msg, size_t length) { .... if (tag & LOG_OUTPUT) { output = true; display = true; if (needTagString) { if (!tagString.empty()) { tagString += ","; } tagString = "VERBOSE"; } } if (tag & LOG_WARNING) { warning = true; display = true; if (needTagString) { if (!tagString.empty()) { tagString += ","; } tagString = "WARNING"; } } .... }

tagString变量在所有位置都被新值覆盖。 很难说是什么问题或为什么要这么做。 也许，'='和'+ ='运算符是混乱的。

此类场所的完整列表：

  • V519'tagString'变量被连续两次赋值。 也许这是一个错误。 检查行：94，96。cmCPackLog.cxx 96
  • V519'tagString'变量被连续两次赋值。 也许这是一个错误。 检查行：104，106。cmCPackLog.cxx 106
  • V519'tagString'变量被连续两次赋值。 也许这是一个错误。 检查行：114，116。cmCPackLog.cxx 116
  • V519'tagString'变量被连续两次赋值。 也许这是一个错误。 检查行：125、127。cmCPackLog.cxx 127

V519连续两次给' aes- > aes_set'变量赋值。 也许这是一个错误。 检查行：4052、4054。archive_string.c 4054

 int archive_mstring_copy_utf8(struct archive_mstring *aes, const char *utf8) { if (utf8 == NULL) { aes->aes_set = 0; // <= } aes->aes_set = AES_SET_UTF8; // <= .... return (int)strlen(utf8); }

强制设置AES_SET_UTF8值看起来可疑。 我认为这样的代码会使任何精炼此片段的开发人员感到困惑。

这段代码已复制到另一个地方：

  • V519连续两次给'aes-> aes_set'变量赋值。 也许这是一个错误。 检查行：4066，4068。archive_string.c 4068

如何在CMake上的项目中查找错误


在本节中，我将简要地告诉您如何使用PVS-Studio检查CMake项目就像一到二到三个一样简单。

Windows / Visual Studio

对于Visual Studio，可以使用CMake GUI或以下命令生成项目文件：

 cmake -G "Visual Studio 15 2017 Win64" ..

接下来，您可以打开.sln文件，并使用Visual Studio 插件检查项目。

Linux / macOS

文件compile_commands.json用于在这些系统上进行检查。 顺便说一下，它可以在不同的构建系统中生成。 这是您在CMake中执行的操作：

 cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=On ..

最后要做的是使用.json文件在目录中运行分析器：

 pvs-studio-analyzer analyze -l /path/to/PVS-Studio.lic -o /path/to/project.log -e /path/to/exclude-path -j<N>

我们还为CMake项目开发了一个模块。 有些人喜欢使用它。 CMake模块及其用法示例可以在我们的GitHub存储库中找到： pvs-studio-cmake-examples 。

结论


大量的CMake用户非常喜欢测试该项目，但是在发布之前，可以使用PVS-Studio之类的静态代码分析工具避免许多问题。

如果您喜欢分析器的结果，但是您的项目不是用C和C ++编写的，那么我想提醒您，分析器还支持使用C＃和Java分析项目。 您可以转到此页面在项目上测试分析仪。

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

More articles:


All Articles