В PVS-Studio появилась поддержка GNU Arm Embedded Toolchain

    GNU Arm Embedded Toolchain + PVS-Studio

    Встраиваемые системы давно и прочно вошли в нашу жизнь. Требования к их стабильности и надежности очень высоки, а исправление ошибок обходится дорого. Поэтому для embedded разработчиков особенно актуально регулярное использование специализированных инструментов для обеспечения качества исходного кода. Эта статья расскажет о появлении поддержки GNU Arm Embedded Toolchain в анализаторе PVS-Studio и дефектах кода, найденных в проекте Mbed OS.

    Введение


    Анализатор PVS-Studio уже поддерживает несколько коммерческих компиляторов для встраиваемых систем, например:


    Теперь к поддержке добавлен еще один инструмент разработчика — GNU Embedded Toolchain.

    GNU Embedded Toolchain — коллекция компиляторов от компании Arm, основанная на GNU Compiler Collection. Первый официальный релиз состоялся в 2012 году, и с тех пор проект развивается вместе с GCC.

    Основное предназначение GNU Embedded Toolchain — генерация кода, работающего на «голом железе» (bare metal), то есть напрямую на процессоре без прослойки в виде операционной системы. В комплект поставки входят компиляторы для C и C++, ассемблер, набор утилит GNU Binutils и библиотека Newlib. Исходный код всех компонентов полностью открыт и распространяется по лицензии GNU GPL. С официального сайта можно скачать версии под Windows, Linux и macOS.

    Mbed OS


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

    Mbed OS + PVS-Studio


    Хотя основной целью статьи является рассказать о поддержке GNU Embedded Toolchain, много про это написать сложно. Тем более, что читатели наших статей наверняка ждут описания каких-то интересных ошибок. Что же, не будем обманывать их ожидания и запустим анализатор на проекте Mbed OS. Это операционная система с открытым исходным кодом, которая разрабатывается при участии компании Arm.

    Официальный сайт: https://www.mbed.com/

    Исходный код: https://github.com/ARMmbed/mbed-os

    Выбор на Mbed OS пал не случайно, вот как описывают проект его авторы:

    Arm Mbed OS is an open source embedded operating system designed specifically for the «things» in the Internet of Things. It includes all the features you need to develop a connected product based on an Arm Cortex-M microcontroller, including security, connectivity, an RTOS and drivers for sensors and I/O devices.

    Это идеальный проект для сборки с помощью GNU Embedded Toolchain, особенно с учетом участия Arm в его разработке. Сразу оговорюсь, что цели найти и показать как можно больше ошибок в конкретном проекте у меня не было, поэтому результаты проверки рассмотрены кратко.

    Ошибки


    В ходе проверки кода Mbed OS анализатор PVS-Studio выдал 693 предупреждения, 86 из них — с приоритетом high. Я не буду подробно рассматривать их все, тем более что многие из них повторяются или не представляют особого интереса. Например, анализатор выдал много предупреждений V547 (Expression is always true/false), относящихся к однотипным фрагментам кода. Анализатор можно настроить, чтобы существенно сократить количество ложных и неинтересных срабатываний, но такой задачи при написании статьи не ставилось. Желающие могут посмотреть пример подобной настройки, описанной в статье "Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний".

    Для статьи я отобрал несколько интересных ошибок, чтобы продемонстрировать работу анализатора.

    Утечки памяти


    Начнем с распространенного класса ошибок в C и C++ — утечек памяти.

    Предупреждение анализатора: V773 CWE-401 The function was exited without releasing the 'read_buf' pointer. A memory leak is possible. cfstore_test.c 565

    int32_t cfstore_test_init_1(void)
    {
       ....
      read_buf = (char*) malloc(max_len);
      if(read_buf == NULL) {
        CFSTORE_ERRLOG(....);
        return ret;
      }
      ....
      while(node->key_name != NULL)
      {
        ....
        ret = drv->Create(....);
        if(ret < ARM_DRIVER_OK){
          CFSTORE_ERRLOG(....);
          return ret;              // <=
        }
      ....
      free(read_buf);
      return ret;
    }

    Классическая ситуация при работе с динамической памятью. Выделенный с помощью malloc буфер используется только внутри функции и освобождается перед выходом. Проблема в том, что этого не происходит, если функция прекращает работу досрочно. Обратите внимание на одинаковый код в блоках if. Скорее всего, автор скопировал верхний фрагмент и забыл добавить вызов free.

    Еще пример, аналогичный предыдущему.

    Предупреждение анализатора: V773 CWE-401 The function was exited without releasing the 'interface' pointer. A memory leak is possible. nanostackemacinterface.cpp 204

    nsapi_error_t Nanostack::add_ethernet_interface(
        EMAC &emac,
        bool default_if,
        Nanostack::EthernetInterface **interface_out,
        const uint8_t *mac_addr)
    {
      ....
      Nanostack::EthernetInterface *interface;
      interface = new (nothrow) Nanostack::EthernetInterface(*single_phy);
      if (!interface) {
        return NSAPI_ERROR_NO_MEMORY;
      }
    
      nsapi_error_t err = interface->initialize();
      if (err) {
        return err;              // <=
      }
    
      *interface_out = interface;
      return NSAPI_ERROR_OK;
    }

    Указатель на выделенную память возвращается через выходной параметр, но только если вызов initialize прошел успешно, а в случае ошибки происходит утечка, потому что локальная переменная interface выходит из области видимости, и указатель попросту теряется. Здесь следовало бы либо вызвать delete, либо хотя бы отдать хранящийся в переменной interface адрес наружу в любом случае, чтобы об этом мог позаботиться вызывающий код.

    Memset


    Использование функции memset часто приводит к ошибкам, примеры связанных с ней проблем можно посмотреть в статье "Самая опасная функция в мире С/С++".

    Рассмотрим такое предупреждение анализатора:

    V575 CWE-628 The 'memset' function processes '0' elements. Inspect the third argument. mbed_error.c 282

    mbed_error_status_t mbed_clear_all_errors(void)
    {
        ....
        //Clear the error and context capturing buffer
        memset(&last_error_ctx, sizeof(mbed_error_ctx), 0);
        //reset error count to 0
        error_count = 0;
        ....
    }

    Программист намеревался обнулить память, занимаемую структурой last_error_ctx, но перепутал местами второй и третий аргумент. В результате 0 байт заполняется значением sizeof(mbed_error_ctx).

    Точно такая же ошибка присутствует сотней строк выше:

    V575 CWE-628 The 'memset' function processes '0' elements. Inspect the third argument. mbed_error.c 123

    Безусловный оператор 'return' в цикле


    Предупреждение анализатора: V612 CWE-670 An unconditional 'return' within a loop. thread_network_data_storage.c 2348

    bool thread_nd_service_anycast_address_mapping_from_network_data (
              thread_network_data_cache_entry_t *networkDataList,
              uint16_t *rlocAddress,
              uint8_t S_id)
    {
      ns_list_foreach(thread_network_data_service_cache_entry_t,
                      curService, &networkDataList->service_list) {
        // Go through all services
        if (curService->S_id != S_id) {
          continue;
        }
        ns_list_foreach(thread_network_data_service_server_entry_t,
                        curServiceServer, &curService->server_list) {
          *rlocAddress = curServiceServer->router_id;
          return true;                     // <=
        }
      }
      return false;
    }

    В этом фрагменте ns_list_foreach — это макрос, который раскрывается в оператор for. Внутренний цикл выполняет не больше одной итерации из-за вызова return сразу после строки, в которой инициализируется выходной параметр функции. Возможно, этот код работает так, как задумано, но использование внутреннего цикла выглядит в этом контексте довольно странно. Скорее всего, инициализация rlocAddress и выход из функции должны выполняться по условию, или от внутреннего цикла можно избавиться.

    Ошибки в условиях


    Как я говорил выше, анализатор выдал довольно большое количество неинтересных предупреждений V547, поэтому я изучал их бегло и выписал для статьи только два случая.

    V547 CWE-570 Expression 'pcb->state == LISTEN' is always false. lwip_tcp.c 689

    enum tcp_state {
      CLOSED      = 0,
      LISTEN      = 1,
      ....
    };
    
    struct tcp_pcb *
    tcp_listen_with_backlog_and_err(struct tcp_pcb *pcb, u8_t backlog, err_t *err)
    {
      ....
      LWIP_ERROR("tcp_listen: pcb already connected",
                 pcb->state == CLOSED,
                 res = ERR_CLSD; goto done);
    
      /* already listening? */
      if (pcb->state == LISTEN) {               // <=
        lpcb = (struct tcp_pcb_listen*)pcb;
        res = ERR_ALREADY;
        goto done;
      }
      ....
    }

    Анализатор считает, что условие pcb->state == LISTEN всегда ложно, давайте разберемся, почему.

    Перед оператором if используется макрос LWIP_ERROR, который по логике своей работы напоминает assert. Его объявление выглядит так:

    #define LWIP_ERROR(message, expression, handler) do { if (!(expression)) { \
      LWIP_PLATFORM_ERROR(message); handler;}} while(0)

    Если условие ложно, макрос сообщает об ошибке и выполняет код, переданный через параметр handler, в этом фрагменте кода — безусловный переход с использованием goto.

    В данном примере проверяется условие 'pcb->state == CLOSED', то есть переход на метку done происходит в случае, когда pcb->state имеет любое другое значение. Оператор if, следующий за вызовом LWIP_ERROR, проверяет pcb->state на равенство LISTEN, но это условие никогда не выполняется, потому что state в этой строке может содержать только значение CLOSED.

    Рассмотрим еще одно предупреждение, связанное с условиями: V517 CWE-570 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 62, 65. libdhcpv6_server.c 62

    static void libdhcpv6_address_generate(....)
    {
      ....
      if (entry->linkType == DHCPV6_DUID_HARDWARE_EUI64_TYPE) // <=
      {
        memcpy(ptr, entry->linkId, 8);
       *ptr ^= 2;
      }
      else if (entry->linkType == DHCPV6_DUID_HARDWARE_EUI64_TYPE)// <=
      {
        *ptr++  = entry->linkId[0] ^ 2;
        *ptr++  = entry->linkId[1];
      ....
      }
    }

    Здесь if и else if проверяют одно и то же условие, в результате чего код в теле else if никогда не выполняется. Такие ошибки часто возникают при написании кода методом 'copy-paste'.

    Ownerless expression


    Посмотрим напоследок на забавный фрагмент кода.

    Предупреждение анализатора: V607 Ownerless expression '& discover_response_tlv'. thread_discovery.c 562

    static int thread_discovery_response_send(
                            thread_discovery_class_t *class,
                            thread_discovery_response_msg_t *msg_buffers)
    {
      ....
      thread_extension_discover_response_tlv_write(
                 &discover_response_tlv, class->version,
                 linkConfiguration->securityPolicy);
      ....
    }

    А теперь давайте взглянем на объявление макроса thread_extension_discover_response_tlv_write:

    #define thread_extension_discover_response_tlv_write \
    ( data, version, extension_bit)\
    (data)

    Макрос раскрывается в аргумент data, то есть его вызов внутри функции thread_discovery_response_send после препроцессирования превращается в выражение (&discover_response_tlv).

    Wait what


    У меня комментариев нет. Наверное, это не ошибка, но такой код всегда вводит меня в состояние, подобному изображению на картинке :).

    Заключение


    Список поддерживаемых в PVS-Studio компиляторов пополнился. Если у вас есть проект, предназначенный для сборки с помощью GNU Arm Embedded Toolchain, предлагаю попробовать проверить его с помощью нашего анализатора. Скачать демонстрационную версию можно здесь. Обратите также внимание на вариант с бесплатной лицензией, который подходит для некоторых небольших проектов.



    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Yuri Minaev. PVS-Studio Now Supports GNU Arm Embedded Toolchain.

    PVS-Studio

    461,00

    Ищем ошибки в C, C++ и C# на Windows, Linux, macOS

    Поделиться публикацией

    Похожие публикации

    Комментарии 25
      +6
      Даа, грустно! Ведь mbedOS вроде как юзает asan clang'овский, на гитхабе хостятся и всякие другие модные слова употребляют. И на С++ пишут, а не на С.
      Но нет, все равно malloc, а не new хотя бы, никаких умных контейнеров, и опять memset вместо std::fill. Фу.

      Единственное, что из статьи не ясно — в чем же именно заключается поддержка GNU Arm Embedded? Я несколько лет назад проверял, что PVS-Studio сходу умеет работать с arm gcc — в конце концов, это ведь просто gcc.
      Видимо, теперь учитывается какая-то специфика, но как раз про это в статье не написано.
        +2
        В ядре у них полно C, так что насчет malloc ничего удивительного.

        А насчет поддержки, фронтенд там целиком от gcc, так что в этом плане практически без изменений. Мы теперь правильно отличаем обычный gcc от того, который для ARM, и учитываем некоторые специфичные для платформы вещи. Плюс мы теперь под всеми системами автоматически его находим и правильно проставляем все настройки для анализа.
          +2
          В ядре у них полно C, так что насчет malloc ничего удивительного.

          Непонятно только зачем у них там С вообще. Но это к ним вопрос, не к вам :)

          учитываем некоторые специфичные для платформы вещи

          А вот тут хотелось бы поподробнее!
            0
            Если не ошибаюсь MBED базируется на RTX
              +1
              Даже не базируется. Само ядро MBED — это такая C++ прослойка, в которую обёрнута старая сишная Keil RTX.
        0
        1) А планируется ли поддержка bare metal или других операционных сред?
        2) Выработаны ли диагностики гонки, связанные с моделью памяти на ARM, которая существенно отличается от x86?
          0
          1) А планируется ли поддержка bare metal или других операционных сред?
          Embedded — очень широкое направление. В приоритете пока поддержка чего-нибудь максимально популярного среди разработчиков и где работа препроцессора не сильно отличается от поддерживаемых нами компиляторов.
          2) Выработаны ли диагностики гонки, связанные с моделью памяти на ARM, которая существенно отличается от x86?
          Эта задача полноценно не решается статическим анализом, поэтому ничего нового мы не делали. А существующих диагностик на эту тему совсем не много.

          +3
          Можно было бы проверить FreeRTOS как одну из самых распространенных открытые ОСРВ для встроенки.
            0
            А ещё vxWorks
              +1
              я так понимаю кроссплатформенная версия имеет уже проприетарную лицензию и стоит денюжек?
                0
                Читать надо. Вполне возможно, что на использование не для встраивания можно и как-то договориться.
                0
                На моей памяти vxWorks всю жизнь была жестоко закрыта. Или я отстал от жизни?
                +1
                ага, или ещё код сгенерированный с помощью CubeMX
                  –1
                  Боюсь анализатор зависнет и комп сгорит от такого треша
                    0
                    Если вспоминать за «сгорит» — было бы любопытно прогнать PVS по ACE/TAO. Особенно по тому, что выдает их IDL компилятор.
                +1
                респект ребята)) очень круто!
                  0
                  Как насчет линаровского тулчейна? www.linaro.org/downloads
                    0
                    Линаровский пока не делали, но там тоже gcc, так что в целом должно работать, только под Linux и macOS придется вручную указывать имена исполняемых файлов компиляторов.
                    +1
                    Здесь следовало бы либо вызвать delete, либо хотя бы отдать хранящийся в переменной interface адрес наружу в любом случае, чтобы об этом мог позаботиться вызывающий код

                    а ещё лучше — std::unique_ptr::release
                    И, да, ребята, категорически умоляю, прячьте объяснения под кат, а то совсем неинтересно становится читать
                      0
                      Здравствуйте, можете по подробнее рассказать про связку PVS-Studio и Keil Embedded Development. Каким образом происходит взаимодействие: имеется некая интеграция в среду разработки или через внешний скрипт?
                        0
                        Добрый день.

                        Для проверки Keil-проектов у нас пока нет интеграции в среды разработки (uVision/Eclipse), поэтому анализ возможен только с помощью прямой интеграции в скрипты сборки или с использованием режимов мониторинга сборки (для Windows и Linux).

                        Вы можете запросить на сайте временный ключ для проверки своего проекта. Задать вопросы вы можете через форму обратной связи, или обменяться почтой через ЛС Хабра для продолжения общения.
                        0
                        Было бы интересно посмотреть на результат проверки RiotOS.
                          +1
                          Присылайте проект в наш Check List на GitHub, откуда мы берём проекты для анализа.
                            0
                            О, за чеклист отдельное спасибо.
                              0
                              Благодарю.

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

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