Сворачиваем CPython вокруг PVS-Studio

Python... язык программирования, не нуждающийся в особом представлении. За удобство в обработке "больших данных" заслуженно получил звание "лучшего Excel". За удобство интеграции в C и C++ код его любит геймдев. А также у него низкий порог вхождения!

Введение

В этой статье мы рассмотрим основную и самую распространённую вариацию интерпретатора Python — CPython. Из названия следует, что она написана на C. Предыдущая проверка была почти девять лет назад, и без уточнений тоже не обошлось. С тех пор язык сильно шагнул вперёд в серии релизов Python 3, и внутренний мир интерпретатора изменялся вместе с ним. API Python/C тоже расширялся, переписывались по несколько раз встроенные модули интерпретатора, а некоторые ушли в историю после стадии DeprecationWarning. И вот однажды...

Установка PVS-Studio

Начнём с установки PVS-Studio. Загрузить установочный дистрибутив можно с этой страницы. Для запуска анализа понадобится лицензия — её триальную версию так же просто получить, буквально по щелчку мыши и быстрому аккорду пальцами по клавиатуре. Установка анализатора тоже пройдёт легко: нам понадобится только одна дополнительная интеграция для Visual Studio 2022. Актуальная на момент написания статьи версия PVS-Studio — 7.35.

Конфигурация сборки и анализа

И вот однажды на наш стол попал готовящийся к выпуску релиз CPython 3.13.3, который мы взяли на коммите e6dfa9d. Обычно я проверяю проекты в "отладочной" (Debug) конфигурации, но в этот раз меня разобрало любопытство и захотелось попробовать запустить анализ на "релизной" (Release), чтобы посмотреть на происходящее в интерпретаторе, поставляемом конечному пользователю.

Современные версии CPython 3 требуют немного повозиться перед запуском сборки под Windows: перейдите в папку PCbuild и запустите из неё файл get_externals.bat. Будут загружены сторонние зависимости, необходимые для сборки интерпретатора.

Анализ найденных ошибок

В той же папке PCbuild находится файл решения pcbuild.sln, настроенный для полной сборки CPython. Когда запускался batch-файл get_externals.bat, в консоли перечислялись имена папок сторонних зависимостей:

D:\Git\cpython\PCbuild>get_externals.bat
Using py -3.12 (found 3.12 with py.exe)
Fetching external libraries...
Fetching bzip2-1.0.8...
Fetching mpdecimal-4.0.0...
Fetching sqlite-3.45.3.0...
Fetching xz-5.2.5...
Fetching zlib-1.3.1...
Fetching external binaries...
Fetching libffi-3.4.4...
Fetching openssl-bin-3.0.15...
Fetching tcltk-8.6.15.0...
Finished.

Не надо перечислять каждую папку для исключения из анализа: достаточно добавить одну папку externals, находящуюся уровнем выше. Чтобы исключить её из анализа при работе в Visual Studio, перейдите в соответствующий раздел настроек: Extensions > PVS-Studio > Options > Don't Check Files. Это не единственный способ изменить исключённые из анализа папки — больше способов описано в нашей документации. Для своего удобства я исключил сторонние зависимости через абсолютный путь:

D:\Git\cpython\externals\

Что ж, лезем внутрь...

MD5 вселяет уверенность...

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

V522 There might be dereferencing of a potential null pointer 'p'. Check lines: 1174, 1173. Hacl_Hash_MD5.c 1174

#ifndef KRML_HOST_MALLOC
#  define KRML_HOST_MALLOC malloc
#endif

Hacl_Streaming_MD_state_32 *Hacl_Hash_MD5_malloc(void)
{
  ....
  Hacl_Streaming_MD_state_32
  *p = (Hacl_Streaming_MD_state_32 *)
        KRML_HOST_MALLOC(sizeof (Hacl_Streaming_MD_state_32));
  p[0U] = s;                                                   // <=
  ....
}

V575 The potential null pointer is passed into 'memcpy' function. Inspect the first argument. Check lines: 1415, 1414. Hacl_Hash_MD5.c 1415

V575 The potential null pointer is passed into 'memcpy' function. Inspect the first argument. Check lines: 1417, 1416. Hacl_Hash_MD5.c 1417

