Мы используем куки, чтобы пользоваться сайтом
было удобно.
Такие компании, как Sony, Microsoft и Nintendo, каждое поколение радуют своих потребителей новыми консолями и различными играми на них. Но вот беда, часть игр является эксклюзивами своих платформ, а приставка иной раз стоит как дорогая комплектующая ПК или как полноценный компьютер. Что же делать? На выручку приходят эмуляторы. Рынок полон подобными проектами, некоторые даже выкладываются в сеть с открытым исходным кодом. Обратим внимание на эмуляторы Nintendo Switch. В сети популярными ответами станут проекты Ryujinx и Yuzu. Давайте проведем проверку кода эмулятора Ryujinx и узнаем, сколько интересных ошибок можно найти с помощью статического анализа.
Ryujinx (имя Ryujinx основано на имени "Ryujin"(Рюдзин) - имя Мифического (Морского Бога) Дракона) - это молодой проект эмулятора Nintendo Switch с открытым исходным кодом, написанный на C#. Этот эмулятор нацелен на обеспечение превосходной точности и производительности, удобного интерфейса.
Проект Ryujinx конкурирует со своим старшим братом Yuzu, написанным на языке C++, код которого уже рассматривался в одной из наших статей. Каждый из этих проектов имеет свои положительные и отрицательные стороны. Но оставим старичка в покое, и взглянем на молодой проект с помощью нашего статического анализатора кода PVS-Studio. Исходный код "Дракона" был взят из его официального репозитория на GitHub.
Начнем обзор ошибок проекта Ryujinx с ошибок, которые могут вызвать исключение NullReferenceException.
Частый случай среди ошибок – использование переменных, значение которых может быть null, без проверки на этот самый null. Или же бывает, как в случае ниже.
V3095 The 'firmwareVersion' object was used before it was verified against null. Check lines: 1157, 1159. MainWindow.cs
private void HandleInstallerDialog(FileChooserDialog fileChooser){
....
string dialogTitle = $"Install Firmware {firmwareVersion.VersionString}";
if (firmwareVersion == null)
{
....
}
....
}Здесь firmwareVersion используется прежде, чем производится его проверка на null, — это может вызвать соответствующую ошибку V3095. Данное сообщение было выдано многократно:
V3080 Possible null dereference. Consider inspecting 'firmwareVersion'. MainWindow.cs 605
public void LoadApplication(string path)
{
....
firmwareVersion = _contentManager.GetCurrentFirmwareVersion();
RefreshFirmwareLabel();
string message =
$"No installed firmware was found but Ryujinx was able to install firmware
{firmwareVersion.VersionString} from the provided game.
\nThe emulator will now start.";
....
}Здесь же переменная firmwareVersion используется без проверки. Если взглянуть на метод GetCurrentFirmwareVersion, то можно увидеть, что есть вероятность того, что вернется не ссылка на объект, а null, что, в свою очередь, приведет к ошибке:
public SystemVersion GetCurrentFirmwareVersion()
{
LoadEntries();
lock (_lock)
{
....
if (romfs.OpenFile(out IFile systemVersionFile,
"/file".ToU8Span(),
OpenMode.Read).IsSuccess())
{
return new SystemVersion(systemVersionFile.AsStream());
}
....
}
return null;
}Ошибки такого типа достаточно часто встречаются в этом проекте:
V3125 The 'Owner' object was used after it was verified against null. Check lines: 1084, 1082. KThread.cs 1084
private void FreeResources()
{
Owner?.RemoveThread(this);
if (_tlsAddress != 0 &&
Owner.FreeThreadLocalStorage(_tlsAddress) != KernelResult.Success)
{
....
}
....
}В этот раз мы видим, что проверка на null у нас есть, но выполняется она только один раз. Хотя переменная используется здесь дважды. При первой встрече с Owner его метод вызывается только в том случае, когда переменная не равна null. При второй же упустили этот момент. Если же Owner в первом случае будет null, то метод просто не вызовется, тогда как во втором случае при попытке вызвать метод может возникнуть ошибка NullReferenceException.
V3105 The 'result' variable was used after it was assigned through null-conditional operator. NullReferenceException is possible. Client.cs 213
private byte[] Receive(int clientId, int timeout = 0)
{
....
var result = _client?.Receive(ref endPoint);
if (result.Length > 0)
{
....
}
....
}В этом фрагменте кода мы видим использование null-conditional оператора для присвоения результата в переменную result. Но в дальнейшем эта переменная не проверяется на null, что может вызвать ошибку в строке с условием, ведь мы не можем измерить длину null.
V3042 Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'data' object Client.cs 254
public void ReceiveLoop(int clientId)
{
....
byte[] data = Receive(clientId);
if (data.Length == 0)
{
continue;
}
....
}Здесь мы видим, что data присваивают результат выполнения функции, но давайте взглянем внутрь неё и узнаем, что она может вернуть:
private byte[] Receive(int clientId, int timeout = 0)
{
....
var result = _client?.Receive(ref endPoint);
if (result.Length > 0)
{
....
}
return result;
....
}Кажется, или мы уже где-то видели этот код? Конечно, ранее описанная ошибка привела к ещё одной.
V3022 Expression 'result != KernelResult.Success' is always false. KMemoryRegionManager.cs 169
private KernelResult AllocatePagesImpl(....)
{
....
KernelResult result = pageList.AddRange(address, blockPagesCount);
if (result != KernelResult.Success)
....
}Итак, первая логическая ошибка сообщает нам, что условие всегда ложно. С чего бы это? Для этого нам нужно провести вскрытие метода AddRange.
public KernelResult AddRange(....)
{
....
return KernelResult.Success;
}Опустив алгоритм метода, остановимся на результате. Вызов return происходит лишь один раз, и, соответственно, единственное возможное значение переменной result лишь одно. Либо метод ещё не закончен, либо произошла излишняя проверка результата выполнения метода. Ошибка V3022 встретилась в проекте далеко не единожды:
V3003 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 26, 30. ZbcSetTableArguments.cs 26
public uint this[int index]
{
get
{
if (index == 0)
{
return element0;
}
else if (index == 1)
{
return element1;
}
else if (index == 2)
{
return element2;
}
else if (index == 2)
{
return element3;
}
throw new IndexOutOfRangeException();
}
}Ошибка повторяющихся условий. Есть несколько возможных причин её появления, старый добрый copy-paste, или же банальная невнимательность. Здесь, скорее всего, второй вариант. Ситуации, вызванные опечатками с использованием чисел 0, 1, 2, частые гости в программировании. Если эта тема вас заинтересовала, можете ознакомиться с ней подробнее в нашей статье
V3022 Expression 'Base == null' is always false. Demangler.cs 2049
private BaseNode ParseExpression()
{
....
BaseNode Base = ParseExpression();
if (Base == null)
{
return null;
}
BaseNode subscript = ParseExpression();
if (Base == null)
{
return null;
}
....
}Итак, выше мы видим аналогичную ошибку, Base дважды проверяется на null. В этот раз её причиной стал, скорее всего, злополучный copy-paste. Из-за этого один и тот же фрагмент содержит и следующую ошибку: 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
Скорее всего второе условие должно было проверить переменную subscript, что в свою очередь убьет сразу двух зайцев:
BaseNode subscript = ParseExpression();
if (subscript == null)
{
return null;
}V3009 It's odd that this method always returns one and the same value of 'ResultCode.Success'. IApplicationFunctions.cs 116
public ResultCode GetDesiredLanguage(ServiceCtx context)
{
....
if (firstSupported > (int)SystemState.TitleLanguage.Chinese)
{
Logger.Warning?.Print(LogClass.ServiceAm,
"Application has zero supported languages");
context.ResponseData.Write(desiredLanguageCode);
return ResultCode.Success;
}
....
return ResultCode.Success;
}В проекте Ryujinx встретилось несколько функций, работающих с набором значений ResultCode. Одну мы уже встретили ранее. Однако ни одна из них не использовала все значения, останавливаясь лишь на Success. Возможно, разработчики ещё не закончили работу, или в функциях закралась ошибка. Из-за этого было необходимо использовать другой результат. Мы уже видели, что другой код, работающий с результатами этих функций, может вызывать ошибки или работать неправильно. Подобные сообщения ещё несколько раз встречались в проекте:
V3064 Potential division by zero. Consider inspecting denominator 'blockWidth'. AstcDecoder.cs 71
public AstcDecoder(
ReadOnlyMemory<byte> inputBuffer,
Memory<byte> outputBuffer,
int blockWidth,
int blockHeight,
int width,
int height,
int depth,
int levels,
int layers)
{
....
if ((uint)blockWidth > 12)
{
throw new ArgumentOutOfRangeException(nameof(blockWidth));
}
if ((uint)blockHeight > 12)
{
throw new ArgumentOutOfRangeException(nameof(blockHeight));
}
....
level.BlockCountX =
(level.ImageSizeX + blockWidth - 1) / blockWidth;
level.BlockCountY =
(level.ImageSizeY + blockHeight - 1) / blockHeight;
....
}Данное сообщение говорит нам о том, что может произойти деление на ноль. Благодаря условию проверки диапазон чисел сжался от 0 до 11, что не исключает возможности присвоения этим переменным нуля. Следует обезопасить этот участок от подобной ошибки.
V3171 The value used as the size of an array could reach -1. Consider inspecting: deviceCount. AudioDevice.cs 133
public string[] ListAudioDeviceName()
{
int deviceCount = _sessions.Length;
if (!_isUsbDeviceSupported)
{
deviceCount--;
}
string[] result = new string[deviceCount];
....
}Здесь ошибка кроется в том, что если _sessions.Length будет равна нулю, то и deviceCount может стать -1, что вызовет ошибку при создании массива. Следует сделать проверку, чтобы избежать этой ситуации.
V3063 A part of conditional expression is always true if it is evaluated: value >= 0. NumberFormatter.cs 96
public static string FormatUint(uint value)
{
if (value <= MaxDecimal && value >= 0)
{
return value.ToString(CultureInfo.InvariantCulture) + "u";
}
....
}Итак, анализатор сообщает нам о том, что условие value >= 0 всегда истинно. Причина проста - диапазон значения типа uint начинается с 0, заканчивая числом 4294967295, т.е. переменные типа uint всегда больше или равны 0. Вывод прост - проверка value здесь излишняя. Так же было обнаружено ещё несколько подобных ситуаций:
V3139 Two or more case-branches perform the same actions. Demangler.cs 2251
private BaseNode ParseExpression()
{
....
case 'm':
_position += 2;
return ParseBinaryExpression("%");
case 'M':
_position += 2;
return ParseBinaryExpression("%");
....
}Старый добрый оператор switch. В данном примере он довольно большой, и нет, ничего страшного, что в один момент начинается путаница. В данном случае возможны два варианта. Либо эти две ветки case должны выполнять одну и туже операцию и их можно объединить. Либо здесь закралась ошибка, ведомая лишь одним разработчикам. Подобных случаев в проекте насчитывается ещё 19.
V3022 Expression 'mainNca != null' is always true. ApplicationLoader.cs 272
public void LoadNsp(string nspFile)
{
....
if (mainNca == null)
{
Logger.Error?.Print(LogClass.Loader,
"Unable to load NSP: Could not find Main NCA");
return;
}
if (mainNca != null)
{
_device.Configuration.ContentManager.ClearAocData();
_device.Configuration.ContentManager.AddAocData(nsp,
nspFile,
mainNca.Header.TitleId,
_device.Configuration.FsIntegrityCheckLevel);
LoadNca(mainNca, patchNca, controlNca);
return;
}
....
}Анализатор сообщает нам, что второе условие в указанном месте всегда истинно. И это очевидно, ведь прямо перед этим выполнялась уже проверка mainNca ровно на противоположное значение. От чего встает вопрос, а нужно ли вообще проводить вторую проверку, если переменная не меняется?
V3022 Expression 'result == null' is always false. Demangler.cs 2906
private BaseNode ParseUnresolvedName(....)
{
....
BaseNode qualifier = ParseSimpleId();
if (qualifier == null)
{
return null;
}
if (result != null)
{
result = new QualifiedName(result, qualifier);
}
else if (isGlobal)
{
result = new GlobalQualifiedName(qualifier);
}
else
{
result = qualifier;
}
if (result == null)
{
return null;
}
....
}В этом участке кода мы можем заметить, что result дважды проверяется на null, однако вторая проверка всегда ложна, ведь в любом случае result присваивается новые производные от класса, BaseNode, который точно не равен null. Проверка на null переменных, которым присвоили новый экземпляр классов встречается ещё несколько раз в проекте:
V3117 Constructor parameter 'context' is not used. IAccountServiceForAdministrator.cs 12
public IAccountServiceForAdministrator(ServiceCtx context,
AccountServiceFlag serviceFlag)
{
_applicationServiceServer = new ApplicationServiceServer(serviceFlag);
}Довольно много сообщений V3117 выводит анализатор в проекте. Подобные случаи вызываются подозрительным кодом. Если аргумент не используется, то зачем его передавать? Возможно, эти функции недоработаны, или же разработчики просто перестраховались. В итоге параметры не понадобились, а убрать просто забыли. В проекте достаточно много подобных фрагментов кода:
V3061 Parameter 'instruction' is always rewritten in method body before being used. EndConditionalBlock.cs 18
public static void Emit(byte[] instruction, CompilationContext context)
{
// 20000000
// Use the conditional begin instruction stored in the stack.
instruction = context.CurrentBlock.BaseInstruction;
....
}А вот здесь ситуации несколько иная. Аргумент передали и даже используют, но не с тем значением, которое передается, ведь сразу на входе instruction перезаписывается. Стоит либо отказаться от передачи ненужного аргумента, либо сделать его необязательным, если его все же нужно будет передавать.
V3030 Recurring check. The 'setFlags' condition was already verified in line 139. InstEmitAluHelper.cs 141
public static void EmitGenericAluStoreA32(....)
{
Debug.Assert(value.Type == OperandType.I32);
if (rd == RegisterAlias.Aarch32Pc && setFlags)
{
if (setFlags)
{
// TODO: Load SPSR etc.
EmitBxWritePc(context, value);
}
else
{
EmitAluWritePc(context, value);
}
....
}
....
}Здесь спорная ситуация. С точки зрения анализатора здесь есть лишняя проверка переменной setFlags. Однако, судя по комментариям разработчиков, этот кусок кода в ветвях условия не доработан. Удалять лишнюю проверку просто так не получится – код в ветвях отличается. Но разобраться с этим кодом нужно уже сейчас. Иначе есть риск, что он будет дописан, как есть, и тогда в коде останется ошибка с недостижимым кодом. А в большем объёме кода её будет ещё сложнее найти.
V3138 String literal contains potential interpolated expression. Consider inspecting: keyHash. CacheCollection.cs 524
public void AddValue(ref Hash128 keyHash, byte[] value)
{
if (IsReadOnly)
{
Logger.Warning?.Print(LogClass.Gpu,
"Trying to add {keyHash} on a read-only cache, ignoring.");
....
}
....
}Здесь кроется маленькая ошибка, которая вместо сообщения об ошибке выведет вместо неё название переменной, в которой та хранится. Разработчик забыл указать доллар ($), что включает форматирование строки.
V3078 Original sorting order will be lost after repetitive call to 'OrderBy' method. Use 'ThenBy' method to preserve the original sorting. ShaderConfig.cs 413
private static TextureDescriptor[] GetTextureOrImageDescriptors(....)
{
....
foreach (var kv in dict.OrderBy(x => x.Key.Indexed)
.OrderBy(x => x.Key.Handle))
{
....
}
....
}Чтобы понять, почему анализатор выдает нам предупреждение в этом месте, следует разобраться в принципах работы сортировки. OrderBy сортирует коллекцию независимо от того, были ли перед этим другие сортировки или нет. В таком случае результат dict.OrderBy(x => x.Key.Indexed).OrderBy(x => x.Key.Handle) равен dict.OrderBy(x => x.Key.Handle). Чтобы сохранить полученную ранее сортировку, необходимо использовать ThenBy. Тогда это позволит сохранить первичную сортировку:
var kv in dict.OrderBy(x => x.Key.Indexed).ThenBy(x => x.Key.Handle)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
public override void PrintLeft(TextWriter writer)
{
foreach (BaseNode node in Nodes)
{
node.PrintLeft(writer);
}
}
public override void PrintRight(TextWriter writer)
{
foreach (BaseNode node in Nodes)
{
node.PrintLeft(writer);
}
}Яркий пример применения любимых (и не очень) copy-paste. Обе функции перебирают коллекции и вызывают для их элементов PrintLeft. И ладно бы это была единственная функция для класса BaseNode, можно было бы убрать лишнюю функцию и радоваться. Но у BaseNode также имеется и PrintRight. Это означает, что вторая функция выполняет неправильную операцию.
По результатам проверки проекта Ryujinx нашим анализатором можно сказать, что проект содержит множество однотипных ошибок. Но пока проект развивается, можно ожидать, что разработчики исправят баги и порадуют пользователей новым функционалом. А пока, если вас заинтересовала тема статического анализа эмуляторов, приглашаю вас ознакомиться со статьей по Yuzu.
0
0
0
Мы ответим вам на
Если вы так и не получили ответ, пожалуйста, проверьте, отфильтровано ли письмо в одну из следующих стандартных папок:
0