Ошибки в роботах: ожидание и реальность

    Робо-ошибки

    Мне кажется, бунт роботов не близок. Я начал писать статью про то, что наша команда приступила к адаптации анализатора кода PVS-Studio для нужд Embedded-разработчиков. Для начала мы поддержали KEIL и IAR. Посмотрев на ошибки в некоторых проектах для встроенных устройств, я полон чувств, которыми хочу поделиться. Проще всего это будет сделать, показав пару картинок и пару примеров ошибок.

    Итак, с чем ассоциируются у нас ошибки в роботах? Воображение обывателя рисует вот такую картину:

    Ожидание

    Однако, когда я вижу в коде операционной системы RT-Thread вот такие ошибки, я думаю, что всё иначе.
    Несколько наиболее простых и забавных ошибок, которые я нашел с помощью анализатора кода PVS-Studio в проекте RT-Thread.

    RT-Thread is an open source IoT operating system from China, which has strong scalability: from a tiny kernel running on a tiny core, for example ARM Cortex-M0, or Cortex-M3/4/7, to a rich feature system running on MIPS32, ARM Cortex-A8, ARM Cortex-A9 DualCore etc. github.com/RT-Thread/rt-thread

    Предупреждения PVS-Studio:

    V560 CWE-571 A part of conditional expression is always true: 0xFFFF0000. peci.c 372
    V560 CWE-571 A part of conditional expression is always true: 0x0000FFFF. peci.c 373

    #define PECI_M0D0C_HITHR_M      0xFFFF0000  // High Threshold
    #define PECI_M0D0C_LOTHR_M      0x0000FFFF  // Low Threshold
    
    void
    PECIDomainConfigGet(....)
    {
      unsigned long ulTemp;
      ....
      ulTemp = HWREG(ulBase + PECI_O_M0D0C + (ulDomain * 4));
      *pulHigh =
        ((ulTemp && PECI_M0D0C_HITHR_M) >> PECI_M0D0C_HITHR_S);
      *pulLow =
        ((ulTemp && PECI_M0D0C_LOTHR_M) >> PECI_M0D0C_LOTHR_S);
    }
    

    Вместо && следовало написать &.

    Предупреждение PVS-Studio:
    V767 Suspicious access to element of 'w' array by a constant index inside a loop. fsl_dcp.c 946

    typedef union _dcp_hash_block
    {
        uint32_t w[DCP_HASH_BLOCK_SIZE / 4];
        uint8_t b[DCP_HASH_BLOCK_SIZE];
    } dcp_hash_block_t;
    
    typedef struct _dcp_hash_ctx_internal
    {
      dcp_hash_block_t blk;
      ....
    } dcp_hash_ctx_internal_t;
    
    status_t DCP_HASH_Init(DCP_Type *base, dcp_handle_t *handle,
                           dcp_hash_ctx_t *ctx, dcp_hash_algo_t algo)
    {
      ....
      dcp_hash_ctx_internal_t *ctxInternal;
      ....
      for (i = 0; i < sizeof(ctxInternal->blk.w) /
                                sizeof(ctxInternal->blk.w[0]); i++)
      {
         ctxInternal->blk.w[0] = 0u;
      }
      ....
    }
    

    Записываем 0 в одну и ту же ячейку массива. Большая часть массива останется неинициализированной. Должно быть написано:

    ctxInternal->blk.w[i] = 0u;

    Предупреждения PVS-Studio:

    V602 CWE-480 Consider inspecting the '(1U < 1)' expression. '<' possibly should be replaced with '<<'. fsl_aipstz.h 69
    V602 CWE-480 Consider inspecting the '(1U < 2)' expression. '<' possibly should be replaced with '<<'. fsl_aipstz.h 70
    V602 CWE-480 Consider inspecting the '(1U < 2)' expression. '<' possibly should be replaced with '<<'. fsl_aipstz.h 71

    typedef enum _aipstz_peripheral_access_control {
      kAIPSTZ_PeripheralAllowUntrustedMaster = 1U,
      kAIPSTZ_PeripheralWriteProtected = (1U < 1),
      kAIPSTZ_PeripheralRequireSupervisor = (1U < 2),
      kAIPSTZ_PeripheralAllowBufferedWrite = (1U < 2)
    } aipstz_peripheral_access_control_t;
    

    Вместо < следовало использовать <<. Тогда константы станут равны различным степеням числа 2.

    Когда Терминатор подъедет к полицейскому участку, чтобы убить Сару Коннор, будет так:

    Реальность

    Так что пока можно спать спокойно.



    Подождите, я ведь собираюсь писать статью, где расскажу, что PVS-Studio будет находить такие ошибки… Упс!
    PVS-Studio 493,58
    Ищем ошибки в C, C++ и C# на Windows, Linux, macOS
    Поделиться публикацией
    Комментарии 24
      +1
      нуу… когда kAIPSTZ_PeripheralAllowUntrustedMaster == kAIPSTZ_PeripheralWriteProtected это пять… :D
        +2
        Так ведь именно эти константы и не равны?
          0
          Метрическая сила! Мозги мои — плавленные сырки :( Посыпаю лысину пеплом
        0

        О нет! Я наивно полагала, что только писишный софт умудряется работать (!) несмотря на тысячи ошибок (но как?). А такое творится ещё и в эмбеддеде! Но как оно работает при этом?

          +1
          Такое, как в статье, не работает. =)
            0
            Наивно полагать, что кривые битовые маски не работают.
              0
              Да кривые битовые маски — это ещё самое наменьшее зло. =)
            0
            Открою секрет… этот код может вообще не вызываться.
              +3
              Открою ужасающий секрет — embedded-софт тестируют как минимум на порядок меньше и хуже, чем любой зачуханный веб-сервер. И пишут его зачастую динозавры, отладчик — это для слабаков, а «если код компилируется, значит, все работает».
              Код на С. Компилируется. Значит, работает.
              Я не шучу, к сожалению.

              И никто этого не замечает, потому что в веб-сервер каждый день тыкают пальцами миллионы пользователей, и любые ошибки выплывают гораздо быстрее.
              –1
              Выбрал ряд предупреждений, которые на мой взгляд могут представлять интерес и выгрузил их в полный HTML-лог (с навигацией по коду). Интересующиеся могут скачать архив rt-thread-html-log.zip и посмотреть, что есть ещё интересного. Заодно, это повод познакомиться с нашим новым форматом отчётов.

              Или можно подождать статью, где я разберу ошибки в проекте RT-Thread и расскажу про поддержку KEIL, IAR.
                0
                Так что там с IAR`ом? В двух словах… Уже есть поддержка или пока ещё ждём?
                  0
                  Доделываем. Будет в следующем релизе.
                    0
                    Встречайте PVS-Studio 6.22:

                    • Добавлена поддержка проверки проектов, использующих компиляторы Keil MDK ARM Compiler 5 и ARM Compiler 6.
                    • Добавлена поддержка проверки проектов, использующих компилятор IAR C/C++ Compiler for ARM.
                      0
                      Спасибо. Как-раз устанавливаю.
                    +1
                    А кейл? Кейл? Кейл-то как? Как он, родненький?
                    Джва года
                    Боже мой, да я пять лет жду такую новость!
                      0
                      Ой, ну не переживайте так. Если хотите, могу (наверное) beta-версию выдать. :)
                        0
                        Если можно, то очень хочу :) Узнайте, пожалуйста!
                          0
                          Выложили бету, можете попробовать.
                            0
                            Спасибо большое!
                            Мне уже неловко вам писать, но… а как ей пользоваться? -_-' Я попробовал запустить standalone и мониторить запуски компилятора, но компиляцию в Кейле она не увидела. В Кейл ничего не интегрировалось (я не особо верил, но вдруг?).

                            Наверное, я поторопился; надо было подождать вашей следующей статьи.
                              0
                              1. Попробуйте запустить Standalone от администратора;
                              2. Какой версии ваш компилятор Keil? 5/6.
                                0
                                Попробовал из под администратора, попробовал и 5 (5.06 update 4 build 422) и 6.6, никаких изменений.
                                  0
                                  Перешли в ЛС для выяснения деталей запуска.
                    +2
                    Нисколько не принижая важность статического анализа кода, считаю необходимым написать, что даже
                    *pulHigh = ((ulTemp & PECI_M0D0C_HITHR_M) >> PECI_M0D0C_HITHR_S);
                    НИКАК не может считаться хорошим стилем по двум причинам:
                    1) используются две константы, которые должны изменяться синхронно, а задаются просто так;
                    2) данный фрагмент используется многократно и ДОЛЖЕН быть заменен inline функцией или даже макросом, при всех недостатках последнего, хуже, чем есть, точно не будет.

                      Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

                      Самое читаемое