تحليل الكود الثابت لمجموعة مكتبة PMDK من Intel والأخطاء التي لا تعتبر أخطاء

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



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



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



باختصار ، PMDK عبارة عن مجموعة من المكتبات والأدوات مفتوحة المصدر المصممة لتبسيط تطوير وتصحيح وإدارة التطبيقات غير المتطايرة التي تدعم الذاكرة. مزيد من التفاصيل هنا: مقدمة PMDK . المصادر هنا: pmdk .



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

رمز خاطئ يعمل

حجم تخصيص الذاكرة

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

int main(int argc, char *argv[])
{
  ....
  struct pool *pop = malloc(sizeof(pop));
  ....
}

تحذير PVS-Studio: V568 من الغريب أن يقوم عامل التشغيل "sizeof ()" بتقييم حجم المؤشر للفئة ، ولكن ليس حجم كائن فئة "pop". use_ctl.c 717



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

struct pool *pop = malloc(sizeof(pool));

أو

struct pool *pop = malloc(sizeof(*pop));

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

struct pool {
  struct ctl *ctl;
};

اتضح أن الهيكل يشغل بالضبط نفس القدر من المؤشر. الأمور جيدة.

طول الخط

دعنا ننتقل إلى الحالة التالية ، حيث تم ارتكاب الخطأ مرة أخرى باستخدام sizeof المشغل .

typedef void *(*pmem2_memcpy_fn)(void *pmemdest, const void *src, size_t len,
    unsigned flags);

static const char *initial_state = "No code.";

static int
test_rwx_prot_map_priv_do_execute(const struct test_case *tc,
  int argc, char *argv[])
{
  ....
  char *addr_map = pmem2_map_get_address(map);
  map->memcpy_fn(addr_map, initial_state, sizeof(initial_state), 0);
  ....
}

تحذير PVS-Studio: V579 [CWE-687] تستقبل وظيفة memcpy_fn المؤشر وحجمه كوسائط. ربما يكون خطأ. افحص الوسيطة الثالثة. pmem2_map_prot.c 513



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



يفترض المبرمج أن حجم المشغل سيحسب حجم السلسلة الحرفية. ولكن ، في الواقع ، يتم حساب حجم المؤشر مرة أخرى.



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



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



السيناريو 1. كان من الضروري نسخ محطة الصفر. إذن فأنا مخطئ ، وهذا ليس خطأ غير ضار لا يظهر نفسه على الإطلاق. لم يتم نسخ 9 بايت ، ولكن فقط 8. لا يوجد صفر طرفي ، ولا يمكن التنبؤ بالنتائج. في هذه الحالة ، يمكن تصحيح الكود عن طريق تغيير تعريف السلسلة الثابتة initial_state على النحو التالي:

static const char initial_state [] = "No code.";

الآن قيمة sizeof (initial_state) هي 9.



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

UT_ASSERTeq(memcmp(addr_map, initial_state, strlen(initial_state)), 0);

كما ترى ، ستعيد وظيفة strlen القيمة 8 ولا يتم تضمين الصفر الطرفي في المقارنة. ثم إنه الحظ حقًا وكل شيء على ما يرام.

التحول قليلا

المثال التالي يتعلق بعملية إزاحة أحادي الاتجاه.

static int
clo_parse_single_uint(struct benchmark_clo *clo, const char *arg, void *ptr)
{
  ....
  uint64_t tmax = ~0 >> (64 - 8 * clo->type_uint.size);
  ....
}

تحذير PVS-Studio: V610 [CWE-758] سلوك غير محدد. تحقق من عامل النقل ">>". المعامل الأيسر "~ 0" سلبي. clo.cpp 205



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

أولوية العمليات

والنظر في الحالة الأخيرة المتعلقة بأولوية العمليات.

#define BTT_CREATE_DEF_SIZE  (20 * 1UL << 20) /* 20 MB */

تحذير PVS-Studio: V634 [CWE-783] أولوية العملية "*" أعلى من أولوية العملية "<<". من الممكن أن يتم استخدام الأقواس في التعبير. bttcreate.c 204



للحصول على ثابت يساوي 20 ميغا بايت قرر المبرمج القيام بما يلي:

• تحولت 1 × 20 بت للحصول على 1048576 ، أي 1 ميجا بايت.
• ضرب 1 ميغا بايت في 20.

بمعنى آخر ، يعتقد المبرمج أن العمليات الحسابية تتم على النحو التالي: (20 * (1UL << 20))



ولكن في الواقع ، فإن أولوية عامل الضرب أعلى من أولوية عامل التحويل ، ويتم تقييم التعبير على النحو التالي: ((20 * 1UL) << 20).



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



لكن هذا الخطأ لن يعبر عن نفسه بأي شكل من الأشكال. لا يهم كيف تكتب:

