# Nintendo Switch: drop test of the Ryujinx emulator's source code

Each generation, companies like Sony, Microsoft and Nintendo delight their consumers with new consoles and different games for them\. Yet there is a caveat \- some games exclusively run on their platforms\. Whereas a console may be worth an expensive PC component or a full\-fledged computer\. So what can you do? Emulators come to the rescue here\. The market is full of similar projects, some are released as open source\. Let us turn our attention to Nintendo Switch emulators\. On the network, Ryujinx and Yuzu projects are among most popular responses\. Let's check the code of the Ryujinx emulator and find out how many interesting bugs can be found using static analysis\.

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

## Introduction

[Ryujinx](https://ryujinx.org/) \(Ryujinx's name is based on the name "Ryujin" \- the name of the Mythic \(Sea God\) Dragon\) is a young open\-source Nintendo Switch emulator project, written in C\#\. This emulator aims to provide superior accuracy and performance, a convenient interface\.

Ryujinx project competes with its older brother Yuzu, written in C\+\+, whose code was already covered in one of our [articles](https://pvs-studio.com/en/blog/posts/cpp/0651/)\. Each of these projects has its positive and negative sides\. But let's leave the old man alone, and take a look at the young project with our static [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/) code analyzer\. The source code of the "Dragon" was taken from its official repository [on GitHub](https://github.com/Ryujinx/Ryujinx)\.

Let's start reviewing Ryujinx project's errors with flaws that may cause _NullReferenceException_\.

## Potential null reference exception

Developers often use variables whose value can be _null_ without checking for this very _null_\. Or something may happen, as in the case below\.

[V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'firmwareVersion' object was used before it was verified against null\. Check lines: 1157, 1159\. MainWindow\.cs

```cpp
private void HandleInstallerDialog(FileChooserDialog fileChooser){
    ....
    
    string dialogTitle = $"Install Firmware {firmwareVersion.VersionString}";

     if (firmwareVersion == null)
    {
        ....
    }
    ....
}
```

_firmwareVersion_ here is used before its check for _null_ \- this may result in a [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) error\. This message has been issued multiple times:

* V3095 The '\_profileEntry\.Text' object was used before it was verified against null\. Check lines: 34, 40\. ProfileDialog\.cs 34
* V3095 The 'owner' object was used before it was verified against null\. Check lines: 161, 166\. KThread\.cs 161
* V3095 The 'owner' object was used before it was verified against null\. Check lines: 1084, 1103\. KThread\.cs 1084
* V3095 The '\_nsdSettings' object was used before it was verified against null\. Check lines: 44, 48\. FqdnResolver\.cs 44
* V3095 The 'texture' object was used before it was verified against null\. Check lines: 354, 362\. TextureBindingsManager\.cs 354

[V3080](https://pvs-studio.com/en/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'firmwareVersion'\. MainWindow\.cs 605

```cpp
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.";
    ....
}
```

Here, the _FirmWareVersion _variable is used without a check\. The _GetCurrentFirmwareVersion_ method shows that we'll get _null_ instead of a reference to an object\. This can lead to an error as well\. 

```cpp

public SystemVersion GetCurrentFirmwareVersion()
{
    LoadEntries();

    lock (_lock)
    {
        ....

        if (romfs.OpenFile(out IFile systemVersionFile,
            "/file".ToU8Span(),
            OpenMode.Read).IsSuccess())
        {
            return new SystemVersion(systemVersionFile.AsStream());
        }
        ....
    }

    return null;
}
```

Errors of this type are quite common for this project:

* V3080 Possible null dereference\. Consider inspecting 'region'\. KMemoryManager\.cs 46
* V3080 Possible null dereference\. Consider inspecting 'node'\. KPageTableBase\.cs 2250
* V3080 Possible null dereference\. Consider inspecting 'node'\. KPageTableBase\.cs 2316
* V3080 Possible null dereference\. Consider inspecting 'node'\. KPageTableBase\.cs 2408
* V3080 Possible null dereference\. Consider inspecting 'dimension'\. Demangler\.cs 361

[V3125](https://pvs-studio.com/en/docs/warnings/v3125/) The 'Owner' object was used after it was verified against null\. Check lines: 1084, 1082\. KThread\.cs 1084

```cpp
private void FreeResources()
{
    Owner?.RemoveThread(this);

    if (_tlsAddress != 0 &&
        Owner.FreeThreadLocalStorage(_tlsAddress) != KernelResult.Success)
    {
      ....
    }
    ....
}
```

This time we see that we have a single check for _null_\. Although the variable is used twice here\. When we first encounter _Owner_, its method is invoked only when the variable is not _null_\. In the second case, this nuance is forgotten\. If _Owner_ is _null_ in the first case, the method won't be called\. As for the second case, whoever tries to call the method will get _NullReferenceException_\. 

[V3105](https://pvs-studio.com/en/docs/warnings/v3105/) The 'result' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. Client\.cs 213

```cpp
private byte[] Receive(int clientId, int timeout = 0)
{
    ....

    var result = _client?.Receive(ref endPoint);

    if (result.Length > 0)
    {
        ....
    }
    ....
}
```

From this code fragment we see how a null\-conditional operator is used to assign the result to the _result_ variable\. This variable isn't checked for _null_ below\. We may get an error in the line with the condition, as we can't estimate the _null_ length\.

[V3042](https://pvs-studio.com/en/docs/warnings/v3042/) Possible NullReferenceException\. The '?\.' and '\.' operators are used for accessing members of the 'data' object Client\.cs 254

```cpp
public void ReceiveLoop(int clientId)
{
    ....
    byte[] data = Receive(clientId);

    if (data.Length == 0)
    {
        continue;
    }
    ....
}
```

Here the author assigns the function result to _data_\. Let's look inside it and find out what it can return:

```cpp
private byte[] Receive(int clientId, int timeout = 0)
{
    ....
    var result = _client?.Receive(ref endPoint);

    if (result.Length > 0)
    {
        ....
    }

    return result;
    ....
}
```

It seems like we've seen this code before, haven't we? The error I described above led to another one\.

## Logic errors

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'result \!\= KernelResult\.Success' is always false\. KMemoryRegionManager\.cs 169

```cpp
private KernelResult AllocatePagesImpl(....)
{
    ....
    KernelResult result = pageList.AddRange(address, blockPagesCount);

    if (result != KernelResult.Success)
    ....
}
```

So, the first logic error tells us that the condition is always false\. Why? Let's look inside the _AddRange_ method\. 

```cpp
public KernelResult AddRange(....)
{
    ....
    return KernelResult.Success;
}
```

We'll omit the method algorithm and focus on the result\. _return_ is called only once\. So, there is only one possible value of the _result_ variable\. Either the method is not finished, or there has been a redundant check of the method result\. We encountered the [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) error many times in the project:

* V3022 Expression 'result \!\= KernelResult\.Success' is always false\. KProcess\.cs 639
* V3022 Expression 'TerminationRequested' is always false\. KThread\.cs 375
* V3022 Expression 'resultCode \=\= ResultCode\.Success' is always true\. IManagerForApplication\.cs 32
* V3022 Expression 'resultCode \=\= ResultCode\.Success' is always true\. IManagerForSystemService\.cs 32
* V3022 Expression 'result \!\= ResultCode\.Success' is always false\. IApplicationFunctions\.cs 376

[V3003](https://pvs-studio.com/en/docs/warnings/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

```cpp
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();
    }
}
```

The error of recurring conditions\. Possible reasons: good old copy\-paste, or corny inattention\. This is likely the second reason here\. Typos with numbers 0, 1, 2 are frequent guests in programming\. If you are interested in this topic, learn more details in the [article](https://pvs-studio.com/en/blog/posts/cpp/0713/)\.

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'Base \=\= null' is always false\. Demangler\.cs 2049

```cpp
private BaseNode ParseExpression()
{
    ....
    BaseNode Base = ParseExpression();
    if (Base == null)
    {
        return null;
    }

    BaseNode subscript = ParseExpression();
    if (Base == null)
    {
        return null;
    }
    ....
}
```

So above we see a similar error, _Base_ is double\-checked for _null_\. This time, it was most likely the ill\-fated copy\-paste\. Because of this, the same fragment contains the following error: [V3021](https://pvs-studio.com/en/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

Most likely the second condition should have checked the _subscript_ variable, which in turn would kill two birds with one stone:

```cpp
BaseNode subscript = ParseExpression();
if (subscript == null)
{
    return null;
}
```

[V3009](https://pvs-studio.com/en/docs/warnings/v3009/) It's odd that this method always returns one and the same value of 'ResultCode\.Success'\. IApplicationFunctions\.cs 116

```cpp
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;
}
```

The Ryujinx project revealed several functions working with the _ResultCode_ set of values\. We've already seen one of them earlier\. However, none of them used all the values, stopping only at _Success_\. The developers may not have finished the job yet, or the functions have caught a bug\. This is why a wrong result was used\. We've already seen that other code that works with the results of these functions may cause errors or work incorrectly\. Similar warnings in the project:

* V3009 It's odd that this method always returns one and the same value of 'ResultCode\.Success'\. IAddOnContentManager\.cs 52
* V3009 It's odd that this method always returns one and the same value of 'ResultCode\.Success'\. ISystemSettingsServer\.cs 30
* V3009 It's odd that this method always returns one and the same value of 'Status\.Success'\. ConsumerBase\.cs 131
* V3009 It's odd that this method always returns one and the same value of 'ResultCode\.Success'\. IBinder\.cs 14
* V3009 It's odd that this method always returns one and the same value of 'true'\. AstcDecoder\.cs 307

[V3064](https://pvs-studio.com/en/docs/warnings/v3064/) Potential division by zero\. Consider inspecting denominator 'blockWidth'\. AstcDecoder\.cs 71

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

This message warns about possible division by zero\. The number range falls in range from 0 to 11 due to the check's condition\. These variables still can be assigned 0\. One has to secure this code fragment from such an error\. 

[V3171](https://pvs-studio.com/en/docs/warnings/v3171/) The value used as the size of an array could reach \-1\. Consider inspecting: deviceCount\. AudioDevice\.cs 133

```cpp
public string[] ListAudioDeviceName()
{
    int deviceCount = _sessions.Length;

    if (!_isUsbDeviceSupported)
    {
        deviceCount--;
    }

    string[] result = new string[deviceCount];
    ....
}
```

Guess where the error hides in this piece\. If _\_sessions\.Length_ is null, _deviceCount_ may equal \-1\. This will cause an error when creating an array\. To avoid this situation, the author should perform a check\.

## Redundant code

[V3063](https://pvs-studio.com/en/docs/warnings/v3063/) A part of conditional expression is always true if it is evaluated: value \>\= 0\. NumberFormatter\.cs 96

```cpp
public static string FormatUint(uint value)
{
    if (value <= MaxDecimal && value >= 0)
    {
        return value.ToString(CultureInfo.InvariantCulture) + "u";
    }
    ....
}
```

So, the analyzer tells us that the _value \>\= 0_ condition is always true\. Here's a simple reason for this\. The _uint_ type range starts at 0, ends with 4294967295\. That is, _uint_ type variables are always greater or equal to 0\. It follows that the _value_ check is simply redundant\. A few other similar situations have also been found:

* V3063 A part of conditional expression is always false if it is evaluated: backendDisconnected\. SoundIoHardwareDeviceDriver\.cs 68
* V3063 A part of conditional expression is always true if it is evaluated: info \!\= null\. SynchronizationManager\.cs 132
* V3063 A part of conditional expression is always false if it is evaluated: flush\. TextureManager\.cs 942

[V3139](https://pvs-studio.com/en/docs/warnings/v3139/) Two or more case\-branches perform the same actions\. Demangler\.cs 2251

```cpp
private BaseNode ParseExpression()
{
    ....
    case 'm':
        _position += 2;
        return ParseBinaryExpression("%");
    case 'm':
        _position += 2;
        return ParseBinaryExpression("%");
    ....
}
```

Good old _switch_ operator\. In this example, it's pretty large\. But it's not about the confusion that may start at a certain point\. There are two possibilities here\. First \- these two _case_ branches must perform the same operation; we can merge branches\. Second \- only authors know about the error hiding here\. There are 19 alike cases in the project\.

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'mainNca \!\= null' is always true\. ApplicationLoader\.cs 272

```cpp
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;
    }

    ....
}
```

The analyzer tells us that the second condition in this place is always true\. It's quite obvious—right before that, _mainNca_ was checked for exactly the opposite value\. Here comes the question \- do we need the second check if the variable doesn't change?

[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'result \=\= null' is always false\. Demangler\.cs 2906

```cpp
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_ is checked twice for _null_\. However, the second check is always false\. Why? The _BaseNode_ class is definitely not _null_\. And anyway, _result_ is assigned objects of classes derived from this class\. So it is a variable that was assigned a new class instance and then checked for _null_\. I found other such cases in the project:

* V3022 Expression 'result \=\= null' is always false\. Demangler\.cs 2848
* V3022 Expression 'result \=\= null' is always false\. Demangler\.cs 2833
* V3022 Expression 'result \=\= null' is always false\. Demangler\.cs 3094
* V3022 Expression 'result \=\= null' is always false\. Demangler\.cs 2930

[V3117](https://pvs-studio.com/en/docs/warnings/v3117/) Constructor parameter 'context' is not used\. IAccountServiceForAdministrator\.cs 12

```cpp
public IAccountServiceForAdministrator(ServiceCtx context,
    AccountServiceFlag serviceFlag)
{
    _applicationServiceServer = new ApplicationServiceServer(serviceFlag);
}
```

The analyzer outputs quite a few [V3117](https://pvs-studio.com/en/docs/warnings/v3117/) warnings\. Cases like this are caused by suspicious code\. If the argument isn't used, then why pass it? Perhaps these functions are underperformed, or developers have simply reinsured\. In the end, the parameters were not needed, but forgotten in code\. There are quite a lot of similar code pieces in the project:

* V3117 Constructor parameter 'context' is not used\. IAccountServiceForApplication\.cs 13
* V3117 Constructor parameter 'context' is not used\. IAccountServiceForSystemService\.cs 11
* V3117 Constructor parameter 'context' is not used\. IDeliveryCacheStorageService\.cs 12
* V3117 Constructor parameter 'memory' is not used\. NvHostAsGpuDeviceFile\.cs 17
* V3117 Constructor parameter 'condition' is not used\. IfBlock\.cs 17

[V3061](https://pvs-studio.com/en/docs/warnings/v3061/) Parameter 'instruction' is always rewritten in method body before being used\. EndConditionalBlock\.cs 18

```cpp
public static void Emit(byte[] instruction, CompilationContext context)
{
    // 20000000

    // Use the conditional begin instruction stored in the stack.
    instruction = context.CurrentBlock.BaseInstruction;
    ....
}
```

Here's a different case\. The argument is passed and even used\. But it's used with another value—_instruction_ is overwritten right at the beginning\. One should either not pass an unnecessary argument or make it optional if it still needs to be passed\.

[V3030](https://pvs-studio.com/en/docs/warnings/v3030/) Recurring check\. The 'setFlags' condition was already verified in line 139\. InstEmitAluHelper\.cs 141

```cpp
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);
        }
        ....
    }
    ....
}
```

It's controversial point here\. From the analyzer's point of view, there's an extra check of the _SetFlags_ variable here\. However, from the developers' comments, this piece of code in the condition branches is incomplete\. The author can't simply delete a redundant check—code in branches differs\. One needs to deal with this code right now\. The fragment may remain as it is, so there will be an error with unreachable code\. It will be even more difficult to find this bug with the growth of code base\. 

[V3138](https://pvs-studio.com/en/docs/warnings/v3138/) String literal contains potential interpolated expression\. Consider inspecting: keyHash\. CacheCollection\.cs 524

```cpp
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.");
        ....
    }
    ....
}
```

Here's a small error\. Instead of an error message, we'll get the variable name where the error is stored\.  The developer forgot to specify a dollar sign \($\), which enables string formatting\. 

[V3078](https://pvs-studio.com/en/docs/warnings/v3078/) Original sorting order will be lost after repetitive call to 'OrderBy' method\. Use 'ThenBy' method to preserve the original sorting\. ShaderConfig\.cs 413

```cpp
private static TextureDescriptor[] GetTextureOrImageDescriptors(....)
{
    ....
    foreach (var kv in dict.OrderBy(x => x.Key.Indexed)
                           .OrderBy(x => x.Key.Handle))
    {
            ....
    }
    ....
}
```

We need to find out why the analyzer triggers here\. To do this, we should look into how sorting works\.  _OrderBy_ sorts the collection no matter if there were other sorts before it or not\.  In such a case, the result of _dict\.OrderBy\(x \=\> x\.Key\.Indexed\)\.OrderBy\(x \=\> x\.Key\.Handle\) _equals _dict\.OrderBy\(x \=\> x\.Key\.Handle\)_\. To save the sorting attained earlier, the developer must use _ThenBy_\. This will keep the primary sorting:

```cpp
var kv in dict.OrderBy(x => x.Key.Indexed).ThenBy(x => x.Key.Handle)
```

## Copy\-paste

[V3013](https://pvs-studio.com/en/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

```cpp
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);
    }
}
```

A prime example of beloved copy\-paste\. Both functions iterate collections and invoke _PrintLeft_ for their elements\. It would be fine if it were the only function of the _BaseNode_class\. One simply could delete the redundant function\. But _BaseNode_ also has _PrintRight_\. This means that the second function performs the wrong operation\.

## Conclusion

So, we checked the Ryujinx project with our analyzer\. The results revealed many similar errors\. While the project is still evolving, we expect developers to fix bugs and delight users with new features\. In the meantime, if you are interested in static analysis checks of emulators, be sure to check out the [article](https://pvs-studio.com/en/blog/posts/cpp/0651/) on Yuzu\.