#ifndef KRML_HOST_CALLOC
#  define KRML_HOST_CALLOC calloc
#endif

Hacl_Streaming_MD_state_32 *
Hacl_Hash_MD5_copy(Hacl_Streaming_MD_state_32 *state)
{
  Hacl_Streaming_MD_state_32 scrut = *state;
  uint32_t *block_state0 = scrut.block_state;
  uint8_t *buf0 = scrut.buf;
  ....
  uint8_t *buf = (uint8_t *)KRML_HOST_CALLOC(64U, sizeof (uint8_t));
  memcpy(buf, buf0, 64U * sizeof (uint8_t));                         // <=
  uint32_t *block_state = (uint32_t *)KRML_HOST_CALLOC(4U, sizeof (uint32_t));
  memcpy(block_state, block_state0, 4U * sizeof (uint32_t));         // <=
  ....
}

Пошло довольно кучно. Нет, эти три срабатывания не являются основной причиной, из-за которой MD5 признали некриптостойким алгоритмом хеширования. Дело в другом: выделение памяти через calloc/malloc не гарантирует возврата указателя на выделенную память.

Функция memcpy с виду безобидная букашка. Настолько безобидная, что тот же MSVC за использование этой функции побьёт SDL-проверками и будет прав. Теоретически memcpy может успеть повредить данные в памяти перед тем, как порвать интерпретатор на части из-за NULL вместо буфера. Реализация этой функции не обязана читать области памяти с начала, и если переданный размер достаточно большой, то перед падением программы она может получить доступ к запрещённой области памяти.

...а управляющие структуры — нет?

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

V547 Expression 'fut->fut_state != STATE_PENDING' is always false. _asynciomodule.c 506

static PyObject *
future_set_exception(asyncio_state *state, FutureObj *fut, PyObject *exc)
{
  PyObject *exc_val = NULL;

  if (fut->fut_state != STATE_PENDING) {           // <=
      PyErr_SetString(state->asyncio_InvalidStateError, "invalid state");
      return NULL;
  }

  if (PyExceptionClass_Check(exc)) {
      exc_val = PyObject_CallNoArgs(exc);
      if (exc_val == NULL) {
        return NULL;
      }
      if (fut->fut_state != STATE_PENDING) {      // <=
          Py_DECREF(exc_val);
          PyErr_SetString(state->asyncio_InvalidStateError, "invalid state");
          return NULL;
      }
  }
  ....
}

Очень частый случай. Уже есть проверка fut->fut_state:

if (fut->fut_state != STATE_PENDING) {           // <=
    PyErr_SetString(state->asyncio_InvalidStateError, "invalid state");
    xreturn NULL;
}

Мы сообщили о неправильном состоянии просто текстом, и теперь хотим проверить, надо ли нам бросать исключение... и снова проверяем значение fut->fut_state? Но первая проверка показала, что там STATE_PENDING! В итоге лишняя ссылка на объект исключения так и будет висеть. Уплощаем и упрощаем код:

if (PyExceptionClass_Check(exc)) {
    exc_val = PyObject_CallNoArgs(exc);
    if (exc_val == NULL) {
      return NULL;
    }
    Py_DECREF(exc_val);
    PyErr_SetString(state->asyncio_InvalidStateError, "invalid state");
    return NULL;
}

Вот теперь порядок. Коллекция схожих срабатываний:

• V547 Expression 'start_state != deque->state' is always false. _collectionsmodule.c 1156

• V547 Expression 'start_state != deque->state' is always false. _collectionsmodule.c 1190

• V547 Expression 'start_state != deque->state' is always false. _collectionsmodule.c 1450

Невозможное невозможно

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

V547 Expression 'error_handler == _Py_ERROR_UNKNOWN' is always false. unicodeobject.c 15765

Осторожно, сейчас будет много контекста:

