Undercover Spy: Checking the ILSpy Source Code Using PVS-Studio

In the blog of the PVS-Studio company you can find more than one article with the results of checks of the source code of various compilers. On the other hand, another class of programs, the class of decompilers, seems to be a bit deprived of PVS-Studio's attention. In order to make the world more harmonious, the C # source code of the ILSpy program, which belongs to the decompiler camp, was checked. Let's see what interesting PVS-Studio could find.





Introduction

, . : - , , . .NET' dotPeek ILSpy. .NET Reflector . , , , . - ILSpy . , , PVS-Studio ILSpy. , , , , "" .





, ILSpy . , . , , Unity Unreal Engine.





, PVS-Studio Unreal Engine, Unity , , . Unity . open-source , . - ( , , ?). , Unity ILSpy ( , ). , ILSpy , C# . , . , Unity , ILSpy.





GitHub': ILSpy is the open-source .NET assembly browser and decompiler.





, - . , - PVS-Studio . .





V3038 The '"'"' argument was passed to 'Replace' method several times. It is possible that other argument should be passed instead. ICSharpCode.Decompiler ReflectionDisassembler.cs 772





private static void WriteSimpleValue(ITextOutput output,
                                     object value, string typeName)
{
  switch (typeName)
  {
    case "string":
      output.Write(  "'"
                   + DisassemblerHelpers
                      .EscapeString(value.ToString())
                      .Replace("'", "\'")                   // <=
                   + "'");
      break;
    case "type":
    ....
  }
  ....
}

      
      



, , : . - - "'" , . "'" "\'" - . , "\'", backslash '@': "\\'" @"\'". Replace :





Replace("'", @"\'")

      
      



1





V3022 Expression 'negatedOp == BinaryOperatorType.Any' is always true. ICSharpCode.Decompiler CSharpUtil.cs





static Expression InvertConditionInternal(Expression condition)
{
  var bOp = (BinaryOperatorExpression)condition;

  if (   (bOp.Operator == BinaryOperatorType.ConditionalAnd)
      || (bOp.Operator == BinaryOperatorType.ConditionalOr))
  {
    ....
  }
  else if (   (bOp.Operator == BinaryOperatorType.Equality)
           || (bOp.Operator == BinaryOperatorType.InEquality) 
           || (bOp.Operator == BinaryOperatorType.GreaterThan)
           || (bOp.Operator == BinaryOperatorType.GreaterThanOrEqual)
           || (bOp.Operator == BinaryOperatorType.LessThan) 
           || (bOp.Operator == BinaryOperatorType.LessThanOrEqual))
  {
    ....
  }
  else
  {
    var negatedOp = NegateRelationalOperator(bOp.Operator);
    if (negatedOp == BinaryOperatorType.Any)                  // <=
      return new UnaryOperatorExpression(....);
    bOp = (BinaryOperatorExpression)bOp.Clone();
    bOp.Operator = negatedOp;
    return bOp;
  }
}

      
      



, negatedOp Any BinaryOperatorType. , NegateRelationalOperator, .





public static BinaryOperatorType NegateRelationalOperator(BinaryOperatorType op)
{
  switch (op)
  {
    case BinaryOperatorType.GreaterThan:
      return BinaryOperatorType.LessThanOrEqual;
    case BinaryOperatorType.GreaterThanOrEqual:
      return BinaryOperatorType.LessThan;
    case BinaryOperatorType.Equality:
      return BinaryOperatorType.InEquality;
    case BinaryOperatorType.InEquality:
      return BinaryOperatorType.Equality;
    case BinaryOperatorType.LessThan:
      return BinaryOperatorType.GreaterThanOrEqual;
    case BinaryOperatorType.LessThanOrEqual:
      return BinaryOperatorType.GreaterThan;
    case BinaryOperatorType.ConditionalOr:
      return BinaryOperatorType.ConditionalAnd;
    case BinaryOperatorType.ConditionalAnd:
      return BinaryOperatorType.ConditionalOr;
  }
  return BinaryOperatorType.Any;
}

      
      



NegateRelationalOperator bOp.Operator , case, BinaryOperatorType.Any. , NegateRelationalOperator , if if else false. , , if if else case NegateRelationalOperator. , NegateRelationalOperator bOp.Operator case, BinaryOperatorType.Any. , negatedOp == BinaryOperatorType.Any true, . :





bOp = (BinaryOperatorExpression)bOp.Clone();
bOp.Operator = negatedOp;
return bOp;

      
      



