Why don't CreateGenericInstMethodSig

[CreateAndInject]

dnlib/src/DotNet/Importer.cs

Lines 490 to 491 in 0b2dc95

	if (methodBase.ContainsGenericParameters)
	return method; // Declaring type is instantiated but method itself is not

This code is from #466 by @wwh1004
@wtfsck @wwh1004 Is it buggy? Why don't CreateGenericInstMethodSig here?

[ElektroKill]

This is not a bug. ContainsGenericParameters returns true if the method contains unspecified generic parameters. If these generic parameters are unspecified, it means that no instantiation signature is provided. Therefore we don’t need to create a GenericInstMethodSig.
https://learn.microsoft.com/en-us/dotnet/api/system.reflection.methodbase.containsgenericparameters?view=net-8.0

[CreateAndInject]

Do you mean we can use MethodDef/MemberRef rather than MethodSpec as operand of call instruction when the called method is generic method?
If that, show a code to see how compiler build such thing.

[wwh1004]

Why do you think the MethodBase existing in reflection must be used as the operand of call? For whatever reason, there is a MethodBase which declaring type that is instantiated but the method itself does not get instantiated. It also cannot be used as the call operand in reflection. Why import as MethodSpec in this case?

#466 has undergone very strict testing to ensure that objects existing in reflection can be mapped to dnlib objects with equivalent semantics. You can retrieve test cases in this PR. It covers all GAC assemblies and very complex generic nested cases that I constructed manually, which includes the situation you mentioned.

foreach (var method in moduleDef.Enumerate<MemberRef>(t => t.IsMethodRef).Cast<IMethod>().Concat(moduleDef.Enumerate<MethodSpec>()).Concat(moduleDef.Enumerate<MethodDef>()))

[CreateAndInject]

For whatever reason, there is a MethodBase which declaring type that is instantiated but the method itself does not get instantiated. It also cannot be used as the call operand in reflection.

@wwh1004 CreateInstance<T>() here IsGenericButNotGenericMethodDefinition, and ContainsGenericParameters, why did you say it cannot be used as operand?

class My<T>
{
    public static void Test()
    {
        Activator.CreateInstance<T>();
    }
}

[wwh1004]

This is a MethodSpec :-)

[CreateAndInject]

Yes, but your code import it as MethodDef/MemberRef

[wwh1004]

I can't get this operand in reflection if I don't provide closed types.

[CreateAndInject]

DynamicTest.zip

I made an example, your code import a generic method as MemberRef, old dnlib work fine:

[wwh1004]

It is not a valid dynamic method. If you invoke it, you will get "InvalidProgramException: Common Language Runtime detected an invalid program". In this case, it is inherently invalid so dnlib is not required to give a valid output.

[wwh1004]

You should check "FCIMPL5(ReflectMethodObject*, ModuleHandle::GetDynamicMethod, ReflectMethodObject *pMethodUNSAFE, ReflectModuleBaseObject *pModuleUNSAFE, StringObject *name, U1Array *sig, Object *resolver)" in coreclr's runtimehandles.cpp. It just copy method body from managed side to unmanaged side. You can even fill in any invalid bytes in DynamicILInfo and GetMethodDescriptor will not throw an exception until you actually compile the method.

[CreateAndInject]

It is not a valid dynamic method. If you invoke it, you will get "InvalidProgramException: Common Language Runtime detected an invalid program". In this case, it is inherently invalid so dnlib is not required to give a valid output.

Let's asssume we want to make an unpacker for a DynamicMethod obfusction, we don't have instantiated generic type, every method is like this, we can't support instantiated type in our unpacker :

[wwh1004]

I think this question is off topic. If you do not pass the required generic parameters, the dynamic method is incomplete and invalid, whcih cannot be compiled by the JIT compiler. Then dnlib does not need support it.

As my last comment said, you can populate any invalid code via DynamicILInfo. How does dnlib give correct import results when the code is invalid? Your example does not mean that the MethodSpec containing open generic arguments that can be represented in System.Reflection api.

[CreateAndInject]

Don't agree, dnlib.DotNet.Emit.DynamicMethodBodyReader is used to decrypt DynamicMethod's code, rather than execute a DynamicMethod, your code will cause DynamicMethodBodyReader no longer work.

[wwh1004]

Then your needs are not related to the original topic. Why does dnlib create a MethodSpec when there are no generic method arguments?

[CreateAndInject]

Many protectors based on DynamicMethod will encrypt all methods like this, so how can we make unpacker for those? Do you think we must support correct generic types for all methods?
dnlib is a reverse lib, and DynamicMethodBodyReader is powerful and designed for decryption porpose rather than emulator, it work fine many years, but your code let it lose efficacy, so what's the meaning of DynamicMethodBodyReader now in dnlib?

class My1<T, U> where T : new()
{
    public static void Test()
    {
        new DynamicMethod("", typeof(void), Type.EmptyTypes).Invoke(null, null);
    }
}

