﻿# Bugs and suspicious places in \.NET 8 source code

Every year, Microsoft releases a new version of \.NET\. This annual event is a great chance to learn about the latest \.NET enhancements as well as to explore the \.NET source code\. Let's take the bull by the horns\!

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

By the way, we have several articles about the latest updates in \.NET and C\#\. If you're interested in the new features Microsoft has added, I suggest you read the following:

* [What's new in \.NET 8?](https://pvs-studio.com/en/blog/posts/csharp/1080/)
* [What's new in C\# 12: overview](https://pvs-studio.com/en/blog/posts/csharp/1074/)

In these articles, we take a look at the key changes that have been added in the latest versions of \.NET and C\#\. Check them out to keep up with the latest updates\.

Moreover, the latest PVS\-Studio 7\.28 release already supports the analysis of \.NET 8 projects\. To examine \.NET 8, we've looked at the release code available on GitHub at this [link](https://github.com/dotnet/runtime/tree/v8.0.0)\.

Before we start to explore the bugs found in \.NET 8, I'd like to tell you a little story\.

As we know, \.NET is huge, and that can be a problem\. Its source code contains a script for generating a solution for \.NET libraries\. I analyzed the solution with the PVS\-Studio console utility\. Then I started to study the report in the IDE I worked in — Visual Studio 2022 — but there was an issue\. When I was trying to navigate through the code in Visual Studio 2022, something unexpected happened: either the IDE restarted or just terminated\. Moreover, the behavior repeated not only when we were navigating through the code with the PVS\-Studio plugin but also when switching files, using 'Go To Definition', etc\.

It complicated the process, but our team quickly found a solution\.

We recently added support for analyzing \.NET projects in VS Code\. We had a separate article about it: "[Using the PVS\-Studio extension for VS Code to effectively handle errors in C\# code](https://pvs-studio.com/en/blog/posts/csharp/1079/)\." We don't encounter the same issues as in Visual Studio 2022 because VS Code is a lightweight code editor\.

The PVS\-Studio window looks like this:

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

\.NET is a powerful platform with high standards for code\. It's written and well\-tested by experienced developers\. However, PVS\-Studio can detect errors even in such an impressive project\.

Now go ahead and explore the detected errors\.

**Fragment 1**

```cpp
private static bool IsRoamingSetting(SettingsProperty setting)
{
  List<KeyValuePair<int, ServiceCallSite>> callSitesByIndex = new();
  ....
  SettingsManageabilityAttribute manageAttr = ....;
  return    manageAttr != null 
         && ((manageAttr.Manageability & SettingsManageability.Roaming) ==
             SettingsManageability.Roaming);
}
```

The PVS\-Studio warning: [V3181](https://pvs-studio.com/en/docs/warnings/v3181/) The result of '&' operator is '0' because the value of 'SettingsManageability\.Roaming' is '0'\. LocalFileSettingsProvider\.cs 411

In this case, the value of the _SettingsManageability\.Roaming_ enumeration constant is 0\. Since the result of a bitwise AND with an operand equal to 0 is always 0, it means that 0 is compared to 0\. The output of the _\(\(manageAttr\.Manageability & SettingsManageability\.Roaming\) \=\= SettingsManageability\.Roaming\)_ expression is always _true_\.

This code fragment is worth paying attention to\.

**Fragment 2**

```cpp
internal DataView(....)
{
  ....
  DataCommonEventSource.Log.Trace("<ds.DataView.DataView|API> %d#, table=%d, 
                                   RowState=%d{ds.DataViewRowState}\n",
                ObjectID, (table != null) ? table.ObjectID : 0, (int)RowState);
  ....
}
```

The PVS\-Studio warning: [V3025](https://pvs-studio.com/en/docs/warnings/v3025/) The 1st argument '"<ds\.DataView\.DataView\|API\> %d\#, table\=%d, RowState\=%d\{ds\.DataViewRowState\}\\n"' is used as incorrect format string inside method\. A different number of format items is expected while calling 'Trace' function\. Arguments not used: 1st, 2nd, 3rd\. DataView\.cs 166, DataCommonEventSource\.cs 45

The analyzer reports an incorrect format string in the first argument of the _Trace_ method\. Let's take a look at the method:

```cpp
internal void Trace<T0, T1, T2>(string format, T0 arg0, T1 arg1, T2 arg2)
{
  if (!Log.IsEnabled()) return;
  Trace(string.Format(format, arg0, arg1, arg2));
}
```

Indeed, the first argument is used as the format string\. Arguments are substituted into this line\. However, the arguments are to be substituted into placeholders of the format \{0\}, \{1\}, etc\. There are no such placeholders in this string\. Using such a format string results in an exception of the _System\.FormatException_ type being thrown, reporting an incorrect format\.

Perhaps another logging method needs to be used\. If we go through other places where the _Trace_ method is used, everything is correct\. The format strings contain markers:

![1095_NET8_Errors/image3.png](https://import.viva64.com/docx/blog/1095_NET8_Errors/image3.png)

**Fragment 3**

```cpp
public static SqlDecimal operator /(SqlDecimal x, SqlDecimal y)
{
  ....
  bScaleD = x._bScale;
  bPrecD = x._bPrec;
  ResScale = Math.Max(x._bScale + y._bPrec + 1, s_cNumeDivScaleMin);
  ResInteger = x._bPrec - x._bScale + y._bScale;
  ResPrec = ResScale + x._bPrec + y._bPrec + 1;               // <=
  MinScale = Math.Min(ResScale, s_cNumeDivScaleMin);

  ResInteger = Math.Min(ResInteger, s_NUMERIC_MAX_PRECISION);
  ResPrec = ResInteger + ResScale;                            // <=
  ....
}
```

The PVS\-Studio warning: [V3008](https://pvs-studio.com/en/docs/warnings/v3008/) The 'ResPrec' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 1689, 1685\. SQLDecimal\.cs 1689

We can see the _ResPrec_ variable is assigned values twice here\.

Since _ResPrec_ isn't used between these two operations, it indicates an error\.

Here are two options:

* One of the assignments is redundant\. It's no big deal, it's just an unnecessary operation, although it's not good\.
* The _ResPrec_ variable should be used between two assignments — and that is a truly unpleasant error\.

**Fragment 4**

```cpp
public override void MoveToAttribute(int i)
{
  ....
  _currentAttrIndex = i;
  if (i < _coreReaderAttributeCount)
  {
    ....
    _validationState = ValidatingReaderState.OnAttribute;
  }
  else
  {
    ....
    _validationState = ValidatingReaderState.OnDefaultAttribute;
  }

  if (_validationState == ValidatingReaderState.OnReadBinaryContent)
  {
    Debug.Assert(_readBinaryHelper != null);
    _readBinaryHelper.Finish();
    _validationState = _savedState;
  }
}
```

The PVS\-Studio warning: [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression '\_validationState \=\= ValidatingReaderState\.OnReadBinaryContent' is always false\. XsdValidatingReader\.cs 1302

PVS\-Studio has detected that the last _if \(\_validationState \=\= ValidatingReaderState\.OnReadBinaryContent\)_ condition is always false\. Let's see why it may happen\.

Let's take a look at the first _if_ statement\. There, the _\_validationState_ field is assigned:

* in then branch_ — ValidatingReaderState\.OnAttribute_
* in else branch_ — ValidatingReaderState\.OnDefaultAttribute_

Therefore, the field value can't be equal to _ValidatingReaderState\.OnReadBinaryContent_, and the code inside _if_ isn't executed\.

**Fragment 5**

```cpp
private static string GetTypeNameDebug(TypeDesc type)
{
  string result;
  TypeDesc typeDefinition = type.GetTypeDefinition();
  if (type != typeDefinition)
  {
    result = GetTypeNameDebug(typeDefinition) + "<";
    for (int i = 0; i < type.Instantiation.Length; i++)
      result += (i == 0 ? "" : ",") + GetTypeNameDebug(type.Instantiation[0]);
    return result + ">";
  }
  else
  {
    ....
  }
  ....
}
```

The PVS\-Studio warning: [V3102](https://pvs-studio.com/en/docs/warnings/v3102/) Suspicious access to element of 'type\.Instantiation' object by a constant index inside a loop\. TypeLoaderEnvironment\.GVMResolution\.cs 32

Here, maybe the following string is created from information about type: _ConsoleApp1\.Program\.MyClass<string, int, double\>_\. However, the _type\.Instantiation_ object is accessed in the loop by a constant index of 0\. It is possible that it works as it should, but it looks odd because _GetTypeNameDebug\(type\.Instantiation\[i\]\)_ is expected\.

I've immediately checked it\. The Visual Studio 2022 debugger displays everything correctly, but, unexceptionally, we can encounter a type displayed with an error somewhere :\)\.

**Fragment 6**

```cpp
Instruction[]? GetArgumentsOnStack (MethodDefinition method)
{
  int length = method.GetMetadataParametersCount ();
  Debug.Assert (length != 0);
  if (stack_instr?.Count < length)
    return null;

  var result = new Instruction[length];
  while (length != 0)
    result[--length] = stack_instr!.Pop ();    // <=

  return result;
}
```

The PVS\-Studio warning: [V3125](https://pvs-studio.com/en/docs/warnings/v3125/) The 'stack\_instr\!' object was used after it was verified against null\. Check lines: 1918, 1913\. UnreachableBlocksOptimizer\.cs 1918

The developer used the '?\.' operator, suggesting that the _stack\_instr_ field could be _null_\. Everything seemed fine, there was a check, but\.\.\. no such luck\. It was possible to dereference a _null_ reference in the specified line\. Most likely, the developer thought that the _stack\_instr?\.Count < length_ expression with _stack\_instr_ equal to _null_ would return _true_, and the method would exit\. But no — the result would be _false_\.

Moreover, the developer suppressed the compiler message about possible dereference of a _null_ reference with '\!'\. They thought that the static analysis of the compiler just failed and didn't understand the check\.

What do you think about nullable context? If you're interested in our opinion or not yet familiar with it, I suggest you read our articles:

* [Help the compiler, and the compiler will help you\. Subtleties of working with nullable reference types in C\#](https://pvs-studio.com/en/blog/posts/csharp/1017/)
* [Nullable Reference will not protect you, and here is the proof](https://pvs-studio.com/en/blog/posts/csharp/0764/)
* [Nullable Reference types in C\# 8\.0 and static analysis](https://pvs-studio.com/en/blog/posts/csharp/0631/)

**Fragment 7**

```cpp
private HierarchyFlags GetFlags (TypeDefinition resolvedType)
{
  if (_cache.TryGetValue (resolvedType, out var flags))
  {
    return flags;
  }

  if (   resolvedType.Name == "IReflect"                // <=
      && resolvedType.Namespace == "System.Reflection") 
  {
    flags |= HierarchyFlags.IsSystemReflectionIReflect;
  }
  ....
  if (resolvedType != null)                             // <=
    _cache.Add (resolvedType, flags);

  return flags;
}
```

The PVS\-Studio warning: [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'resolvedType' object was used before it was verified against null\. Check lines: 34, 55\. TypeHierarchyCache\.cs 34

The _resolvedType_ parameter is used first but is checked for _null_ before being added to the cache\. That's kind of weird\. The analyzer points to _resolvedType\.Name_, but issues will arise even earlier\. The _TryGetValue_ method throws an exception if the first _resolvedType_ argument is _null_\.

**Fragment 8**

```cpp
public static bool IsTypeOf<T> (this TypeReference tr)
{
  var type = typeof (T);
  return tr.Name == type.Name && tr.Namespace == tr.Namespace;
}
```

The PVS\-Studio warning: [V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'tr\.Namespace' to the left and to the right of the '\=\=' operator\. TypeReferenceExtensions\.cs 365

The analyzer has detected that two identical subexpressions are compared here\. It's a simple yet frustrating error\. Instead of being compared to _type\.Namespace_, _tr\.Namespace_ is being compared to _tr\.Namespace_\.

**Fragment 9**

```cpp
public void WriteTo(TextWriter writer, int methodRva, bool dumpRva)
{
  ....
  switch (Flags & CorExceptionFlag.COR_ILEXCEPTION_CLAUSE_KIND_MASK)
  {
    case CorExceptionFlag.COR_ILEXCEPTION_CLAUSE_NONE:
      writer.Write($" CATCH: {0}", ClassName ?? "null");
      break;

    case CorExceptionFlag.COR_ILEXCEPTION_CLAUSE_FILTER:
      writer.Write($" FILTER (RVA {0:X4})",
                   ClassTokenOrFilterOffset + methodRva);
      break;
    ....
  }
  ....
}
```

The PVS\-Studio warning: [V3025](https://pvs-studio.com/en/docs/warnings/v3025/) Incorrect format\. A different number of format items is expected while calling 'Write' function\. Arguments not used: ClassName ?? "null"\. EHInfo\.cs 135

Here is another format string error, but in the _TextWriter_ class\. The developer used the string interpolation '$' character\. It simply substitutes 0 into the string, and the format string will be equal to " CATCH: 0"\. As a result, the text they wanted to substitute for the \{0\} placeholder isn't used\. The same error occurs in the next _case_ method\.

**Fragment 10**

```cpp
public TType ParseType()
{
  CorElementType corElemType = ReadElementType();
  switch (corElemType)
  {
    ....
    case CorElementType.ELEMENT_TYPE_GENERICINST:
    {
      TType genericType = ParseType();
      uint typeArgCount = ReadUInt();
      var outerDecoder = new R2RSignatureDecoder<....>(_provider,
                                                       Context,
                                                       _outerReader, // <=
                                                       _image,
                                                       _offset,
                                                       _outerReader, // <=
                                                       _contextReader);
  }
}
```

The PVS\-Studio warning: [V3038](https://pvs-studio.com/en/docs/warnings/v3038/) The argument was passed to constructor several times\. It is possible that other argument should be passed instead\. ReadyToRunSignature\.cs 707

The _\_outerReader_ argument is passed to the constructor twice\. If we look at the constructor declaration, we can see that the constructor has the _metadataReader_ parameter:

```cpp
public R2RSignatureDecoder(IR2RSignatureTypeProvider<....> provider,
                           TGenericContext context,
                           MetadataReader metadataReader,  // <=
                           byte[] signature,
                           int offset,
                           MetadataReader outerReader,     // <=
                           ReadyToRunReader contextReader,
                           bool skipOverrideMetadataReader = false)
{
  ....
}
```

The _\_metadataReader_ field is available when calling the constructor\. Perhaps it would be better to use this field as the third argument\.

**Bonus fragment 11**

```cpp
protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(....)
{
  bool requiresAlign8 
    =    !largestAlignmentRequired.IsIndeterminate 
      && context.Target.PointerSize == 4
      && context.Target.GetObjectAlignment(....).AsInt > 4 
      && context.Target.PointerSize == 4;
}
```

The PVS\-Studio warning: [V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'context\.Target\.PointerSize \=\= 4' to the left and to the right of the '&&' operator\. MetadataFieldLayoutAlgorithm\.cs 648

The expression checks _context\.Target\.PointerSize \=\= 4_ twice\. In the _GetObjectAlignment_ instance method, _context\.Target\.PointerSize_ isn't changed\. It's possible that something else should be checked here, or maybe it's just an unnecessary check\.

As I've written before, \.NET has high\-quality code\. However, I'm always amazed at some of the errors that are found in such big projects\. They have a well\-oiled development process and experienced developers, but still, there are bugs and suspicious places in the code\. Indeed, it's OK; perfect code doesn't exist, but you can and should strive for it\.

Check your project for oddities and bugs, too\. Try the analyzer at the [link](https://pvs-studio.com/en/pvs-studio/try-free/)\. [Contact us](https://pvs-studio.com/en/about-feedback/) if you have any questions — we will quickly solve any issues :\)\.