static _Py_error_handler
get_error_handler_wide(const wchar_t *errors)
{
    if (errors == NULL || wcscmp(errors, L"strict") == 0) {
        return _Py_ERROR_STRICT;
    }
    if (wcscmp(errors, L"surrogateescape") == 0) {
        return _Py_ERROR_SURROGATEESCAPE;
    }
    if (wcscmp(errors, L"replace") == 0) {
        return _Py_ERROR_REPLACE;
    }
    if (wcscmp(errors, L"ignore") == 0) {
        return _Py_ERROR_IGNORE;
    }
    if (wcscmp(errors, L"backslashreplace") == 0) {
        return _Py_ERROR_BACKSLASHREPLACE;
    }
    if (wcscmp(errors, L"surrogatepass") == 0) {
        return _Py_ERROR_SURROGATEPASS;
    }
    if (wcscmp(errors, L"xmlcharrefreplace") == 0) {
        return _Py_ERROR_XMLCHARREFREPLACE;
    }
    return _Py_ERROR_OTHER;
}

static int
init_fs_codec(PyInterpreterState *interp)
{
    const PyConfig *config = _PyInterpreterState_GetConfig(interp);

    _Py_error_handler error_handler;
    error_handler = get_error_handler_wide(config->filesystem_errors);
    if (error_handler == _Py_ERROR_UNKNOWN) {                              // <=
        PyErr_SetString(PyExc_RuntimeError, "unknown filesystem error handler");
        return -1;
    }
    ....
}

Если вы чувствуете запах подгоревшей чешуи, вы на верном пути :) Если не чувствуете, не бойтесь, сейчас всё покажем.

Функция get_error_handler_wide возвращает что угодно, кроме _Py_ERROR_UNKNOWN, на который и идёт проверка. Из неё невозможно выйти с этим результатом: все пути ветвления заканчиваются на return с каким-либо определением ошибки. Раз у нас богатый выбор сюжетных концовок, почему бы не посмотреть все? Хм-м-м...

typedef enum {
    _Py_ERROR_UNKNOWN=0,          // <=
    _Py_ERROR_STRICT,
    _Py_ERROR_SURROGATEESCAPE,
    _Py_ERROR_REPLACE,
    _Py_ERROR_IGNORE,
    _Py_ERROR_BACKSLASHREPLACE,
    _Py_ERROR_SURROGATEPASS,
    _Py_ERROR_XMLCHARREFREPLACE,
    _Py_ERROR_OTHER
} _Py_error_handler;

Ой, а тут есть одна секретная незадействованная концовка. Можно спекулятивно предположить, что вместо "прочей ошибки" и должна быть "неизвестная ошибка", но когда использовать "неизвестную ошибку", если есть "прочая ошибка"? Возможно, ответ дадут несколько мест, где явно присваивается это состояние, но для рассматриваемого случая они все мимо.

Объективно не нулевой

Сообщение PVS-Studio:

V547 Expression 'capi == NULL' is always false. _datetimemodule.c 7236

/* The C-API is process-global.  This violates interpreter isolation
 * due to the objects stored here.  Thus each of those objects must
 * be managed carefully. */
// XXX Can we make this const?
static PyDateTime_CAPI capi = {
    ....
}

/* Get a new C API by calling this function.
 * Clients get at C API via PyDateTime_IMPORT, defined in datetime.h.
 */
static inline PyDateTime_CAPI *
get_datetime_capi(void)
{
    return &capi;
}

static int
_datetime_exec(PyObject *module)
{
    ....
    /* At last, set up and add the encapsulated C API */
    PyDateTime_CAPI *capi = get_datetime_capi();
    if (capi == NULL) {                            // <=
        goto error;
    }
    ....
}

Посмотрите на вызов get_datetime_capi. Судя по коду, функция возвращает указатель на PyDateTime, и программист предположил, что он может быть нулевым. Взглянем на её реализацию: она просто возвращает указатель на статическую переменную в глобальной области видимости. Нет никаких причин думать, что на момент вызова этой функции ей не будет присвоено значение. Точнее, корректного значения может и не быть, но адрес у этой переменной точно будет. Более того, объявление структур и функций для Python/C расширений статическими — стандартный подход к их написанию, теряться нечему. Если всё ещё не верите, то в ход пойдёт тяжёлая артиллерия — WinDbg. Отладчик, который в первую очередь предназначен для работы на уровне ядра Windows!

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

SDK бьёт по рукам

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

V547 Expression 't != u' is always false. _datetimemodule.c 5245