• (20 * 1 UL << 20)
• (20 * (1 UL << 20))
• ((20 * 1 UL) << 20)

النتيجة هي نفسها دائما ! القيمة المطلوبة دائمًا هي 20971520 ويعمل البرنامج بشكل صحيح تمامًا.

أخطاء أخرى

أقواس في المكان الخطأ

#define STATUS_INFO_LENGTH_MISMATCH 0xc0000004

static void
enum_handles(int op)
{
  ....
  NTSTATUS status;
  while ((status = NtQuerySystemInformation(
      SystemExtendedHandleInformation,
      hndl_info, hi_size, &req_size)
        == STATUS_INFO_LENGTH_MISMATCH)) {
    hi_size = req_size + 4096;
    hndl_info = (PSYSTEM_HANDLE_INFORMATION_EX)REALLOC(hndl_info,
        hi_size);
  }
  UT_ASSERT(status >= 0);
  ....
}

تحذير PVS-Studio: V593 [CWE-783] ضع في اعتبارك مراجعة التعبير من النوع "A = B == C". يتم حساب التعبير على النحو التالي: "أ = (ب == ج)". ut.c 641



ألق نظرة فاحصة هنا:

while ((status = NtQuerySystemInformation(....) == STATUS_INFO_LENGTH_MISMATCH))

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

while ((status = NtQuerySystemInformation(....)) == STATUS_INFO_LENGTH_MISMATCH)

وبسبب هذا الخطأ ، لن يعمل الماكرو UT_ASSERT أبدًا. في الواقع ، يحتوي متغير الحالة دائمًا على نتيجة المقارنة ، أي خطأ (0) أو صواب (1). ومن ثم فإن الشرط ([0..1]> = 0) صحيح دائمًا.

احتمال تسرب الذاكرة

static enum pocli_ret
pocli_args_obj_root(struct pocli_ctx *ctx, char *in, PMEMoid **oidp)
{
  char *input = strdup(in);
  if (!input)
    return POCLI_ERR_MALLOC;

  if (!oidp)
    return POCLI_ERR_PARS;
  ....
}

تحذير PVS-Studio: V773 [CWE-401] تم الخروج من الوظيفة دون تحرير مؤشر "الإدخال". من الممكن حدوث تسرب للذاكرة. pmemobjcli.c 238



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

static enum pocli_ret
pocli_args_obj_root(struct pocli_ctx *ctx, char *in, PMEMoid **oidp)
{
  if (!oidp)
    return POCLI_ERR_PARS;

  char *input = strdup(in);
  if (!input)
    return POCLI_ERR_MALLOC;
  ....
}

أو يمكنك تحرير الذاكرة بشكل صريح:

static enum pocli_ret
pocli_args_obj_root(struct pocli_ctx *ctx, char *in, PMEMoid **oidp)
{
  char *input = strdup(in);
  if (!input)
    return POCLI_ERR_MALLOC;

  if (!oidp)
  {
    free(input);
    return POCLI_ERR_PARS;
  }
  ....
}

تجاوز محتمل

typedef long long os_off_t;

void
do_memcpy(...., int dest_off, ....., size_t mapped_len, .....)
{
  ....
  LSEEK(fd, (os_off_t)(dest_off + (int)(mapped_len / 2)), SEEK_SET);
  ....
}

تحذير PVS-Studio: V1028 [CWE-190] تجاوز محتمل. ضع في اعتبارك صب المعاملات ، وليس النتيجة. memcpy_common.c 62 ليس من المنطقي أن



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



أعتقد أنه سيكون من الأصح أن تكتب مثل هذا:

LSEEK(fd, dest_off + (os_off_t)(mapped_len) / 2, SEEK_SET);

هنا يتم تحويل القيمة غير الموقعة من النوع size_t إلى قيمة موقعة (بحيث لا يوجد تحذير من المترجم). وفي الوقت نفسه ، بالتأكيد لن يكون هناك فائض أثناء الإضافة.

حماية تجاوز غير صحيحة

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

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



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

عدم التحقق من أن الذاكرة قد تم تخصيصها بنجاح

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

static void
remove_extra_node(TOID(struct tree_map_node) *node)
{
  ....
  unsigned char *new_key = (unsigned char *)malloc(new_key_size);
  assert(new_key != NULL);
  memcpy(new_key, D_RO(tmp)->key, D_RO(tmp)->key_size);
  ....
}

تحذير PVS-Studio: V575 [CWE-628] يتم تمرير المؤشر الفارغ المحتمل إلى وظيفة "memcpy". افحص الحجة الأولى. تحقق من السطور: 340 ، 338. rtree_map.c 340 في



مكان آخر لا يوجد حتى تأكيد :

