Geekly articles weekly

البق في حاسبة ويندوز


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

مقدمة


لا أعتقد أننا بحاجة إلى تقديم "الحاسبة" لأنك بالكاد تجد مستخدم Windows لا يعرف ما هو عليه. يمكن لأي شخص الآن تنزيل التعليمات البرمجية المصدر للتطبيق من GitHub واقتراح تحسيناتها .

الوظيفة التالية ، على سبيل المثال ، اجتذبت بالفعل انتباه المجتمع:

void TraceLogger::LogInvalidInputPasted(....) { if (!GetTraceLoggingProviderEnabled()) return; 

  LoggingFields fields{}; fields.AddString(L"Mode", NavCategory::GetFriendlyName(mode)->Data()); fields.AddString(L"Reason", reason); fields.AddString(L"PastedExpression", pastedExpression); fields.AddString(L"ProgrammerNumberBase", GetProgrammerType(...).c_str()); fields.AddString(L"BitLengthType", GetProgrammerType(bitLengthType).c_str()); LogTelemetryEvent(EVENT_NAME_INVALID_INPUT_PASTED, fields); }

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

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

كتذكير ، في حالة فقدك للأخبار المتعلقة بالقدرات الأخرى لأداتنا ، لا يدعم PVS-Studio فقط C و C ++ ولكن C # و Java أيضًا.

مقارنة السلسلة غير صحيحة


تعبير V547 'm_resolvedName == L "en-US"' غير صحيح دائمًا. لمقارنة السلاسل ، يجب استخدام دالة wcscmp (). تعريب آلة حاسبة. 180

 wchar_t m_resolvedName[LOCALE_NAME_MAX_LENGTH]; Platform::String^ GetEnglishValueFromLocalizedDigits(....) const { if (m_resolvedName == L"en-US") { return ref new Platform::String(localizedString.c_str()); } .... }

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

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

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

تسرب الذاكرة في الرمز الأصلي


V773 تم إنهاء الوظيفة دون تحرير المؤشر "مؤقت". تسرب الذاكرة ممكن. CalcViewModel StandardCalculatorViewModel.cpp 529

 void StandardCalculatorViewModel::HandleUpdatedOperandData(Command cmdenum) { .... wchar_t* temp = new wchar_t[100]; .... if (commandIndex == 0) { delete [] temp; return; } .... length = m_selectedExpressionLastData->Length() + 1; if (length > 50) { return; } .... String^ updatedData = ref new String(temp); UpdateOperand(m_tokenPosition, updatedData); displayExpressionToken->Token = updatedData; IsOperandUpdatedUsingViewModel = true; displayExpressionToken->CommandIndex = commandIndex; }

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

استثناء بعيد المنال


يجب أن يتم اشتقاق فئات V702 دائمًا من std :: استثناء (وما شابه ) كـ "عام" (لم يتم تحديد أي كلمة رئيسية ، لذلك يقوم المحول البرمجي بإعدادها على "خاص"). CalcManager CalcException.h 4

 class CalcException : std::exception { public: CalcException(HRESULT hr) { m_hr = hr; } HRESULT GetException() { return m_hr; } private: HRESULT m_hr; };

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

اليوم الضائع


V719 لا يغطي بيان التبديل جميع قيم التعداد 'DateUnit': اليوم. CalcViewModel DateCalculator.cpp 279

 public enum class _Enum_is_bitflag_ DateUnit { Year = 0x01, Month = 0x02, Week = 0x04, Day = 0x08 }; Windows::Globalization::Calendar^ m_calendar; DateTime DateCalculationEngine::AdjustCalendarDate(Windows::Foundation::DateTime date, DateUnit dateUnit, int difference) { m_calendar→SetDateTime(date); switch (dateUnit) { case DateUnit::Year: { .... m_calendar->AddYears(difference); m_calendar->ChangeCalendarSystem(currentCalendarSystem); break; } case DateUnit::Month: m_calendar->AddMonths(difference); break; case DateUnit::Week: m_calendar->AddWeeks(difference); break; } return m_calendar->GetDateTime(); }

