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

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

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

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

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

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

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

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

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

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

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 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 — очень хороший кандидат, чтобы стать первым embedded-проектом, проверенным с помощью PVS-Studio.

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

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

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

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

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

Предупреждение 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 сообщает только о последнем подозрительном присваивании, что связано с некоторыми внутренними особенностями его работы. Однако, если последнее присваивание удалить или исправить, то анализатор начнёт предупреждать о первом присваивании.

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

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.

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

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

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

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

Предупреждение 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) всегда является истинным. Скорее всего, мы имеем дело с очередной опечаткой и, на самом деле, здесь должно быть написано:

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

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

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

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

Предупреждение 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. Это странно, так как в переменной хранится результат осмысленных вычисления. Скорее всего, код должен быть таким:

Предупреждение 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 записывается всё время в нулевой элемент массива, в то время как остальные элементы остаются неинициализированными.

• 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 раза, но допустил опечатку. Возможно, следовало написать условие цикла так:

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

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

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

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

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

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

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

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

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