static void
calc_pi_mt(void)
{
  ....
  HANDLE *workers = (HANDLE *) malloc(sizeof(HANDLE) * pending);
  for (i = 0; i < pending; ++i) {
    workers[i] = CreateThread(NULL, 0, calc_pi,
      &tasks[i], 0, NULL);
    if (workers[i] == NULL)
      break;
  }
  ....
}

تحذير PVS-Studio: V522 [CWE-690] قد يكون هناك إشارة مرجعية لمؤشر فارغ محتمل "عمال". تحقق من الأسطر: 126 ، 124



. لذلك لا أرى أي سبب لإدراجهم جميعًا في المقالة.



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

كود الشم

استدعاء مزدوج CloseHandle

static void
prepare_map(struct pmem2_map **map_ptr,
  struct pmem2_config *cfg, struct pmem2_source *src)
{
  ....
  HANDLE mh = CreateFileMapping(....);
  ....
  UT_ASSERTne(CloseHandle(mh), 0);
  ....
}

تحذير PVS-Studio: V586 [CWE-675] يتم استدعاء وظيفة "CloseHandle" مرتين لإلغاء تخصيص نفس المورد. pmem2_map.c 76



بالنظر إلى هذا الرمز وتحذير PVS-Studio ، من الواضح أنه لا يوجد شيء واضح. أين يمكنني الاتصال بـ CloseHandle مرة أخرى ؟ للعثور على الإجابة ، دعنا نلقي نظرة على تنفيذ ماكرو UT_ASSERTne .

#define UT_ASSERTne(lhs, rhs)\
  do {\
    /* See comment in UT_ASSERT. */\
    if (__builtin_constant_p(lhs) && __builtin_constant_p(rhs))\
      UT_ASSERT_COMPILE_ERROR_ON((lhs) != (rhs));\
    UT_ASSERTne_rt(lhs, rhs);\
  } while (0)

لم يصبح الأمر أكثر وضوحا. ما هو UT_ASSERT_COMPILE_ERROR_ON ؟ ما هو UT_ASSERTne_rt ؟



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

do {
  if (0 && 0) (void)((CloseHandle(mh)) != (0));
  ((void)(((CloseHandle(mh)) != (0)) ||
    (ut_fatal(".....", 76, __FUNCTION__, "......: %s (0x%llx) != %s (0x%llx)",
              "CloseHandle(mh)", (unsigned long long)(CloseHandle(mh)), "0",
              (unsigned long long)(0)), 0))); } while (0);

دعنا نزيل الشرط الخاطئ دائمًا (0 && 0) وبشكل عام ، كل شيء غير ذي صلة. اتضح:

((void)(((CloseHandle(mh)) != (0)) ||
  (ut_fatal(...., "assertion failure: %s (0x%llx) != %s (0x%llx)",
            ....., (unsigned long long)(CloseHandle(mh)), .... ), 0)));

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



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

عدم اتساق واجهة التنفيذ (إزالة الثابت)

static int
status_push(PMEMpoolcheck *ppc, struct check_status *st, uint32_t question)
{
  ....
  } else {
    status_msg_info_and_question(st->msg);            // <=
    st->question = question;
    ppc->result = CHECK_RESULT_ASK_QUESTIONS;
    st->answer = PMEMPOOL_CHECK_ANSWER_EMPTY;
    PMDK_TAILQ_INSERT_TAIL(&ppc->data->questions, st, next);
  }
  ....
}

يعرض المحلل الرسالة: V530 [CWE-252] مطلوب استخدام قيمة الإرجاع للوظيفة "status_msg_info_and_question". check_util.c 293



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

static inline int
status_msg_info_and_question(const char *msg)
{
  char *sep = strchr(msg, MSG_SEPARATOR);
  if (sep) {
    *sep = ' ';
    return 0;
  }
  return -1;
}

عند استدعاء دالة strchr ، تحدث إزالة ضمنية للثابت. الحقيقة هي أنه في C يتم الإعلان عنها على النحو التالي:

char * strchr ( const char *, int );

ليس الحل الأفضل. لكن لغة C ما هي عليه :).



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



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

static inline int
status_msg_info_and_question(char *msg)
{
  char *sep = strchr(msg, MSG_SEPARATOR);
  if (sep) {
    *sep = ' ';
    return 0;
  }
  return -1;
}

لذلك تصبح النوايا أكثر وضوحًا على الفور ، وسيظل المحلل صامتًا.

كود معقد للغاية

static struct memory_block
heap_coalesce(struct palloc_heap *heap,
  const struct memory_block *blocks[], int n)
{
  struct memory_block ret = MEMORY_BLOCK_NONE;

  const struct memory_block *b = NULL;
  ret.size_idx = 0;
  for (int i = 0; i < n; ++i) {
    if (blocks[i] == NULL)
      continue;
    b = b ? b : blocks[i];
    ret.size_idx += blocks[i] ? blocks[i]->size_idx : 0;
  }
  ....
}

