jbevain / cecil

Cecil is a library to inspect, modify and create .NET programs and libraries.
MIT License
2.77k stars 630 forks source link

Resolving generic type creates Scope System.Private.CoreLib resulting in MissingMethodExceptions #766

Closed lovettchris closed 3 years ago

lovettchris commented 3 years ago

The following generic type:

TypeReference genericType = module.ImportReference(typeof(System.Runtime.CompilerServices.TaskAwaiter<>))

(where module is a loaded Net5.0 assembly) The typeDef Scope is set to System.Private.CoreLib.dll which seems broken because when I try and call a method on this type I get MissingMethodException.

When I import a type that returns one of these generics the Scope on that return type is System.Runtime:

image

Interestingly the loaded assembly that contains this module has the following cached referenced assemblies, which do include System.Private.CoreLib even though ilspy does not show this dependency.

image

This is what I see in ilspy:

image

jbevain commented 3 years ago

Hi,

It's hard to say what's going wrong.

Could you extract a repro that we could investigate?

Thanks!

ltrzesniewski commented 3 years ago

Isn't this caused by the call to the ImportReference(Type) overload instead of ImportReference(TypeReference)?

jbevain commented 3 years ago

@ltrzesniewski that's likely the case, but if you execute the Cecil code and the patched code on the same framework (net50 here), I think you could expect the SR overloads to work.

lovettchris commented 3 years ago

Doesn't take much to repro, just load a .NET 5.0 assembly and do this:

  using (var assembly = AssemblyDefinition.ReadAssembly(assemblyPath))
  {
      var module = assembly.MainModule;
      TypeReference genericType = module.ImportReference(typeof(System.Runtime.CompilerServices.TaskAwaiter<>));
      Console.WriteLine(genericType.Scope.Name);
  }

and it prints "System.Private.CoreLib" which I believe is wrong.

jbevain commented 3 years ago

@lovettchris If I execute this (no Cecil in there) in .NET 5.0:

using System;
using System.Runtime.CompilerServices;

Console.WriteLine(typeof(TaskAwaiter<>).Assembly.GetName());

I get

System.Private.CoreLib, Version=5.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e

So maybe that's not the root cause of your issue here.

Note that TaskAwaiter<> is an open generic type, you're not going to be able to use it as is in Cecil without instantiating it.

lovettchris commented 3 years ago

Sorry, I should have provided a complete repro. See attached zip file. Build the solution and run it, and then run the command line "verify.cmd" which runs ilverify. You wills see the error:

[IL]: Error [MissingMethod]: [D:\Temp\CecilTest\CecilImportTest\bin\Debug\net5.0\RewriteCecilLibrary.dll : 
.<<TestUncontrolledTaskAwaiter>b__0_0>d::MoveNext()] Missing method 'TestRuntime.TaskAwaiter`1<Int32> TestRuntime.TaskAwaiter.Wrap(TestRuntime.TaskAwaiter`1<Int32>)'

What this code is trying to do is intercept "GetAwaiter()" calls by wrapping them in a wrapper object, TestRuntime.TaskAwaiter.

CecilTest.zip

PS: it would be lovely if some day I didn't have to write all that ImportGenericMethodInstance and ImportGenericTypeInstance code...

jbevain commented 3 years ago

Yeah I think there's something wrong in the way you create the reference to the generic method.

I've taken the one I use in Cecil's tests and adapted your code:

using System.Linq;
using Mono.Cecil;
using Mono.Cecil.Cil;
using System;
using System.IO;

namespace CecilImportTest
{
    class Program
    {
        static void Main(string[] args)
        {
            var assemblyPath = System.IO.Path.GetFullPath(@"..\..\..\..\CecilLibrary\bin\Debug\net5.0\CecilLibrary.dll");
            using (var assembly = AssemblyDefinition.ReadAssembly(assemblyPath))
            {
                foreach (var module in assembly.Modules)
                {
                    foreach (var type in module.GetTypes())
                    {
                        RewriteType(type);
                    }
                }

                var writeParams = new WriterParameters()
                {
                    SymbolWriterProvider = new PortablePdbWriterProvider()
                };

                var outputPath = System.IO.Path.GetFullPath("RewriteCecilLibrary.dll");
                assembly.Write(outputPath, writeParams);
            }
        }

