Какой язык программирования начать изучать? Python или Ruby? Какой лучше? Django или Ruby on Rails? Такие вопросы часто можно найти на форумах ИТ по всему миру. Предлагаю сравнивать не сами языки, а их эталонные реализации: CPython и MRI. В этой статье мы расскажем об ошибках, которые PVS-Studio обнаружил в этих проектах.
Введение
Для анализа были взяты последние версии исходного кода из репозиториев (Ruby, Python). Проверка проекта проводилась с помощью статического анализатора кода PVS-Studio v6.06. Python можно легко скомпилировать в Visual Studio; для Ruby вы можете использовать автономную версию в режиме мониторинга компиляции.
Ярких ошибок было не так уж и много: большинство предупреждений связано с использованием макросов, которые расширяются в довольно подозрительный с точки зрения анализатора код, но довольно невинный с точки зрения разработчика. Можно было бы начать долгую дискуссию о том, приносят ли макросы вред или пользу, но мы можем точно сказать, что анализатору они не очень нравятся. Чтобы избавиться от надоедливого макроса, есть возможность подавить ложные срабатывания. Достаточно написать:
//-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 Выражение s-› sock_fd ‹0 всегда ложно. Значение беззнакового типа никогда не бывает <0. socketmodule.c 655
Тип SOCKET в Windows беззнаковый, поэтому сравнивать его с null бессмысленно. Проверка функции socket () вернула правильный дескриптор, необходимо сравнить его значение с INVALID_SOCKET. Стоит отметить, что это сравнение будет корректно работать в Linux, потому что там в качестве типа сокета используется подписанный тип int, а значение -1 указывает на ошибку. Тем не менее для проверки лучше использовать специальные макросы или константы.
Еще несколько подобных проверок, за которые анализатор выдавал предупреждения.
- V547 Выражение «s-› sock_fd ‹0» всегда ложно. Значение беззнакового типа никогда не <0. _ssl.c 1702
- V547 Выражение «sock-› sock_fd ‹0» всегда ложно. Значение беззнакового типа никогда не <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 Есть идентичные подвыражения ‘(c ==‘ ‘)’ слева и справа от оператора ‘||’. a_print.c 77
Типичный пример ошибки, возникшей в результате копирования-вставки. Довольно часто при использовании большого количества скопированных блоков внимание программиста падает, и он забывает изменить в них переменную или константу. Например, в этом случае в большом условном выражении программист перепутал значения, с которыми сравнивается переменная 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 Используется потенциально неинициализированный указатель sigint_event. 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 Неопределенное поведение. Проверьте оператор смены ‘‹(0xffffffffffffffffLL) отрицательный. 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 Излишнюю проверку можно упростить. Оператор || окружен противоположными выражениями quotetabs и ! Quotetabs. binascii.c 1453
Этот фрагмент не ошибочен, тем не менее, к нему следует присмотреться. Предупреждение - это в основном рекомендация: выражение ‘A || (! A && B) ’ можно упростить до ‘ A || B ’:, что упростит чтение этого довольно сложного кода.
Подобные предупреждения:
- V728 Чрезмерную проверку можно упростить. Оператор ‘||’ окружен противоположными выражениями ‘! Type’ и ‘type’. digest.c 167
- V728 Чрезмерную проверку можно упростить. Оператор «||» окружен противоположными выражениями «! Cipher» и «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 Рассмотрите возможность проверки выражения atype! = - 1 && atype == 5. Выражение является чрезмерным или содержит опечатку. 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 Переменной env ›version присваиваются значения дважды подряд. Возможно, это ошибка. Проверить строки: 907, 908. cms_env.c 908
Трудно сказать, что имел в виду автор при написании этого кода. Возможно, здесь опущено else. На этом этапе нет смысла использовать if, поскольку значение переменной ‘env-› version ’ в любом случае будет перезаписано.
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 Указатель self-› extra использовался до того, как он был проверен на соответствие nullptr. Проверьте линии: 917, 923. _elementtree.c 917
Это традиционная ошибка, связанная с разыменованием нулевого указателя, которая встречается почти в каждом проекте. Во-первых, в выражении ‘def-› m_slots ’ программист получил доступ по некоторому адресу, а затем выяснилось, что этот адрес мог быть нулевым. В результате проверка на nullptr не будет работать, так как у нас будет разыменование нулевого указателя, что приведет, например, к неопределенному поведению программы и ее сбою.
Рубин
Фрагмент 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 Указатель привязки использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 377, 382. vm.c 377
Аналогичная ошибка также была обнаружена в проекте Ruby. Проверка ‘if (bind)’ не поможет, потому что ссылка на bind была разыменована немного раньше в коде. Таких предупреждений было более 30, поэтому перечислять их все здесь нет смысла.
Фрагмент N2
static int
code_page_i(....)
{
table = realloc(table, count * sizeof(*table));
if (!table) return ST_CONTINUE;
....
}
Возможная утечка V701 realloc (): когда realloc () не может выделить память, исходный указатель table теряется. Рассмотрите возможность присвоения realloc () временному указателю. 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 Преобразование типа DWORD в тип BOOLEAN может привести к потере старших битов. Ненулевое значение может стать ЛОЖЬ. 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 Значение ptr присутствует с обеих сторон оператора ==. Выражение неверное или его можно упростить. string.c 7211
В обеих частях сравнения есть добавление ptr, поэтому его можно удалить:
if (start == len)
Но, скорее всего, в этом фрагменте нет ошибки. Однако мы довольно часто видим, что программисты сравнивают две разные переменные в таких выражениях. Вот почему такие сравнения всегда стоит пересматривать.
Общие результаты
Проанализировав все предупреждения общей диагностики анализа и убрав все ложные срабатывания, я пришел к следующему выводу о плотности ошибок:
Большинство предупреждений в Ruby было выдано диагностикой V610 (369 предупреждений!), Но даже если их исключить, ситуация мало что изменится: Python опережает Ruby по количеству подозрительных фрагментов.
Самой частой диагностикой оказалась V595 - в Python было 17 предупреждений, а в Ruby - 37.
Конечно, гораздо интереснее посмотреть на коэффициент плотности ошибок. Python также значительно опережает Ruby в этой категории. Вот результаты оценок, представленные в виде таблицы:
Может показаться, что количество ошибок довольно велико. Но это не так. Во-первых, не все ошибки критичны. Например, уже упоминавшаяся диагностика V610 выявляет ошибки с точки зрения языка C ++. Однако на практике для набора компиляторов результат всегда может быть правильным. Хотя эти ошибки по-прежнему являются ошибками, они никоим образом не влияют на программу. Во-вторых, нужно учитывать размер кода. Поэтому можно сказать, что качество этих проектов достаточно высокое. На данный момент это может быть довольно субъективным, потому что ранее мы не оценивали плотность ошибок этих проектов. Мы постараемся сделать это в будущем, чтобы потом сравнить результаты проверок.
Вывод
Python и Ruby чрезвычайно популярны: миллионы разработчиков используют их для написания кода. Трудно найти большое количество ошибок в проекте, когда он так активно используется, регулярно тестируется другим инструментом статического анализа (оба проекта проверяются Coverity) и имеет поддержку сообщества. Тем не менее, PVS-Studio удалось найти несколько подозрительных фрагментов. Мы должны понимать, что это регулярные проверки, которые могут значительно облегчить жизнь программистам. В идеале ошибка должна быть исправлена до того, как изменения попадут в репозиторий и выпущены - и здесь лучше всего может помочь статический анализатор.
Предлагаю запустить PVS-Studio на своих проектах.
Обновление. Эта статья содержит несколько неточностей. Пожалуйста, смотрите обновленную версию здесь: http://www.viva64.com/ru/b/0418/