تحذير PVS-Studio: V547 [CWE-571] التعبير "كتل [i]" دائمًا صحيح. heap.c 1054



إذا كتل [أنا] == NULL ، ثم يستمر سيتم تشغيل بيان وسوف تبدأ حلقة التكرار التالي. لذلك ، فإن إعادة فحص كتل العناصر [i] لا معنى لها والمشغل الثلاثي غير ضروري. يمكن تبسيط الكود:

....
for (int i = 0; i < n; ++i) {
  if (blocks[i] == NULL)
    continue;
  b = b ? b : blocks[i];
  ret.size_idx += blocks[i]->size_idx;
}
....

استخدام مريب لمؤشر فارغ

void win_mmap_fini(void)
{
  ....
  if (mt->BaseAddress != NULL)
    UnmapViewOfFile(mt->BaseAddress);
  size_t release_size =
    (char *)mt->EndAddress - (char *)mt->BaseAddress;
  void *release_addr = (char *)mt->BaseAddress + mt->FileLen;
  mmap_unreserve(release_addr, release_size - mt->FileLen);
  ....
}

تحذير PVS-Studio: V1004 [CWE-119] تم استخدام المؤشر '(char *) mt-> BaseAddress' بشكل غير آمن بعد التحقق من أنه مقابل nullptr. الاختيار خطوط: 226، 235. win_mmap.c 235



و mt-> BaseAddress المؤشر يمكن أن يكون باطلا، كما يدل على ذلك الاختيار:

if (mt->BaseAddress != NULL)

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

size_t release_size =
  (char *)mt->EndAddress - (char *)mt->BaseAddress;

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

أسماء مختصرة للمتغيرات العالمية

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

static struct critnib *c;

تحذيرات PVS-Studio لهذه المتغيرات:

• V707 يعتبر إعطاء أسماء مختصرة للمتغيرات العالمية ممارسة سيئة. يُقترح إعادة تسمية متغير "ri". map.c 131
• V707 يعتبر إعطاء أسماء مختصرة للمتغيرات العالمية ممارسة سيئة. يُقترح إعادة تسمية متغير "c". 56
• V707 يعتبر إعطاء أسماء مختصرة للمتغيرات العالمية ممارسة سيئة. يُقترح إعادة تسمية متغير "Id". obj_list.h 68
• V707 يعتبر إعطاء أسماء مختصرة للمتغيرات العالمية ممارسة سيئة. يُقترح إعادة تسمية متغير "Id". obj_list.c 34

غريب

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

void
do_memmove(char *dst, char *src, const char *file_name,
    size_t dest_off, size_t src_off, size_t bytes,
    memmove_fn fn, unsigned flags, persist_fn persist)
{
  ....
  /* do the same using regular memmove and verify that buffers match */
  memmove(dstshadow + dest_off, dstshadow + dest_off, bytes / 2);
  verify_contents(file_name, 0, dstshadow, dst, bytes);
  verify_contents(file_name, 1, srcshadow, src, bytes);
  ....
}

تحذير PVS-Studio: V549 [CWE-688] الوسيطة الأولى لوظيفة "memmove" تساوي الوسيطة الثانية. memmove_common.c 71



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

• كنت أرغب في "لمس" كتلة الذاكرة. لكن هل سيحدث هذا في الواقع؟ هل سيقوم مترجم التحسين بإزالة الكود الذي ينسخ كتلة الذاكرة إلى نفسها؟
• هذا نوع من اختبار الوحدة لوظيفة memmove .
• يحتوي الرمز على خطأ مطبعي.

وإليك مقتطف غريب بنفس الدرجة في نفس الوظيفة:

void
do_memmove(char *dst, char *src, const char *file_name,
    size_t dest_off, size_t src_off, size_t bytes,
    memmove_fn fn, unsigned flags, persist_fn persist)
{
  ....
  /* do the same using regular memmove and verify that buffers match */
  memmove(dstshadow + dest_off, srcshadow + src_off, 0);
  verify_contents(file_name, 2, dstshadow, dst, bytes);
  verify_contents(file_name, 3, srcshadow, src, bytes);
  ....
}

تحذير PVS-Studio: V575 [CWE-628] تعالج وظيفة "memmove" العناصر "0". افحص الوسيطة الثالثة. memmove_common.c 82



تحرك الدالة 0 بايت. ما هذا؟ اختبار الوحدة؟ خطأ مطبعي؟



بالنسبة لي ، هذا الرمز غير مفهوم وغريب.

لماذا استخدام محللات الكود؟

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



شكرآ لك على أهتمامك!

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