PVS-Studio نظرت إلى محرك Red Dead Redemption - Bullet

صورة 4

في الوقت الحاضر ، على سبيل المثال ، تطوير الألعاب ، لم تعد هناك حاجة إلى تنفيذ فيزياء الكائنات بشكل مستقل من البداية ، نظرًا لوجود عدد كبير من المكتبات لهذا الغرض. تم استخدام Bullet مرة واحدة بنشاط في العديد من ألعاب AAA ، ومشاريع الواقع الافتراضي ، والمحاكاة المختلفة ، والتعلم الآلي. نعم ، ولا يزال يستخدم اليوم ، على سبيل المثال ، أحد محركات Red Dead Redemption و Red Dead Redemption 2. لذلك فلماذا لا تحقق من Bullet مع PVS-Studio لمعرفة الأخطاء التي يمكن أن يكتشفها التحليل الثابت في مثل هذا المشروع الكبير ، المتعلقة محاكاة الفيزياء.

يتم توزيع هذه المكتبة مجانًا ، بحيث يمكن للجميع استخدامها اختياريًا في مشاريعهم. بالإضافة إلى Red Dead Redemption ، يتم استخدام محرك الفيزياء هذا أيضًا في صناعة الأفلام لإنشاء مؤثرات خاصة. على سبيل المثال ، شارك في تصوير شيرلوك هولمز من قبل جاي ريتشي لحساب الاصطدامات.

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

للاحماء


مثال 1:

لنبدأ بخطأ مضحك:

V624 ربما يكون هناك خطأ مطبعي في ثابت "3.141592538". حاول استخدام ثابت M_PI من <math.h>. PhysicsClientC_API.cpp 4109

B3_SHARED_API void b3ComputeProjectionMatrixFOV(float fov, ....) { float yScale = 1.0 / tan((3.141592538 / 180.0) * fov / 2); .... } 

خطأ مطبعي صغير في الرقم Pi (3.141592653 ...) ، مفقودًا الرقم "6" في الموضع السابع في الجزء الكسري.

الصورة 1
ربما لن يؤدي الخطأ في المكان العشري العشري إلى عواقب ملموسة ، ولكن لا يزال يتعين عليك استخدام ثوابت المكتبة الحالية بدون أخطاء مطبعية. بالنسبة لـ Pi ، يوجد ثابت M_PI من رأس math.h.

نسخ لصق


مثال 2:

في بعض الأحيان يسمح لك المحلل بالعثور على الخطأ بشكل غير مباشر. لذلك ، على سبيل المثال ، يتم هنا تمرير ثلاث وسائط ذات صلة هي halfExtentsX ، و halfExtentsY ، و halfExtentsZ إلى الوظيفة ، لكن الأخيرة لا تستخدم في أي مكان في الوظيفة. قد تلاحظ أنه عند استدعاء الأسلوب addVertex ، يتم استخدام halfExtentsY المتغير مرتين. لذلك ربما يكون هذا خطأ في نسخ اللصق وهنا يجب استخدام وسيطة منسية.

لا يتم استخدام المعلمة V751 "halfExtentsZ" داخل جسم الوظيفة. TinyRenderer.cpp 375

 void TinyRenderObjectData::createCube(float halfExtentsX, float halfExtentsY, float halfExtentsZ, ....) { .... m_model->addVertex(halfExtentsX * cube_vertices_textured[i * 9], halfExtentsY * cube_vertices_textured[i * 9 + 1], halfExtentsY * cube_vertices_textured[i * 9 + 2], cube_vertices_textured[i * 9 + 4], ....); .... } 

مثال 3:

عثر المحلل أيضًا على الجزء المثير للاهتمام التالي ، وسأحضره أولاً في شكله الأصلي.

صورة 11

انظر هذا السطر الأخير؟

صورة 12

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

أصدر المحلل التحذيرات التالية على هذا الخط.

