Каждый год выходит новая версия .NET, и .NET 9 не стал исключением. Как и в прошлый раз, мы попробуем свои силы в поиске ошибок в исходном коде .NET. Погнали копаться в исходниках!

Наша команда, как и многие C# разработчики, следит за релизами и нововведениями .NET. Конечно же, мы подготовили для вас несколько материалов на эту тематику. Предлагаю прочитать, что было добавлено в .NET 9 и C# 13:

• Что нового в .NET 9?

• Обзор нововведений в C# 13

В этих статьях мы рассматриваем основные нововведения, которые были внедрены в последних релизах .NET и C#.

Но мы следим за релизами .NET не только из-за новых фич. Одна из наших задач — поддержка нового .NET в статическом анализаторе кода PVS-Studio. В этот раз поддержка .NET 9 была достаточно интересная, и по итогу была написана статья про трудности, с которыми мы столкнулись: "Как обновить библиотеку и утонуть в задаче. Обновление Roslyn и PVS-Studio 7.34". Конечно, поддержка .NET 9 реализована, и вы уже можете попробовать анализатор на своих проектах.

Перейдём к анализу исходного кода .NET 9. Для проведения анализа исходников использовался релизный код, который доступен на GitHub по ссылке.

В прошлом году я рассказывал, что для анализа .NET 8 мне пришлось перейти с Visual Studio 2022 на VS Code. Связано это с тем, что VS 2022 просто не выдерживала размеров .sln .NET 8. Она перезагружалась или просто завершала работу. И мне повезло, что мы очень вовремя выпустили плагин PVS-Studio для VS Code.

В этом году я сразу решил провести анализ через VS Code, но ради интереса попробовал поработать через VS 2022. И знаете что? Я не словил перезагрузок или аварийных завершений работы, но выполнять навигацию по коду всё ещё было достаточно трудно. Всё работало достаточно медленно, но это определённо прогресс. Сказываются постоянные обновления VS 2022, направленные на повышение производительности. Посмотрим, что будет в следующем году :)

А теперь переходим к рассмотрению обнаруженных ошибок.

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

struct StackValue
{
  ....
  public override bool Equals(object obj)
  {
    if (Object.ReferenceEquals(this, obj))
      return true;

    if (!(obj is StackValue))
      return false;

    var value = (StackValue)obj;
    return    this.Kind == value.Kind 
           && this.Flags == value.Flags 
           && this.Type == value.Type;
  }
}

Видите проблему? А она есть!

V3161 Comparing value type variables with 'ReferenceEquals' is incorrect because 'this' will be boxed. ILImporter.StackValue.cs 164

В этом случае выполняется сравнение по ссылке объекта значимого типа. Метод ReferenceEquals принимает параметр типа Object, и при передаче значимого типа будет произведена упаковка. Созданная в куче ссылка не будет равна никакой другой, и методвсегда будет возвращать false.

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

private static async Task<JArray> GetRootHiddenChildren(...., JObject root)
{
  var rootValue = root?["value"] ?? root["get"];

  if (rootValue?["subtype"]?.Value<string>() == "null")
    return new JArray();
  ....
}

V3042 Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'root' object MemberObjectsExplorer.cs 109

Анализатор обнаружил совместное использование операторов ? и .:

root?["value"] ?? root["get"]

В нашем случае root проверяется на null с помощью ?. Вот только если root будет равен null, то будет выброшено исключение NullReferenceException при выполнении root["get"].

Причём подобных предупреждений достаточно много. Где-то оператор ?. выглядит просто излишним, а в некоторых фрагментах кода возможны ошибки из-за совместного использования ?. и ..

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

Есть ещё похожие ошибки.

public async Task<JObject> GetValueFromObject(JToken objRet, ....)
{
  if (objRet["value"]?["className"]?.Value<string>() == "System.Exception")
  {
    if (DotnetObjectId
          .TryParse(objRet?["value"]?["objectId"]?.Value<string>(), ....))
    {
      ....
    }
  }
}

V3095 The 'objRet' object was used before it was verified against null. Check lines: 61, 63. MemberReferenceResolver.cs 61

Здесь objRet сначала используют в условии первого if, а потом проверяют на равенство null в условии второго if.

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

