﻿# Ryujinx: повторная проверка эмулятора Nintendo Switch с помощью PVS\-Studio

Популярность Nintendo Switch не угасает, эксклюзивные игры получают награды, и желание в них поиграть только растет\. Однако возможность опробовать портативную приставку есть не у каждого\. Решает эту проблему эмулятор консоли Nintendo Switch — Ryujinx\. Сегодня проверим его код с помощью анализатора PVS\-Studio\.

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

## О проекте

Ryujinx — это эмулятор Nintendo Switch, нацеленный, по заявлению разработчиков, на точность, производительность и удобный интерфейс\. Проект написан на C\#, активно разрабатывается и доступен в [репозитории на GitHub](https://github.com/Ryujinx/Ryujinx)\.

Среди эмуляторов Nintendo Switch есть и конкуренты для Ryujinx — это проект Yuzu, написанный на языке C\+\+\. Он рассматривался в одной из наших [статей](https://pvs-studio.ru/ru/blog/posts/cpp/0651/) по проверке проектов\. 

В [блоге](https://pvs-studio.ru/ru/blog/posts/?tag=GameDev) PVS\-Studio про эмулятор Ryujinx уже была [статья](https://pvs-studio.ru/ru/blog/posts/csharp/0840/) о проверке, но с тех пор прошло много времени: появилось множество новых функций, а с ними и новые ошибки\. О них мы сегодня и поговорим\.

## Разбор ошибок

### Невнимательность, опечатки, copy\-paste

Подобные ошибки легко пропустить\. А ещё они могут как не влиять на работоспособность программы, так и менять её логику — как повезёт\. 

**Issue 1**

Давайте и начнём с проверки на внимательность\.

Далее приведу большой фрагмент кода\. Сможете найти в нём подвох?

```cpp
public static PrimitiveType Convert(this PrimitiveTopology topology)
{
  switch (topology)
  {
    case PrimitiveTopology.Points:
      return PrimitiveType.Points;
    case PrimitiveTopology.Lines:
       return PrimitiveType.Lines;
    case PrimitiveTopology.LineLoop:
       return PrimitiveType.LineLoop;
    case PrimitiveTopology.LineStrip:
       return PrimitiveType.LineStrip;
    case PrimitiveTopology.Triangles:
       return PrimitiveType.Triangles;
    case PrimitiveTopology.TriangleStrip:
       return PrimitiveType.TriangleStrip;
    case PrimitiveTopology.TriangleFan:
       return PrimitiveType.TriangleFan;
    case PrimitiveTopology.Quads:
       return PrimitiveType.Quads;
    case PrimitiveTopology.QuadStrip:
       return PrimitiveType.QuadStrip;
    case PrimitiveTopology.Polygon:
       return PrimitiveType.TriangleFan;
    case PrimitiveTopology.LinesAdjacency:
       return PrimitiveType.LinesAdjacency;
    case PrimitiveTopology.LineStripAdjacency:
       return PrimitiveType.LineStripAdjacency;
    case PrimitiveTopology.TrianglesAdjacency:
       return PrimitiveType.TrianglesAdjacency;
    case PrimitiveTopology.TriangleStripAdjacency:
       return PrimitiveType.TriangleStripAdjacency;
    case PrimitiveTopology.Patches:
       return PrimitiveType.Patches;
  }
  ....
}
```





<details>
   <summary>Предупреждение PVS\\\-Studio</summary>

[V3139](https://pvs-studio.ru/ru/docs/warnings/v3139/) Two or more case\-branches perform the same actions\. EnumConversion\.cs 448


</details>


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

Если вдруг у вас заболели глаза, дам подсказку:

```cpp
switch (topology)
{
  ....
  case PrimitiveTopology.TriangleFan:
    return PrimitiveType.TriangleFan;
  ....
  case PrimitiveTopology.Polygon:
    return PrimitiveType.TriangleFan;
  ....
}
```

Это классическая ошибка copy\-paste\. Во второй _case_\-секции должно возвращаться значение _PrimitiveType\.Polygon_\. Убедимся в том, что оно есть в перечислении _PrimitiveType_:

```cpp
public enum PrimitiveType
{
  ....
  TriangleFan = 6,
  ....
  Polygon = 9,
  ....
}
```

Copy\-paste — это удобно и быстро, но когда фрагмент становится всё больше и больше, также растёт и возможность ошибиться\. 

**Issue 2**

Рассмотрим подозрительный фрагмент кода:

```cpp
private void YesButton_Clicked(object sender, EventArgs args)
{
  ....
  Window.Functions = _mainWindow.Window.Functions =
    WMFunction.All & WMFunction.Close;
  ....
}
```

Предупреждение PVS\-Studio: [V3182](https://pvs-studio.ru/ru/docs/warnings/v3182/) The result of 'WMFunction\.All & WMFunction\.Close' expression is '0'\. It is possible that the '\|' operator should be used instead\. UpdateDialog\.cs 69

Как мы видим из предупреждения, анализатор ругается на оператор '&'\. Чтобы понять почему, давайте взглянем на перечисление _WMFunction_:

```cpp
[Flags]
public enum WMFunction
{
  All = 0x1,
  Resize = 0x2,
  Move = 0x4,
  Minimize = 0x8,
  Maximize = 0x10,
  Close = 0x20
}
```

Мы имеем дело с битовыми флагами, но в данном случае реализация объединения не будет работать: результатом операции побитового И \(&\) для значений _WMFunction\.All_ и _WMFunction\.Close_ будет ноль\.

Анализатор сразу даёт нам подсказку о решении данной проблемы\. Корректная реализация объединения флагов выглядит следующим образом:

```cpp
Window.Functions = _mainWindow.Window.Functions =
  WMFunction.All | WMFunction.Close;
```

**Issue 3**

Ещё один интересный случай:

```cpp
private BaseNode ParseSpecialName(....)
{
  switch (....)
  {
    case 'C':
      _position += 2;
      BaseNode firstType = ParseType();
      if (   firstType == null 
          || ParseNumber(true).Length == 0 
          || !ConsumeIf("_"))
      {
        return null;
      }
      BaseNode secondType = ParseType();
      return new CtorVtableSpecialName(secondType, firstType); // <=
  }
}
```

Предупреждение PVS\-Studio: [V3066](https://pvs-studio.ru/ru/docs/warnings/v3066/) Possible incorrect order of arguments passed to 'CtorVtableSpecialName' constructor: 'secondType' and 'firstType'\. Demangler\.cs 803

Порядок аргументов выглядит странно, но не будем гадать и проверим метод:

```cpp
public CtorVtableSpecialName(BaseNode firstType, BaseNode secondType) :
  base(NodeType.CtorVtableSpecialName)
{
  _firstType  = firstType;
  _secondType = secondType;
}
```

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

Но как понять со стороны, задумка это или ошибка? Комментарии могли бы помочь, однако их нет\. 

**Примечание\.** Помимо новых срабатываний остались неисправленные ошибки с прошлой проверки:

* [V3021](https://pvs-studio.ru/ru/docs/warnings/v3021/) There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless Demangler\.cs 2043
* [V3013](https://pvs-studio.ru/ru/docs/warnings/v3013/) It is odd that the body of 'PrintLeft' function is fully equivalent to the body of 'PrintRight' function \(10, line 18\)\. PackedTemplateParameter\.cs 10

Разбор этих ошибок вы можете прочитать в этой [статье](https://pvs-studio.ru/ru/blog/posts/csharp/0840/)\.

### NullReferenceException и все\-все\-все

Распространённый паттерн — подозрительное взаимодействие с потенциально нулевыми ссылками\. 

**Issue 4**

Рассмотрим случай опасного порядка действий: 

```cpp
public Result Initialize(....)
{
  ....
  if (type == ThreadType.User)
  {
    if (owner.AllocateThreadLocalStorage(out _tlsAddress) 
        != Result.Success)
    {
      return KernelResult.OutOfMemory;
    }

    MemoryHelper.FillWithZeros(owner.CpuMemory, 
                               _tlsAddress,
                               KTlsPageInfo.TlsEntrySize);
  }

  if (owner != null)
  {
    Owner = owner;
    owner.IncrementReferenceCount();
    owner.IncrementThreadCount();
    ....
  }
  else
  {
    is64Bits = true;
  }
}
```

Предупреждение PVS\-Studio: [V3095](https://pvs-studio.ru/ru/docs/warnings/v3095/) The 'owner' object was used before it was verified against null\. Check lines: 169, 174\. KThread\.cs 169

Нас интересует переменная _owner_\. Она проверяется на _null_, но перед этим используется в методе без проверок\. Этот код выглядит подозрительно\. 

А вот есть ли здесь ошибка, или между переменными _owner_ и _type_ есть некая связь, и исключения не будет — вопрос\. Со стороны сказать сложно, здесь разработчикам должно быть виднее\. Однако код в любом случае подозрительный\.

Кстати, анализатор сработал не совсем корректно, так как указал на обращение _owner\.CpuMemory_, а не _owner\.AllocateThreadLocalStorage_\. Здесь нам есть над чем поработать\. :\)

**Issue 5**

А теперь наоборот\. Давайте взглянем на следующий код:

```cpp
private void RenderLoop()
{
  ....
  _renderer?.Window?.SetAntiAliasing(....);
  _renderer?.Window?.SetScalingFilter(....);
  _renderer?.Window?.SetScalingFilterLevel(....);
  ....
  _renderer.Window.SetSize(....);
  ....
}
```

Предупреждение PVS\-Studio: [V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/) The '\_renderer\.Window' object was used after it was verified against null\. Check lines: 882, 877\. AppHost\.cs 882

В этот раз разработчик проверяет _\_renderer\.Window_ с помощью оператора условного доступа — '?\.', но после этого данная проверка опускается\. 

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

**Примечание\.** Помимо новых, вновь нашлись неисправленные с прошлой проверки предупреждения:

* [V3095](https://pvs-studio.ru/ru/docs/warnings/v3095/) The 'IManager\.NsdSettings' object was used before it was verified against null\. Check lines: 37, 41\. FqdnResolver\.cs 37
* [V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/) The 'Owner' object was used after it was verified against null\. Check lines: 1308, 1306\. KThread\.cs 1308
* [V3105](https://pvs-studio.ru/ru/docs/warnings/v3105/) The 'result' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. Client\.cs 214

**Issue 6**

Посмотрим на следующий метод, который анализатор посчитал подозрительным: 

```cpp
public (uint, bool) GetFormat(....)
{
  TextureSpecializationState state = 
    GetTextureSpecState(stageIndex, handle, cbufSlot).Value;
  ....
}
```

С первого взгляда проблемы не видно, но дьявол, как говорится, кроется в деталях\. Обратите внимание на предупреждение:

[V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference of method return value\. Consider inspecting: GetTextureSpecState\(\.\.\.\)\. ShaderSpecializationState\.cs 408

Пойдём по пути пользователя анализатора и посмотрим на метод изнутри:

```cpp
private Box<....> GetTextureSpecState(....)
{
  TextureKey key = new TextureKey(stageIndex, handle, cbufSlot);
  if (_textureSpecialization.TryGetValue(key,
        out Box<TextureSpecializationState> state))
  {
    return state;
  }
  return null;
}
```

Так как существует сценарий возвращения _null_, хорошей идеей будет добавить проверку\.

Стоит заметить, что подобный код встречается несколько раз:

* [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference of method return value\. Consider inspecting: GetTextureSpecState\(\.\.\.\)\. ShaderSpecializationState\.cs 408
* [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference of method return value\. Consider inspecting: GetTextureSpecState\(\.\.\.\)\. ShaderSpecializationState\.cs 420
* [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference of method return value\. Consider inspecting: GetTextureSpecState\(\.\.\.\)\. ShaderSpecializationState\.cs 431Ф

Если хотите больше узнать о _NullReferenceException_, советую ознакомиться с [этой статьёй](https://pvs-studio.ru/ru/blog/posts/csharp/1049/)\. Например, можете проверить, все ли перечисленные способы столкнуться с NRE вы помните\. 

### null из прошлого

Неисправленные ошибки прошлого стали причиной возникновения новых\.

**Issue 7**

В предыдущей статье о проверке данного проекта было описано следующее предупреждение:

```cpp
public void LoadApplication(string path, bool isFirmwareTitle)
{
  ....
  firmwareVersion = _contentManager.GetCurrentFirmwareVersion();
  string message = $"{firmwareVersion.VersionString}.....";
  GtkDialog.CreateInfoDialog($"{firmwareVersion.VersionString} .... ");
  ....
}
```

Предупреждение PVS\-Studio: [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'firmwareVersion'\. MainWindow\.cs 734

Проблема заключалась в том, что переменная _firmwareVersion_ может иметь значение _null,_ но проверка перед использованием отсутствует\. Рассмотрим метод _GetCurrentFirmwareVersion_:

```cpp
public SystemVersion GetCurrentFirmwareVersion()
{
  LoadEntries();
  lock (_lock)
  {
    ....
    if (....)
    {
      return new SystemVersion(systemVersionFile.AsStream());
    }
    ....
  }
  return null; // <=
}
```

Есть вероятность возвращения _null_, что, в свою очередь, может привести к ошибке\.

Помимо рассмотренного предупреждения, возникло новое: 

```cpp
public async Task<bool> LoadGuestApplication(){
  ....
  firmwareVersion = ContentManager.GetCurrentFirmwareVersion();


  await ContentDialogHelper.CreateInfoDialog(
    LocaleManager.Instance.UpdateAndGetDynamicValue(
      ....,  
      firmwareVersion.VersionString),                  // <=
    LocaleManager.Instance.UpdateAndGetDynamicValue(
      ...., 
      firmwareVersion.VersionString),                  // <=
    ....);               
  ....
}
```

Предупреждение PVS\-Studio: [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'firmwareVersion'\. AppHost\.cs 541

Стоит добавить, что ниже по коду находится уже безопасное использование переменной _firmwareVersion_:

```cpp
public async Task<bool> LoadGuestApplication()
{
  ....
  firmwareVersion = ContentManager.GetCurrentFirmwareVersion();
  ....
  Logger.Notice.Print(
    LogClass.Application, 
    $"Using Firmware Version: {firmwareVersion?.VersionString}");
}
```

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

Надеемся, эта статья поможет разработчикам улучшить их проект\. 

Отметим, что с прошлой проверки прошло около двух лет, но некоторые места так и не были исправлены\. Советуем разработчикам Ryujinx обратить на них внимание и ознакомиться с предыдущей [статьей](https://pvs-studio.ru/ru/blog/posts/csharp/0840/)\.

Если вам интересны статьи про проверку игровых проектов, предлагаю ознакомиться с нашим [блогом](https://pvs-studio.ru/ru/blog/posts/?tag=GameDev)\.

А если вас заинтересовала возможность проверить свой код с помощью PVS\-Studio, скачать и попробовать его можно [здесь](https://pvs-studio.ru/ru/pvs-studio/try-free/)\.