# Ошибки и подозрительные места в исходниках \.NET 10

Чуть больше полугода назад состоялся релиз \.NET 10\. Остались ли дефекты в исходном коде проекта, спустя месяцы после релиза? Постараемся сегодня найти ответ на этот вопрос\. Приятного чтения\!

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

Новая версия платформы принесла множество изменений: улучшения производительности, обновления библиотек, новые API и очередные доработки компилятора и рантайма\. Но, как показывает практика, даже код проектов такого уровня не застрахован от ошибок\.

Поэтому мы решили проверить исходный код \.NET 10 \(версии [10\.0\.8](https://github.com/dotnet/runtime/tree/v10.0.8)\) с помощью статического анализатора [PVS\-Studio](https://pvs-studio.ru/ru/)\.

В процессе анализа удалось обнаружить несколько подозрительных участков кода, потенциальных ошибок и просто любопытных мест, заслуживающих внимания разработчиков\. В статье мы рассмотрим наиболее интересные предупреждения анализатора\.

Как было сказано ранее, с момента релиза \.NET 10 прошло уже немало времени\. Однако освежить в памяти нововведения не будет лишним:

* [Что нового в \.NET 10?](https://pvs-studio.ru/ru/blog/posts/csharp/1308/);
* [Обзор нововведений в C\# 14](https://pvs-studio.ru/ru/blog/posts/csharp/1301/)\.

Перед разбором кода \.NET10, хочется сказать, что список языков, поддерживаемых PVS\-Studio, расширяется\. Теперь в режиме раннего доступа можно протестировать работу анализатора для Go, JavaScript и TypeScript\. [Здесь](https://pvs-studio.ru/ru/pvs-studio-eap/) можно записаться на тестирование\.

Приступим к разбору кода\!

## Всегда ложь

**Фрагмент кода 1**

```cpp
public static void ReflectionWriteValue(....)
{
  ....
  if (memberValue == null)                                      // <=
  {
    context.WriteNull(xmlWriter,
                    memberType, 
                    DataContract.IsTypeSerializable(memberType));
  }
  else
  {
    PrimitiveDataContract? primitiveContract = ....;
    if (   primitiveContract != null 
        && primitiveContract.UnderlyingType != Globals.TypeOfObject 
        && !writeXsiType)
    {
      primitiveContract.WriteXmlValue(xmlWriter, memberValue, context);
    }
    else
    {
      if (   memberValue == null                                // <=
          && (   memberType == Globals.TypeOfObject
              || (originValueIsNullableOfT && memberType.IsValueType)))
      {
        context.WriteNull(xmlWriter,
                          memberType,
                          DataContract.IsTypeSerializable(memberType));
      }
      else
      {
        ReflectionInternalSerialize(xmlWriter,
                                    context,
                                    memberValue!,
                                    memberValue!.GetType()
                                               .TypeHandle
                                               .Equals(memberType.TypeHandle),
                                    writeXsiType, 
                                    memberType,
                                    originValueIsNullableOfT);
      }
    }
  }
  ....
}
```

Предупреждение PVS\-Studio: [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'memberValue \=\= null && \(memberType \=\= Globals\.TypeOfObject \|\| \(originValueIsNullableOfT && memberType\.IsValueType\)\)' is always false\. [ReflectionClassWriter\.cs 94](https://github.com/dotnet/runtime/blob/v10.0.8/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/ReflectionClassWriter.cs#L94)

Обратите внимание на `if` верхнего уровня\. В нём на `null` проверяется `memberValue`\. Следовательно, в `else` блоке этого `if` переменная `memberValue` не `null`\. Однако в этом блоке есть проверка:

```cpp
if (   memberValue == null 
    && (   memberType == Globals.TypeOfObject
        || (originValueIsNullableOfT && memberType.IsValueType)))
```

Данное условие всегда ложно, так как проверка `memberValue` на `null` является одним из операндов оператора `&&`\. 

## Ошибка приоритетов

**Фрагмент кода 2**

```cpp
protected override string GetName(NodeFactory factory)
{
  return   "Reflectable delegate type: " 
         + _delegateType?.ToString() ?? "All delegates";
}
```

Предупреждение PVS\-Studio: [V3123](https://pvs-studio.ru/ru/docs/warnings/v3123/) Perhaps the '??' operator works in a different way than it was expected\. Its priority is lower than priority of other operators in its left part\. [ReflectedDelegateNode\.cs 38](https://github.com/dotnet/runtime/blob/v10.0.8/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedDelegateNode.cs#L38)

Достаточно часто при написании кода, разработчики не учитывают, что оператор `??` имеет один из самых низких приоритетов\. Это может негативно повлиять на логику работы программы\. К счастью, в данном примере логика работы не пострадала, однако проблема всё равно есть\. Если при выполнении метода `GetName` поле `delegateType` будет `null`, то вместо ожидаемого сообщения: "Reflectable delegate type: All delegates", мы получим: "Reflectable delegate type: "\. Это обусловлено тем, что приоритет `+` выше, чем `??`\. Для исправления нужно взять часть выражения в скобки:

```cpp
"Reflectable delegate type: " + (_delegateType?.ToString() ?? "All delegates");
```

## Перепутали

**Фрагмент кода 3**

```cpp
public ISymbolNode GenericLookupHelper(
    CORINFO_RUNTIME_LOOKUP_KIND runtimeLookupKind,
    ReadyToRunHelperId helperId,
    object helperArgument,
    GenericContext methodContext)
{
  switch (helperId)
  {
    case ReadyToRunHelperId.TypeHandle:
      return GenericLookupTypeHelper(
          runtimeLookupKind,
          ReadyToRunFixupKind.TypeHandle,
          helperArgument,
          methodContext);

    case ReadyToRunHelperId.MethodHandle:
      return GenericLookupMethodHelper(
          runtimeLookupKind,
          ReadyToRunFixupKind.MethodHandle,
          (MethodWithToken)helperArgument,
          methodContext);

    case ReadyToRunHelperId.MethodEntry:
      return GenericLookupMethodHelper(
          runtimeLookupKind,
          ReadyToRunFixupKind.MethodEntry,
          (MethodWithToken)helperArgument,
          methodContext);

    case ReadyToRunHelperId.MethodDictionary:  // <=
      return GenericLookupMethodHelper(
          runtimeLookupKind,
          ReadyToRunFixupKind.MethodHandle,    // <=
          (MethodWithToken)helperArgument,
          methodContext);

    case ReadyToRunHelperId.TypeDictionary:
      return GenericLookupTypeHelper(
          runtimeLookupKind,
          ReadyToRunFixupKind.TypeDictionary,
          (TypeDesc)helperArgument,
          methodContext);

    case ReadyToRunHelperId.VirtualDispatchCell:
      return GenericLookupMethodHelper(
          runtimeLookupKind,
          ReadyToRunFixupKind.VirtualEntry,
          (MethodWithToken)helperArgument,
          methodContext);

    case ReadyToRunHelperId.FieldHandle:
      return GenericLookupFieldHelper(
          runtimeLookupKind,
          ReadyToRunFixupKind.FieldHandle,
          (FieldWithToken)helperArgument,
          methodContext);

    default:
      throw new NotImplementedException(helperId.ToString());
  }
}
```

Предупреждение PVS\-Studio: [V3139](https://pvs-studio.ru/ru/docs/warnings/v3139/) Two or more case\-branches perform the same actions\. [ReadyToRunSymbolNodeFactory\.cs 583](https://github.com/dotnet/runtime/blob/v10.0.8/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs#L583)

Код содержит множество однотипных операций, записанных в одном формате\. Неудивительно, что здесь присутствует крайне подозрительный момент\. Если `helperId` соответствует `ReadyToRunHelperId.MethodDictionary`, то в метод `GenericLookupMethodHelper` передаётся `ReadyToRunFixupKind.MethodHandle`\. Выглядит не совсем корректно, при условии, что в перечислении `ReadyToRunFixupKind` есть элемент `MethodDictionary`\. Кажется, что именно он должен передаваться в `GenericLookupMethodHelper`, если `helperId` соответствует `ReadyToRunHelperId.MethodDictionary`\. 

**Фрагмент кода 4**

```cpp
partial class MutableModule
{
  ....

  public int CompareTo(MutableModule other)
  {
    return _index.CompareTo(_index);
  }
}
```

Предупреждение PVS\-Studio: [V3062](https://pvs-studio.ru/ru/docs/warnings/v3062/) An object '\_index' is used as an argument to its own method\. Consider checking the first actual argument of the 'CompareTo' method\. [MutableModule\.Sorting\.cs 19](https://github.com/dotnet/runtime/blob/v10.0.8/src/coreclr/tools/aot/ILCompiler.ReadyToRun/TypeSystem/Mutable/MutableModule.Sorting.cs#L19)

В методе `CompareTo` поле `_index` сравнивается само с собой\. Скорее всего, в качестве аргумента должно передаваться `other._index`\.

**Фрагмент кода 5**

```cpp
public void ComputeReturnValueTreatment(....)
{
  ....

  if (descriptor.passedInRegisters)
  {
    if (descriptor.eightByteCount == 1)
    {
      if (descriptor.eightByteClassifications0 
                == SystemVClassificationType.SystemVClassificationTypeSSE)
      {
        // Structs occupying just one eightbyte are treated as int / double
        fpReturnSize = sizeof(double);
      }
    }
    else
    {
      // Size of the struct is 16 bytes
      fpReturnSize = 16;
      // The lowest two bits of the size encode 
      // the order of the int and SSE fields
      if (descriptor.eightByteClassifications0                            // <=
               == SystemVClassificationType.SystemVClassificationTypeSSE)
      {
        fpReturnSize += 1;
      }

      if (descriptor.eightByteClassifications0                            // <=
               == SystemVClassificationType.SystemVClassificationTypeSSE)
      {
        fpReturnSize += 2;
      }
    }

    break;
  }

  ....
}
```

Предупреждение PVS\-Studio: [V3029](https://pvs-studio.ru/ru/docs/warnings/v3029/) The conditional expressions of the 'if' statements situated alongside each other are identical\. Check lines: 364, 369\. [TransitionBlock\.cs 364](https://github.com/dotnet/runtime/blob/v10.0.8/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TransitionBlock.cs#L364)

Анализатор сообщает о последовательно идущих `if` с одинаковыми условиями\. Это действительно так, `descriptor.eightByteClassifications0` дважды сравнивается с `SystemVClassificationType.SystemVClassificationTypeSSE`\. На мысль о том, что в одном из случаев проверка ошибочна, наталкивает наличие поля `eightByteClassifications1` у `descriptor`\. Скорее всего, именно оно должно использоваться в одной из проверок\.

## Точно не null?

**Фрагмент кода 6**

```cpp
public static bool SubstEqualTypeArrays(TypeArray taDst,
                                        TypeArray taSrc,
                                        TypeArray typeArgsCls,
                                        TypeArray typeArgsMeth)
{
  // Handle the simple common cases first.
  if (taDst == taSrc || (taDst != null && taDst.Equals(taSrc))) // <=
  {
    ....
    return true;
  }
  if (taDst.Count != taSrc.Count)                               // <=
    return false;
  if (taDst.Count == 0)
    return true;

  var ctx = new SubstContext(typeArgsCls, typeArgsMeth, true);
  if (ctx.IsNop)
  {
    return false;
  }

  for (int i = 0; i < taDst.Count; i++)
  {
    if (!SubstEqualTypesCore(taDst[i], taSrc[i], ctx))
      return false;
  }

  return true;
}
```

Предупреждение PVS\-Studio: [V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/) The 'taDst' object was used after it was verified against null\. Check lines: 354, 344\. [TypeManager\.cs 354](https://github.com/dotnet/runtime/blob/v10.0.8/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Types/TypeManager.cs#L354)

В условии первого `if` параметр `taDst` проверяется на `null`\. Следовательно, предполагается, что этот параметр может иметь значение `null`\. Однако в условии следующего `if` параметр `taDst` разыменовывается без проверки: `taDst.Count`\. Если `taDst` будет `null`, а `taSrc` не `null`, то будет выброшено исключение типа `NullReferenceException`\.

**Фрагмент кода 7**

```cpp
internal void WriteXmlnsAttribute(XmlDictionaryString ns)
{
  if (dictionaryWriter != null)
  {
    if (ns != null) 
      dictionaryWriter.WriteXmlnsAttribute(null, ns);
  }
  else
    WriteXmlnsAttribute(ns.Value);
}
```

Предупреждение PVS\-Studio: [V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/) The 'ns' object was used and was verified against null in different execution branches\. Check lines: 71, 67\. [XmlWriterDelegator\.cs 71](https://github.com/dotnet/runtime/blob/v10.0.8/src/libraries/System.Private.DataContractSerialization/src/System/Runtime/Serialization/XmlWriterDelegator.cs#L71)

Проблема схожа с предыдущей\. В блоке `then` первого `if` параметр `ns` перед использованием проверяется на `null`\. В блоке `else` этой проверки уже нет\. Возможно, параметр `ns` как\-то связан с полем `dictionaryWriter`, что позволяет не проверять `ns` на `null`, если `dictionaryWriter` — `null`\. Однако подтверждения этому найти не удалось\.

На данный момент выглядит так, что если `dictionaryWriter` и `ns` будут иметь значение `null`, то при обращении к `ns.Value` выбросится исключение типа `NullReferenceException`\.

**Фрагмент кода 8**

```cpp
private struct ModuleAndIntValueKey : IEquatable<ModuleAndIntValueKey>
{
  public bool Equals(ModuleAndIntValueKey other) => 
         IntValue == other.IntValue 
      && (   (Module == null && other.Module == null) 
          ||  Module.Equals(other.Module));
}
```

Предупреждение PVS\-Studio: [V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/) The 'Module' object was used after it was verified against null\. Check lines: 180, 180\. [ReadyToRunCodegenNodeFactory\.cs 180](https://github.com/dotnet/runtime/blob/v10.0.8/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs#L180)

При сравнении объектов типа `ModuleAndIntValueKey` проверяются их поля `Module`:

```cpp
(Module == null && other.Module == null) ||  Module.Equals(other.Module)
```

Если возникнет ситуация, в которой `Module` — null, а `other.Module` не `null`, то будет выброшено исключение типа `NullReferenceException`\. Так как левый операнд оператора `||` будет `false` условие будет вычисляться дальше, что приведёт к разыменованию `Module` при вызове `Equals`\.

Для исправления ситуации, нужно добавить проверку на `null`, перед обращением к `Module`\.

**Фрагмент кода 9**

```cpp
public bool IsInheritanceChainLayoutFixedInCurrentVersionBubble(TypeDesc type)
{
  // This method is not expected to be called for value types
  Debug.Assert(!type.IsValueType);

  if (type.IsObject)
    return true;

  if (!IsLayoutFixedInCurrentVersionBubble(type))
  {
    return false;
  }

  type = type.BaseType;

  if (type != null)
  {
    return false;

    while (!type.IsObject && type != null)
    {
      if (!IsLayoutFixedInCurrentVersionBubble(type))
      {
        return false;
      }
      type = type.BaseType;
    }
  }

    return true;
}
```

Предупреждение PVS\-Studio: [V3027](https://pvs-studio.ru/ru/docs/warnings/v3027/) The variable 'type' was utilized in the logical expression before it was verified against null in the same logical expression\. [ReadyToRunCodegenCompilation\.cs 596](https://github.com/dotnet/runtime/blob/v10.0.8/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs#L596)

В условии `while` сначала обращаются к свойству `IsObject` переменной `type`, после чего проверяют `type` на `null`\. Кажется, что такая проверка малоэффективна\. Если `type` всё же окажется `null`, то будет выброшено исключение типа `NullReferenceException`\. 

Для исправления нужно либо поменять местами проверку на `null` и обращение, либо, если `type` никогда не `null`, убрать эту самую проверку\.

## Сравнение с подвохом

**Фрагмент кода 10**

```cpp
public class PropertyPseudoDesc : TypeSystemEntity
{
  ....

  public static bool operator ==(PropertyPseudoDesc a, PropertyPseudoDesc b) 
              => a._type == b._type && a._handle == b._handle;

  ....
}
```

Предупреждение PVS\-Studio: [V3115](https://pvs-studio.ru/ru/docs/warnings/v3115/) Passing 'null' to '\=\=' operator should not result in 'NullReferenceException'\. [PropertyPseudoDesc\.cs 89](https://github.com/dotnet/runtime/blob/v10.0.8/src/coreclr/tools/Common/Compiler/PropertyPseudoDesc.cs#L89)

Если при сравнении объектов типа `PropertyPseudoDesc` один из операндов будет `null`, выбросится исключение типа 'NullReferenceException'\. Чтобы избежать подобной ситуации, перед обращением к полям операндов, нужно проверить, что они не `null`\.

## Заключение

В ходе проверки исходного кода \.NET 10 мы убедились, что даже спустя полгода после релиза в проекте сохраняются дефекты и потенциально проблемные участки\. Часть из них относится к классическим ошибкам: неочевидные условия, избыточные проверки, возможные NRE и неоднозначные ветвления логики\. Такие проблемы сложно заметить при беглом код\-ревью, особенно в крупной кодовой базе уровня платформы\.

Каждый год мы проверяем актуальную версию \.NET:

* [Проверка \.NET 9](https://pvs-studio.ru/ru/blog/posts/csharp/1216/);
* [Проверка \.NET 8](https://pvs-studio.ru/ru/blog/posts/csharp/1095/);
* [Проверка \.NET 7](https://pvs-studio.ru/ru/blog/posts/csharp/1015/);
* [Проверка \.NET 6](https://pvs-studio.ru/ru/blog/posts/csharp/0903/)\.

Также регулярно публикуем проверки других популярных проектов\. Если хотите всегда быть в курсе новых проверок, [подпишитесь](https://pvs-studio.ru/ru/subscribe/) на нашу рассылку\.

Если вы хотите самостоятельно проверить проект с помощью PVS\-Studio, то попробовать анализатор можете по [ссылке](https://pvs-studio.ru/ru/pvs-studio/try-free/)\!