Какой язык программирования начать учить: Python или Ruby? Что из них лучше? Django или Ruby on Rails? Такие вопросы можно часто встретить на IT форумах всего мира. Я же предлагаю сравнить не сами языки, а их эталонные реализации: CPython и MRI. О том, какие ошибки в их интерпретаторах смог найти PVS-Studio, и пойдёт речь в этой статье.

Введение

Для анализа были взяты самые свежие исходники из основных репозиториев (Ruby, Python). Проверка проводилась с помощью статического анализатора кода PVS-Studio версии 6.06. Python отлично собирается в Visual Studio, а для Ruby можно использовать Standalone версию в режиме мониторинга вызова компилятора.

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

//-V:RB_TYPE_P:501

И все срабатывания диагностики V501, в которых фигурирует макрос RB_TYPE_P пропадут.

Python

Фрагмент N1

#ifdef MS_WINDOWS
typedef SOCKET SOCKET_T;
#else
typedef int SOCKET_T;
#endif
typedef struct {
  PyObject_HEAD
  SOCKET_T sock_fd; /* Socket file descriptor */
  ....
} PySocketSockObject;

static int
internal_select(PySocketSockObject *s,
                int writing,
                _PyTime_t interval,
                int connect)
{
  ....
  if (s->sock_fd < 0) // <=
    return 0;
  ....
}

V547 Expression 's->sock_fd < 0' is always false. Unsigned type value is never < 0. socketmodule.c 655

Тип SOCKET в Windows является беззнаковым, поэтому сравнение его с нулём бессмысленно. Для того, чтобы проверить, вернула ли функция socket() корректный дескриптор, необходимо сравнить его значение с INVALID_SOCKET. Стоит отметить, что в Linux данное сравнение работало бы правильно, так как там в качестве типа сокета используется знаковый int и об ошибке сигнализирует значение -1. Тем не менее, для проверки лучше использовать специальные макросы или константы.

Аналогичные проверки, на которые было выдано предупреждение:

• V547 Expression 's->sock_fd < 0' is always false. Unsigned type value is never < 0. _ssl.c 1702
• V547 Expression 'sock->sock_fd < 0' is always false. Unsigned type value is never < 0. _ssl.c 2018

Фрагмент N2

int ASN1_PRINTABLE_type(const unsigned char *s, int len)
{
  int c;
  int ia5 = 0;
  ....
  if (!(((c >= 'a') && (c <= 'z')) ||
        ((c >= 'A') && (c <= 'Z')) ||
        (c == ' ') ||                   // <=
        ((c >= '0') && (c <= '9')) ||
        (c == ' ') || (c == '\'') ||    // <=
        (c == '(') || (c == ')') ||
        (c == '+') || (c == ',') ||
        (c == '-') || (c == '.') ||
        (c == '/') || (c == ':') ||
        (c == '=') || (c == '?')))
    ia5 = 1;
  ....
}

V501 There are identical sub-expressions '(c == ' ')' to the left and to the right of the '||' operator. a_print.c 77

Типичный пример ошибки, возникшей в результате Copy-Paste. Нередко, при большом количестве скопированных блоков, программист теряет внимание и забывает поменять одну переменную или константу в одном их них. Так и в этом случае в большом условном выражении перепутали значения, с которыми сравнивается переменная c. Точно сказать нельзя, но похоже на то, что забыли символ двойных кавычек '"'.

Фрагмент N3

static PyObject *
semlock_acquire(SemLockObject *self, PyObject *args, PyObject *kwds)
{
  ....
  HANDLE handles[2], sigint_event;
  ....
  /* prepare list of handles */
  nhandles = 0;
  handles[nhandles++] = self->handle;
  if (_PyOS_IsMainThread()) {
    sigint_event = _PyOS_SigintEvent();
    assert(sigint_event != NULL);
    handles[nhandles++] = sigint_event;
  }

  /* do the wait */
  Py_BEGIN_ALLOW_THREADS
  if (sigint_event != NULL) //<=
    ResetEvent(sigint_event);
  ....
}

V614 Potentially uninitialized pointer 'sigint_event' used. semaphore.c 120

В случае, когда функция _PyOS_IsMainThread() вернёт false, указатель sigint_event останется неинициализированным. Это приведёт к неопределённому поведению. Такую ошибку можно легко проглядеть в отладочной версии, где указатель, скорее всего, будет инициализирован нулём.

Фрагмент N4

#define BN_MASK2 (0xffffffffffffffffLL)
int BN_mask_bits(BIGNUM *a, int n)
{
  ....
  a->d[w] &= ~(BN_MASK2 << b); //<=
  ....
}

V610 Undefined behavior. Check the shift operator '<<'. The left operand '(0xffffffffffffffffLL)' is negative. bn_lib.c 796

