Ошибки и подозрительные места в исходниках .NET 8
Каждый год выходит новая версия .NET. Это событие не только предоставляет нам возможность познакомиться с последними улучшениями в самом .NET и нововведениями в языке, но и даёт повод исследовать исходный код .NET. Нужно воспользоваться этим шансом!
Кстати говоря, у нас уже есть несколько статей, посвящённых последним обновлениям в мире .NET и C#. Если вас интересует, что в этот раз добавили Microsoft, рекомендую вам заглянуть в следующие материалы:
В этих статьях мы рассматриваем ключевые изменения, которые появились в последних версиях .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 контексту? Если интересно наше мнение, или если вы ещё не знакомы с данным механизмом, то предлагаю почитать наши статьи:
Фрагмент кода 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.