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 تم استخدام مؤشر 'this-> BuildFileStream' قبل أن يتم التحقق منه مقابل nullptr. خطوط التحقق: 133 ، 134. cmMakefileTargetGenerator.cxx 133

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

المؤشر هذا-> تم إلغاء تحديد BuildFileStream مباشرة قبل التحقق من صلاحيته. لم هذا يسبب أي مشاكل لأحد؟ يوجد أدناه مثال آخر على هذا المقتطف. انها مصنوعة تماما مثل نسخة كربونية. ولكن في الواقع ، هناك الكثير من التحذيرات V595 ومعظمها ليست واضحة جدا. من تجربتي ، أستطيع أن أقول إن تصحيح التحذيرات من هذا التشخيص يستغرق أطول وقت.

  • V595 تم استخدام مؤشر 'this-> FlagFileStream' قبل أن يتم التحقق منه مقابل nullptr. خطوط التحقق: 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]); } .... }

يخفي هذا الجزء من التعليمات البرمجية عدة مشاكل في وقت واحد. عند الوصول إلى صفيفات الطول والطول ، قد ينفد فهرس الصفيف ، حيث كتب المطورون عامل التشغيل ">" بدلاً من "> =" أعلاه. بدأ هذا الاختيار يفوتك قيمة غير مقبولة. هنا ليس لدينا سوى نمط خطأ كلاسيكي يسمى 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 ، إذا كانت الدالة testRun-> StartTest ترجع إلى حقيقة . عند تنفيذ فرع رمز آخر ، يتم تحرير هذه الذاكرة في الوظيفة 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++; .... }

* حرف مقارنة مع فارغة لا لزوم لها. يعتمد شرط حلقة الاثناء فقط على ما اذا كان الحرف يساوي مساحة أم لا. هذا ليس خطأ ، ولكن تعقيد غير ضروري من التعليمات البرمجية.

V592 تم تضمين التعبير بأقواس مرتين: ((تعبير)). زوج واحد من الأقواس غير ضروري أو وجود خطأ مطبعي. 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); } .... }

يحذر المحلل من أن عملية النفي ربما يجب إخراجها من الأقواس. يبدو أنه لا يوجد مثل هذا الخطأ هنا - فقط الأقواس المزدوجة غير الضرورية. لكن على الأرجح ، يوجد خطأ منطقي في الكود.

يتم تنفيذ عامل التشغيل المستمر فقط في حالة إذا كانت قائمة الاختبارات هذا> TestsToRun غير فارغة وغياب cnt فيها. من المنطقي افتراض أنه إذا كانت قائمة الاختبارات فارغة ، فيجب إجراء نفس الإجراء. على الأرجح ، يجب أن تكون الحالة كما يلي:

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

V592 تم تضمين التعبير بأقواس مرتين: ((تعبير)). زوج واحد من الأقواس غير ضروري أو وجود خطأ مطبعي. 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) تتحقق من أن القيمة تم تعيينها في تكوين المشروع. على الأرجح ، يكون العامل التكميلي متكررًا ، كما هو الحال في كلتا الحالتين ، من الصحيح تعيين نفس قيم النوع والمستوى.

V728 يمكن تبسيط عملية التحقق المفرطة. The '(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 يمكن تبسيط عملية التحقق المفرطة. The '(A && B) || تعبير (! A &&! B) 'يعادل التعبير' bool (A) == bool (B) '. cmCTestTestHandler.cxx 702
  • V728 يمكن تبسيط عملية التحقق المفرطة. The '(A &&! B) || تعبير (! A && B) 'يعادل التعبير' bool (A)! = Bool (B) '. digest_sspi.c 443
  • V728 يمكن تبسيط عملية التحقق المفرطة. The '(A &&! B) || تعبير (! A && B) 'يعادل التعبير' bool (A)! = Bool (B) '. tcp.c 1295
  • V728 يمكن تبسيط عملية التحقق المفرطة. The '(A &&! B) || تعبير (! A && B) 'يعادل التعبير' bool (A)! = Bool (B) '. testDynamicLoader.cxx 58
  • V728 يمكن تبسيط عملية التحقق المفرطة. The '(A &&! B) || تعبير (! A && B) 'يعادل التعبير' bool (A)! = Bool (B) '. testDynamicLoader.cxx 65
  • V728 يمكن تبسيط عملية التحقق المفرطة. The '(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


في هذا القسم ، سوف أخبرك بإيجاز كيفية التحقق من مشاريع CMake باستخدام PVS-Studio بنفس سهولة واحد وثلاثة أو ثلاثة.

ويندوز / البصرية ستوديو

لبرنامج Visual Studio ، يمكنك إنشاء ملف مشروع باستخدام CMake GUI أو الأمر التالي:

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

بعد ذلك ، يمكنك فتح ملف .sln والتحقق من المشروع باستخدام البرنامج المساعد لبرنامج Visual Studio.

لينكس / ماك

يتم استخدام ملف 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- أمثلة .

استنتاج


يعد جمهور كبير من مستخدمي CMake رائعًا لاختبار المشروع ، ولكن يمكن تجنب العديد من المشكلات قبل الإصدار باستخدام أدوات تحليل الشفرة الثابتة ، مثل PVS-Studio .

إذا كنت تحب نتائج المحلل ، ولكن لم تتم كتابة مشروعك في C و C ++ ، أود أن أذكر أن المحلل يدعم أيضًا تحليل المشروعات في C # و Java. يمكنك اختبار محلل مشروعك بالانتقال إلى هذه الصفحة .

Source: https://habr.com/ru/post/ar464145/

More articles:


All Articles