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

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

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

لسوء الحظ ، خذلنا GitHub ولم نسمح بإعداد مقالة ذكية رائعة حول هذا الموضوع. يحتوي GitHub نفسه أيضًا على خلل (أو ميزة) لا يسمح لك بالبحث عن التعليقات في طلبات السحب في المشروعات المكتوبة بلغات برمجة معينة فقط. حسنًا ، أو "لا أعرف كيف أطبخها". بغض النظر عن ما أقوله لك للبحث عن تعليقات في مشاريع في C ++ أو C # أو Java ، يتم عرض النتائج بجميع اللغات ، بما في ذلك 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 كالوسيطة الثانية ، فسيتم إلغاء تحديدها في النص الأساسي للدالة. والنتيجة مؤسفة ولا يمكن التنبؤ بها .

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

ستصدر 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 بت a أكبر من 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.

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