Несмотря на то, что код в большинстве случаев работает, данное выражение по стандарту является неопределённым поведением. Подробнее о сдвигах отрицательных чисел вы можете прочитать в статье Андрея Карпова "Не зная брода, не лезь в воду. Часть третья". Стоит ли избегать конструкций, результат которых не гарантирован стандартом, решать вам, но лучше от такого отказаться, о чём нас анализатор и предупреждает.

static PyObject *
binascii_b2a_qp_impl(PyModuleDef *module,
                     Py_buffer *data,
                     int quotetabs,
                     int istext,
                     int header)
{
  Py_ssize_t in, out;
  const unsigned char *databuf;
  ....
  if ((databuf[in] > 126) ||
      (databuf[in] == '=') ||
      (header && databuf[in] == '_') ||
      ((databuf[in] == '.') && (linelen == 0) &&
      (databuf[in+1] == '\n' || databuf[in+1] == '\r' ||
                                 databuf[in+1] == 0)) ||
      (!istext && ((databuf[in] == '\r') ||
                   (databuf[in] == '\n'))) ||
      ((databuf[in] == '\t' || databuf[in] == ' ') &&
           (in + 1 == datalen)) ||
      ((databuf[in] < 33) &&
       (databuf[in] != '\r') && (databuf[in] != '\n') &&
       (quotetabs ||
      (!quotetabs && ((databuf[in] != '\t') && // <=
             (databuf[in] != ' '))))))
  {
  ....
  }
  ....
}

V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions 'quotetabs' and '!quotetabs'. binascii.c 1453

Данный фрагмент не является ошибочным, тем не менее, стоит рассмотреть его отдельно. Предупреждение носит во многом рекомендательный характер: выражение вида 'A || (!A && B)' можно и нужно упростить до 'A || B': это немного облегчит чтение и без того переусложнённого кода.

Аналогичные предупреждения:

• V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!type' and 'type'. digest.c 167
• V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!cipher' and 'cipher'. evp_enc.c 120

Фрагмент N5

static int dh_cms_set_peerkey(....)
{
  ....
  int atype;
  ....
  /* Only absent parameters allowed in RFC XXXX */
  if (atype != V_ASN1_UNDEF && atype == V_ASN1_NULL)
    goto err;
   ....
}

V590 Consider inspecting the 'atype != — 1 && atype == 5' expression. The expression is excessive or contains a misprint. dh_ameth.c 670

Как ни странно, ошибки в логических выражениях очень часто встречаются даже в больших проектах. Логическое выражение из этого фрагмента является избыточным: его можно упростить до 'atype == V_ASN1_NULL'. Скорее всего, исходя из контекста, ошибки здесь нет, но выглядит такой код подозрительно.

Фрагмент N6

static void cms_env_set_version(CMS_EnvelopedData *env)
{
  ....
  if (env->originatorInfo || env->unprotectedAttrs)
    env->version = 2;
  env->version = 0;
}

V519 The 'env->version' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 907, 908. cms_env.c 908

Сложно сказать, что изначально подразумевалось автором этого кода. Возможно, здесь пропущен else. Сейчас смысла в if нет, так как значение переменой 'env->version' будет в любом случае перезаписано.

Фрагмент N7

int
_PyState_AddModule(PyObject* module, struct PyModuleDef* def)
{
  PyInterpreterState *state;
  if (def->m_slots) {
    PyErr_SetString(PyExc_SystemError,
        "PyState_AddModule called on module with slots");
    return -1;
  }
  state = GET_INTERP_STATE();
  if (!def)
    return -1;
  ....
}

V595 The 'self->extra' pointer was utilized before it was verified against nullptr. Check lines: 917, 923. _elementtree.c 917

Традиционная ошибка, связанная с разыменовыванием нулевого указателя, которая встречается практически в каждом проекте. Сначала в выражении 'def->m_slots' обратились по какому-то адресу, а потом оказалось, что этот адрес мог быть нулевым. В итоге проверка на nullptr не работает, так как произойдет разыменование нулевого указателя, что приведёт к неопределённому поведению программы, например к её падению.

Ruby

Фрагмент N1

static void
vm_set_main_stack(rb_thread_t *th, const rb_iseq_t *iseq)
{
  VALUE toplevel_binding = rb_const_get(rb_cObject,
              rb_intern("TOPLEVEL_BINDING"));
  rb_binding_t *bind;
  rb_env_t *env;

  GetBindingPtr(toplevel_binding, bind);
  GetEnvPtr(bind->env, env);

  vm_set_eval_stack(th, iseq, 0, &env->block);

  /* save binding */
  if (bind && iseq->body->local_size > 0) {
    bind->env = vm_make_env_object(th, th->cfp);
  }
}

