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 مرات من المخطط. دعنا نشرح أين هو "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; };

اكتشف المحلل استخدام مؤشر المؤشر غير المهيأ . وهذا نشأ بسبب الخطأ المطبعي المعتاد. عند استدعاء وظيفة 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 بإرجاع 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 مع محطة الصفر غير ضرورية. تتوقف حالة حلقة while على ما إذا كانت الشخصية تساوي مساحة أم لا. هذا ليس خطأ ، ولكن تعقيد إضافي من التعليمات البرمجية.

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

 oid 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. يمكنك تجربة المحلل في مشروعك بالانتقال إلى هذه الصفحة.



إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فالرجاء استخدام الرابط الخاص بالترجمة: Svyatoslav Razmyslov. CMake: الحالة التي تكون فيها جودة المشروع أمرًا لا يغتفر .

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

More articles:


All Articles