Bugs and suspicious places in .NET 8 source code
Tags: dotnet, programming, csharp
Published: 2023-12-22 11:20:28

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\!

![](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:

![](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:

![](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/?utm_source=website&utm_medium=habr&utm_campaign=article&utm_content=1095)\. [Contact us](https://pvs-studio.com/en/about-feedback/) if you have any questions — we will quickly solve any issues :\)\.