V501 هناك تعبيرات فرعية مماثلة 'rotmat.Column1 (). Norm () <1.0001' إلى اليسار وإلى يمين عامل التشغيل '&&'. LinearR4.cpp 351

V501 هناك تعبيرات فرعية متطابقة '0.9999 <rotmat.Column1 (). Norm ()' إلى اليسار وإلى يمين المشغل '&&'. LinearR4.cpp 351

إذا كتبنا كل شيء في نموذج "جدولي" مرئي ، فسيصبح من الواضح أن عمليات التحقق نفسها تنطبق على العمود 1 . توضح المقارنات الأخيرة اثنين أن هناك Column1 و Column2 . على الأرجح ، يجب أن تحقق المقارنات الثالثة والرابعة قيمة العمود 2 .

  Column1().Norm() < 1.0001 && 0.9999 < Column1().Norm() && Column1().Norm() < 1.0001 && 0.9999 < Column1().Norm() &&(Column1() ^ Column2()) < 0.001 && (Column1() ^ Column2()) > -0.001 

في هذا النموذج ، تصبح المقارنة نفسها أكثر وضوحًا.

مثال 4:

خطأ من نفس النوع:

V501 هناك تعبيرات فرعية متطابقة 'cs.m_fJacCoeffInv [0] == 0' إلى اليسار وإلى يمين عامل التشغيل '&&'. b3CpuRigidBodyPipeline.cpp 169

 float m_fJacCoeffInv[2]; static inline void b3SolveFriction(b3ContactConstraint4& cs, ....) { if (cs.m_fJacCoeffInv[0] == 0 && cs.m_fJacCoeffInv[0] == 0) { return; } .... } 

في هذه الحالة ، يتم التحقق من عنصر الصفيف نفسه مرتين. على الأرجح ، كان يجب أن يكون الشرط كما يلي: cs.m_fJacCoeffInv [0] == 0 && cs.m_fJacCoeffInv [1] == 0 . هذا مثال كلاسيكي على خطأ نسخ اللصق.

مثال 5:

تم اكتشاف عيب آخر:

V517 استخدام نمط "if (A) {...} آخر إذا تم اكتشاف (A) {...}". هناك احتمال لوجود خطأ منطقي. خطوط الفحص: 79 ، 112. main.cpp 79

 int main(int argc, char* argv[]) { .... while (serviceResult > 0) { serviceResult = enet_host_service(client, &event, 0); if (serviceResult > 0) { .... } else if (serviceResult > 0) { puts("Error with servicing the client"); exit(EXIT_FAILURE); } .... } .... } 

ترجع الدالة enet_host_service ، والتي يتم تعيين نتيجة لها إلى serviceResult ، واحدة إذا نجحت و -1 إذا فشلت. على الأرجح ، إذا كان الفرع يجب أن يكون قد استجاب للقيمة السلبية لـ serviceResult ، ولكن تم تكرار شرط التحقق. على الأرجح هذا أيضًا خطأ في نسخ اللصق.

تحذير محلل مماثل ، لا معنى لوصف في المقال:

V517 استخدام نمط "if (A) {...} آخر إذا تم اكتشاف (A) {...}". هناك احتمال لوجود خطأ منطقي. خطوط الفحص: 151 ، 190. PhysicsClientUDP.cpp 151

ما وراء الحدود: تجاوز حدود المصفوفة


مثال 6:

أحد الأشياء غير السارة للبحث عن الأخطاء هو الخروج من الصفيف. غالبًا ما يحدث هذا الخطأ بسبب الفهرسة المعقدة في الحلقة.

هنا ، في حالة الحلقة ، يقتصر متغير dofIndex على 128 من أعلاه ، و dof إلى 4 ، شامل. لكن m_desiredState يحتوي أيضًا على 128 عنصرًا فقط. نتيجة لذلك ، يمكن أن يؤدي الفهرس [dofIndex + dof] إلى تدفق الصفيف.

