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

Vib-OS – World's First Vibecoded AI Operating System

На глаза попалась новость "ИИ навайбкодил операционную систему. Как результат, DOOM не запускается, интернет не включается" о проекте Vib-OS. Решил глянуть, что там, полистав код и запустив статический анализатор PVS-Studio.

Проект на самом деле очень маленький. Операционная система — это только звучит громко. Посчитаем размер. Для начала я убрал оттуда заимствованные элементы, т. е. файлы, в которых нет "vibeos", "vibcode" и "vib-os". Затем ресурсы, представленные в виде массивов:

unsigned char doom1_wad[] = {
  0x49, 0x57, 0x41, 0x44, 0xf0, 0x04, 0x00, 0x00, 0xb4, 0xb7, 0x3f, 0x00,
  0x00, 0x00, 0x00, 0x1f, 0x17, 0x0b, 0x17, 0x0f, 0x07, 0x4b, 0x4b, 0x4b,
  0xff, 0xff, 0xff, 0x1b, 0x1b, 0x1b, 0x13, 0x13, 0x13, 0x0b, 0x0b, 0x0b,
  ....

В итоге осталось около 110 файлов, содержащих 35 тыс. строк кода на C. Это очень мало, и я не думал, что там вообще будет что-то, о чём можно написать. Однако быстро выяснилось, что я вижу в нём как ошибки, так и кринжовые моменты, про которые хочется рассказать.

Многословие

В глаза сразу бросается, что код растянут. Это только занимает место и временами мешает его пониманию человеком. Например, я бы не стал так писать, а воспользовался функцией sprintf:

char seq[32];
int s = 0;
seq[s++] = ':';
seq[s++] = ' ';
seq[s++] = 'i';
seq[s++] = 'c';
seq[s++] = 'm';
seq[s++] = 'p';
seq[s++] = '_';
seq[s++] = 's';
seq[s++] = 'e';
seq[s++] = 'q';
seq[s++] = '=';
seq[s++] = '0' + i;
seq[s++] = ' ';
seq[s++] = 't';
seq[s++] = 't';
seq[s++] = 'l';
seq[s++] = '=';
seq[s++] = '6';
seq[s++] = '4';
seq[s++] = ' ';
seq[s++] = 't';
seq[s++] = 'i';
seq[s++] = 'm';
seq[s++] = 'e';
seq[s++] = '=';
/* Random-ish time 10-50ms */
int time_ms = 15 + (i * 7) % 30;
seq[s++] = '0' + (time_ms / 10);
seq[s++] = '0' + (time_ms % 10);
seq[s++] = ' ';
seq[s++] = 'm';
seq[s++] = 's';
seq[s++] = '\n';
seq[s] = '\0';

Другой пример. В doom_libc.c реализованы функции:

int isdigit(int c) { return c >= '0' && c <= '9'; }
int isupper(int c) { return c >= 'A' && c <= 'Z'; }
int islower(int c) { return c >= 'a' && c <= 'z'; }

При этом ниже в том же файле символы местами проверяются "в лоб" без использования этих функций:

if (*s >= '0' && *s <= '9') digit = *s - '0';
else if (*s >= 'a' && *s <= 'z') digit = *s - 'a' + 10;
else if (*s >= 'A' && *s <= 'Z') digit = *s - 'A' + 10;

I Like to Move It, Move It

Постоянно встречается однотипный код копирования байтов, который так и напрашивается на замену на memcpy, srtcpy и т. п. Одних только одинаковых функций копирования строк я встретил четыре штуки.

static void str_copy(char *dst, const char *src, int max) {
  int i = 0;
  while (src[i] && i < max - 1) {
    dst[i] = src[i];
    i++;
  }
  dst[i] = '\0';
}

static void str_cpy(char *dst, const char *src, int max) {
  int i = 0;
  while (src[i] && i < max - 1) {
    dst[i] = src[i];
    i++;
  }
  dst[i] = '\0';
}

static void strcpy_safe(char *dst, const char *src, size_t max) {
  size_t i = 0;
  while (src[i] && i < max - 1) {
    dst[i] = src[i];
    i++;
  }
  dst[i] = '\0';
}

static inline char *strncpy_safe(char *dst, const char *src, size_t n) {
    size_t i;
    for (i = 0; i < n - 1 && src[i]; i++) {
        dst[i] = src[i];
    }
    dst[i] = '\0';
    return dst;
}

Постоянно в разных местах "вручную" копируются различные буферы:

uint8_t *src = buf + offset_in_block;
uint8_t *dst = (uint8_t *)inode;
for (size_t i = 0; i < sizeof(struct ext4_inode); i++) {
  dst[i] = src[i];
}

uint8_t *src = sb_buf;
uint8_t *dst = (uint8_t *)&fs->sb;
for (size_t i = 0; i < sizeof(struct ext4_superblock); i++) {
  dst[i] = src[i];
}

uint8_t *src = (uint8_t *)ptr;
uint8_t *dst = (uint8_t *)new_ptr;
for (size_t i = 0; i < old_size; i++) {
  dst[i] = src[i];
}

Все эти четверостишья можно заменить на memcpy или, по крайней мере, на одну нормально написанную функцию копирования. Тем более что временами делается попытка скопировать буфер с претензией на оптимизацию:

size_t i = 0;
size_t fast_count = count64 & ~7UL;
for (; i < fast_count; i += 8) {
  dst[i] = src[i];
  dst[i + 1] = src[i + 1];
  dst[i + 2] = src[i + 2];
  dst[i + 3] = src[i + 3];
  dst[i + 4] = src[i + 4];
  dst[i + 5] = src[i + 5];
  dst[i + 6] = src[i + 6];
  dst[i + 7] = src[i + 7];
}
for (; i < count64; i++) {
  dst[i] = src[i];
}

Чувствуется, что ИИ не лень код писать и он очень любит копировать данные туда-сюда. Вот, например, функция bt_set_local_name просто перекладывает строку в буфер размером 248 байт и передаёт её дальше в hci_send_cmd.

int bt_set_local_name(const char *name)
{
    uint8_t params[248] = {0};
    int len = 0;
    
    while (name[len] && len < 247) {
        params[len] = name[len];
        len++;
    }
    
    return hci_send_cmd(HCI_OP_WRITE_LOCAL_NAME, params, 248);
}

Посмотрим, что происходит со строкой дальше:

static int hci_send_cmd(uint16_t opcode, void *params, uint8_t plen)
{
    uint8_t buf[256];
    
    buf[0] = HCI_COMMAND_PKT;
    
    struct hci_command_hdr *hdr = (struct hci_command_hdr *)&buf[1];
    hdr->opcode = opcode;
    hdr->plen = plen;
    
    if (plen > 0 && params) {
        for (int i = 0; i < plen; i++) {
            buf[4 + i] = ((uint8_t *)params)[i];
        }
    }
    
    /* TODO: Send via USB bulk endpoint */
    printk(KERN_DEBUG "BT: Send cmd opcode=0x%04x len=%d\n", opcode, plen);
    
    return 0;
}

Формируется новый буфер из специального заголовка и переданной строки. Дальше этот буфер пока не используется, но суть не в этом. Непонятно, зачем вообще был нужен промежуточный буфер в bt_set_local_name. Код можно сократить, попутно ускорив его.

Баги

А что там про ошибочки? Из-за длинных функций и "пухлости" кода просматривать некоторые предупреждения статического анализатора сложно. Впрочем, в некоторых случаях ошибки очевидны.

Бессмысленные проверки

void term_execute_command(struct terminal *term, const char *cmd) {
  ....
  /* Built-in commands */
  if (str_starts_with(cmd, "clear")) {
    ....
  } else if (str_starts_with(cmd, "help")) {
  ....
  } else if (str_starts_with(cmd, "ping ")) { // <=
    term_puts(term, "Pinging ");
    term_puts(term, cmd + 5);
    term_puts(term, "...\n");

    char *ip_str = (char *)cmd + 5;
    uint32_t ip = 0;
    int octet = 0;
    int shift = 24;
    ....
  } else if (str_starts_with(cmd, "browser")) {
  ....
  }
  else if (str_starts_with(cmd, "ping ")) { // <=
    const char *host = cmd + 5;
    while (*host == ' ')
      host++;

    term_puts(term, "PING ");
    term_puts(term, host);
    term_puts(term, " (10.0.2.15): 56 data bytes\n");
    ....
  }
  ....
}

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

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

Другой похожий баг:

static void draw_window(struct window *win) {
  ....
  if (win->title[0] == 'C' && win->title[1] == 'a' && win->title[2] == 'l') {
  ....
  /* Clock window */
  else if (win->title[0] == 'C' && win->title[1] == 'l' &&
           win->title[2] == 'o') {
    int center_x = content_x + content_w / 2;
    int center_y = content_y + content_h / 2;
    int radius = 60;
    char *ip_str = (char *)cmd + 5;
    uint32_t ip = 0;
    int octet = 0;
    int shift = 24;
    ....
  }
  /* Game window */
  else if (win->title[0] == 'G' && win->title[1] == 'a' &&
           win->title[2] == 'm') {
  ....
  /* Clock */
  else if (win->title[0] == 'C' && win->title[1] == 'l' &&
           win->title[2] == 'o') {
    int cx = content_x + content_w / 2;
    int cy = content_y + content_h / 2;
    int r = (content_w < content_h ? content_w : content_h) / 2 - 16;  
    /* Draw Clock Face */
    gui_draw_circle(cx, cy, r, 0xF0F0F0, true);  /* Face */
    gui_draw_circle(cx, cy, r, 0x808080, false); /* Outline */
    gui_draw_circle(cx, cy, 3, 0x000000, true);  /* Center dot */
    ....
  }
  ....
}

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

Проверка для блоков кода /* Clock window */ и /* Clock */ идентична:

if (win->title[0] == 'C' && win->title[1] == 'l' && win->title[2] == 'o')

Соответственно, перед нами ошибка — недостижимый код.

Дублирование кода

Встречаются продублированные блоки кода:

static void fm_on_mouse(struct window *win, int x, int y, int buttons) {
  ....
  /* Initialize dimensions */
  fctx.slot_w = 80;
  fctx.slot_h = 70;
  fctx.win_w = win->width - 40; /* Match wrapped logic in render callback */
  fctx.slot_w = 80;
  fctx.slot_h = 70;
  fctx.win_w = win->width - 40;
  ....
}

Одно из предупреждений PVS-Studio: V519 The 'fctx.slot_w' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1242, 1245. window.c 1245

Другой случай:

static void draw_menu_bar(void) {
  ....
  /* WiFi Icon (Static Connected) */
  {
    int wx = primary_display.width - 86;
    int wy = 12;
    /* Draw arcs using simple lines/pixels */
    /* Center dot */
    gui_draw_rect(wx, wy + 6, 2, 2, 0xFFFFFF);
    /* Middle arc */
    gui_draw_line(wx - 3, wy + 3, wx, wy, 0xFFFFFF);
    gui_draw_line(wx, wy, wx + 3, wy + 3, 0xFFFFFF);
    /* Top arc */
    gui_draw_line(wx - 6, wy, wx, wy - 3, 0xFFFFFF);
    gui_draw_line(wx, wy - 3, wx + 6, wy, 0xFFFFFF);
  }

  /* WiFi Icon (Static Connected) */
  {
    int wx = primary_display.width - 86;
    int wy = 12;
    /* Draw arcs using simple lines/pixels */
    /* Center dot */
    gui_draw_rect(wx, wy + 6, 2, 2, 0xFFFFFF);
    /* Middle arc */
    gui_draw_line(wx - 3, wy + 3, wx, wy, 0xFFFFFF);
    gui_draw_line(wx, wy, wx + 3, wy + 3, 0xFFFFFF);
    /* Top arc */
    gui_draw_line(wx - 6, wy, wx, wy - 3, 0xFFFFFF);
    gui_draw_line(wx, wy - 3, wx + 6, wy, 0xFFFFFF);
  }
  ....
}

Предупреждение PVS-Studio: V760 Two identical blocks of text were found. The second block begins from line 2558. window.c 2543

Следующий фрагмент демонстрирует, что опечатку может допустить не только человек:

static void draw_window(struct window *win) {
  ....
  if (btn_char == '/' || btn_char == '*' || btn_char == '-' ||
      btn_char == '+' || btn_char == '=') {
    /* Orange operator buttons */
    bg = 0xFF9F0A;
    fg = 0xFFFFFF;
  } else if (btn_char == 'C' || btn_char == '+' || btn_char == '%') {
    /* Light gray function buttons */
    bg = 0xA5A5A5;
    fg = 0x000000;
  } else {
  ....
}

Предупреждение PVS-Studio: V560 A part of conditional expression is always false: btn_char == '+'. window.c 1713

Здесь что-то не так с условием. Возможно, оно должно быть другим. Нет смысла повторно проверять переменную на равенство символу +.

Выравнивание данных

Напоследок опасные игры с выравниванием данных:

/* Optimized memcpy for scanlines */
static inline void fast_memcpy_line(uint32_t *dst, uint32_t *src, int width) {
  /* Use 64-bit copies for better performance */
  uint64_t *d64 = (uint64_t *)dst;
  uint64_t *s64 = (uint64_t *)src;
  int count = width / 2;

  for (int i = 0; i < count; i++) {
    d64[i] = s64[i];
  }

  /* Handle odd pixel */
  if (width & 1) {
    dst[width - 1] = src[width - 1];
  }
}

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

• V1032 The pointer 'dst' is cast to a more strictly aligned pointer type. window.c 3108

• V1032 The pointer 'src' is cast to a more strictly aligned pointer type. window.c 3109

Этот код может работать, но он очень опасен. Если входные указатели окажутся выровнены не по 64-битной границе, возникнет неопределённое поведение.

Произойдёт это или нет, затрудняюсь сказать. Если честно, я просто поленился разобрать, как будет работать код, вызывающий функцию fast_memcpy_line, и какое будет выравнивание. Вот этот код:

static void blit_region(int x, int y, int w, int h) {
  if (!primary_display.backbuffer || !primary_display.framebuffer)
    return;

  /* Clip to screen bounds */
  if (x < 0) {
    w += x;
    x = 0;
  }
  if (y < 0) {
    h += y;
    y = 0;
  }
  if (x + w > (int)primary_display.width)
    w = primary_display.width - x;
  if (y + h > (int)primary_display.height)
    h = primary_display.height - y;
  if (w <= 0 || h <= 0)
    return;

  int pitch_pixels = primary_display.pitch / 4;

  for (int row = y; row < y + h; row++) {
    uint32_t *src = primary_display.backbuffer + row * pitch_pixels + x;
    uint32_t *dst = primary_display.framebuffer + row * pitch_pixels + x;
    fast_memcpy_line(dst, src, w);
  }
}

В любом случае код выглядит очень опасным и незащищённым от неправильно использования. Лучше так не делать. Подробнее тему выравнивания данных и связанных с этим ошибок моя коллега недавно разбирала в статье "Тихий враг или молчаливый союзник: коротко о выравнивании в C++": часть 1, часть 2.

Заключение

На этом пока всё. Если у вас есть на примете открытый код vibe-проекта, то оставьте в комментариях ссылку. Будем потихоньку смотреть, что там интересного.

Скучный вывод. Пишете вы код руками или создаёте его с помощью ИИ, чтобы он был качественным и надёжным, вы должны делать обзоры кода, использовать анализатор PVS-Studio и другие практики РБПО.

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Let's dig into some vibe code.