# Проверяем эмулятор RPCS3 с помощью PVS\-Studio

RPCS3 – интересный проект, который эмулирует консоль PS3\. Он активно развивается: недавно была новость о том, что он научился запускать все игры из своего каталога\. Это хороший повод для проверки – посмотрим, какие ошибки остались после исправлений\.

![0886_rpcs3_ru/image1.png](https://import.viva64.com/docx/blog/0886_rpcs3_ru/image1.png)

## Введение

Проект довольно увесистый\. Он насчитывает около 300 тысяч строк кода на C\+\+ и к тому же тянет за собой большое количество внешних зависимостей, в числе которых есть:

* llvm – инструментарий для написания компиляторов и утилит\. К слову, недавно мы проверяли [LLVM 13](https://pvs-studio.ru/ru/blog/posts/cpp/0871/);
* ffmpeg – библиотека для работы с медиафайлами;
* curl – нужна для взаимодействия с сетью и работы с протоколом HTTP;
* zlib – библиотека сжатия данных, использующая алгоритм DEFLATE\.

Для GUI части также понадобится Qt, однако он берётся из системы\. Полный список зависимостей можно посмотреть на скриншоте:

![0886_rpcs3_ru/image2.png](https://import.viva64.com/docx/blog/0886_rpcs3_ru/image2.png)

Что примечательно, используемый стандарт языка C\+\+  – свежий C\+\+20\. PVS\-Studio хорошо справился с проверкой такого современного кода, ведь мы активно работаем над поддержкой нововведений\. Да, есть некоторые недоработки, но мы их постепенно исправим\. В целом проверка проекта послужила хорошим тестом поддержки новых языковых конструкций\.

Проект использует сборочную систему CMake\. К сожалению, во время сборки у меня возникли проблемы – GCC 11\.2 отказывался компилировать какую\-то constexpr\-конструкцию, однако Clang 12 справился со сборкой отлично\. Сборку я производил под devel\-версией Ubuntu, так что, возможно, это проблема дистрибутива\.

Полная процедура сборки и проверки под Linux в режиме [межмодульного анализа](https://pvs-studio.ru/ru/blog/posts/cpp/0851/) выглядит так:

```cpp
cmake -S. -Bbuild -DCMAKE_EXPORT_COMPILE_COMMANDS=On -DCMAKE_BUILD_TYPE=Debug \
          -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
cmake --build build -j$(nproc)
pvs-studio-analyzer analyze -f ./build/compile_commands.json -j`nproc` \
          -o pvs.log -e 3rdparty/ -e llvm/ --intermodular
```

Что же, проект проверен – время перейти к ошибкам\!

## Не пиши в std, брат

[V1061](https://pvs-studio.ru/ru/docs/warnings/v1061/) Extending the 'std' namespace may result in undefined behavior\. shared\_ptr\.hpp 1131

```cpp
namespace std
{
  template <typename T>
  void swap(stx::single_ptr<T>& lhs, stx::single_ptr<T>& rhs) noexcept
  {
    lhs.swap(rhs);
  }

  template <typename T>
  void swap(stx::shared_ptr<T>& lhs, stx::shared_ptr<T>& rhs) noexcept
  {
    lhs.swap(rhs);
  }
}
```

Стандарт C\+\+ явно запрещает определение пользовательских шаблонов функций в пространстве имён _std_, а C\+\+20 также запрещает определение специализаций шаблонов функций\. Определение пользовательской функции _swap_ – частый случай такой ошибки\. В этой ситуации следует поступать таким образом:

* определить функцию _swap_ в том же пространстве имён, где определён класс \(_stx_\);
* в блоке, где понадобится вызов _swap_, нужно написать директиву _using std::swap;_
* вызов swap должен быть без указания пространства имён _std_, т\.е\. неквалифицированным: _swap\(obj1, obj2\);_

При таком подходе работает механизм Argument\-Dependent Lookup \(ADL\), в результате чего находится функция _swap_, которую мы определили рядом с классом\. А пространство имён _std_ не изменено\.

## Удалённый memset

[V597](https://pvs-studio.ru/ru/docs/warnings/v597/) The compiler could delete the 'memset' function call, which is used to flush 'cty' object\. The memset\_s\(\) function should be used to erase the private data\. aes\.cpp 596

```cpp
/*
 * AES key schedule (decryption)
 */
int aes_setkey_dec(....)
{
    aes_context cty;

    // ....

done:
    memset( &cty, 0, sizeof( aes_context ) );

    return( 0 );
}
```

Частая ошибка – вызов _memset_ будет удалён компилятором во время оптимизаций, а приватные данные останутся лежать в памяти\. Да, в случае кода эмулятора вряд ли это представляет какую\-либо угрозу утечки данных, но тем не менее ошибка присутствует\.

Также были найдены ещё случаи такой ошибки:

* [V597](https://pvs-studio.ru/ru/docs/warnings/v597/) The compiler could delete the 'memset' function call, which is used to flush 'tmpbuf' buffer\. The memset\_s\(\) function should be used to erase the private data\. sha1\.cpp 371
* [V597](https://pvs-studio.ru/ru/docs/warnings/v597/) The compiler could delete the 'memset' function call, which is used to flush 'ctx' object\. The memset\_s\(\) function should be used to erase the private data\. sha1\.cpp 396

## Избыточная проверка

[V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'rawcode \=\= CELL\_KEYC\_KPAD\_NUMLOCK' is always false\. cellKb\.cpp 126

```cpp
enum Keys
{
  // ....
  CELL_KEYC_KPAD_NUMLOCK          = 0x53,
  // ....
};

u16 cellKbCnvRawCode(u32 arrange, u32 mkey, u32 led, u16 rawcode)
{
  // ....

  // CELL_KB_RAWDAT
  if (rawcode <= 0x03
      || rawcode == 0x29
      || rawcode == 0x35
      || (rawcode >= 0x39 && rawcode <= 0x53)    // <=
      || rawcode == 0x65
      || rawcode == 0x88
      || rawcode == 0x8A
      || rawcode == 0x8B)
  {
    return rawcode | 0x8000;
  }

  const bool is_alt = mkey & (CELL_KB_MKEY_L_ALT | CELL_KB_MKEY_R_ALT);
  const bool is_shift = mkey & (CELL_KB_MKEY_L_SHIFT | CELL_KB_MKEY_R_SHIFT);
  const bool is_caps_lock = led & (CELL_KB_LED_CAPS_LOCK);
  const bool is_num_lock = led & (CELL_KB_LED_NUM_LOCK);

  // CELL_KB_NUMPAD

  if (is_num_lock)
  {
    if (rawcode == CELL_KEYC_KPAD_NUMLOCK)  return 0x00 | 0x4000; // <=
    if (rawcode == CELL_KEYC_KPAD_SLASH)    return 0x2F | 0x4000;
    if (rawcode == CELL_KEYC_KPAD_ASTERISK) return 0x2A | 0x4000;
    if (rawcode == CELL_KEYC_KPAD_MINUS)    return 0x2D | 0x4000;
    if (rawcode == CELL_KEYC_KPAD_PLUS)     return 0x2B | 0x4000;
    if (rawcode == CELL_KEYC_KPAD_ENTER)    return 0x0A | 0x4000;
    if (rawcode == CELL_KEYC_KPAD_0)        return 0x30 | 0x4000;
    if (rawcode >= CELL_KEYC_KPAD_1 && rawcode <= CELL_KEYC_KPAD_9)
      return (rawcode - 0x28) | 0x4000;
  }
}
```

Здесь ошибка кроется в первом условии: оно перекрывает условие ниже, которое проверяет значение переменной _rawcode_ на равенство константе _CELL\_KEYC\_KPAD\_NUMLOCK_\. Значение _CELL\_KEYC\_KPAD\_NUMLOCK_ соответствует числу 0x53 – это число попадает под первое условие, которое в результате выполнения выходит из функции, так что нижний _if_ никогда не выполнится\.

Ошибка может быть в двух местах – либо в первом условии не учтено значение константы, либо сама константа определена неверно\.

## Выход за границу массива

[V557](https://pvs-studio.ru/ru/docs/warnings/v557/) Array underrun is possible\. The value of 'month \+ \- 1' index could reach \-1\. cellRtc\.cpp 1470

```cpp
error_code cellRtcGetDaysInMonth(s32 year, s32 month)
{
  cellRtc.todo("cellRtcGetDaysInMonth(year=%d, month=%d)", year, month);

  if ((year < 0) || (month < 0) || (month > 12))
  {
    return CELL_RTC_ERROR_INVALID_ARG;
  }

  if (is_leap_year(year))
  {
    return not_an_error(DAYS_IN_MONTH[month + 11]);
  }

  return not_an_error(DAYS_IN_MONTH[month + -1]); // <=
}
```

В этом случае в операторе return обращение к массиву _DAYS\_IN\_MONTH_ может произойти по индексу \-1, т\.к\. значение аргумента _month_ может быть равно 0\.

Скорее всего, ошибка находится в первом условии – судя по коду, отсчет месяцев ведётся с единицы, а условие проверяет на то, что _month_ меньше 0, когда надо _month < 1_\.

Эта ошибка напомнила мне интересный случай из проекта protobuf: [31 февраля](https://pvs-studio.ru/ru/blog/posts/cpp/0550/)\.

## Ошибка копипасты

[V519](https://pvs-studio.ru/ru/docs/warnings/v519/) The 'evnt\-\>color\.white\_x' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 51, 52\. sys\_uart\.cpp 52

```cpp
struct av_get_monitor_info_cmd : public ps3av_cmd
{
  bool execute(....) override
  {
    // ....
    evnt->color.blue_x = 0xFFFF;
    evnt->color.blue_y = 0xFFFF;
    evnt->color.green_x = 0xFFFF;
    evnt->color.green_y = 0xFFFF;
    evnt->color.red_x = 0xFFFF;
    evnt->color.red_y = 0xFFFF;
    evnt->color.white_x = 0xFFFF;
    evnt->color.white_x = 0xFFFF; // <=
    evnt->color.gamma = 100;
    // ....
  {
};
```

Классическая ошибка: при написании функции скопировали строку и забыли в ней поменять нужную переменную\. Глазами заметить ошибку здесь сложно, но статический анализатор отлично справляется в таких случаях\.

## Повторяющаяся проверка

[V581](https://pvs-studio.ru/ru/docs/warnings/v581/) The conditional expressions of the 'if' statements situated alongside each other are identical\. Check lines: 4225, 4226\. PPUTranslator\.cpp 4226

```cpp
void PPUTranslator::MTFSFI(ppu_opcode_t op)
{
  SetFPSCRBit(op.crfd * 4 + 0, m_ir->getInt1((op.i & 8) != 0), false);
  if (op.crfd != 0) SetFPSCRBit(op.crfd * 4 + 1,
                                m_ir->getInt1((op.i & 4) != 0), false);
  if (op.crfd != 0) SetFPSCRBit(op.crfd * 4 + 2,
                                m_ir->getInt1((op.i & 2) != 0), false);
  SetFPSCRBit(op.crfd * 4 + 3, m_ir->getInt1((op.i & 1) != 0), false);

  if (op.rc) SetCrFieldFPCC(1);
}
```

Похоже на ещё одну ошибку копипасты – видимо, скопировали условие и забыли его поменять, однако then\-часть теперь другая\.

Что интересно, это не единственный случай такой ошибки\. В коде было ещё одно срабатывание:

* [V581](https://pvs-studio.ru/ru/docs/warnings/v581/) The conditional expressions of the 'if' statements situated alongside each other are identical\. Check lines: 758, 759\. RSXThread\.cpp 759

## Ошибка в цикле

[V560](https://pvs-studio.ru/ru/docs/warnings/v560/) A part of conditional expression is always true: i \!\= 1\. PPUTranslator\.cpp 4252

```cpp
void PPUTranslator::MTFSF(ppu_opcode_t op)
{
  const auto value = GetFpr(op.frb, 32, true);

  for (u32 i = 16; i < 20; i++)
  {
    if (i != 1 && i != 2 && (op.flm & (128 >> (i / 4))) != 0)
    {
      SetFPSCRBit(i, Trunc(m_ir->CreateLShr(value, i ^ 31),
                  GetType<bool>()), false);
    }
  }

  if (op.rc) SetCrFieldFPCC(1);
}
```

Проверка переменной _i_ на равенство 1 и 2 никогда не выполнится – цикл работает с числами от 16 до 20\. Возможно, этот код переписывался и индексы забыли поменять на корректные\.

## Разыменование указателя перед проверкой

[V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'cached\_dest' pointer was utilized before it was verified against nullptr\. Check lines: 3059, 3064\. texture\_cache\.h 3059

```cpp
template <typename surface_store_type, typename blitter_type, typename ...Args>
blit_op_result upload_scaled_image(....)
{
  // ....

  if (!use_null_region) [[likely]]
  {
    // Do preliminary analysis
    typeless_info.analyse();

    blitter.scale_image(cmd, vram_texture, dest_texture, src_area, dst_area,
                        interpolate, typeless_info);
  }
  else
  {
    cached_dest->dma_transfer(cmd, vram_texture, src_area, // <=
                              dst_range, dst.pitch);
  }

  blit_op_result result = true;

  if (cached_dest) // <=
  {
    result.real_dst_address = cached_dest->get_section_base();
    result.real_dst_size = cached_dest->get_section_size();
  }
  else
  {
    result.real_dst_address = dst_base_address;
    result.real_dst_size = dst.pitch * dst_dimensions.height;
  }

  return result;
}
```

Ещё один частый паттерн – сначала используется указатель, а затем проверяется\. Опять же, возможно, что ошибка появилась в процессе изменений этого фрагмента кода\.

## Проверка на null результата new

[V668](https://pvs-studio.ru/ru/docs/warnings/v668/) There is no sense in testing the 'movie' pointer against null, as the memory was allocated using the 'new' operator\. The exception will be generated in the case of memory allocation error\. movie\_item\.h 56

```cpp
void init_movie(const QString& path)
{
  if (path.isEmpty() || !m_icon_callback) return;

  if (QMovie* movie = new QMovie(path); movie && movie->isValid())
  {
    m_movie = movie;
  }
  else
  {
    delete movie;
    return;
  }

  QObject::connect(m_movie, &QMovie::frameChanged, m_movie, m_icon_callback);
}
```

Проверка на nullptr здесь бессмысленна: при вызове _new_ в случае ошибки будет брошено исключение _std::bad\_alloc_\. Если бросать исключение не требуется, то следует использовать конструкцию [std::nothrow](https://en.cppreference.com/w/cpp/memory/new/nothrow) – тогда как раз вернётся нулевой указатель\.

Ещё места с подобной ошибкой:

* [V668](https://pvs-studio.ru/ru/docs/warnings/v668/) There is no sense in testing the 'm\_render\_creator' pointer against null, as the memory was allocated using the 'new' operator\. The exception will be generated in the case of memory allocation error\. emu\_settings\.cpp 75
* [V668](https://pvs-studio.ru/ru/docs/warnings/v668/) There is no sense in testing the 'trophy\_slider\_label' pointer against null, as the memory was allocated using the 'new' operator\. The exception will be generated in the case of memory allocation error\. trophy\_manager\_dialog\.cpp 216

## Утечка памяти

[V773](https://pvs-studio.ru/ru/docs/warnings/v773/) The function was exited without releasing the 'buffer' pointer\. A memory leak is possible\. rsx\_debugger\.cpp 380

```cpp
u8* convert_to_QImage_buffer(rsx::surface_color_format format,
                             std::span<const std::byte> orig_buffer,
                             usz width, usz height) noexcept
{
  u8* buffer = static_cast<u8*>(std::malloc(width * height * 4));
  if (!buffer || width == 0 || height == 0)
  {
    return nullptr;
  }
  for (u32 i = 0; i < width * height; i++)
  {
    // depending on original buffer, the colors may need to be reversed
    const auto &colors = get_value(orig_buffer, format, i);
    buffer[0 + i * 4] = colors[0];
    buffer[1 + i * 4] = colors[1];
    buffer[2 + i * 4] = colors[2];
    buffer[3 + i * 4] = 255;
  }
  return buffer;
}
```

В начале функции видим выделение памяти через _malloc_, и если вернулся _nullptr_, то выходим\. Пока всё хорошо\. Но дальше идут проверки параметров _width_ и _height_ – они происходят уже после выделения памяти, и в случае успеха из функции также возвращается _nullptr_\. Да, при равенстве этих переменных нулю malloc будет выделять 0 байт, однако стандарт говорит, что в этом случае может вернуться как _nullptr_, так и валидный указатель, который нельзя разыменовывать\. Но освободить его все равно нужно, к тому же, _free_ способна принимать и нулевой указатель\. Так что исправление может выглядеть таким образом:

```cpp
if (!buffer || width == 0 || height == 0)
{
  std::free(buffer)
  return nullptr;
}
```

Либо проверки на 0 можно вообще убрать – цикл в этом случае не выполнится:

```cpp
if (!buffer)
{
  return nullptr;
}
for (u32 i = 0; i < width * height; i++)
{
  // ....
}
return buffer;
```

## Некорректная проверка размера

[V557](https://pvs-studio.ru/ru/docs/warnings/v557/) Array overrun is possible\. The 'pad' index is pointing beyond array bound\. pad\_thread\.cpp 191

```cpp
void pad_thread::SetRumble(const u32 pad, u8 largeMotor, bool smallMotor)
{
  if (pad > m_pads.size())
    return;

  if (m_pads[pad]->m_vibrateMotors.size() >= 2)
  {
    m_pads[pad]->m_vibrateMotors[0].m_value = largeMotor;
    m_pads[pad]->m_vibrateMotors[1].m_value = smallMotor ? 255 : 0;
  }
}
```

При проверке входных данных использован оператор \> вместо \>\=, в результате чего значение _pad_ может быть равно размеру контейнера _m\_pads_, поэтому при дальнейшем обращении к контейнеру есть вероятность выхода за границу\.

## Сдвиг не в ту сторону

[V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'current\_version < threshold\_version' is always false\. Unsigned type value is never < 0\. device\.cpp 91

```cpp
void physical_device::create(VkInstance context,
                             VkPhysicalDevice pdev,
                             bool allow_extensions)
{
  else if (get_driver_vendor() == driver_vendor::NVIDIA)
  {
#ifdef _WIN32
    // SPIRV bugs were fixed in 452.28 for windows
    const u32 threshold_version = (452u >> 22) | (28 >> 14);
#else
    // SPIRV bugs were fixed in 450.56 for linux/BSD
    const u32 threshold_version = (450u >> 22) | (56 >> 14);
#endif
    // Clear patch and revision fields
    const auto current_version = props.driverVersion & ~0x3fffu;
    if (current_version < threshold_version)
    {
      rsx_log.error(....);
    }
  }
}
```

Константа _threshold\_version_ всегда будет равна 0, т\.к\. вместо сдвига влево используется сдвиг вправо\. Сдвиг вправо эквивалентен делению на степень двойки – в нашем случае на 2^22 и 2^14 соответственно\. Очевидно, что числа из выражений меньше этих степеней, поэтому результат будет равен 0\.

Похоже, что это место было скопировано из кода декодирования значения версии, но при копировании поменять операторы просто забыли\.

## Заключение

В результате проверки анализатор нашёл в проекте ошибки разного рода: традиционно были выявлены опечатки, а также есть логические ошибки, где код просто не тестировался\. Надеемся, что эта проверка поможет исправить пару багов, а также желаем разработчикам эмулятора хорошей поддержки игр и отличной производительности\. А вы можете попробовать [триальную версию](https://pvs-studio.ru/ru/pvs-studio/try-free/) анализатора PVS\-Studio и посмотреть, какие ошибки вы найдёте у себя\. А если вы разрабатываете какую\-то открытую игру, эмулятор либо просто open\-source проект, то рассмотрите вариант [бесплатного лицензирования](https://pvs-studio.ru/ru/blog/posts/0614/)\.