# Самая красивая ошибка, которую я нашёл с помощью PVS\-Studio в 2024 году
Сразу предупреждаю, мои вкусы очень специфичны\. Красота ошибки в том, что человеку её очень сложно найти\. Я не верю, что её можно заметить при обзоре кода\. Если только заранее знать, что она есть, и искать её целенаправленно\.
Ошибку я нашёл в проекте [DPDK](https://github.com/DPDK/dpdk)\. В нём есть и другие ошибки, но про них потом\. Они меркнут перед этим алмазом\. Только не ждите чего\-то эдакого\. Ошибка проста до безобразия\. Вот только найти её, просматривая код, ой как непросто\. Собственно, попробуйте сами\.
Для этого сначала изучите список именованных констант:
enum dbg\\\_status
```cpp
enum dbg_status {
DBG_STATUS_OK,
DBG_STATUS_APP_VERSION_NOT_SET,
DBG_STATUS_UNSUPPORTED_APP_VERSION,
DBG_STATUS_DBG_BLOCK_NOT_RESET,
DBG_STATUS_INVALID_ARGS,
DBG_STATUS_OUTPUT_ALREADY_SET,
DBG_STATUS_INVALID_PCI_BUF_SIZE,
DBG_STATUS_PCI_BUF_ALLOC_FAILED,
DBG_STATUS_PCI_BUF_NOT_ALLOCATED,
DBG_STATUS_INVALID_FILTER_TRIGGER_DWORDS,
DBG_STATUS_NO_MATCHING_FRAMING_MODE,
DBG_STATUS_VFC_READ_ERROR,
DBG_STATUS_STORM_ALREADY_ENABLED,
DBG_STATUS_STORM_NOT_ENABLED,
DBG_STATUS_BLOCK_ALREADY_ENABLED,
DBG_STATUS_BLOCK_NOT_ENABLED,
DBG_STATUS_NO_INPUT_ENABLED,
DBG_STATUS_NO_FILTER_TRIGGER_256B,
DBG_STATUS_FILTER_ALREADY_ENABLED,
DBG_STATUS_TRIGGER_ALREADY_ENABLED,
DBG_STATUS_TRIGGER_NOT_ENABLED,
DBG_STATUS_CANT_ADD_CONSTRAINT,
DBG_STATUS_TOO_MANY_TRIGGER_STATES,
DBG_STATUS_TOO_MANY_CONSTRAINTS,
DBG_STATUS_RECORDING_NOT_STARTED,
DBG_STATUS_DATA_DIDNT_TRIGGER,
DBG_STATUS_NO_DATA_RECORDED,
DBG_STATUS_DUMP_BUF_TOO_SMALL,
DBG_STATUS_DUMP_NOT_CHUNK_ALIGNED,
DBG_STATUS_UNKNOWN_CHIP,
DBG_STATUS_VIRT_MEM_ALLOC_FAILED,
DBG_STATUS_BLOCK_IN_RESET,
DBG_STATUS_INVALID_TRACE_SIGNATURE,
DBG_STATUS_INVALID_NVRAM_BUNDLE,
DBG_STATUS_NVRAM_GET_IMAGE_FAILED,
DBG_STATUS_NON_ALIGNED_NVRAM_IMAGE,
DBG_STATUS_NVRAM_READ_FAILED,
DBG_STATUS_IDLE_CHK_PARSE_FAILED,
DBG_STATUS_MCP_TRACE_BAD_DATA,
DBG_STATUS_MCP_TRACE_NO_META,
DBG_STATUS_MCP_COULD_NOT_HALT,
DBG_STATUS_MCP_COULD_NOT_RESUME,
DBG_STATUS_RESERVED0,
DBG_STATUS_SEMI_FIFO_NOT_EMPTY,
DBG_STATUS_IGU_FIFO_BAD_DATA,
DBG_STATUS_MCP_COULD_NOT_MASK_PRTY,
DBG_STATUS_FW_ASSERTS_PARSE_FAILED,
DBG_STATUS_REG_FIFO_BAD_DATA,
DBG_STATUS_PROTECTION_OVERRIDE_BAD_DATA,
DBG_STATUS_DBG_ARRAY_NOT_SET,
DBG_STATUS_RESERVED1,
DBG_STATUS_NON_MATCHING_LINES,
DBG_STATUS_INSUFFICIENT_HW_IDS,
DBG_STATUS_DBG_BUS_IN_USE,
DBG_STATUS_INVALID_STORM_DBG_MODE,
DBG_STATUS_OTHER_ENGINE_BB_ONLY,
DBG_STATUS_FILTER_SINGLE_HW_ID,
DBG_STATUS_TRIGGER_SINGLE_HW_ID,
DBG_STATUS_MISSING_TRIGGER_STATE_STORM,
MAX_DBG_STATUS
};
```
Теперь изучите массив строк:
static const char \\\* const s\\\_status\\\_str\\\[\\\] \\\= \\\.\\\.\\\.\\\.
```cpp
static const char * const s_status_str[] = {
/* DBG_STATUS_OK */
"Operation completed successfully",
/* DBG_STATUS_APP_VERSION_NOT_SET */
"Debug application version wasn't set",
/* DBG_STATUS_UNSUPPORTED_APP_VERSION */
"Unsupported debug application version",
/* DBG_STATUS_DBG_BLOCK_NOT_RESET */
"The debug block wasn't reset since the last recording",
/* DBG_STATUS_INVALID_ARGS */
"Invalid arguments",
/* DBG_STATUS_OUTPUT_ALREADY_SET */
"The debug output was already set",
/* DBG_STATUS_INVALID_PCI_BUF_SIZE */
"Invalid PCI buffer size",
/* DBG_STATUS_PCI_BUF_ALLOC_FAILED */
"PCI buffer allocation failed",
/* DBG_STATUS_PCI_BUF_NOT_ALLOCATED */
"A PCI buffer wasn't allocated",
/* DBG_STATUS_INVALID_FILTER_TRIGGER_DWORDS */
"The filter/trigger constraint dword offsets are not "
"enabled for recording",
/* DBG_STATUS_VFC_READ_ERROR */
"Error reading from VFC",
/* DBG_STATUS_STORM_ALREADY_ENABLED */
"The Storm was already enabled",
/* DBG_STATUS_STORM_NOT_ENABLED */
"The specified Storm wasn't enabled",
/* DBG_STATUS_BLOCK_ALREADY_ENABLED */
"The block was already enabled",
/* DBG_STATUS_BLOCK_NOT_ENABLED */
"The specified block wasn't enabled",
/* DBG_STATUS_NO_INPUT_ENABLED */
"No input was enabled for recording",
/* DBG_STATUS_NO_FILTER_TRIGGER_256B */
"Filters and triggers are not allowed in E4 256-bit mode",
/* DBG_STATUS_FILTER_ALREADY_ENABLED */
"The filter was already enabled",
/* DBG_STATUS_TRIGGER_ALREADY_ENABLED */
"The trigger was already enabled",
/* DBG_STATUS_TRIGGER_NOT_ENABLED */
"The trigger wasn't enabled",
/* DBG_STATUS_CANT_ADD_CONSTRAINT */
"A constraint can be added only after a filter was "
"enabled or a trigger state was added",
/* DBG_STATUS_TOO_MANY_TRIGGER_STATES */
"Cannot add more than 3 trigger states",
/* DBG_STATUS_TOO_MANY_CONSTRAINTS */
"Cannot add more than 4 constraints per filter or trigger state",
/* DBG_STATUS_RECORDING_NOT_STARTED */
"The recording wasn't started",
/* DBG_STATUS_DATA_DID_NOT_TRIGGER */
"A trigger was configured, but it didn't trigger",
/* DBG_STATUS_NO_DATA_RECORDED */
"No data was recorded",
/* DBG_STATUS_DUMP_BUF_TOO_SMALL */
"Dump buffer is too small",
/* DBG_STATUS_DUMP_NOT_CHUNK_ALIGNED */
"Dumped data is not aligned to chunks",
/* DBG_STATUS_UNKNOWN_CHIP */
"Unknown chip",
/* DBG_STATUS_VIRT_MEM_ALLOC_FAILED */
"Failed allocating virtual memory",
/* DBG_STATUS_BLOCK_IN_RESET */
"The input block is in reset",
/* DBG_STATUS_INVALID_TRACE_SIGNATURE */
"Invalid MCP trace signature found in NVRAM",
/* DBG_STATUS_INVALID_NVRAM_BUNDLE */
"Invalid bundle ID found in NVRAM",
/* DBG_STATUS_NVRAM_GET_IMAGE_FAILED */
"Failed getting NVRAM image",
/* DBG_STATUS_NON_ALIGNED_NVRAM_IMAGE */
"NVRAM image is not dword-aligned",
/* DBG_STATUS_NVRAM_READ_FAILED */
"Failed reading from NVRAM",
/* DBG_STATUS_IDLE_CHK_PARSE_FAILED */
"Idle check parsing failed",
/* DBG_STATUS_MCP_TRACE_BAD_DATA */
"MCP Trace data is corrupt",
/* DBG_STATUS_MCP_TRACE_NO_META */
"Dump doesn't contain meta data - it must be provided in image file",
/* DBG_STATUS_MCP_COULD_NOT_HALT */
"Failed to halt MCP",
/* DBG_STATUS_MCP_COULD_NOT_RESUME */
"Failed to resume MCP after halt",
/* DBG_STATUS_RESERVED0 */
"",
/* DBG_STATUS_SEMI_FIFO_NOT_EMPTY */
"Failed to empty SEMI sync FIFO",
/* DBG_STATUS_IGU_FIFO_BAD_DATA */
"IGU FIFO data is corrupt",
/* DBG_STATUS_MCP_COULD_NOT_MASK_PRTY */
"MCP failed to mask parities",
/* DBG_STATUS_FW_ASSERTS_PARSE_FAILED */
"FW Asserts parsing failed",
/* DBG_STATUS_REG_FIFO_BAD_DATA */
"GRC FIFO data is corrupt",
/* DBG_STATUS_PROTECTION_OVERRIDE_BAD_DATA */
"Protection Override data is corrupt",
/* DBG_STATUS_DBG_ARRAY_NOT_SET */
"Debug arrays were not set "
"(when using binary files, dbg_set_bin_ptr must be called)",
/* DBG_STATUS_RESERVED1 */
"",
/* DBG_STATUS_NON_MATCHING_LINES */
"Non-matching debug lines - in E4, all lines must be of "
"the same type (either 128b or 256b)",
/* DBG_STATUS_INSUFFICIENT_HW_IDS */
"Insufficient HW IDs. Try to record less Storms/blocks",
/* DBG_STATUS_DBG_BUS_IN_USE */
"The debug bus is in use",
/* DBG_STATUS_INVALID_STORM_DBG_MODE */
"The storm debug mode is not supported in the current chip",
/* DBG_STATUS_OTHER_ENGINE_BB_ONLY */
"Other engine is supported only in BB",
/* DBG_STATUS_FILTER_SINGLE_HW_ID */
"The configured filter mode requires a single Storm/block input",
/* DBG_STATUS_TRIGGER_SINGLE_HW_ID */
"The configured filter mode requires that all the constraints of a "
"single trigger state will be defined on a single Storm/block input",
/* DBG_STATUS_MISSING_TRIGGER_STATE_STORM */
"When triggering on Storm data, the Storm to trigger on must be specified"
};
```
Последнее — код, который превращает именованную константу в строчку:
```cpp
const char *qed_dbg_get_status_str(enum dbg_status status)
{
return (status < MAX_DBG_STATUS) ?
s_status_str[status] : "Invalid debug status";
}
```
**Где ошибка?**
Нашли?
На самом деле есть намёк, за который можно теоретически зацепиться\. В массиве все строки отделяются одной пустой строкой, кроме одного места\. Но начнём мы с предупреждения анализатора кода PVS\-Studio:
[V557](https://pvs-studio.ru/ru/docs/warnings/v557/) Array overrun is possible\. The value of 'status' index could reach 58\. qede\_debug\.c 7149
Оно указывает на место, где из массива по индексу извлекается указатель на строку:
```cpp
return (status < MAX_DBG_STATUS) ?
s_status_str[status] : "Invalid debug status";
```
Вначале я поставил неправильный скучный диагноз\. По невнимательности я подумал, что передо мной [off\-by\-one error](https://cwe.mitre.org/data/definitions/193.html)\. Уж очень это похоже на типовую неправильную проверку, которую я встречал много раз в разных проектах\.
Суть классического антипаттерна\. Есть _enum_, последний элемент которого используется как количество элементов в нём\.
```cpp
enum Efoo {
A,
B,
C,
COUNT
};
```
Константам в _enum_ присваиваются значения, начиная с 0\. Соответственно, вспомогательная константа COUNT окажется равна 3, что соответствует количеству основных именованных констант\.
Есть какой\-то массив, где каждой именованной константе что\-то соответствует:
```cpp
char Cfoo[] = { 'A', 'B', 'C' };
```
Часто допускают баг, когда проверяют, что индекс не выходит за границу массива:
```cpp
char Convert(unsigned id)
{
return (id > COUNT) ? 0 : Cfoo[id];
}
```
Проверка работает неправильно: если _id_ окажется равен _COUNT_, то произойдёт выход за границу массива\. Для таких ошибок анализатор PVS\-Studio выдаёт предупреждение аналогично рассмотренному ранее\.
Правильным будет один из следующих вариантов:
```cpp
return (id >= COUNT) ? 0 : Cfoo[id]; // OK
return (id < COUNT) ? Cfoo[id] : 0; // OK
```
Ничего интересного\. Расходимся\. В статью можно выписывать только само предупреждение рядом с другими ошибками выхода за границу, но сам баг не разбирать в статье про проверку DPDK\.
**И тут в последний момент — СТОП\!**
**ЗДЕСЬ ЖЕ ВЕДЬ ПРАВИЛЬНАЯ ПРОВЕРКА НАПИСАНА\!**
```cpp
return (status < MAX_DBG_STATUS) ?
s_status_str[status] : "Invalid debug status";
```
Теперь непонятно и интересно\! Почему анализатор выдал предупреждение? Ложное срабатывание? Это вряд ли, тут ему негде запутаться\.
Засучив рукава, я начинаю внимательно изучать код\. Вот оно\! Пропущена строка для константы _DBG\_STATUS\_NO\_MATCHING\_FRAMING\_MODE_\.
В enum:
```cpp
DBG_STATUS_INVALID_FILTER_TRIGGER_DWORDS,
DBG_STATUS_NO_MATCHING_FRAMING_MODE,
DBG_STATUS_VFC_READ_ERROR,
```
В массиве:
```cpp
/* DBG_STATUS_INVALID_FILTER_TRIGGER_DWORDS */
"The filter/trigger constraint dword offsets are not "
"enabled for recording",
/* DBG_STATUS_VFC_READ_ERROR */
"Error reading from VFC",
```
Обратите внимание на разделитель, состоящий из двух пустых строчек\. Я упоминал об этом артефакте раньше\. Здесь что\-то пошло не так\. Возможно, произошёл какой\-то сбой при заполнении массива, возможно, строчку случайно удалили, возможно, неудачный мёрж веток, или что\-то ещё\.
Впрочем, это сейчас, когда найден баг, две пустые строчки выглядят подозрительно\. При обзоре никто не обратит на них внимание\. А даже случайно заметив, просто удалят одну из них для красоты\. Никто не скажет, "да тут небось что\-то пропустили, пойдём проверим" :\)
В результате этой ошибки:
* возможен выход за границу массива;
* функция возвращает неправильные строки для всех констант, начиная с _DBG\_STATUS\_NO\_MATCHING\_FRAMING\_MODE_\.
На мой взгляд, это красиво\. Спасибо за внимание\. Регулярно проверяйте код используя [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/download/)\.
**P\.S\.**
Коллега предложил добавить в статью, что подобной ошибки можно избежать, используя проверки этапа компиляции\. [Примеры кода с _static\_assert_ для C и C\+\+](https://godbolt.org/z/rWefjahMc)\. Для C\+\+ получается ловчее с вычислением размеров массивов, но это уже другая [история](https://pvs-studio.ru/ru/blog/posts/cpp/1112/)\. Пожалуй, везде такие проверки добавлять — перебор\. Но вот для таких больших массивов, почему бы и нет\.