V557 تجاوز الصفيف هو ممكن. قد تصل قيمة مؤشر "dofIndex + dof" إلى 130. PhysicsClientC_API.cpp 968

 #define MAX_DEGREE_OF_FREEDOM 128 double m_desiredState[MAX_DEGREE_OF_FREEDOM]; B3_SHARED_API int b3JointControl(int dofIndex, double* forces, int dofCount, ....) { .... if ( (dofIndex >= 0) && (dofIndex < MAX_DEGREE_OF_FREEDOM ) && dofCount >= 0 && dofCount <= 4) { for (int dof = 0; dof < dofCount; dof++) { command->m_sendState.m_desiredState[dofIndex+dof] = forces[dof]; .... } } .... } 

مثال 7:

خطأ مشابه ، لكن الآن يؤدي الجمع إلى أنه ليس عند فهرسة صفيف ، ولكن في حالة. إذا كان اسم الملف أطول فترة ممكنة ، فسيتم كتابة الصفر الطرفي خارج الصفيف ( خطأ خارج عن طريق الخطأ ). بطبيعة الحال ، سيكون متغير len مساوياً لـ MAX_FILENAME_LENGTH فقط في حالات استثنائية ، لكن هذا لا يلغي الخطأ ، ولكن يجعل حدوثه نادر الحدوث.

V557 تجاوز الصفيف هو ممكن. قد تصل قيمة مؤشر 'len' إلى 1024. PhysicsClientC_API.cpp 5223

 #define MAX_FILENAME_LENGTH MAX_URDF_FILENAME_LENGTH 1024 struct b3Profile { char m_name[MAX_FILENAME_LENGTH]; int m_durationInMicroSeconds; }; int len = strlen(name); if (len >= 0 && len < (MAX_FILENAME_LENGTH + 1)) { command->m_type = CMD_PROFILE_TIMING; strcpy(command->m_profile.m_name, name); command->m_profile.m_name[len] = 0; } 

قياس مرة واحدة - قطع سبع مرات


مثال 8:

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

V807 انخفاض الأداء. حاول إنشاء مؤشر لتجنب استخدام تعبير 'm_app-> m_renderer-> getActiveCamera () بشكل متكرر. InverseKinematicsExample.cpp 315

 virtual void resetCamera() { .... if (....) { m_app->m_renderer->getActiveCamera()->setCameraDistance(dist); m_app->m_renderer->getActiveCamera()->setCameraPitch(pitch); m_app->m_renderer->getActiveCamera()->setCameraYaw(yaw); m_app->m_renderer->getActiveCamera()->setCameraPosition(....); } } 

هنا يتم إعادة استخدام سلسلة الاتصال نفسها ، والتي يمكن استبدالها بمؤشر واحد.

مثال 9:

V810 انخفاض الأداء. تم استدعاء الدالة 'btCos (euler_out.pitch)' عدة مرات باستخدام وسيطات متطابقة. ربما يجب حفظ النتيجة في متغير مؤقت ، والذي يمكن استخدامه أثناء استدعاء وظيفة "btAtan2". btMatrix3x3.h 576

V810 انخفاض الأداء. تم استدعاء الدالة 'btCos (euler_out2.pitch)' عدة مرات باستخدام وسيطات متطابقة. ربما يجب حفظ النتيجة في متغير مؤقت ، والذي يمكن استخدامه أثناء استدعاء وظيفة "btAtan2". btMatrix3x3.h 578

 void getEulerZYX(....) const { .... if (....) { .... } else { .... euler_out.roll = btAtan2(m_el[2].y() / btCos(euler_out.pitch), m_el[2].z() / btCos(euler_out.pitch)); euler_out2.roll = btAtan2(m_el[2].y() / btCos(euler_out2.pitch), m_el[2].z() / btCos(euler_out2.pitch)); euler_out.yaw = btAtan2(m_el[1].x() / btCos(euler_out.pitch), m_el[0].x() / btCos(euler_out.pitch)); euler_out2.yaw = btAtan2(m_el[1].x() / btCos(euler_out2.pitch), m_el[0].x() / btCos(euler_out2.pitch)); } .... } 