, : V3142 Unreachable code detected. It is possible that an error is present. ICSharpCode.Decompiler CSharpUtil.cs 81





2





V3022 Expression 'pt != null' is always true. ICSharpCode.Decompiler FunctionPointerType.cs 168





public override IType VisitChildren(TypeVisitor visitor)
{
  ....
  IType[] pt = (r != ReturnType) ? new IType[ParameterTypes.Length] : null;
  ....
  if (pt == null)
    return this;
  else
    return new FunctionPointerType(
      module, CallingConvention, CustomCallingConventions,
      r, ReturnIsRefReadOnly,
      pt != null ? pt.ToImmutableArray() : ParameterTypes,    // <=
      ParameterReferenceKinds);
}

      
      



- else , pt null. pt null, . , if else return. , :





public override IType VisitChildren(TypeVisitor visitor)
{
  ....
  IType[] pt = (r != ReturnType) ? new IType[ParameterTypes.Length] : null;
  ....
  if (pt == null)
    return this;
  else
    return new FunctionPointerType(
      module, CallingConvention, CustomCallingConventions,
      r, ReturnIsRefReadOnly,
      pt.ToImmutableArray(), ParameterReferenceKinds);
}

      
      



3





V3022 Expression 'settings.LoadInMemory' is always true. ICSharpCode.Decompiler CSharpDecompiler.cs 394





static PEFile LoadPEFile(string fileName, DecompilerSettings settings)
{
  settings.LoadInMemory = true;
  return new PEFile(
    fileName,
    new FileStream(fileName, FileMode.Open, FileAccess.Read),
    streamOptions: settings.LoadInMemory ?                           // <=
      PEStreamOptions.PrefetchEntireImage : PEStreamOptions.Default,
    metadataOptions: settings.ApplyWindowsRuntimeProjections ? 
        MetadataReaderOptions.ApplyWindowsRuntimeProjections :
        MetadataReaderOptions.None
  );
}

      
      



. settings.LoadInMemory, , true, . :





public bool LoadInMemory {
  get { return loadInMemory; }
  set {
      if (loadInMemory != value)
      {
        loadInMemory = value;
        OnPropertyChanged();
      }
  }
}

      
      



, - .





4





V3022 Expression 'ta' is always not null. The operator '??' is excessive. ICSharpCode.Decompiler ParameterizedType.cs 354





public IType VisitChildren(TypeVisitor visitor)
{
  ....
  if (ta == null)
      return this;
  else
      return new ParameterizedType(g, ta ?? typeArguments);     // <=
}

      
      



null coalescing . else ta null. , ?? .





31 V3022.





1





V3025 Incorrect format. A different number of format items is expected while calling 'Format' function. Arguments not used: End. ICSharpCode.Decompiler Interval.cs 269





public override string ToString()
{
  if (End == long.MinValue)
  {
    if (Start == long.MinValue)
      return string.Format("[long.MinValue..long.MaxValue]", End); // <=
    else
      return string.Format("[{0}..long.MaxValue]", Start);
  }
  else if (Start == long.MinValue)
  {
    return string.Format("[long.MinValue..{0})", End);
  }
  else
  {
    return string.Format("[{0}..{1})", Start, End);
  }
}

      
      



string.Format . End, , , {0}. , return , . , , , string.Format . , , .





2





V3025 Incorrect format. A different number of format items is expected while calling 'AppendFormat' function. Arguments not used: angle. ILSpy.BamlDecompiler XamlPathDeserializer.cs 177





public static string Deserialize(BinaryReader reader)
{
  ....
  var sb = new StringBuilder();
  ....
  sb.AppendFormat(CultureInfo.InvariantCulture,
                  "A{0} {2:R} {2} {3} {4}",
                  size, angle, largeArc ? '1' : '0',
                  sweepDirection ? '1' : '0', pt1);
  ....
}

      
      



angle. , AppendFormat, - , {1} {2}, . , : "A{0} {1:R} {2} {3} {4}".





1





V3095 The 'roslynProject' object was used before it was verified against null. Check lines: 96, 97. ILSpy.AddIn OpenILSpyCommand.cs 96





protected Dictionary<string, detectedreference=""> GetReferences(....)
{
  ....
  var roslynProject =  owner.Workspace
                            .CurrentSolution
                            .GetProject(projectReference.ProjectId);
  var project = FindProject(owner.DTE.Solution
                                 .Projects.OfType<envdte.project>(),
                            roslynProject.FilePath);              // <=

  if (roslynProject != null && project != null)           // <=
  ....
}

      
      