        private static void RewriteType(TypeDefinition type)
        {
            foreach(var method in type.Methods)
            {
                RewriteMethod(method);
            }
        }

        private static void RewriteMethod(MethodDefinition method)
        {
            Instruction instruction = method.Body.Instructions.FirstOrDefault();
            while (instruction != null)
            {
                instruction = RewriteInstruction(method, instruction);
                instruction = instruction.Next;
            }
        }

        private static Instruction RewriteInstruction(MethodDefinition method, Instruction instruction)
        {
            if ((instruction.OpCode == OpCodes.Call || instruction.OpCode == OpCodes.Callvirt) && instruction.Operand is MethodReference methodReference)
            {
                if (methodReference.Name == "GetAwaiter")
                {
                    var module = methodReference.Module;
                    var declaringType = methodReference.DeclaringType;
                    if (declaringType is GenericInstanceType gt)
                    {
                        var processor = method.Body.GetILProcessor();

                        TypeDefinition providerType = module.ImportReference(typeof(TestRuntime.TaskAwaiter)).Resolve();
                        MethodDefinition genericMethod = providerType.Methods.FirstOrDefault(m => m.Name == "Wrap" && m.HasGenericParameters);

                        TypeReference argType = gt.GenericArguments.FirstOrDefault().GetElementType();

                        MethodReference wrapReference = module.ImportReference(genericMethod);
                        var wrapMethod = MakeGenericMethod(wrapReference, argType);

                        Instruction newInstruction = Instruction.Create(OpCodes.Call, wrapMethod);
                        processor.InsertAfter(instruction, newInstruction);
                        instruction = newInstruction;
                    }
                }
            }
            return instruction;
        }

        public static TypeReference MakeGenericType(TypeReference self, params TypeReference[] arguments)
        {
            if (self.GenericParameters.Count != arguments.Length)
                throw new ArgumentException();

            var instance = new GenericInstanceType(self);
            foreach (var argument in arguments)
                instance.GenericArguments.Add(argument);

            return instance;
        }

        public static MethodReference MakeGenericMethod(MethodReference self, params TypeReference[] arguments)
        {
            if (self.GenericParameters.Count != arguments.Length)
                throw new ArgumentException();

            var instance = new GenericInstanceMethod(self);
            foreach (var argument in arguments)
                instance.GenericArguments.Add(argument);

            return instance;
        }

        public static MethodReference MakeGeneric(MethodReference self, params TypeReference[] arguments)
        {
            var reference = new MethodReference(self.Name, self.ReturnType, MakeGenericType(self.DeclaringType, arguments))
            {
                HasThis = self.HasThis,
                ExplicitThis = self.ExplicitThis,
                ReturnType = self.ReturnType,
                CallingConvention = self.CallingConvention,
            };

            foreach (var parameter in self.Parameters)
                reference.Parameters.Add(new ParameterDefinition(parameter.ParameterType));

            foreach (var generic_parameter in self.GenericParameters)
                reference.GenericParameters.Add(new GenericParameter(generic_parameter.Name, reference));

            return reference;
        }

        public static FieldReference MakeGeneric(FieldReference self, params TypeReference[] arguments)
        {
            return new FieldReference(self.Name, self.FieldType, MakeGenericType(self.DeclaringType, arguments));
        }
    }
}

Now ilverify doesn't complain about a missing method. It does however complain because the instrumentation is not finished (the code is trying to assign the return value of Wrap to a TaskAwaiter and they're two completely different types.

lovettchris commented 3 years ago

Yes, I skipped a bunch of other rewrite logic for variables, etc. So wow your code is much simpler, thanks and all my tests are still passing. I seemed to have gone off into the weeds with bugs I was running into thinking I had to also "make" all the types referenced by the generic (method parameter types and return types and so on). But somehow your magic avoids all that and things still seem to work... thanks for taking the time to help me out!

jbevain commented 3 years ago

Cool, happy to hear this works for you!