PVS-Studio теперь поддерживает набор встроенных инструментов GNU Arm

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

Введение

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

• Встраиваемый верстак IAR
• Инструменты разработки Keil Embedded для Arm
• Инструменты генерации кода TI ARM

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

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

Основная цель GNU Embedded Toolchain — генерация кода, ориентированного на голое железо, то есть кода, предназначенного для работы на ЦП напрямую, без операционной системы. В состав пакета входят компиляторы C и C++, ассемблер, GNU Binutils и библиотека Newlib. Все компоненты с открытым исходным кодом; они распространяются под лицензией GNU GPL. Вы можете загрузить готовые версии наборов инструментов для Windows, Linux и macOS с официального сайта.

Мбед ОС

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

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

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

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

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

Arm Mbed OS — это встроенная операционная система с открытым исходным кодом, разработанная специально для «вещей» в Интернете вещей. Он включает в себя все функции, необходимые для разработки подключенного продукта на базе микроконтроллера Arm Cortex-M, включая безопасность, возможность подключения, ОСРВ и драйверы для датчиков и устройств ввода-вывода.

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

Проблемы

В результате запуска PVS-Studio на исходном коде ОС Mbed было выдано 693 предупреждения, 86 из которых имели высокий приоритет. Многие из них далеко не интересны, поэтому я не буду описывать их все. Например, было много предупреждений V547 (Выражение всегда истинно/ложно), исходящих из похожих фрагментов кода. Конечно, есть способ настроить анализатор, чтобы сильно уменьшить количество ложных или просто неинтересных сообщений, но это не имело отношения к моей цели. Если вы хотите увидеть пример такой настройки, обратитесь к статье Характеристики PVS-Studio Analyzer на примере основных библиотек EFL, 10–15% ложных срабатываний.

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

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

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

Предупреждение анализатора: V773 CWE-401 Произошел выход из функции без освобождения указателя read_buf. Возможна утечка памяти. 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 Произошел выход из функции без отпускания указателя интерфейс. Возможна утечка памяти. 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;
}

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

Мемсет

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

Давайте проверим это предупреждение:

V575 CWE-628 Функция memset обрабатывает элементы 0. Проверьте третий аргумент. 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 Функция memset обрабатывает элементы 0. Проверьте третий аргумент. mbed_error.c 123

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

Предупреждение анализатора: V612 CWE-670 Безусловный возврат внутри цикла. 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 Выражение pcb-›state == LISTEN всегда ложно. 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 — условие, которое никогда не выполняется потому что состояние в этой строке может быть равно только ЗАКРЫТО.

Еще одно предупреждение, связанное с условиями: V517 CWE-570 Обнаружено использование паттерна if (A) {…} else if (A) {…}. Существует вероятность наличия логической ошибки. Проверьте строки: 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 недостижимым. Подобные ошибки часто связаны с методом программирования копировать-вставить.

Бесхозное выражение

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

Предупреждение анализатора: V607 Бесхозяйное выражение ‘& 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) после предварительной обработки.

У меня нет дальнейших комментариев. Возможно, здесь нет никакой ошибки, но такой код всегда делает меня похожим на картинку выше :).

Вывод

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