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

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

Ryujinx — это эмулятор Nintendo Switch, нацеленный, по заявлению разработчиков, на точность, производительность и удобный интерфейс. Проект написан на C#, активно разрабатывается и доступен в репозитории на GitHub.

Среди эмуляторов Nintendo Switch есть и конкуренты для Ryujinx — это проект Yuzu, написанный на языке C++. Он рассматривался в одной из наших статей по проверке проектов.

В блоге PVS-Studio про эмулятор Ryujinx уже была статья о проверке, но с тех пор прошло много времени: появилось множество новых функций, а с ними и новые ошибки. О них мы сегодня и поговорим.

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

Issue 1

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

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

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; } .... }

Предупреждение PVS-Studio: V3139 Two or more case-branches perform the same actions. EnumConversion.cs 448

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

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

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

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

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

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

Issue 2

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

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

Предупреждение PVS-Studio: 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:

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

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

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

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

Issue 3

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

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 Possible incorrect order of arguments passed to 'CtorVtableSpecialName' constructor: 'secondType' and 'firstType'. Demangler.cs 803

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

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

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

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

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

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 It is odd that the body of 'PrintLeft' function is fully equivalent to the body of 'PrintRight' function (10, line 18). PackedTemplateParameter.cs 10

Разбор этих ошибок вы можете прочитать в этой статье.

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

Issue 4

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

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 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. Здесь нам есть над чем поработать. :)

Полная версия разбора ниже по ссылке

pvs-studio.ru

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

#opensource #staticanalysis #csharp #bugs #эмулятор

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

О проекте

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

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

NullReferenceException и все-все-все