static long long
local(long long u)
{
    struct tm local_time;
    time_t t;
    u -= epoch;
    t = u;
    if (t != u) {
        PyErr_SetString(PyExc_OverflowError,
        "timestamp out of range for platform time_t");
        return -1;
    }
    ....
}

Обсуждение этого срабатывания превратилось в 38-минутный спор на повышенных тонах, и это не шутка для подогрева интереса. В чём проблема?

Читатель уже догадывается, что я скажу про:

t = u;
if (t != u) { .... }

И он будет мыслить в правильном направлении, мол, что за безобразие, сравниваем только что присвоенное значение с самим собой, ведь time_t уже long long! А вот не всё так просто.

Тип time_t принудительно задаётся 64-битным, начиная с Visual Studio 2005. Нельзя использовать 32-битный time_t на 64-битной платформе, о чём нам говорит стандартный заголовочный файл corecrt.h из Universal C Runtime. Visual Studio использует UCRT с Visual Studio 2015.

#if defined _USE_32BIT_TIME_T && defined _WIN64
    #error You cannot use 32-bit time_t (_USE_32BIT_TIME_T) with _WIN64
#endif

#ifndef _CRT_NO_TIME_T
    #ifdef _USE_32BIT_TIME_T
        typedef __time32_t time_t;
    #else
        typedef __time64_t time_t;
    #endif
#endif

Visual Studio 2005 тоже это запрещает, о чём свидетельствует похожая цепочка директив препроцессора в стандартном заголовочном файле crtdefs.h из Microsoft Visual C Runtime (MSVCRT).

#ifdef  _USE_32BIT_TIME_T
#ifdef  _WIN64
#error You cannot use 32-bit time_t (_USE_32BIT_TIME_T) with _WIN64
#undef  _USE_32BIT_TIME_T
#endif
#else
#if     _INTEGRAL_MAX_BITS < 64
#define _USE_32BIT_TIME_T
#endif
#endif

Но одно дело MSVC, а если взять MinGW? Там будет та же самая картина из MSVC, просто форматирование отличается. Проблемы могут начаться, если time_t не определён в 64-битное целое число, но это уже совсем другая история.

Дезориентация в пространстве

Пожалуй, хватит жести. Давайте немного отдохнём и посмотрим на что-нибудь попроще. Например, сразу три сообщения PVS-Studio:

V523 The 'then' statement is equivalent to the 'else' statement. crossinterp.c 1416

void
_PyXI_FreeNamespace(_PyXI_namespace *ns)
{
    ....
    if (interpid == PyInterpreterState_GetID(PyInterpreterState_Get())) {
        _sharedns_free(ns);
    }
    else {
        // If we weren't always dynamically allocating the cross-interpreter
        // data in each item then we would need to using a pending call
        // to call _sharedns_free(), to avoid the race between freeing
        // the shared namespace and releasing the XI data.
        _sharedns_free(ns);
    }
}

V524 It is odd that the body of 'FStar_UInt128_u32_combine_' function is fully equivalent to the body of 'FStar_UInt128_u32_combine' function. FStar_UInt128_Verified.h 309

static inline
uint64_t FStar_UInt128_u32_combine(uint64_t hi, uint64_t lo)  // <=
{
  return lo + (hi << FStar_UInt128_u32_32);
}

static inline
uint64_t FStar_UInt128_u32_combine_(uint64_t hi, uint64_t lo) // <=
{
  return lo + (hi << FStar_UInt128_u32_32);
}

V1048 The 'val' variable was assigned the same value. blob.c 491

static int
ass_subscript_index(pysqlite_Blob *self, PyObject *item, PyObject *value)
{
    ....
    long val = PyLong_AsLong(value);
    if (val == -1 && PyErr_Occurred()) {
        PyErr_Clear();
        val = -1;                          // <=
    }
    ....
}

У всех трёх примеров есть общая проблема — случайно сорвавшаяся рука или неожиданное сбитие с мысли.

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

Второй вариант проблемы наталкивает на вопрос: сбитие кого с мысли, писателя или читателя, — особенно при рассмотрении второго фрагмента кода. Да, это inline, да, в теле функции буквально один и тот же текст, компилятор может встроить её и никто не заметит подвоха. Вот только он не обязан это делать. Предположим, что встраивания тела функции не произошло. Обе функции также static, и если не сработает inline, то это будут обычные обращения к двум разным статическим функциям.

