Статический анализатор кода PVS-Studio 6.22 адаптирован для ARM-компиляторов (Keil, IAR)

    Embedded bugs

    PVS-Studio — это статический анализатор кода для поиска ошибок и потенциальных уязвимостей в коде программ на языке C, C++ и C#. Мы давно радуем читателей нашего блога проверкой открытых проектов и разбором найденных ошибок. Наши статьи имеют потенциал стать более интересными, так как анализатор научился проверять код встроенных устройств. Мы поддержали несколько ARM-компиляторов, про которые подробнее вы узнаете из статьи. Ошибки во встроенных устройствах и роботах могут быть более зрелищными, чем в прикладных программах. Ошибка во встроенном устройстве — это не просто падение/зависание программы или неправильная картинка. Это сошедший с ума Wi-Fi-чайник, который будет кипятить воду, пока она не выкипит и не сработает термопредохранитель. В общем, с ошибками в мире embedded-систем всё обстоит куда интереснее и страшнее.

    Мой самый зрелищный баг


    За свою карьеру программиста я сделал немало ошибок в коде. Однако ошибки в каком-то смысле были скучными. Что-то не так работало, где-то разыменовывался нулевой указатель и так далее. Да, это были настоящие ошибки, которые требовали исправления. Однако самое яркое впечатление от собственной ошибки я получил, развлекаясь с самодельными роботами.

    В робототехнике я любитель и все мои поделки носят исключительно экспериментально-развлекательный характер. Одной из поделок стало создание четырёх небольших роботов, управляемых с дистанционных пультов и умеющих играть в робофутбол и «ловлю мышей». Не буду вдаваться в детали, отмечу только, что они умели: ездить, бить по мячику, хватать клешней, издавать звуки, мигать светодиодами. Собственно, чтобы не быть голословным, вот один из роботов.

    Бот


    Бот реализован на базе микроконтроллера ATmega8A (8 Кбайт Flash, 512 байт EEPROM, 1 Кбайт RAM). В первом варианте программы один из таймеров микроконтроллера генерировал прерывание, в обработчике которого читались команды с пульта дистанционного управления. Если были какие-то команды, то они записывались в FIFO-буфер, из которого затем извлекались и исполнялись в главном цикле программы. Команды были такие, как: едем вперёд/назад; разворачиваемся влево/вправо; едем вперёд, поворачивая немного влево; хватаем мышку; пинаем мячик и так далее.

    На самом деле, я всё переусложнил. Позже я избавился от FIFO-буфера и вообще написал всё более просто и красиво.

    Теперь представьте: я заливаю в микроконтроллер новую программу, включаю робота, и… Неожиданно робот начинает жить своей собственной жизнью!

    Оно живое!


    Бот хаотично носится по полу, щелкает клешнёй, толкает несуществующий мячик, мигает. Причём, действия совершенно мне непонятны. В роботе просто нет кода, который, на мой взгляд, мог бы стать причиной таких действий.

    Это было самое большое впечатление от ошибки в программе, полученное мною за все мои годы программирования. Одно дело, когда программа падает из-за переполнения стека, и совсем другое, когда перед тобой носится чокнутый робот, которого ты сам сделал и даже не понимаешь, как такое может быть. Жаль, что я не догадался заснять это действо и свои эмоции на заднем плане :).

    После недолгого разбирательства выяснилась, что я допустил одну из самых классических программистских ошибок: переменная, хранящая количество необработанных команд в FIFO-буфере, оказалась неинициализированной. Робот начал выполнять случайную последовательность команд, читая данные из буфера, а также данные, находящиеся уже после буфера.

    Зачем я рассказал эту историю? Чтобы просто показать, что ошибки в программах микроконтроллеров могут быть более зрелищными, и я надеюсь, что смогу порадовать в будущем читателя интересными публикациями. Теперь же давайте вернемся к основной теме статьи, посвященной выходу новой версии анализатора PVS-Studio.

    PVS-Studio 6.22


    В новой версии анализатора PVS-Studio 6.22 наша команда доработала инфраструктуру для проверки проектов следующего типа:
    1. Появилась поддержка ARM Compiler 5 и ARM Compiler 6 в составе среды Keil uVision 5.
    2. Компиляторов ARM Compiler 5 и ARM Compiler 6 в составе среды Keil DS-MDK.
    3. Мы поддерживаем IAR C/C++ Compiler for ARM в составе среды IAR Embedded Workbench.

    Проект RT-Thread


    Для демонстрации новых возможностей PVS-Studio мне понадобился открытый проект, и выбор пал на RT-Thread. Этот проект можно собрать в режиме gcc/keil/iar. С целью дополнительного тестирования анализатора мы проверили его как в режиме Keil, так и в режиме IAR. Отчёты получились практически одинаковыми, так что я даже не помню, с каким из них я в дальнейшем работал.

    Давайте теперь немного поговорим о самом проекте RT-Thread.

    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.

    Официальный сайт: rt-thread.org.

    Исходный код: rt-thread.

    Думаю, операционная система RT-Thread — очень хороший кандидат, чтобы стать первым embedded-проектом, проверенным с помощью PVS-Studio.

    Ошибки, замеченные в проекте RT-Thread


    Я бегло просмотрел отчёт анализатора PVS-Studio и отобрал 95 предупреждений, представляющих, на мой взгляд, наибольший интерес. Вы можете ознакомиться с ними, скачав архив rt-thread-html-log.zip с полным HTML-отчётом. Этот формат мы реализовали недавно, и не все пользователи знают про него. Поэтому я решил воспользоваться подходящей возможностью, чтобы вновь написать о нём. Вот как выглядит этот отчёт, открытый в Firefox:

    PVS-Studio Full HTML log


    Отчёт сделан по аналогии с HTML отчётом, который генерирует анализатор Clang. Отчёт хранит в себе часть исходного кода, и вы можете сразу увидеть, к какому фрагменту программы относятся предупреждения. Просмотр одного из предупреждений выглядит следующим образом:

    Просмотр предупреждения в HTML отчёте


    Нет смысла рассматривать в статье все 95 предупреждений, так как многие из них похожи. В статье приведу только 14 фрагментов кода, которые мне показались по какой-то причине заслуживающими описания.

    Примечание. Я мог пропустить важные предупреждения, указывающие на серьёзные ошибки. Поэтому прошу разработчиков RT-Thread не полагаться только на мой отчёт, содержащий 95 предупреждений, а провести анализ проекта самостоятельно. Вдобавок мне кажется, мы не разобрались как следует с проектом RT-Thread и проверили только какую-то его часть.

    Фрагмент N1. CWE-562: Return of Stack Variable Address


    void SEMC_GetDefaultConfig(semc_config_t *config)
    {
      assert(config);
    
      semc_axi_queueweight_t queueWeight; /*!< AXI queue weight. */
      semc_queuea_weight_t queueaWeight;
      semc_queueb_weight_t queuebWeight;
    
      ....
    
      config->queueWeight.queueaWeight = &queueaWeight;
      config->queueWeight.queuebWeight = &queuebWeight;
    }

    Предупреждение PVS-Studio: V506 CWE-562 Pointer to local variable 'queuebWeight' is stored outside the scope of this variable. Such a pointer will become invalid. fsl_semc.c 257

    Функция записывает во внешнюю структуру адреса двух локальных переменных (queueaWeight и queuebWeight). После выхода из функции переменные перестают существовать, но структура по-прежнему будет хранить и использовать указатели на эти уже несуществующие объекты. Фактически, указатели указывают в какое-то место стека, в котором может быть всё что угодно. Это очень неприятная ошибка безопасности.

    Анализатор PVS-Studio сообщает только о последнем подозрительном присваивании, что связано с некоторыми внутренними особенностями его работы. Однако, если последнее присваивание удалить или исправить, то анализатор начнёт предупреждать о первом присваивании.

    Фрагмент N2. CWE-570: Expression is Always False


    #define CAN_FIFO0   ((uint8_t)0x00U)   /*!< receive FIFO0 */
    #define CAN_FIFO1   ((uint8_t)0x01U)   /*!< receive FIFO1 */
    
    uint8_t can_receive_message_length(uint32_t can_periph,
                                       uint8_t fifo_number)
    {
      uint8_t val = 0U;
        
      if(CAN_FIFO0 == fifo_number){
        val = (uint8_t)(CAN_RFIFO0(can_periph) & CAN_RFIFO_RFL0_MASK);
      }else if(CAN_FIFO0 == fifo_number){
        val = (uint8_t)(CAN_RFIFO1(can_periph) & CAN_RFIFO_RFL0_MASK);
      }else{
        /* illegal parameter */
      }
      return val;
    }

    Предупреждение PVS-Studio: V517 CWE-570 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 525, 527. gd32f4xx_can.c 525

    Если аргумент fifo_number не равен CAN_FIFO0, то функция всегда возвращает 0. Код, скорее всего, писался с помощью Copy-Paste и по невнимательности в скопированном фрагменте забыли заменить константу CAN_FIFO0 на CAN_FIFO1.

    Фрагмент N3. CWE-571: Expression is Always True


    #define PECI_M0D0C_HITHR_M      0xFFFF0000  // High Threshold
    #define PECI_M0D0C_LOTHR_M      0x0000FFFF  // Low Threshold
    #define PECI_M0D0C_HITHR_S      16
    #define PECI_M0D0C_LOTHR_S      0
    
    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:

    • 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

    Две досадные опечатки: вместо двух операторов & дважды использовали оператор &&.

    Из-за этого переменной pulHigh всегда будет присваиваться значение 0, а переменной pulLow будет присвоено значение 0 или 1. Это явно не то, что задумывал программист.

    Пояснение для тех, кто слабо знаком с языком C. Результатом выражения (ulTemp && PECI_M0D0C_xxxxx_M) всегда является 0 или 1. Далее 0 или 1 сдвигаются вправо. При сдвиге вправо значения 0/1 на 16 бит всегда будет получаться 0. При сдвиге значения 0/1 на 0 бит по-прежнему получается 0 или 1.

    Фрагмент N4. CWE-480: Use of Incorrect Operator


    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;

    Единорог-facepalm


    Предупреждения 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

    Именованные константы должны были являться степенями двойки и быть равны следующим значениям: 1, 2, 4, 4. Но вместо оператора << программист случайно написал <. В результате получаются следующие значения:

    • kAIPSTZ_PeripheralAllowUntrustedMaster = 1
    • kAIPSTZ_PeripheralWriteProtected = 0
    • kAIPSTZ_PeripheralRequireSupervisor = 1
    • kAIPSTZ_PeripheralAllowBufferedWrite = 1

    Фрагмент N5. CWE-834: Excessive Iteration


    static int ft5x06_dump(void)
    {
      uint8_t i;
      uint8_t reg_value;
        
      DEBUG_PRINTF("[FTS] Touch Chip\r\n");
            
      for (i = 0; i <= 255; i++)
      {
        _ft5x06_read(i, &reg_value, 1);
        
        if (i % 8 == 7)
          DEBUG_PRINTF("0x%02X = 0x%02X\r\n", i, reg_value);
        else
          DEBUG_PRINTF("0x%02X = 0x%02X ", i, reg_value);
      }
      DEBUG_PRINTF("\n");
       
      return 0;
    }

    Предупреждение PVS-Studio: V654 CWE-834 The condition 'i <= 255' of loop is always true. drv_ft5x06.c 160

    Переменные типа uint8_t могут хранить значения в диапазоне [0..255], поэтому условие i <= 255 всегда истинно. Из-за этого цикл будет бесконечно распечатывать отладочные данные.

    Фрагмент N6. CWE-571: Expression is Always True


    #define RT_CAN_MODE_NORMAL              0
    #define RT_CAN_MODE_LISEN               1
    #define RT_CAN_MODE_LOOPBACK            2
    #define RT_CAN_MODE_LOOPBACKANLISEN     3
    
    static rt_err_t control(struct rt_can_device *can,
                            int cmd, void *arg)
    {
      ....
      case RT_CAN_CMD_SET_MODE:
        argval = (rt_uint32_t) arg;
        if (argval != RT_CAN_MODE_NORMAL ||
            argval != RT_CAN_MODE_LISEN ||
            argval != RT_CAN_MODE_LOOPBACK ||
            argval != RT_CAN_MODE_LOOPBACKANLISEN)
        {
          return RT_ERROR;
        }
        if (argval != can->config.mode)
        {
          can->config.mode = argval;
          return bxcan_set_mode(pbxcan->reg, argval);
        }
        break;
      ....
    }

    Предупреждение PVS-Studio: V547 CWE-571 Expression is always true. Probably the '&&' operator should be used here. bxcan.c 1171

    Случай RT_CAN_CMD_SET_MODE всегда обрабатывается неверно. Дело в том, что условие вида (x !=0 || x != 1 || x != 2 || x != 3) всегда является истинным. Скорее всего, мы имеем дело с очередной опечаткой и, на самом деле, здесь должно быть написано:

    if (argval != RT_CAN_MODE_NORMAL &&
        argval != RT_CAN_MODE_LISEN &&
        argval != RT_CAN_MODE_LOOPBACK &&
        argval != RT_CAN_MODE_LOOPBACKANLISEN)

    Фрагмент N7. CWE-687: Function Call With Incorrectly Specified Argument Value


    void MCAN_SetSTDFilterElement(CAN_Type *base,
      const mcan_frame_filter_config_t *config,
      const mcan_std_filter_element_config_t *filter,
      uint8_t idx)
    {
      uint8_t *elementAddress = 0;
      elementAddress = (uint8_t *)(MCAN_GetMsgRAMBase(base) +
                                   config->address + idx * 4U);
      memcpy(elementAddress, filter, sizeof(filter));
    }

    Анализатор указывает на ошибку сразу двумя разными предупреждениями:

    • V579 CWE-687 The memcpy function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. fsl_mcan.c 418
    • V568 It's odd that 'sizeof()' operator evaluates the size of a pointer to a class, but not the size of the 'filter' class object. fsl_mcan.c 418

    Функция memcpy копирует не всю структуру типа mcan_std_filter_element_config_t, а только её часть, равную размеру одного указателя.

    Фрагмент N8. CWE-476: NULL Pointer Dereference


    Не обошлось в коде RT-Thread без ошибок, когда указатель разыменовывается до его проверки. Это очень распространённый тип ошибки.

    static rt_size_t rt_sdcard_read(rt_device_t dev,
                                    rt_off_t    pos,
                                    void       *buffer,
                                    rt_size_t   size)
    {
      int i, addr;
      struct dfs_partition *part =
        (struct dfs_partition *)dev->user_data;
    
      if (dev == RT_NULL)
      {
        rt_set_errno(-EINVAL);
        return 0;
      }
      ....
    }

    Предупреждение PVS-Studio: V595 CWE-476 The 'dev' pointer was utilized before it was verified against nullptr. Check lines: 497, 499. sdcard.c 497

    Фрагмент N9. CWE-563: Assignment to Variable without Use


    static void enet_default_init(void)
    {
      ....
      reg_value = ENET_DMA_BCTL;
      reg_value &= DMA_BCTL_MASK;
      reg_value = ENET_ADDRESS_ALIGN_ENABLE 
                 |ENET_ARBITRATION_RXTX_2_1
                 |ENET_RXDP_32BEAT |ENET_PGBL_32BEAT 
                 |ENET_RXTX_DIFFERENT_PGBL
                 |ENET_FIXED_BURST_ENABLE |ENET_MIXED_BURST_DISABLE
                 |ENET_NORMAL_DESCRIPTOR;
      ENET_DMA_BCTL = reg_value; 
      ....
    }

    Предупреждение PVS-Studio: V519 CWE-563 The 'reg_value' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 3427, 3428. gd32f4xx_enet.c 3428

    Присваивание reg_value = ENET_ADDRESS_ALIGN_ENABLE|.... перетирает предыдущее значение переменной reg_value. Это странно, так как в переменной хранится результат осмысленных вычисления. Скорее всего, код должен быть таким:

    reg_value = ENET_DMA_BCTL;
    reg_value &= DMA_BCTL_MASK;
    reg_value |= ENET_ADDRESS_ALIGN_ENABLE 
               |ENET_ARBITRATION_RXTX_2_1
               |ENET_RXDP_32BEAT |ENET_PGBL_32BEAT 
               |ENET_RXTX_DIFFERENT_PGBL
               |ENET_FIXED_BURST_ENABLE |ENET_MIXED_BURST_DISABLE
               |ENET_NORMAL_DESCRIPTOR;

    Фрагмент N10. CWE-665: Improper Initialization


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

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

    Анализатор не смог сопоставить с этим предупреждением никакой CWE ID, однако по смыслу это должно быть CWE-665: Improper Initialization.

    В цикле значение 0 записывается всё время в нулевой элемент массива, в то время как остальные элементы остаются неинициализированными.

    Фрагмент N11. CWE-571: Expression is Always True


    static void at91_mci_init_dma_read(struct at91_mci *mci)
    {
      rt_uint8_t i;
      ....
      for (i = 0; i < 1; i++) 
      {
        /* Check to see if this needs filling */
        if (i == 0) 
        {
          if (at91_mci_read(AT91_PDC_RCR) != 0) 
          {
            mci_dbg("Transfer active in current\n");
            continue;
          }
        }
        else {
          if (at91_mci_read(AT91_PDC_RNCR) != 0)
          {
            mci_dbg("Transfer active in next\n");
            continue;
          }
        }
    
        length = data->blksize * data->blks;
        mci_dbg("dma address = %08X, length = %d\n",
                data->buf, length);
    
        if (i == 0) 
        {
          at91_mci_write(AT91_PDC_RPR, (rt_uint32_t)(data->buf));
          at91_mci_write(AT91_PDC_RCR, .....);
        }
        else 
        {
          at91_mci_write(AT91_PDC_RNPR, (rt_uint32_t)(data->buf));
          at91_mci_write(AT91_PDC_RNCR, .....);
        }
      }
      ....
    }

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

    • V547 CWE-571 Expression 'i == 0' is always true. at91_mci.c 196
    • V547 CWE-571 Expression 'i == 0' is always true. at91_mci.c 215

    Тело цикла выполняется ровно один раз. Это не имеет смысла. Зачем тогда вообще писать цикл?

    Более того, так как в теле цикла переменная i всегда равна 0, то некоторые условия всегда истинные и часть кода никогда не выполняется.

    Мне кажется, на самом деле, разработчик планировал, чтобы тело цикла выполнялось 2 раза, но допустил опечатку. Возможно, следовало написать условие цикла так:

    for (i = 0; i <= 1; i++) 

    В этом случае код функции обретёт смысл.

    Фрагмент N12. CWE-457: Use of Uninitialized Variable


    Прощу прощения, что приведу большой фрагмент тела функции. Это необходимо, чтобы продемонстрировать, что переменная k действительно нигде не инициализируется перед чтением из неё значения.

    void LCD_PutPixel (LCD_PANEL panel, uint32_t X_Left,
                       uint32_t Y_Up, LcdPixel_t color)
    {
      uint32_t k;
      uint32_t * pWordData = NULL;
      uint8_t*   pByteData = NULL;
      uint32_t  bitOffset;
      uint8_t*   pByteSrc = (uint8_t*)&color;
      uint8_t  bpp = bits_per_pixel[lcd_config.lcd_bpp];
      uint8_t  bytes_per_pixel = bpp/8;
      uint32_t start_bit;
      
      if((X_Left >= lcd_hsize)||(Y_Up >= lcd_vsize))
        return;
    
      if(panel == LCD_PANEL_UPPER)
        pWordData = (uint32_t*) LPC_LCD->UPBASE +
                                LCD_GetWordOffset(X_Left,Y_Up);
      else
        pWordData = (uint32_t*) LPC_LCD->LPBASE +
                                LCD_GetWordOffset(X_Left,Y_Up);
        
      bitOffset = LCD_GetBitOffset(X_Left,Y_Up);
      pByteData = (uint8_t*) pWordData;
      pByteData += bitOffset/8;
        
      start_bit =  bitOffset%8;
    
      if(bpp < 8)
      {
        uint8_t bit_pos = start_bit;
        uint8_t bit_ofs = 0;
        for(bit_ofs = 0;bit_ofs <bpp; bit_ofs++,bit_pos++)
        {
          *pByteData &= ~ (0x01 << bit_pos);
          *pByteData |=
            ((*pByteSrc >> (k+bit_ofs)) & 0x01) << bit_pos;   // <=
        }
      }
      ....
    }

    Предупреждение PVS-Studio: V614 CWE-457 Uninitialized variable 'k' used. lpc_lcd.c 510

    Переменная k нигде не инициализируется до момента её использования в выражении:

    *pByteData |= ((*pByteSrc >> (k+bit_ofs)) & 0x01) << bit_pos;

    Фрагмент N13. CWE-670: Always-Incorrect Control Flow Implementation


    HAL_StatusTypeDef FMC_SDRAM_SendCommand(....)
    {
      ....
    
      /* wait until command is send */
      while(HAL_IS_BIT_SET(Device->SDSR, FMC_SDSR_BUSY))
      {
        /* Check for the Timeout */
        if(Timeout != HAL_MAX_DELAY)
        {
          if((Timeout == 0)||((HAL_GetTick() - tickstart) > Timeout))
          {
            return HAL_TIMEOUT;
          }
        }     
        
        return HAL_ERROR;
      }
      
      return HAL_OK;  
    }

    Предупреждение PVS-Studio: V612 CWE-670 An unconditional 'return' within a loop. stm32f7xx_ll_fmc.c 1029

    Тело цикла выполняется не более одного раза. Это очень подозрительно, так как для получения такого же поведения логичнее было бы использовать оператор if. Скорее всего, здесь какая-то логическая ошибка.

    Фрагмент N14. Прочее


    Как я уже говорил ранее, я привёл в статье только некоторые ошибки. Полный список выбранных мною предупреждений можно найти в HTML-отчёте (архив с отчётом: rt-thread-html-log.zip).

    Помимо явных ошибок, я оставил в отчёте и предупреждения, которые указывают на подозрительный код. Я не уверен, есть в коде ошибка или нет, но этот код однозначно следует проверить разработчикам RT-Thread. Приведу пример одного такого предупреждения.

    typedef unsigned long  rt_uint32_t;
    static rt_err_t lpc17xx_emac_init(rt_device_t dev)
    {
      ....
      rt_uint32_t regv, tout, id1, id2;
      ....
      LPC_EMAC->MCFG = MCFG_CLK_DIV20 | MCFG_RES_MII;
      for (tout = 100; tout; tout--);
      LPC_EMAC->MCFG = MCFG_CLK_DIV20;
      ....
    }

    Предупреждение PVS-Studio: V529 CWE-670 Odd semicolon ';' after 'for' operator. emac.c 182

    С помощью цикла программист осуществил маленькую задержку. Анализатор, пусть и косвенно, обращает на это наше внимание.

    В моём мире оптимизирующих компиляторов это явная ошибка. Компиляторы просто выкинут этот цикл и никакой задержки не будет, т.к. tout — это обыкновенная не volatile переменная. Как дело обстоит в embedded-мире я не знаю, но подозреваю, что код всё равно некорректен или, по крайней мере, ненадёжен. Даже если компилятор не выкидывает такие циклы, непонятно, сколько по времени будет длиться задержка и будет ли она достаточна.

    Насколько я знаю, в подобных системах есть функции типа sleep_us. Их и следует использовать для маленьких задержек. Компилятор вполне может превратить вызов функции sleep_us в обыкновенный простой цикл, но это уже особенности реализации. Руками же писать такие циклы задержки некрасиво и опасно.

    Заключение


    Приглашаю читателей проверить проекты для встраиваемых систем, в разработке которых они принимают участие. Мы впервые поддержали ARM-компиляторы, и могут быть накладки. Поэтому прошу не стесняться и по всем возникшим вопросам обращаться к нам в поддержку.

    Вы можете скачать демонстрационную версию PVS-Studio здесь.

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

    Правильный робот


    Спасибо всем за внимание и безбажных вам роботов!

    Дополнительные ссылки


    Эта статья привлечёт новую аудиторию. Поэтому тем, кто ещё не был знаком с инструментом PVS-Studio, предлагаю ознакомиться со следующими статьями:

    1. Документация. Как запустить PVS-Studio в Linux.
    2. Андрей Карпов. Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний.
    3. Андрей Карпов. Дискуссия о статическом анализе кода.
    4. Андрей Карпов. Как 10 лет назад начинался проект PVS-Studio.
    5. Андрей Карпов. Статический анализ как часть процесса разработки Unreal Engine.
    6. Сергей Хренов. PVS-Studio как плагин для SonarQube.
    7. Евгений Рыжков. Философия статического анализа кода: у нас 100 программистов, анализатор нашел мало ошибок, он бесполезен?
    8. Сергей Васильев. Как PVS-Studio может помочь в поиске уязвимостей?
    9. Андрей Карпов. Статья о статическом анализе кода для менеджеров, которую не стоит читать программистам.
    10. Андрей Карпов. Как и почему статические анализаторы борются с ложными срабатываниями.
    11. Всеволод Лутовинов. Встраиваем PVS-Studio в Eclipse CDT (Linux).
    12. Андрей Кузнецов. Встраиваем PVS-Studio в Anjuta DevStudio (Linux).


    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Static Code Analyzer PVS-Studio 6.22 Now Supports ARM Compilers (Keil, IAR).
    PVS-Studio 281,89
    Ищем ошибки в C, C++ и C# на Windows, Linux, macOS
    Поделиться публикацией
    Комментарии 24
    • 0
      А arm-*-*-gcc не поддерживается сознательно и принципиально?
      • +1
        Мы только начали адаптацию анализатора для ARM. Если Вы из этой сферы, то представляете, сколько нам ещё предстоит проделать работы по поддержке того или иного ARM-компилятора.
        • 0
          Почему не поддерживается, очень даже поддерживается, standalone-версия прекрасно перехватывает вызовы arm-gcc.
          • 0
            Хм, не совсем понял, причем тут перехват вызовов. Просто gcc не назван в числе поддерживаемых компиляторов, что я понял так, что анализатор может не поддерживать его особенности.
            • 0
              Для меня «не поддерживается» — это когда PVS вообще не понимает, что с этим компилятором делать. Насколько я понимаю, PVS-Studio анализирует не исходники напрямую, а уже препроцессированные файлы. И если компилятор не поддерживается, то сделать с этим вообще ничего нельзя было. Вот как с Кейлом и IAR'ом до сих пор было.
              А arm-gcc поддерживается, я могу запустить компиляцию и PVS-Studio перехватит вызовы и проведет анализ; потому что формат препроцессированных файлов такой же, как у обычного gcc.

              Вероятно, какие-то очень специфичные вещи при этом могут анализироваться неправильно, но я с таким не сталкивался.
              • 0
                Понятно, спасибо! Но хотелось бы получить ответ от команды PVS.
                • 0
                  Просто gcc не назван в числе поддерживаемых компиляторов

                  Что именно Вы имеете ввиду, GCC (Linux), MinGW, GNU Arm Embedded Toolchain?
                  • 0
                    А arm-*-*-gcc не поддерживается сознательно и принципиально?

                    Странный вопрос. В контексте Keil/IAR, очевидно, GNU Arm Embedded Toolchain. Если сможете сказать, поддерживается ли arm-linux*-eabi[hf] и другие компиляторы (имеются в виду arm-специфические нюансы) — тоже интересно.
                    • 0
                      Давайте так. Если у кого-то, что-то не работает, то просим писать в поддержку и мы постараемся оперативно помочь и выдать новую beta-версию при необходимости. Не хочется вести дискуссию на тему поддерживается/не поддерживается. Всё очень быстро будет меняться. И вот через пару месяцев кто-то почитает комментарии и расстроится, что нет поддержки gcc X. А она уже месяц как есть.
                      • –2
                        Т.е. поставщик коммерческого ПО не может сказать, в конкретной конфигурации будет работать или нет?!
                        • 0
                          Может. Но нет смысла делать это публично, поскольку это будет вводить в заблуждение. Мы не раз жалели, что где-то оставляли какую-то подобную информацию, а потом она вновь и вновь всплывает, хотя всё уже изменилось. :) Место такой информации у нас на сайте в разделе описания продукта и в ответах по почте клиентам и потенциальным пользователям.

                          Напишите пожалуйста в саппорт, что нам конкретно не работает, и мы постараемся решить проблему и поможем с проверкой проекта.
                      • 0
                        Как заметил Amomum, анализатор уже запускается на arm-*-*-gcc, выдавая некоторые результаты. Работа ещё не закончаена, поэтому о поддержке не объвлено.
          • +5

            Было бы здорово аналогично проверить FreeRTOS.

            • 0
              Поддерживаю — весьма популярная RTOS.
            • +1
              В копилку зрелищных багов при программирование микроконтроллеров: geektimes.ru/post/295893
              • +1
                Насколько я знаю, в подобных системах есть функции типа sleep_us. Их и следует использовать для маленьких задержек. Компилятор вполне может превратить вызов функции sleep_us в обыкновенный простой цикл, но это уже особенности реализации. Руками же писать такие циклы задержки некрасиво и опасно.


                Очень далеко не везде. Обычно производители контроллеров поставляют библиотеки, сфокусированные на упрощение работы с периферией контроллера, а формирование задержки больше алгоритмическая вещь. Чаще всего реализации этих sleep_us ищутся на форумах производителя.
                Что же касается приведённого for-а — правила работы оптимизатора в мире эмбеддеда ничем не отличаются и при -O2 это цикл будет выкинут. Что бы это избежать, можно поставить _NOP. Ну или да, volatile на счётчик. Но и то и другое не годится в качестве реализации sleep_us, так как очень неточно (сильно зависит от оптимизации, требует отключения прерывания и тд). На армах я обычно использую реализацию на основе DWT счётчика.

                А в целом спасибо. Я как раз прогнал проект, над которым работаю, через IAR-ровский анализатор, разочаровался в его качестве и начал задумываться, чем бы его ещё прогнать. Наверно теперь попробую PVS.
                • 0
                  А для для компиляторов для PIC от Microchip не планируете? ;)
                  • 0
                    Пока не знаем. Зависит от того, будут ли подобные вопросы.
                    • 0
                      Небольшой оффтоп: а как вообще принимается решение о поддержке нового языка/ОС/компилятора? По обращениям потенциальных или текущих заказчиков, по инициативной разработке сотрудника, как-то еще? Почему-то кажется, что запросы на хабре или ситуации типа «у нас было 2 мешка травы...» имеют минорное значение.
                      • 0
                        Основное, это вопросы от потенциальных пользователей «а не поддержваете ли вы ...?». Плюс своё видение ситуации и своих возможностей.
                        • 0
                          Спасибо. Жаль, что с CppCat неудобно получилось.
                  • 0
                    здравствуйте. почему давно от вас ничего не слышно на ЛОРе?
                    • +1
                      Бан по причине, что там всем интереснее пьяные линуксоиды, а не обсуждение программистских тем. :)
                    • 0
                      Огорчает невозможность запуска на 32 разрядных версиях Windows.

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

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