أهم 10 أخطاء في مشاريع C ++ في 2020

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



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

المركز العاشر: قسمة مودولو بواحد

V1063 العملية النمطية بـ 1 لا معنى لها. ستكون النتيجة دائمًا صفرًا. 631

void Act() override {
  ....
  // If the value type is a vector, and we allow vector select,
  // then in 50% of the cases generate a vector select.
  if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 1)) {
    unsigned NumElem =
        cast<FixedVectorType>(Val0->getType())->getNumElements();
    CondTy = FixedVectorType::get(CondTy, NumElem);
  }
  ....
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

أراد المطور الحصول على قيمة عشوائية بين 0 و 1 باستخدام تقسيم modulo. ومع ذلك ، فإن عملية مثل X٪ 1 ستعيد دائمًا 0. في هذه الحالة ، سيكون من الصحيح إعادة كتابة الشرط على النحو التالي:

if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 2))
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

تم تضمين هذا الخطأ في الجزء العلوي من المقالة: " Checking Clang 11 with PVS-Studio ".

المركز التاسع: أربعة شيكات

أصدر PVS-Studio أربعة تحذيرات للقسم التالي من الكود:

• V560 جزء من التعبير الشرطي يكون دائمًا صحيحًا: x> = 0. editor.cpp 1137
• V560 جزء من التعبير الشرطي يكون دائمًا صحيحًا: y> = 0. editor.cpp 1137
• V560 جزء من التعبير الشرطي يكون دائمًا صحيحًا: x <40. editor.cpp 1137
• V560 جزء من التعبير الشرطي يكون دائمًا صحيحًا: y <30. editor.cpp 1137

