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

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

Кстати говоря, у нас уже есть несколько статей, посвящённых последним обновлениям в мире .NET и C#. Если вас интересует, что в этот раз добавили Microsoft, рекомендую вам заглянуть в следующие материалы:

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

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

В этих статьях мы рассматриваем ключевые изменения, которые появились в последних версиях .NET и C#. Приглашаю вас прочитать, чтобы быть в курсе последних событий.

Кроме того, в последнем релизе PVS-Studio 7.28 уже реализована поддержка анализа проектов, использующих .NET 8. Для проведения анализа исходников использовался релизный код .NET 8, который доступен на GitHub по ссылке.

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

Как известно, .NET огромен, и это может создавать проблемы. В исходниках есть скрипт, который позволяет сгенерировать решение для .NET библиотек. Это решение я проанализировал с помощью консольной утилиты PVS-Studio. Отчёт же я принялся изучать в IDE, в которой я работаю — Visual Studio 2022, но возникла проблема. При попытках навигации по коду в Visual Studio 2022 происходило нечто непредвиденное: либо происходила перезагрузка IDE, либо она просто завершала свою работу. Причём такое поведение повторяется не только при навигации по коду с помощью плагина PVS-Studio, но и при обычном переключении между файлами, использовании ‘Go To Definition’ и т. д.

Это усложнило работу, но выход нашёлся быстро.

Не так давно у нас появилась поддержка анализа .NET проектов в VS Code. Про это есть отдельная статья: «Использование расширения VS Code «PVS-Studio» для эффективной борьбы с ошибками в C# коде«. Учитывая, что VS Code представляет собой легковесный редактор кода, подобных трудностей, с которыми мы столкнулись в Visual Studio 2022, там не возникло.

Вот так выглядит окно PVS-Studio в Visual Studio Code:

.NET — мощная платформа, которая имеет высокие стандарты для кода, пишется настоящими профессионалами и хорошо тестируется. Однако даже в таком крутом проекте PVS-Studio способен найти ошибки.

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

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

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

Предупреждение PVS-Studio: V3181 The result of ‘&’ operator is ‘0’ because the value of ‘SettingsManageability.Roaming’ is ‘0’. LocalFileSettingsProvider.cs 411

В данном случае значение константы перечисления SettingsManageability.Roaming равно 0. Поскольку результат побитового «И» с операндом 0 всегда равен 0, получается, что 0 сравнивается с 0. Выходит, что результатом выражения ((manageAttr.Manageability & SettingsManageability.Roaming) == SettingsManageability.Roaming всегда является true.

Разработчикам стоит обратить внимание на этот код.

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

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

Предупреждение PVS-Studio: 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

Анализатор сообщает о некорректной строке формата в первом аргументе метода Trace. Посмотрим на этот метод:

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

Действительно, первый аргумент используется в качестве строки формата. В эту строку подставляются аргументы. Вот только аргументы должны подставляться в плейсхолдеры вида {0}, {1} и т. д. В данной строке подобные плейсхолдеры отсутствуют. В итоге использования такой строки формата будет выброшено исключение типа System.FormatException о некорректном формате.

Возможно, нужно использовать какой-то другой метод логирования. Если пройтись по другим местам использования метода Trace, то там всё используется корректно, и строки формата содержат маркеры:

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

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;                            // <=   .... } 

Предупреждение PVS-Studio: V3008 The ‘ResPrec’ variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1689, 1685. SQLDecimal.cs 1689

В данном фрагменте видно, что происходит двойное присваивание в переменную ResPrec.

Поскольку между этими двумя операциями ResPrec не используется, это свидетельствует об ошибке.

Здесь два варианта:

• Одно из присваиваний является лишним — ничего страшного, просто лишняя операция, хотя это и нехорошо;

• Между двумя присваиваниями переменная ResPrec должна использоваться — вот это уже будет неприятной ошибкой.

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

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

Предупреждение PVS-Studio: V3022 Expression ‘_validationState == ValidatingReaderState.OnReadBinaryContent’ is always false. XsdValidatingReader.cs 1302

PVS-Studio обнаружил, что последнее условие if (_validationState == ValidatingReaderState.OnReadBinaryContent) всегда будет ложным. Давайте разбираться почему.

Взглянем на первый оператор if. В нём полю _validationState присваивается:

• в then ветви — ValidatingReaderState.OnAttribute

• в else ветви — ValidatingReaderState.OnDefaultAttribute

Поэтому значение поля не может быть равно ValidatingReaderState.OnReadBinaryContent, и код внутри if не выполняется.

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

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   {     ....   }   .... } 

Предупреждение PVS-Studio: V3102 Suspicious access to element of ‘type.Instantiation’ object by a constant index inside a loop. TypeLoaderEnvironment.GVMResolution.cs 32