في هذه الحالة ، يمكنك إنشاء متغيرين وتخزين القيم التي يتم إرجاعها بواسطة الدالة btCos لـ euler_out.pitch و euler_out2.pitch فيها ، بدلاً من استدعاء الدالة أربع مرات لكل وسيطة.

تسرب


مثال 10:

تم العثور على الكثير من الأخطاء من النوع التالي في المشروع:

تم إنهاء نطاق الرؤية V773 لمؤشر 'المستورد' دون تحرير الذاكرة. تسرب الذاكرة ممكن. SerializeSetup.cpp 94

 void SerializeSetup::initPhysics() { .... btBulletWorldImporter* importer = new btBulletWorldImporter(m_dynamicsWorld); .... fclose(file); m_guiHelper->autogenerateGraphicsObjects(m_dynamicsWorld); } 

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

إليك قوانينك


مثال 11:

يظهر الخطأ التالي في الكود بسبب حقيقة أن قواعد C ++ لا تتوافق دائمًا مع القواعد الرياضية أو "الفطرة السليمة". إشعار لنفسك حيث الخطأ في مقتطف صغير من التعليمات البرمجية؟

 btAlignedObjectArray<btFractureBody*> m_fractureBodies; void btFractureDynamicsWorld::fractureCallback() { for (int i = 0; i < numManifolds; i++) { .... int f0 = m_fractureBodies.findLinearSearch(....); int f1 = m_fractureBodies.findLinearSearch(....); if (f0 == f1 == m_fractureBodies.size()) continue; .... } .... } 

يقوم المحلل بإنشاء التحذير التالي:

V709 تم العثور على مقارنة مشبوهة: 'f0 == f1 == m_fractureBodies.size ()'. تذكر أن 'a == b == c' لا تساوي 'a == b && b == c'. btFractureDynamicsWorld.cpp 483

يبدو أن الشرط يتحقق من أن f0 تساوي f1 وتساوي عدد العناصر في m_fractureBodies . يبدو أن هذه المقارنة يجب أن تتحقق مما إذا كانت f0 و f1 في نهاية صفيف m_fractureBodies ، لأنها تحتوي على موضع الكائن الذي تم العثور عليه بواسطة طريقة findLinearSearch () . ومع ذلك ، في الواقع ، يتحول هذا التعبير إلى اختبار لمعرفة ما إذا كانت f0 و f1 متساوية ، ثم إلى التحقق مما إذا كان m_fractureBodies.size () يساوي نتيجة f0 == f1 . نتيجة لذلك ، تتم مقارنة المعامل الثالث بـ 0 أو 1 هنا.

خطأ جميل! ولحسن الحظ ، نادر جدًا. حتى الآن التقينا بها فقط في مشروعين مفتوحين ، ومن المثير للاهتمام ، كلهم ​​كانوا مجرد محركات ألعاب.

مثال 12:

عند العمل مع الأوتار ، غالبًا ما يكون من الأفضل استخدام التسهيلات التي توفرها فئة السلسلة . لذلك ، للحالتين التاليتين ، من الأفضل استبدال strlen (MyStr.c_str ()) و val = "" بـ MyStr.length () و val.clear () ، على التوالي.

V806 انخفاض الأداء. يمكن إعادة كتابة تعبير النوع strlen (MyStr.c_str ()) كـ MyStr.length (). RobotLoggingUtil.cpp 213

 FILE* createMinitaurLogFile(const char* fileName, std::string& structTypes, ....) { FILE* f = fopen(fileName, "wb"); if (f) { .... fwrite(structTypes.c_str(), strlen(structTypes.c_str()), 1, f); .... } .... } 