من المشكوك فيه أن بيان التبديل لا يحتوي على حالة DateUnit :: Day . لهذا السبب ، لن تتم إضافة قيمة اليوم إلى التقويم (المتغير m_calendar ) ، على الرغم من أن التقويم لديه أسلوب AddDays .

الحالات المشبوهة الأخرى مع تعداد آخر:

  • V719 لا يغطي بيان التبديل جميع قيم التعداد 'eANGLE_TYPE': ANGLE_RAD. CalcManager trans.cpp 109
  • V719 لا يغطي بيان التبديل جميع قيم التعداد 'eANGLE_TYPE': ANGLE_RAD. CalcManager trans.cpp 204
  • V719 لا يغطي بيان التبديل جميع قيم التعداد 'eANGLE_TYPE': ANGLE_RAD. CalcManager trans.cpp 276

مقارنة مشبوهة من الأرقام الحقيقية


V550 مقارنة دقيقة غريبة: نسبة = عتبة. من الأفضل استخدام مقارنة مع الدقة المحددة: fabs (A - B) <Epsilon. آلة حاسبة AspectRatioTrigger.cpp 80

 void AspectRatioTrigger::UpdateIsActive(Size sourceSize) { double numerator, denominator; .... bool isActive = false; if (denominator > 0) { double ratio = numerator / denominator; double threshold = abs(Threshold); isActive = ((ratio > threshold) || (ActiveIfEqual && (ratio == threshold))); } SetActive(isActive); }

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

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

  • V550 مقارنة دقيقة غريبة. من الأفضل استخدام مقارنة مع الدقة المحددة: fabs (A - B) <Epsilon. CalcManager UnitConverter.cpp 752
  • V550 مقارنة دقيقة غريبة: stod (roundedString)! = 0.0. من الأفضل استخدام مقارنة مع الدقة المحددة: fabs (A - B)> إبسيلون. CalcManager UnitConverter.cpp 778
  • V550 مقارنة دقيقة غريبة. من الأفضل استخدام مقارنة مع الدقة المحددة: fabs (A - B) <Epsilon. CalcManager UnitConverter.cpp 790
  • V550 مقارنة دقيقة غريبة: stod (roundedString)! = 0.0. من الأفضل استخدام مقارنة مع الدقة المحددة: fabs (A - B)> إبسيلون. CalcManager UnitConverter.cpp 820
  • V550 مقارنة دقيقة غريبة: conversionTable [m_toType] .ratio == 1.0. من الأفضل استخدام مقارنة مع الدقة المحددة: fabs (A - B) <Epsilon. CalcManager UnitConverter.cpp 980
  • V550 مقارنة دقيقة فردية: conversionTable [m_toType] .offset == 0.0. من الأفضل استخدام مقارنة مع الدقة المحددة: fabs (A - B) <Epsilon. CalcManager UnitConverter.cpp 980
  • V550 مقارنة دقيقة فردية: returnValue! = 0. من الأفضل استخدام مقارنة مع الدقة المحددة: fabs (A - B)> Epsilon. CalcManager UnitConverter.cpp 1000
  • V550 مقارنة دقيقة غريبة: sizeToUse! = 0.0. من الأفضل استخدام مقارنة مع الدقة المحددة: fabs (A - B)> إبسيلون. CalcViewModel LocalizationService.cpp 270
  • V550 مقارنة دقيقة غريبة: sizeToUse! = 0.0. من الأفضل استخدام مقارنة مع الدقة المحددة: fabs (A - B)> إبسيلون. CalcViewModel LocalizationService.cpp 289
  • V550 مقارنة دقيقة غريبة: sizeToUse! = 0.0. من الأفضل استخدام مقارنة مع الدقة المحددة: fabs (A - B)> إبسيلون. CalcViewModel LocalizationService.cpp 308
  • V550 مقارنة دقيقة غريبة: sizeToUse! = 0.0. من الأفضل استخدام مقارنة مع الدقة المحددة: fabs (A - B)> إبسيلون. CalcViewModel LocalizationService.cpp 327
  • V550 مقارنة دقيقة فردية: stod (stringToLocalize) == 0. من الأفضل استخدام مقارنة بدقة محددة: fabs (A - B) <Epsilon. CalcViewModel UnitConverterViewModel.cpp 388

تسلسل وظيفة مشبوهة