FilePath roslynProject - , roslynProject null, null. NullReferenceException. FilePath, null- , FindProject null .





2





V3095 The 'listBox' object was used before it was verified against null. Check lines: 46, 52. ILSpy FlagsFilterControl.xaml.cs 46





public override void OnApplyTemplate()
{
  base.OnApplyTemplate();

  listBox = Template.FindName("ListBox", this) as ListBox;
  listBox.ItemsSource = FlagGroup.GetFlags(....);         // <=

  var filter = Filter;

  if (filter == null || filter.Mask == -1)
  {
    listBox?.SelectAll();                                 // <=
  }
}

      
      



. ItemsSource - , listBox null, null- listBox. listBox .





10 V3095. :





  • V3095 The 'pV' object was used before it was verified against null. Check lines: 761, 765. ICSharpCode.Decompiler TypeInference.cs 761





  • V3095 The 'pU' object was used before it was verified against null. Check lines: 882, 886. ICSharpCode.Decompiler TypeInference.cs 882





  • V3095 The 'finalStore' object was used before it was verified against null. Check lines: 261, 262. ICSharpCode.Decompiler TransformArrayInitializers.cs 261





  • V3095 The 'definitionDeclaringType' object was used before it was verified against null. Check lines: 93, 104. ICSharpCode.Decompiler SpecializedMember.cs 93





  • V3095 The 'TypeNamespace' object was used before it was verified against null. Check lines: 84, 88. ILSpy.BamlDecompiler XamlType.cs 84





  • V3095 The 'property.Getter' object was used before it was verified against null. Check lines: 1676, 1684. ICSharpCode.Decompiler CSharpDecompiler.cs 1676





  • V3095 The 'ev.AddAccessor' object was used before it was verified against null. Check lines: 1709, 1717. ICSharpCode.Decompiler CSharpDecompiler.cs 1709





  • V3095 The 'targetType' object was used before it was verified against null. Check lines: 1614, 1657. ICSharpCode.Decompiler CallBuilder.cs 1614





, PVS-Studio ILSpy, , . PVS-Studio, , .





1





V3139 Two or more case-branches perform the same actions. ILSpy Images.cs 251





protected override ImageSource GetBaseImage(MemberIcon icon)
{
  ImageSource baseImage;
  switch (icon)
  {
    case MemberIcon.Field:
      baseImage = Images.Field;
      break;
    case MemberIcon.FieldReadOnly:
      baseImage = Images.FieldReadOnly;
      break;
    case MemberIcon.Literal:
      baseImage = Images.Literal;             // <=
      break;
    case MemberIcon.EnumValue:
      baseImage = Images.Literal;             // <=
      break;
    case MemberIcon.Property:
      baseImage = Images.Property;
      break;
    case MemberIcon.Indexer:
      baseImage = Images.Indexer;
      break;
    case MemberIcon.Method:
      baseImage = Images.Method;
      break;
    case MemberIcon.Constructor:
      baseImage = Images.Constructor;
      break;
    case MemberIcon.VirtualMethod:
      baseImage = Images.VirtualMethod;
      break;
    case MemberIcon.Operator:
      baseImage = Images.Operator;
      break;
    case MemberIcon.ExtensionMethod:
      baseImage = Images.ExtensionMethod;
      break;
    case MemberIcon.PInvokeMethod:
      baseImage = Images.PInvokeMethod;
      break;
    case MemberIcon.Event:
      baseImage = Images.Event;
      break;
    default:
      throw new ArgumentOutOfRangeException(nameof(icon), 
                 $"MemberIcon.{icon} is not supported!");
  }

  return baseImage;
}

      
      



, . , icon MemberIcon.EnumValue, baseImage case Images.EnumValue. , , .





2





V3139 Two or more case-branches perform the same actions. ICSharpCode.Decompiler CSharpConversions.cs 829





bool ImplicitConstantExpressionConversion(ResolveResult rr, IType toType)
{
  ....
  switch (toTypeCode)
  {
    case TypeCode.SByte:
      return val >= SByte.MinValue && val <= SByte.MaxValue;
    case TypeCode.Byte:
      return val >= Byte.MinValue && val <= Byte.MaxValue;
    case TypeCode.Int16:
      return val >= Int16.MinValue && val <= Int16.MaxValue;
    case TypeCode.UInt16:
      return val >= UInt16.MinValue && val <= UInt16.MaxValue;
    case TypeCode.UInt32:
      return val >= 0;                 // <=
    case TypeCode.UInt64:
      return val >= 0;                 // <=
  }
  ....
}

      
      