int editorclass::at( int x, int y )
{
  if(x<0) return at(0,y);
  if(y<0) return at(x,0);
  if(x>=40) return at(39,y);
  if(y>=30) return at(x,29);

  if(x>=0 && y>=0 && x<40 && y<30)
  {
      return contents[x+(levx*40)+vmult[y+(levy*30)]];
  }
  return 0;
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

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



دخل هذا الخطأ إلى الجزء العلوي من المقال: " VVVVVV ؟؟؟ VVVVVV !!! ".

المركز الثامن: حذف بدلاً من حذف []

V611 تم تخصيص الذاكرة باستخدام عامل التشغيل "T [] الجديد ولكن تم تحريرها باستخدام عامل التشغيل" delete ". ضع في اعتبارك فحص هذا الرمز. ربما يكون من الأفضل استخدام "delete [] poke_data؛". CCDDE.CPP 410

BOOL Send_Data_To_DDE_Server (char *data, int length, int packet_type)
{
  ....
  char *poke_data = new char [length + 2*sizeof(int)]; // <=
  ....
  if(DDE_Class->Poke_Server( .... ) == FALSE) {
    CCDebugString("C&C95 - POKE failed!\n");
    DDE_Class->Close_Poke_Connection();
    delete poke_data;                                  // <=
    return (FALSE);
  }

  DDE_Class->Close_Poke_Connection();

  delete poke_data;                                    // <=

  return (TRUE);
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

اكتشف المحلل خطأ يتعلق بحقيقة أنه تم تخصيص الذاكرة وتحريرها بطرق غير متوافقة. لتحرير الذاكرة المخصصة لمصفوفة ، استخدم عامل حذف [] ، وليس حذف .



وصل هذا الخطأ إلى القمة من المقالة: " Command & Conquer Game Code: Bugs from the 90s. Volume II ".

المركز السابع: منطقة عازلة خارج الحدود

لنلقِ نظرة على وظيفة net_hostname_get التي سيتم استخدامها بعد ذلك.

#if defined(CONFIG_NET_HOSTNAME_ENABLE)
const char *net_hostname_get(void);
#else
static inline const char *net_hostname_get(void)
{
  return "zephyr";
}
#endif
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

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

static inline const char *net_hostname_get(void)
{
  return "zephyr";
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

ترجع الدالة مؤشرًا إلى مصفوفة من 7 بايت (ضع في الاعتبار الصفر الطرفي في نهاية السلسلة).



الآن دعونا نلقي نظرة على الكود الذي يؤدي إلى تجاوز المصفوفة.

static int do_net_init(void)
{
  ....
  (void)memcpy(hostname, net_hostname_get(), MAX_HOSTNAME_LEN);
  ....
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

تحذير PVS-Studio: V512 [CWE-119] ستؤدي استدعاء وظيفة "memcpy" إلى أن يصبح المخزن المؤقت "net_hostname_get ()" خارج النطاق. log_backend_net.c 114



بعد المعالجة المسبقة ، يتم توسيع MAX_HOSTNAME_LEN على النحو التالي:

(void)memcpy(hostname, net_hostname_get(),
    sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx"));
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

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



وصل هذا الخطأ إلى قمة المقال: " التحقيق في جودة كود نظام التشغيل Zephyr ".

المركز السادس: شيء غريب جدا

static char *mntpt_prepare(char *mntpt)
{
  char *cpy_mntpt;

  cpy_mntpt = k_malloc(strlen(mntpt) + 1);
  if (cpy_mntpt) {
    ((u8_t *)mntpt)[strlen(mntpt)] = '\0';
    memcpy(cpy_mntpt, mntpt, strlen(mntpt));
  }
  return cpy_mntpt;
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

تحذير PVS-Studio: V575 [CWE-628] وظيفة "memcpy" لا تنسخ السلسلة بأكملها. استخدم وظيفة "strcpy / strcpy_s" للحفاظ على المحطة فارغة. shell.c 427



حاول شخص ما عمل تناظرية لوظيفة strdup ، لكنهم فشلوا.



لنبدأ بتحذير المحلل. تقول أن وظيفة memcpy تنسخ السطر ، لكنها لن تنسخ الصفر الطرفي ، وهو أمر مريب للغاية.



يبدو أن هذه المحطة 0 منسوخة هنا:

((u8_t *)mntpt)[strlen(mntpt)] = '\0';
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

لكن لا! إليك خطأ مطبعي يتسبب في نسخ الطرف صفر في نفسه! لاحظ أن الكتابة ستذهب إلى صفيف mntpt ، وليس cpy_mntpt . نتيجة لذلك ، تُرجع الدالة mntpt_prepare سلسلة فارغة طرفية غير كاملة.



في الواقع ، أراد المبرمج أن يكتب مثل هذا:

((u8_t *)cpy_mntpt)[strlen(mntpt)] = '\0';
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

ومع ذلك ، لا يزال من غير الواضح لماذا تم ذلك بهذه الصعوبة! يمكن تبسيط هذا الرمز إلى ما يلي:

static char *mntpt_prepare(char *mntpt)
{
  char *cpy_mntpt;

  cpy_mntpt = k_malloc(strlen(mntpt) + 1);
  if (cpy_mntpt) {
    strcpy(cpy_mntpt, mntpt);
  }
  return cpy_mntpt;
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

وصل هذا الخطأ إلى الجزء العلوي من المقالة المذكورة أعلاه: " فحص جودة كود نظام تشغيل Zephyr ".

المركز الخامس: الحماية من الفائض غير الصحيح

V547 [CWE-570] التعبير "rel_wait <0" خطأ دائمًا. قيمة النوع غير الموقعة ليست أبدًا <0. os_thread_windows.c 359

static DWORD
get_rel_wait(const struct timespec *abstime)
{
  struct __timeb64 t;
  _ftime64_s(&t);
  time_t now_ms = t.time * 1000 + t.millitm;
  time_t ms = (time_t)(abstime->tv_sec * 1000 +
    abstime->tv_nsec / 1000000);

  DWORD rel_wait = (DWORD)(ms - now_ms);

  return rel_wait < 0 ? 0 : rel_wait;
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

في هذه الحالة ، يكون المتغير rel_wait من نوع DWORD غير الموقعة . هذا يعني أن المقارنة rel_wait <0 لا معنى لها ، لأن النتيجة صحيحة دائمًا.



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



تم تضمين الخطأ في الجزء العلوي من المقالة: " تحليل ثابت لرمز مجموعة مكتبات PMDK من Intel والأخطاء التي ليست أخطاء ."

المركز الرابع: لا تكتب إلى std يا أخي

V1061 قد يؤدي توسيع مساحة الاسم "std" إلى سلوك غير معرف. 210- نورة

// Dirty hack because g++ 4.6 at least wants
// to do a bunch of copy operations.
namespace std {
inline void iter_swap(util::SizedIterator first,
                      util::SizedIterator second)
{
  util::swap(*first, *second);
}
} // namespace std
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

المقالة التي تم أخذ المشغل منها: " تحليل كود مشروع DeepSpeech أو لماذا لا يجب أن تكتب في مساحة الاسم std " توضح بالتفصيل لماذا لا يجب عليك القيام بذلك.

المركز الثالث: شريط التمرير الذي فشل

V501 . توجد تعبيرات فرعية متطابقة على يسار ويمين عامل التشغيل "-": المخزن المؤقت الارتفاع - المخزن المؤقت الارتفاع TermControl.cpp 592

bool TermControl::_InitializeTerminal()
{
  ....
  auto bottom = _terminal->GetViewport().BottomExclusive();
  auto bufferHeight = bottom;

  ScrollBar().Maximum(bufferHeight - bufferHeight);
  ScrollBar().Minimum(0);
  ScrollBar().Value(0);
  ScrollBar().ViewportSize(bufferHeight);
  ....
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

هذا ما يسمى "إطلاق التاريخ". في هذه الحالة ، بسبب خطأ ، لم يعمل شريط التمرير في Windows Terminal. تمت كتابة مقال كامل بناءً على هذا الخطأ ، حيث أجرى زميلي بحثًا واكتشف سبب حدوث ذلك. هل انت مهتم؟ ها هو: " شريط التمرير الذي تعذر ".

المركز الثاني: الخلط بين نصف القطر والارتفاع

ومرة أخرى سنتحدث عن عدة تحذيرات للمحلل:

• V764 ترتيب غير صحيح للوسيطات التي تم تمريرها إلى وظيفة "CreateWheel": "الارتفاع" و "نصف القطر". StandardJoints.cpp 791
• V764 ترتيب غير صحيح للوسيطات التي تم تمريرها إلى وظيفة "CreateWheel": "الارتفاع" و "نصف القطر". StandardJoints.cpp 833
• V764 ترتيب غير صحيح للوسيطات التي تم تمريرها إلى وظيفة "CreateWheel": "الارتفاع" و "نصف القطر". StandardJoints.cpp 884

فيما يلي استدعاءات الوظائف:

NewtonBody* const wheel = CreateWheel (scene, origin, height, radius);
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

وهذا شكل إعلانها:

static NewtonBody* CreateWheel (DemoEntityManager* const scene,
  const dVector& location, dFloat radius, dFloat height)
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

تم عكس الحجج في استدعاءات الوظائف.



وصل هذا الخطأ إلى أعلى المقالة: " إعادة فحص Newton Game Dynamics باستخدام محلل PVS-Studio الثابت ".

المركز الأول: محو النتيجة

V519 يتم تخصيص قيم لمتغير "color_name" مرتين على التوالي. ربما هذا خطأ. تحقق من الأسطر: 621 ، 627. string.cpp 627

static bool parseNamedColorString(const std::string &value,
                                  video::SColor &color)
{
  std::string color_name;
  std::string alpha_string;

  size_t alpha_pos = value.find('#');
  if (alpha_pos != std::string::npos) {
    color_name = value.substr(0, alpha_pos);
    alpha_string = value.substr(alpha_pos + 1);
  } else {
    color_name = value;
  }

  color_name = lowercase(value); // <=

  std::map<const std::string, unsigned>::const_iterator it;
  it = named_colors.colors.find(color_name);
  if (it == named_colors.colors.end())
    return false;
  ....
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

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



ومع ذلك ، في الدالة الصغيرة () ، لا يتم تحويل السلسلة الناتجة نفسها إلى حالة صغيرة ، ولكن وسيطة الوظيفة الأصلية. نتيجة لذلك ، سنفقد ببساطة اللون الذي كان يجب أن يعيده parseNamedColorString () .

color_name = lowercase(color_name);
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

وصل هذا الخطأ إلى أعلى المقالة: " PVS-Studio: تحليل طلبات السحب في Azure DevOps باستخدام وكلاء مستضافين ذاتيًا ".

استنتاج

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



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



أوجه انتباهكم أيضًا إلى المقالات التي تحتوي على أهم 10 أخطاء في ++ C في السنوات الماضية: 2016 ، 2017 ، 2018 ، 2019 .

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