V1020 تم إنهاء الوظيفة دون استدعاء وظيفة "TraceLogger :: GetInstance (). LogNewWindowCreationEnd". خطوط التحقق: 396 ، 375. الحاسبة App.xaml.cpp 396

 void App::OnAppLaunch(IActivatedEventArgs^ args, String^ argument) { .... if (!m_preLaunched) { auto newCoreAppView = CoreApplication::CreateNewView(); newCoreAppView->Dispatcher->RunAsync(....([....]() { TraceLogger::GetInstance().LogNewWindowCreationBegin(....); // <= Begin .... TraceLogger::GetInstance().LogNewWindowCreationEnd(....); // <= End })); } else { TraceLogger::GetInstance().LogNewWindowCreationBegin(....); // <= Begin ActivationViewSwitcher^ activationViewSwitcher; auto activateEventArgs = dynamic_cast<IViewSwitcherProvider^>(args); if (activateEventArgs != nullptr) { activationViewSwitcher = activateEventArgs->ViewSwitcher; } if (activationViewSwitcher != nullptr) { activationViewSwitcher->ShowAsStandaloneAsync(....); TraceLogger::GetInstance().LogNewWindowCreationEnd(....); // <= End TraceLogger::GetInstance().LogPrelaunchedAppActivatedByUser(); } else { TraceLogger::GetInstance().LogError(L"Null_ActivationViewSwitcher"); } } m_preLaunched = false; .... }

يقوم V1020 Diagnostic بفحص الكودات البرمجية والبحث عن الفروع باستدعاء وظيفة مفقودة باستخدام الاستدلال.

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

اختبارات غير موثوقة


V621 فكّر في فحص عامل التشغيل "for". من المحتمل أن يتم تنفيذ الحلقة بشكل غير صحيح أو لن يتم تنفيذها على الإطلاق. آلة حاسبةوحدة اختبارات وحدةالمعاينعرضنموذج وحدة UnitTests.cpp 500

 public enum class NumbersAndOperatorsEnum { .... Add = (int) CM::Command::CommandADD, // 93 .... None = (int) CM::Command::CommandNULL, // 0 .... }; TEST_METHOD(TestButtonCommandFiresModelCommands) { .... for (NumbersAndOperatorsEnum button = NumbersAndOperatorsEnum::Add; button <= NumbersAndOperatorsEnum::None; button++) { if (button == NumbersAndOperatorsEnum::Decimal || button == NumbersAndOperatorsEnum::Negate || button == NumbersAndOperatorsEnum::Backspace) { continue; } vm.ButtonPressed->Execute(button); VERIFY_ARE_EQUAL(++callCount, mock->m_sendCommandCallCount); VERIFY_IS_TRUE(UCM::Command::None == mock->m_lastCommand); } .... }

اكتشف المحلل وجود حلقة لا تعمل على الإطلاق ، مما يعني أن الاختبارات لا تنفذ أيضًا. القيمة الأولية لزر عداد الحلقة (93) أكبر من القيمة النهائية (0) مباشرة من البداية.

V760 تم العثور على كتلتين متطابقتين من النص. يبدأ الكتلة الثانية من السطر 688. CalculatorUnitTests UnitConverterViewModelUnitTests.cpp 683

 TEST_METHOD(TestSwitchAndReselectCurrentlyActiveValueDoesNothing) { shared_ptr<UnitConverterMock> mock = make_shared<UnitConverterMock>(); VM::UnitConverterViewModel vm(mock); const WCHAR * vFrom = L"1", *vTo = L"234"; vm.UpdateDisplay(vFrom, vTo); vm.Value2Active = true; // Establish base condition VERIFY_ARE_EQUAL((UINT)1, mock->m_switchActiveCallCount); VERIFY_ARE_EQUAL((UINT)1, mock->m_sendCommandCallCount); VERIFY_ARE_EQUAL((UINT)1, mock->m_setCurUnitTypesCallCount); vm.Value2Active = true; VERIFY_ARE_EQUAL((UINT)1, mock->m_switchActiveCallCount); VERIFY_ARE_EQUAL((UINT)1, mock->m_sendCommandCallCount); VERIFY_ARE_EQUAL((UINT)1, mock->m_setCurUnitTypesCallCount); }

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

V601 القيمة "الخاطئة" يتم ضمنيًا ضمنا لنوع الأعداد الصحيحة. تفقد الحجة الثانية. CalculatorUnitTests CalcInputTest.cpp 352

 Rational CalcInput::ToRational(uint32_t radix, int32_t precision) { .... } TEST_METHOD(ToRational) { .... auto rat = m_calcInput.ToRational(10, false); .... }

تسمى الدالة ToRational بقيمة Boolean false ، بينما المعلمة المقابلة من النوع int32_t وتسمى الدقة .

قررت تتبع القيمة أسفل الرمز ورأيت أنه تم تمريرها بعد ذلك إلى وظيفة StringToRat :

 PRAT StringToRat(...., int32_t precision) { .... }

ثم إلى StringToNumber :

 PNUMBER StringToNumber(...., int32_t precision) { .... stripzeroesnum(pnumret, precision); .... }

إليك نص الوظيفة الهدف:

 bool stripzeroesnum(_Inout_ PNUMBER pnum, long starting) { MANTTYPE *pmant; long cdigits; bool fstrip = false; pmant=pnum->mant; cdigits=pnum->cdigit; if ( cdigits > starting ) // <= { pmant += cdigits - starting; cdigits = starting; } .... }

يُطلق على متغير الدقة الآن البدء ويشارك في تعبير cdigits> start ، وهو أمر مشبوه للغاية لأن false تم تمريره كقيمة أصلية.

التكرار


V560 جزء من التعبير الشرطي صحيح دائمًا: NumbersAndOperatorsEnum :: None! = Op. CalcViewModel UnitConverterViewModel.cpp 991

 void UnitConverterViewModel::OnPaste(String^ stringToPaste, ViewMode mode) { .... NumbersAndOperatorsEnum op = MapCharacterToButtonId(*it, canSendNegate); if (NumbersAndOperatorsEnum::None != op) // <= { .... if (NumbersAndOperatorsEnum::None != op && // <= NumbersAndOperatorsEnum::Negate != op) { .... } .... } .... }

تمت مقارنة متغير المرجع بالفعل بالقيمة NumbersAndOperatorsEnum :: None ، بحيث يمكن إزالة التحقق المكرر.

V728 يمكن تبسيط عملية التحقق المفرطة. The '(A && B) || تعبير (! A &&! B) 'يعادل التعبير' bool (A) == bool (B) '. Calculator Calculator.xaml.cpp 239

 void Calculator::AnimateCalculator(bool resultAnimate) { if (App::IsAnimationEnabled()) { m_doAnimate = true; m_resultAnimate = resultAnimate; if (((m_isLastAnimatedInScientific && IsScientific) || (!m_isLastAnimatedInScientific && !IsScientific)) && ((m_isLastAnimatedInProgrammer && IsProgrammer) || (!m_isLastAnimatedInProgrammer && !IsProgrammer))) { this->OnStoryboardCompleted(nullptr, nullptr); } } }

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

 if ( m_isLastAnimatedInScientific == IsScientific && m_isLastAnimatedInProgrammer == IsProgrammer) { this->OnStoryboardCompleted(nullptr, nullptr); }

V524 من الغريب أن يكون نص وظيفة "ConvertBack" مكافئًا تمامًا لجسم وظيفة "تحويل". آلة حاسبة BooleanNegationConverter.cpp 24

 Object^ BooleanNegationConverter::Convert(....) { (void) targetType; // Unused parameter (void) parameter; // Unused parameter (void) language; // Unused parameter auto boxedBool = dynamic_cast<Box<bool>^>(value); auto boolValue = (boxedBool != nullptr && boxedBool->Value); return !boolValue; } Object^ BooleanNegationConverter::ConvertBack(....) { (void) targetType; // Unused parameter (void) parameter; // Unused parameter (void) language; // Unused parameter auto boxedBool = dynamic_cast<Box<bool>^>(value); auto boolValue = (boxedBool != nullptr && boxedBool->Value); return !boolValue; }

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

الخاتمة


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

مرحبًا بك في تنزيل PVS-Studio وجربه على "الحاسبة" الخاصة بك. :-)

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

More articles:


All Articles