, , . case TypeCode.UInt32 TypeCode.UInt64 , :





bool ImplicitConstantExpressionConversion(ResolveResult rr, IType toType)
{
  switch (toTypeCode)
  {
      ....
      case TypeCode.UInt32:
      case TypeCode.UInt64:
        return val >= 0;
  }
  ....
}

      
      



2 V3139:





  • V3139 Two or more case-branches perform the same actions. ICSharpCode.Decompiler EscapeInvalidIdentifiers.cs 85





  • V3139 Two or more case-branches perform the same actions. ICSharpCode.Decompiler TransformExpressionTrees.cs 370





V3083 Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. ILSpy MainWindow.xaml.cs 787class ResXResourceWriter : IDisposable





void assemblyList_Assemblies_CollectionChanged(....)
{
  ....
  if (CurrentAssemblyListChanged != null)
    CurrentAssemblyListChanged(this, e);      // <=
}

      
      



, , , . , , , , - , NullReferenceException. CurrentAssemblyListChanged null (, ), NullReferenceException. , , :





void assemblyList_Assemblies_CollectionChanged(....)
{
  ....
  CurrentAssemblyListChanged?.Invoke(this, e);
}

      
      



PVS-Studio 8 , .





V3146 Possible null dereference. The 'FirstOrDefault' can return default null value. ILSpy.BamlDecompiler BamlResourceEntryNode.cs 76





bool LoadBaml(AvalonEditTextOutput output, CancellationToken cancellationToken)
{
  var asm = this.Ancestors().OfType<assemblytreenode>()
                            .FirstOrDefault().LoadedAssembly;       // <=
  ....
  return true;
}

      
      



, OfType, FirstOrDefault AssemblyTreeNode. , , , , , , FirstOrDefault - null . LoadedAssembly NullReferenceException. , null- :





bool LoadBaml(AvalonEditTextOutput output, CancellationToken cancellationToken)
{
  var asm = this.Ancestors().OfType<assemblytreenode>()
                            .FirstOrDefault()?.LoadedAssembly;     // <=
  ....
  return true;
}

      
      



, , FirstOrDefault null. , First FirstOrDefault, , . , , InvalidOperationException ( NullReferenceException FirstOrDefault ) : "Sequence contains no elements".





V3105 The 'm' variable was used after it was assigned through null-conditional operator. NullReferenceException is possible. ILSpy MethodVirtualUsedByAnalyzer.cs 137





static bool ScanMethodBody(IMethod analyzedMethod, 
                           IMethod method, MethodBodyBlock methodBody)
{
  ....
  var mainModule = (MetadataModule)method.ParentModule;
  ....
  switch (member.Kind)
  {
    case HandleKind.MethodDefinition:
    case HandleKind.MethodSpecification:
    case HandleKind.MemberReference:
      var m = (mainModule.ResolveEntity(member, genericContext) as IMember)
              ?.MemberDefinition;
      if (   m.MetadataToken == analyzedMethod.MetadataToken               // <=
          && m.ParentModule.PEFile == analyzedMethod.ParentModule.PEFile)  // <=
      {
        return true;
      }
      break;
  }
  ....
}

      
      



m null- , , , m null. , null- m, NullReferenceException. null- :





static bool ScanMethodBody(IMethod analyzedMethod, 
                           IMethod method, MethodBodyBlock methodBody)
{
  ....
  var mainModule = (MetadataModule)method.ParentModule;
  ....
  switch (member.Kind)
  {
    case HandleKind.MethodDefinition:
    case HandleKind.MethodSpecification:
    case HandleKind.MemberReference:
      var m = (mainModule.ResolveEntity(member, genericContext) as IMember)
              ?.MemberDefinition;
      if (   m?.MetadataToken == analyzedMethod.MetadataToken
          && m?.ParentModule.PEFile == analyzedMethod.ParentModule.PEFile)
      {
        return true;
      }
      break;
  }
  ....
}

      
      



V3070 Uninitialized variable 'schema' is used when initializing the 'ResourceSchema' variable. ICSharpCode.Decompiler ResXResourceWriter.cs 63





class ResXResourceWriter : IDisposable
{
  ....
  public static readonly string ResourceSchema = schema;
  ....
  static string schema = ....;
  ....
}

      
      



. , Mono, , - . Mono, ResourceSchema schema, schema - null. ResXResourceWriter.cs, , Mono. ILSpy . . , .





, , , , , (, Replace ). . / , , , . .





, : Ilya Gainulin. A Spy Undercover: PVS-Studio to Check ILSpy Source Code.








All Articles