Третья проблема возникла, вероятно, на почве сомнений в собственной документации. Функция PyLong_AsLong возвращает -1, если вывести из PyObject* число типа long не удалось. Это функция из стабильного ABI, изменение этого поведения маловероятно.

По стопам прошлых проверок

"Дежавю?" — спросил я себя, читая следующие сообщения от PVS-Studio:

V781 The value of the 'i' index is checked after it was used. Perhaps there is a mistake in program logic. lexer.c 143

static int
set_fstring_expr(struct tok_state* tok, struct token *token, char c) {
  tokenizer_mode *tok_mode = TOK_GET_MODE(tok);
  ....
  if (hash_detected) {
    Py_ssize_t input_length =
      tok_mode->last_expr_size - tok_mode->last_expr_end;

    ....

    for (i = 0, j = 0; i < input_length; i++) {
      if (tok_mode->last_expr_buffer[i] == '#') {
        // Skip characters until newline or end of string
        while (   tok_mode->last_expr_buffer[i] != '\0'
               && i < input_length) {                           // <=
          ....
        }
....
}

Не показалось. Такое уже было в x64dbg. Доподлинно неизвестно, будет ли input_length всегда меньше i, и не возникнет ли чтение за пределами массива, если условие не будет соблюдено.

V1109 The 'PathRemoveFileSpecW' function is deprecated. Consider switching to an equivalent newer function. pyshellext.cpp 464

class DECLSPEC_UUID(CLASS_GUID) PyShellExt : public RuntimeClass<
  RuntimeClassFlags<ClassicCom>,
  IDropTarget,
  IPersistFile
>
{
public:
  STDMETHODIMP Load(LPCOLESTR pszFileName, DWORD dwMode) {
    ....
    if (!PathRemoveFileSpecW(target_dir)) {                     // <=
        OutputDebugStringW(
          L"PyShellExt::Load - failed to remove filespec from target"
        );
        return E_FAIL;
    }
    ....
  }
}

Срабатывание диагностики V1109 в дебаггере тоже было, но на другую функцию. Для расширения оболочки проводника Windows это срабатывание актуально по другой причине: на официальном сайте прямо сказано, что Python 3.13 не поддерживается на Windows 7 и более старых версиях системы.

Вместо устаревшей функции Microsoft рекомендует использовать PathCchRemoveFileSpec, доступную для использования с Windows 8. Преимущество новой функции с функциональной стороны — возможность работы с UNC-путями, в первую очередь с сетевыми папками. С технической стороны — исключён риск переполнения буфера. Функцию уже используют в CPython и вполне возможно, что актуализация используемых функций Windows API растянулась во времени.

Заключение

Сложно комментировать происходящее человеку, пишущему в свободное время преимущественно на Python 2.6/2.7 :)

За последние несколько лет в Python 3 произошло очень много изменений, обсуждаемых многими критиками. Одних возмущает увеличение количества синтаксического сахара, других радует возможность в официальном CPython выключить глобальную блокировку интерпретатора. Обеспечить безопасность самого интерпретатора и стандартных библиотек, безусловно, важный пункт, который надо учитывать в процессе разработки, иначе знаменитые "батарейки" рискуют "протечь".

Интерпретатор языка программирования с простым синтаксисом сложно устроен изнутри, и с возрастом проекта эта сложность неизбежно увеличивается. По забавному совпадению, в обсуждении одного из запросов на слияние появился вопрос про применение статического анализа, хотя бы Clang-Tidy, и сразу же следует ответ, что "сложно настроить линтер, который будет игнорировать существующий код и работать только с изменённым". PVS-Studio не линтер, но умеет проводить анализ модифицированных файлов, коммитов и запросов на слияние. А если у вас есть свой проект с открытым исходным кодом, то вы можете получить бесплатную лицензию на год с возможностью продления в будущем.

Поговаривают, что Python — это про чистый код, поэтому всем коллективом пожелаем именно его Python Software Foundation и участникам разработки от сообщества!

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Taras Shevchenko. Curling CPython around PVS-Studio.