Предположу, что в данном фрагменте кода из информации о типе формируется запись следующего вида: ConsoleApp1.Program.MyClass<string, int, double>. Вот только в цикле обращаются к объекту type.Instantiation по константному индексу, равному 0. Не исключено, что работает всё как надо, но выглядит очень странно. Ожидаешь увидеть GetTypeNameDebug(type.Instantiation[i]).

И да, я сразу пошёл и проверил, в дебаггере Visual Studio 2022 всё отображается корректно, но не исключено, что где-то можно встретить отображение типа с ошибкой :).

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

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

Предупреждение PVS-Studio: V3125 The ‘stack_instr!’ object was used after it was verified against null. Check lines: 1918, 1913. UnreachableBlocksOptimizer.cs 1918

Разработчик использовал оператор ‘?.’, подразумевая, что поле stack_instr может быть null. И вроде бы всё хорошо, есть проверка, но не тут-то было. В указанной строчке возможно разыменование нулевой ссылки. Скорее всего, разработчик подумал, что выражение stack_instr?.Count < length при stack_instr равным null вернёт true, и произойдёт выход из метода, но нет — результатом будет false.

Более того, разработчик подавил сообщение компилятора о возможном разыменовании null ссылки с помощью ‘!’, т.к. подумал, что статический анализ компилятора просто не справился и не понял проверки.

А как вы относитесь к nullable контексту? Если интересно наше мнение, или если вы ещё не знакомы с данным механизмом, то предлагаю почитать наши статьи:

• Помоги компилятору, и он поможет тебе. Тонкости работы с nullable reference типами в C#

• Nullable Reference не защищают, и вот доказательства

• Nullable Reference типы в C# 8.0 и статический анализ

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

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

Предупреждение PVS-Studio: V3095 The ‘resolvedType’ object was used before it was verified against null. Check lines: 34, 55. TypeHierarchyCache.cs 34

Параметр resolvedType сначала используют, но перед добавлением в кэш проверяют на null. Странно как-то выходит. Анализатор указал на resolvedType.Name, но программа упадёт даже раньше. Метод TryGetValue выбросит исключение, если первый аргумент resolvedType будет null.

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

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

Предупреждение PVS-Studio: V3001 There are identical sub-expressions ‘tr.Namespace’ to the left and to the right of the ‘==’ operator. TypeReferenceExtensions.cs 365

Анализатор выявил, что в данном коде сравниваются два одинаковых подвыражения. Простая, но обидная ошибка. tr.Namespace сравнивается с tr.Namespace, а должен с type.Namespace.

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

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

Предупреждение PVS-Studio: V3025 Incorrect format. A different number of format items is expected while calling ‘Write’ function. Arguments not used: ClassName ?? «null». EHInfo.cs 135

Ещё одна ошибка со строкой формата, но в этот раз для класса TextWriter. Разработчик использовал символ интерполяции строк ‘$’. В строку просто подставится число 0, и строка формата станет равна » CATCH: 0″. В итоге текст, который хотели подставить вместо плейсхолдера {0}, не используется. Такая же ошибка и в следующем case.

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

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

Предупреждение PVS-Studio: V3038 The argument was passed to constructor several times. It is possible that other argument should be passed instead. ReadyToRunSignature.cs 707

Аргумент _outerReader передаётся в конструктор два раза. Если взглянуть на объявление конструктора, то можно увидеть, что конструктор имеет параметр с именем metadataReader:

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

В момент вызова конструктора доступно поле _metadataReader. Возможно, в качестве третьего аргумента стоит использовать именно его.

Фрагмент кода 11 — бонус

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

Предупреждение PVS-Studio: V3001 There are identical sub-expressions ‘context.Target.PointerSize == 4’ to the left and to the right of the ‘&&’ operator. MetadataFieldLayoutAlgorithm.cs 648

В выражении два раза проверяется context.Target.PointerSize == 4. В экземпляром методе GetObjectAlignment изменение context.Target.PointerSize не происходит. Возможно, что здесь должно проверяться что-то ещё, а может это просто лишняя проверка.

Как я уже писал ранее, .NET имеет код высокого качества. И тем не менее, я не перестаю удивляться некоторым ошибкам, которые находятся в проектах подобной величины. Отлаженный процесс разработки, разработчики высшего класса, но всё равно встречаются ошибки и странности в коде. Безусловно, это нормально, идеального кода не существует, но к нему можно и нужно стремиться.

Предлагаю и вам проверить свой проект на наличие странностей и ошибок. Попробовать анализатор можно по ссылке. Пишите нам, если будут вопросы — мы оперативно решаем все возникшие проблемы :).

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