V595 The 'bind' pointer was utilized before it was verified against nullptr. Check lines: 377, 382. vm.c 377

Не избежали похожей ошибки и в проекте Ruby. Проверка 'if (bind)' не спасёт от беды, так как bind был разыменован выше по коду. Всего таких предупреждений в Ruby нашлось больше 30, приводить их все нет смысла.

Фрагмент N2

static int
code_page_i(....)
{
  table = realloc(table, count * sizeof(*table));
  if (!table) return ST_CONTINUE;
  ....
}

V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'table' is lost. Consider assigning realloc() to a temporary pointer. file.c 169

В этом участке кода значение realloc сохраняется в ту же переменную, которая используется в качестве аргумента. В случае, когда realloc вернёт nullptr, исходное значение указателя будет утеряно, что приведёт к утечке памяти.

Фрагмент N3

static int
w32_symlink(UINT cp, const char *src, const char *link)
{
  ....
  BOOLEAN ret;

  typedef DWORD (WINAPI *create_symbolic_link_func)
                               (WCHAR*, WCHAR*, DWORD);
  static create_symbolic_link_func create_symbolic_link =
         (create_symbolic_link_func)-1;

  ....
  ret = create_symbolic_link(wlink, wsrc, flag);
  ALLOCV_END(buf);

  if (!ret) {
    int e = GetLastError();
    errno = map_errno(e);
    return -1;
  }
  return 0;
}

V724 Converting type 'DWORD' to type 'BOOLEAN' can lead to a loss of high-order bits. Non-zero value can become 'FALSE'. win32.c 4974

Тип BOOLEAN используется в WinAPI в качестве логического типа. Он объявлен следующим образом:

typedef unsigned char BYTE;
typedef BYTE BOOLEAN;

DWORD является 32-битным беззнаковым числом. Поэтому, если привести, например, значение DWORD 0xffffff00 к BOOLEAN (или любое другое, у которого младший бит равен нулю), то оно станет равно 0, то есть FALSE.

Фрагмент N4

static VALUE
rb_str_split_m(int argc, VALUE *argv, VALUE str)
{
  ....
  char *ptr = RSTRING_PTR(str);
  long len = RSTRING_LEN(str);
  long start = beg;
  ....
  if (ptr+start == ptr+len)
    start++;
  ....
}

V584 The 'ptr' value is present on both sides of the '==' operator. The expression is incorrect or it can be simplified. string.c 7211

В обеих частях сравнения присутствует прибавление ptr, следовательно, его можно опустить:

if (start == len)

Скорее всего в этом фрагменте нет ошибки. Тем не менее, часто в таких выражениях на самом деле хотят сравнить две разные переменные. Поэтому лучше обращать внимания на подобные сравнения.

Итоги

Проанализировав все выданные предупреждения диагностик общего назначения и убрав все ложные срабатывания, я пришёл к следующему распределению ошибок:



Большая часть срабатываний в Ruby пришлась на предупреждение V610 (369 срабатываний!), но даже если их исключить, то ситуация не изменится: в СPython количество подозрительных мест всё равно меньше.

Наиболее частой оказалась диагностика V595: в коде Python она встретилась 17 раз, в коде Ruby – 37.

Но интереснее всего соотношение плотности ошибок. По этой характеристике Python также отличается в лучшую сторону. С результатами расчётов можно ознакомиться в таблице:



Может показаться, что ошибок многовато. Это не так. Во-первых, не все из найденных ошибок критичны. Например, упомянутая ранее диагностика V610, выявляет ошибки с точки зрения языка С++. Однако на практике для используемого набора компиляторов результат может быть всегда правильным. Хотя от этого ошибки не перестают быть ошибками, они никак в данный момент не сказываются на работе программы. Во-вторых, нужно учитывать размер проверенного кода. Поэтому можно говорить о высоком качестве этих проектов. Пока эта оценка субъективна, так как ранее мы не вычисляли плотность ошибок у проверенных проектов. Но постараемся это делать в дальнейшем, чтобы впоследствии можно было делать сравнения.

Заключение

Языки Python и Ruby очень популярны: на них пишут миллионы разработчиков со всего мира. При такой активности проектов и комьюнити, хорошем качестве кода, отличном тестировании и использовании статического анализа (оба проекта проверяются с помощью Coverity) сложно найти большое количество ошибок. Тем не менее, PVS-Studio удалось найти несколько подозрительных мест. Но стоит понимать, что именно регулярная проверка кода может серьёзно облегчить жизнь разработчикам. Лучше всего править ошибку сразу до того, как изменения попадут в репозиторий и в релиз — статический анализатор в этом может помочь, как никто другой.

Предлагаю всем желающим попробовать PVS-Studio на своих проектах.

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Pavel Belikov. Python and Ruby implementations compared by the error density.