لم يتم العثور على أخطاء تحليل التحليل الثابت لأنه لا يتم استخدامه

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

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

لسوء الحظ ، خذلنا GitHub ولم نتمكن من تقديم مقال فخم كبير حول هذا الموضوع. يحتوي GitHub نفسه على خلل (أو ميزة) لا يسمح لك بالبحث عن تعليقات طلبات السحب في مشاريع مكتوبة بلغات برمجة معينة فقط. أو لا أعرف كيف أطبخها. على الرغم من أنني أقوم بالبحث عن التعليقات في مشاريع C و C ++ و C # ، يتم تقديم النتائج لجميع اللغات ، بما في ذلك PHP و Python و JavaScript وغيرها. نتيجة لذلك ، أثبت البحث عن الحالات المناسبة أنه ممل للغاية ، وسأطرح فقط بعض الأمثلة. ومع ذلك ، فهي كافية لإثبات فائدة أدوات تحليل الشفرة الثابتة عند استخدامها بانتظام.

ماذا لو تم اكتشاف الخطأ في المرحلة المبكرة؟ الإجابة بسيطة: لن يضطر المبرمجون إلى الانتظار حتى يظهروا أنفسهم ، ثم يبحثون عن التعليمات البرمجية المعيبة وتصحيحها.

دعونا نلقي نظرة على الأخطاء التي كان بإمكان PVS-Studio اكتشافها على الفور:

المثال الأول مأخوذ من مشروع 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(); } } 

احتوى هذا الرمز على خطأ ، حيث يقوم PVS-Studio بإصدار تحذير على الفور إلى:

V591 يجب أن ترجع الدالة Non-void قيمة. ModFunctions.cpp 44

لا تحتوي الوظيفة المذكورة أعلاه على بيان الإرجاع ، لذلك ستُرجع قيمة غير محددة رسميًا. لم يستخدم المبرمج محلل الكود ، لذلك كان عليه أن يبحث عن الأخطاء من تلقاء نفسه. الوظيفة بعد التحرير:

 // 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; } 

الغريب ، في الالتزام ، قام المؤلف بتمييز الخطأ على أنه حرج: " خطأ حرج ثابت حيث لم يتم إرجاع وظائف API ".

في الالتزام الثاني من تاريخ مشروع 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) { ... } } 

قام المؤلف بتصحيح سطر واحد فقط. استبدل التعبير

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

للواحد التالي

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

في إصدار التعليمات البرمجية القديم لم يكن هناك أي التحقق من وجود مؤشر فارغ. إذا حدث ذلك بحيث يتم تمرير مؤشر فارغ إلى الدالة mc6809dis_direct كالوسيطة الثانية ، فسوف يحدث dereference في نص الدالة. والنتيجة مؤسفة ولا يمكن التنبؤ بها .

يعد dereference للمؤشر الخالي أحد أكثر الأنماط شيوعًا التي تحدثنا عنها: "إنه ليس خطأ حرجًا. الذي يهتم أنه مزدهر في الرمز؟ إذا حدث dereference ، فإن البرنامج سوف يتعطل بهدوء وهذا كل شيء ". من الغريب والمحزن أن نسمع هذا من مبرمجي C ++ ، لكن الحياة تحدث.

على أي حال ، في هذا المشروع تحولت مثل هذا التراجع إلى خطأ ، حيث يخبرنا موضوع الالتزام: " Bug fix --- NULL dereference ".

إذا كان مطور المشروع قد استخدم PVS-Studio ، لكان قد فحص التحذير ووجده منذ شهرين ونصف. هذا هو عندما تم تقديم الخطأ. هنا التحذير:

V595 تم استخدام مؤشر 'cpu' قبل أن يتم التحقق منه ضد nullptr. خطوط الفحص: 1814 ، 1821. mc6809dis.c 1814

وبالتالي ، قد تم إصلاح الخلل في وقت ظهوره ، الأمر الذي كان سيوفر وقت المطور وأعصابه :).

تم العثور على مثال لإصلاح آخر مثير للاهتمام في مشروع libmorton.

الكود المطلوب إصلاحه:

 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 } 

في تحريره ، يستبدل مبرمج التعبير " firstbit_location + = 32 " بـ " * firstbit_location + = 32 ". توقع المبرمج إضافة 32 إلى قيمة المتغير المشار إليها بواسطة مؤشر firstbit_location ، ولكن تمت إضافة 32 إلى المؤشر نفسه. لم يتم استخدام القيمة التي تم تغييرها للمؤشر في أي مكان ولم تعد القيمة المتغيرة المتوقعة كما هي.

ستصدر PVS-Studio تحذيرًا لهذا الرمز:

V1001 يتم تعيين المتغير "firstbit_location" ولكن لا يتم استخدامه بحلول نهاية الوظيفة. morton_common.h 22

حسنًا ، ما هو الشيء السيئ في التعبير المعدل وغير المستخدم؟ التشخيص V1001 لا يبدو أنه يهدف إلى اكتشاف الأخطاء الخطيرة بشكل خاص. على الرغم من ذلك ، وجد خطأً هامًا أثر على منطق البرنامج.

علاوة على ذلك ، اتضح أن هذا الخطأ لم يكن من السهل العثور عليه! لم يكن موجودًا في البرنامج فقط منذ إنشاء الملف ، ولكنه شهد أيضًا العديد من التعديلات في الخطوط المجاورة ووجد في المشروع لما يصل إلى 3 (!) سنوات! كل هذا الوقت تم كسر منطق البرنامج ولم ينجح بالطريقة التي توقعها المطورون. إذا كانوا قد استخدموا PVS-Studio ، لكان قد تم اكتشاف الخلل قبل ذلك بكثير.

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

V629 النظر في فحص تعبير 'vma-> vm_pgoff << 12'. تحويل البت لقيمة 32 بت مع توسع لاحق إلى نوع 64 بت. mmu.c 1795

لقد قمت بسحب إعلانات المتغيرات ، المستخدمة في التعبير " 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; } 

إذا كانت قيمة المتغير 32 بت أكبر من 0xFFFFF ، سيكون 12 بت أعلى قيمة غير فارغة واحدة على الأقل. بعد إزاحة هذا المتغير إلى اليسار ، ستفقد هذه البتات المهمة ، مما يؤدي إلى معلومات غير صحيحة مكتوبة ب x.

للقضاء على فقدان البتات العالية ، نحتاج أولاً إلى الإدلاء بنوع طويل غير موقَّع وفقط بعد هذا التحول:

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

بهذه الطريقة ، سيتم دائمًا كتابة القيمة الصحيحة بالسلطة الفلسطينية.

سيكون ذلك على ما يرام ، لكن هذا الخطأ ، وهو نفس المثال الأول من المقالة ، تبين أنه مهم أيضًا. لقد كتب المؤلف عن ذلك في التعليق. علاوة على ذلك ، وجد هذا الخطأ طريقه إلى عدد هائل من المشاريع. لتقدير حجم المأساة تمامًا ، أقترح النظر في عدد النتائج عند البحث عن هذا الخطأ في GitHub. مخيف ، أليس كذلك؟

لذلك اتبعت طريقة جديدة لإظهار فوائد استخدام محلل الكود الثابت العادي. آمل أن تستمتع به. قم بتنزيل وتجربة محلل الكود الثابت PVS-Studio للتحقق من المشاريع الخاصة بك. في وقت كتابة هذا التقرير ، كان لديه حوالي 700 من قواعد التشخيص المطبقة للكشف عن مجموعة متنوعة من أنماط الخطأ. يدعم C ، C ++ ، C # و Java.