Artem Rovenskii

Jan 29 2025

Tags:

#CSharp

Bugs and suspicious places in .NET 9 source code

Jan 29 2025

Author: Artem Rovenskii

Every year, Microsoft releases a new version of .NET, and .NET 9 is already delivered! Like the last time, we dive into hunting errors in .NET source code. Let's see what we uncover this time!

Our team, like many C# developers, keeps an eye on .NET releases and new features, so we've put together some content on this topic! I suggest checking out what's been added in .NET 9 and C# 13:

These articles explore the key features introduced in the latest versions of .NET and C#.

However, we don't just monitor .NET releases for new features—we focus on supporting the latest .NET versions in the PVS-Studio static analyzer. When we implemented support for .NET 9, we faced some interesting challenges. So, we wrote an article about them: How to update library and get swamped with this task. Roslyn and PVS-Studio 7.34 update. Now .NET 9 is fully supported, and you can start using the analyzer in your projects right away.

Let's move on to analyzing the .NET 9 source code; we'll explore the release code available on GitHub.

Last year, I told you that I had to switch from Visual Studio 2022 to VS Code to analyze .NET 8 because VS 2022 couldn't handle the massive .sln of .NET 8—it either restarted or crashed repeatedly. I was lucky that the PVS-Studio plugin for VS Code was released just in time.

This year, I immediately decided to analyze it via VS Code, but for the sake of interest I also gave VS 2022 a shot. And guess what? No restarts or crashes, but code navigation remained challenging enough. Continuous performance enhancements made a noticeable impact—let's see how it will be next year :)

Now let's explore the detected errors.

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

Do you think there are no problems? Oh no, there definitely are.

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

In this scenario, a value type object is compared by reference. The ReferenceEquals method accepts parameters of the Object type. When a value type is passed, it gets boxed. The reference created on the heap won't match any other reference, meaning the method will always return false.

Fragment 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

The analyzer has detected a combination of the ? and . operators:

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

In our case, root is checked for null with ?. However, if root is equal to null, a NullReferenceException will be thrown when root["get"] is executed.

There are many such warnings: sometimes the ?. operator may be just redundant, while in other cases, combining ?. and . leads to potential code errors.

Fragment 3

There are other similar errors.

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

Here, objRet is first used in the first if statement, and then checked for null in the second if statement.

Fragment 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

It's a common error pattern, but honestly, I didn't expect to see it in the .NET code. In a foreach loop, the enumerable collection is checked twice for null via the ?. operator. But foreach doesn't work with null, which will lead to an exception being thrown.

Here's a link to an article dedicated to such errors: The ?. operator in foreach will not protect from NullReferenceException.

Fragment 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

The analyzer has detected that the Trim return value isn't used in the code. The developer may have overlooked the fact that the method creates a new string instead of modifying the existing one.

Fragment 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

Here's an example of a broken cache—the analyzer has detected an overwritten value variable. Regardless of whether value has been previously retrieved from the cache, an attribute will continue to be searched. This issue may have occurred if developers left the code after adding the cache or code debugging.

Fragment 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

It's an easy yet frustrating error—likely a copy-paste. Instead of comparing argType.IsPointer to paramType.IsPointer, it's compared to itself.

Fragment 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

It's quite a strange condition that's always true and doesn't make any sense.

Fragment 9

Here is a little challenge: can you spot the error in the following snippet?

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

Can't find it? There it is!

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

I hope you've given your eyes a good workout, and your keen eyesight hasn't let you down. In case TypeCode.UInt64 instead of value.ToInt64, developers should have used the existing ToUInt64() method. This may be a copy-paste error.

Fragment 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 finds that case with MemberAttributes.Private is unreachable.

Let's look at the definition of the MemberAttributes enumeration:

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
}

When using any of the values in the MemberAttributes enumeration with MemberAttributes.ScopeMask and the bitwise logical operator &, developers can't get the MemberAttributes.Private value.

Conclusion

We'll wrap it up here. Of course, I haven't demonstrated all the detected errors because it'd take us a long time. As usual, we'll notify developers about bugs found by creating an issue on GitHub.

As a tradition, I encourage you to analyze your own project. Any code has errors and suspicious areas, but of course, the error density increases with the number of code lines. So, the analyzer will be an invaluable ally for large-scale projects.

Try the PVS-Studio analyzer here.

Good luck!

#CSharp