class My2<T, U> where T : Stream, ICloneable
{
    public static void Test()
    {
        new DynamicMethod("", typeof(void), Type.EmptyTypes).Invoke(null, null);
    }
}

class My3<T, U> where T : My1<int, T>
{
    public static void Test()
    {
        new DynamicMethod("", typeof(void), Type.EmptyTypes).Invoke(null, null);
    }
}

[wwh1004]

I still don't know why does dnlib should create a MethodSpec when there are no generic method arguments.

Assume that GTYPE is a type containing gp and GMETHOD is a method containing gp in GTYPE.
typeof(GTYPE) is imported as TypeRef.
typeof(GTYPE).MakeGenericType(typeof(int)) is imported as TypeSpec.
typeof(GTYPE).MakeGenericType(typeof(int)).GetMethod("GMETHOD") is imported as MemberRef.
typeof(GTYPE).MakeGenericType(typeof(int)).GetMethod("GMETHOD").MakeGenericMethod(typeof(int)) is imported as MethodSpec.
This is exactly the right mapping.

[CreateAndInject]

Since DynamicMethodBodyReader is designed as Reader, it contains a RestoreMethod rather than ExecuteMethod. dnlib is reverse lib, we use it to restore IL instructions rather than execute any code.

[CreateAndInject]

By the way, we can't use instantiated type for decryption purpose, otherwise DynamicMethodBodyReader will generate instantiated type also, eg:

class My<T>
{
    public static void Test()
    {
        Activator.CreateInstance<int>(); // DynamicMethodHelper.ConvertFrom(typeof(My<int>).GetMethod("Test"))
    }
}

We should and must use uninstantiated type.

[CreateAndInject]

typeof(GTYPE).MakeGenericType(typeof(int)).GetMethod("GMETHOD") is imported as MemberRef.

@wwh1004 As you said, the imported result here can't be an operand, so how can we use the result? Where can this imported result appeared?

[wwh1004]

I think our point of disagreement is whether the behavior of the existence of open generic parameters for instantiated generic
method in .NET reflection is valid. It seems to me that this is not supposed to happen in .NET reflection, and your example would be due to a quirk or bug in .NET reflection that allows a method have unassigned generic parameters but not be a generic definition. I don't remember the purpose of two lines of code in the PR you mentioned, it was too long ago. I don't have the time to do more tests and investigate on why this quirk can happen. If it has no side effects, remove those two lines of code.

[CreateAndInject]

I still don't know why does dnlib should create a MethodSpec when there are no generic method arguments.

Assume that GTYPE is a type containing gp and GMETHOD is a method containing gp in GTYPE. typeof(GTYPE) is imported as TypeRef. typeof(GTYPE).MakeGenericType(typeof(int)) is imported as TypeSpec. typeof(GTYPE).MakeGenericType(typeof(int)).GetMethod("GMETHOD") is imported as MemberRef. typeof(GTYPE).MakeGenericType(typeof(int)).GetMethod("GMETHOD").MakeGenericMethod(typeof(int)) is imported as MethodSpec. This is exactly the right mapping.

var m = typeof(GTYPE).MakeGenericType(typeof(int)).GetMethod("GMETHOD");
Why do you think Import(m) => MemberRef is exactly the right mapping, and Import(m) => MethodSpec is the wrong mapping?
I want to know where can we use such imported MemberRef. seems such MemberRef is always useless in dnlib.

[CreateAndInject]

I viewed AsmResolver, it uses MethodSpec in this case also as old dnlib.

[CreateAndInject]

If it has no side effects, remove those two lines of code.

@wwh1004 This issue isn't the only issue, seems these 2 PR #452 #466 have serious bugs.

Another example:
Change code in DynamicTest.zip as following, your code will generate the operand of call Do() as MethodDef, but it should be a MemberRef whose DeclaringType is TypeSpec with GenericInstSig.

class My<T>
{
    public static void Test()
    {
        Do();
    }

    public static void Do()
    {
    }
}

I don't know why do you change ImportDeclaringType twice and mark it as obsolete, old dnlib work fine in this case.

[wwh1004]

I don't think the lack of support for importing an invalid method body is a serious bug. You can send a PR to fix it instead arguing whether this is a bug or not.

[CreateAndInject]

A System.Type: My<T>
B TypeRef: My`1
C TypeSpec: My<T>
GetHashCode(A) == GetHashCode(B) != GetHashCode(C)

So:
var methodInfo = typeof(My<>).GetMethod("Test");
var methodTypeSpec = Import(methodInfo);
GetHashCode(methodInfo) != GetHashCode(methodTypeSpec)

Solution:

1. Don't care the different HashCode
2. Let GetHashCode(A) == GetHashCode(B) == GetHashCode(C)
3. Let GetHashCode(methodInfo) == GetHashCode(methodTypeSpec) but keep GetHashCode(A) == GetHashCode(B) != GetHashCode(C)

@wtfsck How do you think?