Порівнюємо реалізацію мов Python і Ruby за щільності помилок

Яку мову програмування почати вчити: 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.

Прочитали статтю і є питання?Часто до наших статей задають одні і ті ж питання. Відповіді на них ми зібрали тут: Відповіді на питання читачів статей про PVS-Studio, версія 2015. Будь ласка, ознайомтеся зі списком.

Джерело: Хабрахабр

0 коментарів

Тільки зареєстровані та авторизовані користувачі можуть залишати коментарі.