private async Task<bool> GetFrames(....)
{
  ....
  foreach (JObject frame in orig_callframes
                              .Value["result"]?["value"]?["frames"])
  {
    ....
  }
  ....
}

V3153 Enumerating the result of null-conditional access operator can lead to NullReferenceException. Consider inspecting: ["value"]?["frames"]. FirefoxMonoProxy.cs 982

Достаточно распространённый паттерн ошибки, но, если честно, в коде .NET я не ожидал его увидеть. В цикле foreach перечисляемую коллекцию дважды проверяют на равенство null с помощью оператора ?.. А ведь foreach не работает с null, из-за чего будет выброшено исключение.

Традиционно даю ссылку на статью, где мы рассматриваем подобные ошибки:

"Использование оператора ?. в foreach: защита от NullReferenceException, которая не работает"

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

private string GetXCodeDevRoot()
{
  string path = "";
  string output;

  if (!File.Exists ("/usr/bin/xcode-select"))
  {
    throw new Exception(....);
  }

  try
  {
    (int exitCode, output) = Utils.TryRunProcess(....);

    output.Trim();                      // <=
    if (Directory.Exists(output))                
    { .... }
  }
}

V3010 The return value of function 'Trim' is required to be utilized. AppleSdk.cs 107

Здесь анализатор предупреждает о том, что в коде не используется возвращаемое значение метода Trim. Видимо, разработчик забыл, что этот метод создаёт новую строку, а не изменяет текущую.

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

private bool HasAssemblyDisableRuntimeMarshallingAttribute(Assembly assembly)
{
  if (!_assemblyDisableRuntimeMarshallingAttributeCache
          .TryGetValue(assembly, out var value))
  {
    _assemblyDisableRuntimeMarshallingAttributeCache[assembly] =
      value = assembly
                .GetCustomAttributesData()
                .Any(d => d.AttributeType.Name == 
                          "DisableRuntimeMarshallingAttribute");
  }

  value = assembly.GetCustomAttributesData()
                   .Any(d => d.AttributeType.Name ==
                             "DisableRuntimeMarshallingAttribute");

  return value;
}

V3008 The 'value' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 207, 202. PInvokeCollector.cs 207

А вот пример неработающего кэша. Анализатор обнаружил перезаписывание переменной value. Независимо от того, смогли ли взять value из кэша, всё равно выполняется операция поиска атрибутов. Выглядит, как случайно оставленный код после добавления кэша или отладки кода.

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

private static bool MatchesTheFilter(....)
{
  ....
  for (int i = 0; i < parameterTypes.Length; i++)
  {
    Type? argType = argumentTypes[i];
    Type? paramType = parameterTypes[i];

    if (argType.IsArray != paramType.IsArray ||
        argType.IsByRef != paramType.IsByRef ||
        argType.IsPointer != argType.IsPointer) // <=
    {
      return false;
    }
  ....
  }
}

V3001 There are identical sub-expressions 'argType.IsPointer' to the left and to the right of the '!=' operator. TypeBuilderImpl.cs 734

Простая, но такая обидная ошибка. Видимо, ошибка copy-paste. argType.IsPointer сравнивается сам с собой, а должен с paramType.IsPointer.

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

private void ImplReadElement()
{
  if (3 != _docState || 9 != _docState)
  {
    switch (_docState)
    {
      ....
    }
  }
}

V3022 Expression '3 != _docState || 9 != _docState' is always true. Probably the '&&' operator should be used here. XmlBinaryReader.cs 3026

Достаточно странное условие, которое всегда true. Как вы сами понимаете, оно бесполезно.

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

Попробуйте самостоятельно найти ошибку в следующем коде:

public static void SetAsIConvertible(this ref ComVariant variant,
                                     IConvertible value)
{
  TypeCode tc = value.GetTypeCode();
  CultureInfo ci = CultureInfo.CurrentCulture;

  switch (tc)
  {
    case TypeCode.Empty: break;
    case TypeCode.Object: 
      variant = ComVariant.CreateRaw(....); break;
    case TypeCode.DBNull: 
      variant = ComVariant.Null; break;
    case TypeCode.Boolean: 
      variant = ComVariant.Create<bool>(....)); break;
    case TypeCode.Char: 
      variant = ComVariant.Create<ushort>(value.ToChar(ci)); break;
    case TypeCode.SByte: 
      variant = ComVariant.Create<sbyte>(value.ToSByte(ci)); break;
    case TypeCode.Byte: 
      variant = ComVariant.Create<byte>(value.ToByte(ci)); break;
    case TypeCode.Int16: 
      variant = ComVariant.Create(value.ToInt16(ci)); break;
    case TypeCode.UInt16: 
      variant = ComVariant.Create(value.ToUInt16(ci)); break;
    case TypeCode.Int32: 
      variant = ComVariant.Create(value.ToInt32(ci)); break;
    case TypeCode.UInt32: 
      variant = ComVariant.Create(value.ToUInt32(ci)); break;
    case TypeCode.Int64: 
      variant = ComVariant.Create(value.ToInt64(ci)); break;
    case TypeCode.UInt64: 
      variant = ComVariant.Create(value.ToInt64(ci)); break;
    case TypeCode.Single: 
      variant = ComVariant.Create(value.ToSingle(ci)); break;
    case TypeCode.Double: 
      variant = ComVariant.Create(value.ToDouble(ci)); break;
    case TypeCode.Decimal: 
      variant = ComVariant.Create(value.ToDecimal(ci)); break;
    case TypeCode.DateTime: 
      variant = ComVariant.Create(value.ToDateTime(ci)); break;
    case TypeCode.String: 
      variant = ComVariant.Create(....); break;

    default:
      throw new NotSupportedException();
  }
}

Видите ошибку? А она есть!

case TypeCode.Int64: 
  variant = ComVariant.Create(value.ToInt64(ci)); break;
case TypeCode.UInt64: 
  variant = ComVariant.Create(value.ToInt64(ci)); break; // <=

Смогли её найти? Надеюсь, вы размяли свои глаза, и зоркость вас не подвела. В case TypeCode.UInt64 вместо выражения value.ToInt64 нужно использовать метод ToUInt64(), который существует. Вероятно, эта ошибка произошла из-за Copy Paste.

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

protected override void OutputMemberScopeModifier(
MemberAttributes attributes)
{
  switch (attributes & MemberAttributes.ScopeMask)
  {
    case MemberAttributes.Abstract:
      Output.Write("MustOverride ");
      break;
    case MemberAttributes.Final:
      Output.Write("");
      break;
    ....
    case MemberAttributes.Private:            // <=
      Output.Write("Private ");
      break;
    ....
  }
}

V3202 Unreachable code detected. The 'case' value is out of range of the match expression. VBCodeGenerator.cs 580

А здесь анализатор PVS-Studio считает, что case с MemberAttributes.Private недостижим.

Посмотрим на определение перечисления MemberAttributes:

public enum MemberAttributes
{
  Abstract = 0x0001,
  Final = 0x0002,
  Static = 0x0003,
  Override = 0x0004,
  Const = 0x0005,
  New = 0x0010,
  Overloaded = 0x0100,
  Assembly = 0x1000,
  FamilyAndAssembly = 0x2000,
  Family = 0x3000,
  FamilyOrAssembly = 0x4000,
  Private = 0x5000,
  Public = 0x6000,
  AccessMask = 0xF000,
  ScopeMask = 0x000F,
  VTableMask = 0x00F0
}

При использовании любого из значений перечисления MemberAttributes с MemberAttributes.ScopeMask и побитового логического оператора & невозможно получить значение MemberAttributes.Private.

Заключение

Вот мы и закончили. Конечно, это не все ошибки, которые были обнаружены. На перечисление всех у нас бы ушло очень много времени. Как обычно, мы сообщим разработчикам о найденных ошибках через issue на GitHub.

Как обычно, предлагаю вам проверить свой проект. Ошибки и странности есть в любом коде, но, конечно, плотность ошибок возрастает с ростом числа строк кода. Так что анализатор будет более полезен для больших проектов. Попробовать анализатор PVS-Studio можно по ссылке.

Удачи!

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Artem Rovenskii. Bugs and suspicious places in .NET 9 source code.