V815 انخفاض الأداء. فكّر في استبدال التعبير 'val = ""' بـ "val.clear () '. b3CommandLineArgs.h 40

 void addArgs(int argc, char **argv) { .... std::string val; .... val = ""; .... } 

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

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

أخطاء وجدت من قبلنا


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

لم يكن هناك الكثير من طلبات السحب في المستودع ، والكثير منها يرتبط بالمنطق الداخلي للمحرك. ولكن كانت هناك أيضًا أخطاء يمكن للمحلل اكتشافها.

مثال 13:

 char m_deviceExtensions[B3_MAX_STRING_LENGTH]; void b3OpenCLUtils_printDeviceInfo(cl_device_id device) { b3OpenCLDeviceInfo info; b3OpenCLUtils::getDeviceInfo(device, &info); .... if (info.m_deviceExtensions != 0) { .... } } 

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

V600 النظر في فحص الشرط. مؤشر 'info.m_deviceExtensions' دائمًا لا يساوي NULL. b3OpenCLUtils.cpp 551

مثال 14:

هل يمكنك على الفور معرفة المشكلة في الوظيفة التالية؟

 inline void Matrix4x4::SetIdentity() { m12 = m13 = m14 = m21 = m23 = m24 = m13 = m23 = m41 = m42 = m43 = 0.0; m11 = m22 = m33 = m44 = 1.0; } 

يقوم المحلل بإنشاء التحذيرات التالية:

V570 يتم تعيين نفس القيمة مرتين للمتغير "m23". LinearR4.h 627

V570 يتم تعيين نفس القيمة مرتين للمتغير "m13". LinearR4.h 627

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

 m12 = m13 = m14 = m21 = m23 = m24 = m31 = m32 = m34 = m41 = m42 = m43 = 0.0; 

مثال 15:

الخطأ التالي في أحد شروط الدالة btSoftBody :: addAeroForceToNode () أدى إلى خلل صريح. وفقًا للتعليق في طلب السحب ، تم تطبيق القوات على أشياء من الجانب الخطأ.

 struct eAeroModel { enum _ { V_Point, V_TwoSided, .... END }; }; void btSoftBody::addAeroForceToNode(....) { .... if (....) { if (btSoftBody::eAeroModel::V_TwoSided) { .... } .... } .... } 

يمكن لـ PVS-Studio أيضًا العثور على هذا الخطأ وعرض التحذير التالي:

V768 يتم استخدام ثابت التعداد 'V_TwoSided' كمتغير من نوع Boolean. btSoftBody.cpp 542

الاختيار الصحيح هو كالتالي:

 if (m_cfg.aeromodel == btSoftBody::eAeroModel::V_TwoSided) { .... } 

بدلاً من معادلة خاصية الكائن لأحد العدادات ، تم التحقق من العداد V_TwoSided نفسه.

من الواضح أنني لم أشاهد جميع طلبات السحب ، لأنني لم أقم بتعيين هذه المهمة بنفسي. أردت فقط أن أوضح أن الاستخدام المنتظم لمحلل الكود الثابت يمكنه اكتشاف الأخطاء في مرحلة مبكرة للغاية. هذه هي الطريقة الصحيحة لاستخدام تحليل الكود الثابت. يجب أن يكون التحليل الثابت مضمنًا في عملية DevOps ويكون عامل التصفية الأساسي للأخطاء. كل هذا موصوف جيدًا في المقال " تضمين التحليل الثابت في العملية ، ولا تبحث عن أخطاء به ".

استنتاج


صورة 6

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

إذا كنت تريد أن تكون دائمًا على علم بأخبار وأحداث فريقنا ، فقم بالاشتراك في خدماتنا الاجتماعية. الشبكات: Instagram و Twitter و Vkontakte و Telegram .



إذا كنت ترغب في مشاركة هذه المقالة مع جمهور ناطق باللغة الإنجليزية ، فيرجى استخدام رابط الترجمة: PVS-Studio ، بحث في Red Dead Redemption's Bullet Engine

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


All Articles