From 822af4ad8b7020034363e02d5ceb6790651b8bdc Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Tue, 18 Mar 2025 14:31:27 -0500 Subject: [PATCH 1/7] Support HasThis calling convention in PersistedAssemblyBuilder --- .../src/System.Reflection.Emit.csproj | 1 + .../Reflection/Emit/ModuleBuilderImpl.cs | 11 ++- .../Emit/SignatureCallingConventionEx.cs | 28 ++++++++ .../AssemblySaveAssemblyBuilderTests.cs | 68 +++++++++++++++++++ .../tests/System.Reflection.Emit.Tests.csproj | 1 + 5 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureCallingConventionEx.cs diff --git a/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj b/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj index 57b328a0bbde26..b26efbe3edc6dd 100644 --- a/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj +++ b/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj @@ -28,6 +28,7 @@ + diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs index 02663bfa182d8d..ca04883f19723f 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs @@ -802,14 +802,19 @@ private static bool IsInstance(CallingConventions callingConvention) => internal static SignatureCallingConvention GetSignatureConvention(CallingConventions callingConvention) { - SignatureCallingConvention convention = SignatureCallingConvention.Default; + SignatureCallingConventionEx convention = (SignatureCallingConventionEx)SignatureCallingConvention.Default; if ((callingConvention & CallingConventions.VarArgs) != 0) { - convention = SignatureCallingConvention.VarArgs; + convention = (SignatureCallingConventionEx)SignatureCallingConvention.VarArgs; } - return convention; + if ((callingConvention & CallingConventions.HasThis) != 0) + { + convention |= SignatureCallingConventionEx.HasThis; + } + + return (SignatureCallingConvention)convention; } private MemberInfo GetOriginalMemberIfConstructedType(MemberInfo memberInfo) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureCallingConventionEx.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureCallingConventionEx.cs new file mode 100644 index 00000000000000..a017c92bbeeec4 --- /dev/null +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureCallingConventionEx.cs @@ -0,0 +1,28 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Reflection.Metadata; + +namespace System.Reflection.Emit +{ + /// + /// Extensions to the public System.Reflection.Metadata.SignatureCallingConvention enum. + /// + [Flags] + internal enum SignatureCallingConventionEx : byte + { + /// + /// Indicates the presence of a "this" parameter. + /// + HasThis = 0x20, + + // Other values based on ECMA-335: + // 0x0A: GenericInst (generic method instantiation) + // 0x0B: NativeVarArg + // 0x0C: Max (first invalid calling convention) + // 0x0F: Mask for SignatureCallingConvention values + // 0x10: Generic (generic method signature with explicit number of type arguments) + // 0x40: ExplicitThis ("this" parameter is explicitly in the signature) + } +} diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs index 45f926b1a67bd7..432cabfef0e707 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs @@ -303,6 +303,74 @@ public void AssemblyWithDifferentTypes() } } + [Fact] + public unsafe void AssemblyWithInstanceBasedFunctionPointer() + { + byte[] assemblyData = GenerateMethodInPersistedAssembly(); + using MemoryStream stream = new MemoryStream(assemblyData); + TestAssemblyLoadContext testAssemblyLoadContext = new(); + + // Verify the assembly is properly generated and the runtime supports the IL. + try + { + Assembly assembly = testAssemblyLoadContext.LoadFromStream(stream); + Type generatedType = assembly.GetType("MyGeneratedType")!; + Assert.NotNull(generatedType); + MethodInfo generatedMethod = generatedType.GetMethod("GetGuid")!; + Assert.NotNull(generatedMethod); + Func generatedMethodToCall = generatedMethod.CreateDelegate>(); + + // Call the property getter through the generated method. + IntPtr fn = typeof(MyClassWithGuidProperty).GetProperty(nameof(MyClassWithGuidProperty.MyGuid))!.GetGetMethod().MethodHandle.GetFunctionPointer(); + Guid guid = Guid.NewGuid(); + MyClassWithGuidProperty obj = new (guid); + Assert.Equal(guid, generatedMethodToCall(obj, fn)); + } + finally + { + testAssemblyLoadContext.Unload(); + } + + static unsafe byte[] GenerateMethodInPersistedAssembly() + { + AssemblyName assemblyName = new("MyAssembly"); + PersistedAssemblyBuilder ab = new(assemblyName, typeof(object).Assembly); + ModuleBuilder moduleBuilder = ab.DefineDynamicModule("MyModule"); + + // Define a type with a method that calls the instance-based function pointer through calli. + TypeBuilder typeBuilder = moduleBuilder.DefineType("MyGeneratedType", TypeAttributes.Public | TypeAttributes.Class); + MethodBuilder methodBuilder = typeBuilder.DefineMethod( + "GetGuid", + MethodAttributes.Public | MethodAttributes.Static, + typeof(Guid), + new Type[] { typeof(object), typeof(IntPtr) }); + + ILGenerator il = methodBuilder.GetILGenerator(); + il.Emit(OpCodes.Ldarg_0); // this + il.Emit(OpCodes.Ldarg_1); // fn + il.EmitCalli(OpCodes.Calli, CallingConventions.HasThis, typeof(Guid), null, null); + il.Emit(OpCodes.Ret); + + typeBuilder.CreateType(); + + MetadataBuilder metadataBuilder = ab.GenerateMetadata(out BlobBuilder ilStream, out BlobBuilder _); + ManagedPEBuilder peBuilder = new ManagedPEBuilder(PEHeaderBuilder.CreateLibraryHeader(), new MetadataRootBuilder(metadataBuilder), ilStream); + BlobBuilder blob = new BlobBuilder(); + peBuilder.Serialize(blob); + return blob.ToArray(); + } + } + + private class MyClassWithGuidProperty + { + public MyClassWithGuidProperty(Guid guid) + { + MyGuid = guid; + } + + public Guid MyGuid { get; init; } + } + void CheckCattr(IList attributes) { CustomAttributeData cattr = attributes.First(a => a.AttributeType.Name == nameof(AttributeUsageAttribute)); diff --git a/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj b/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj index 598d0516beb0f4..0552a0ae8f6473 100644 --- a/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj +++ b/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj @@ -4,6 +4,7 @@ $(NetCoreAppCurrent) true true + true From 972d0ac91c1739765b268c4963a61f4d1409f2ac Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Tue, 18 Mar 2025 17:18:02 -0500 Subject: [PATCH 2/7] Add support for ExplicitThis --- .../src/System.Reflection.Emit.csproj | 1 - .../Reflection/Emit/ModuleBuilderImpl.cs | 17 ++++++----- .../Emit/SignatureCallingConventionEx.cs | 28 ------------------- 3 files changed, 8 insertions(+), 38 deletions(-) delete mode 100644 src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureCallingConventionEx.cs diff --git a/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj b/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj index b26efbe3edc6dd..57b328a0bbde26 100644 --- a/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj +++ b/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj @@ -28,7 +28,6 @@ - diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs index ca04883f19723f..26d43dbaa1083e 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs @@ -800,21 +800,20 @@ private BlobBuilder GetMethodArrayMethodSignature(ArrayMethod method) => Metadat private static bool IsInstance(CallingConventions callingConvention) => callingConvention.HasFlag(CallingConventions.HasThis) || callingConvention.HasFlag(CallingConventions.ExplicitThis) ? true : false; - internal static SignatureCallingConvention GetSignatureConvention(CallingConventions callingConvention) + internal static SignatureCallingConvention GetSignatureConvention(CallingConventions callingConventions) { - SignatureCallingConventionEx convention = (SignatureCallingConventionEx)SignatureCallingConvention.Default; + SignatureCallingConvention convention = SignatureCallingConvention.Default; - if ((callingConvention & CallingConventions.VarArgs) != 0) + if ((callingConventions & CallingConventions.VarArgs) != 0) { - convention = (SignatureCallingConventionEx)SignatureCallingConvention.VarArgs; + convention = SignatureCallingConvention.VarArgs; } - if ((callingConvention & CallingConventions.HasThis) != 0) - { - convention |= SignatureCallingConventionEx.HasThis; - } + // CallingConventions.HasThis (0x20) and ExplicitThis (0x40) can use a bitwise OR with SignatureCallingConvention. + const byte Mask = (byte)(CallingConventions.HasThis | CallingConventions.ExplicitThis); + convention = (SignatureCallingConvention)((byte)convention | (unchecked((byte)callingConventions) & Mask)); - return (SignatureCallingConvention)convention; + return convention; } private MemberInfo GetOriginalMemberIfConstructedType(MemberInfo memberInfo) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureCallingConventionEx.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureCallingConventionEx.cs deleted file mode 100644 index a017c92bbeeec4..00000000000000 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureCallingConventionEx.cs +++ /dev/null @@ -1,28 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Reflection.Metadata; - -namespace System.Reflection.Emit -{ - /// - /// Extensions to the public System.Reflection.Metadata.SignatureCallingConvention enum. - /// - [Flags] - internal enum SignatureCallingConventionEx : byte - { - /// - /// Indicates the presence of a "this" parameter. - /// - HasThis = 0x20, - - // Other values based on ECMA-335: - // 0x0A: GenericInst (generic method instantiation) - // 0x0B: NativeVarArg - // 0x0C: Max (first invalid calling convention) - // 0x0F: Mask for SignatureCallingConvention values - // 0x10: Generic (generic method signature with explicit number of type arguments) - // 0x40: ExplicitThis ("this" parameter is explicitly in the signature) - } -} From 8f231b0719812b104357cda66fdf57fe1217a27f Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Thu, 20 Mar 2025 16:32:41 -0500 Subject: [PATCH 3/7] Initial attempt to add ExplicitThis support and tests --- .../Reflection/Emit/DynamicILGenerator.cs | 10 +++- .../Reflection/Emit/RuntimeILGenerator.cs | 10 +++- .../System/Reflection/Emit/SignatureHelper.cs | 12 +++-- .../tests/DynamicILInfoTests.cs | 54 ++++++++++++++++++- .../System/Reflection/Emit/ILGeneratorImpl.cs | 4 +- .../AssemblySaveAssemblyBuilderTests.cs | 29 +++++++--- 6 files changed, 101 insertions(+), 18 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicILGenerator.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicILGenerator.cs index 4c02d9abbb8056..ee3f6a3e46fb05 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicILGenerator.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicILGenerator.cs @@ -193,17 +193,23 @@ public override void EmitCalli(OpCode opcode, // If there is a non-void return type, push one. if (returnType != typeof(void)) stackchange++; + // Pop off arguments if any. if (parameterTypes != null) stackchange -= parameterTypes.Length; + // Pop off vararg arguments. if (optionalParameterTypes != null) stackchange -= optionalParameterTypes.Length; - // Pop the this parameter if the method has a this parameter. - if ((callingConvention & CallingConventions.HasThis) == CallingConventions.HasThis) + + // Pop the this parameter if the method has an implicit this parameter. + if ((callingConvention & CallingConventions.HasThis) == CallingConventions.HasThis && + (callingConvention & CallingConventions.ExplicitThis) == 0) stackchange--; + // Pop the native function pointer. stackchange--; + UpdateStackSize(OpCodes.Calli, stackchange); int token = GetTokenForSig(sig.GetSignature(true)); diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs index 8976805080d145..b2f0814ca64db3 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs @@ -536,17 +536,23 @@ public override void EmitCalli(OpCode opcode, CallingConventions callingConventi // If there is a non-void return type, push one. if (returnType != typeof(void)) stackchange++; + // Pop off arguments if any. if (parameterTypes != null) stackchange -= parameterTypes.Length; + // Pop off vararg arguments. if (optionalParameterTypes != null) stackchange -= optionalParameterTypes.Length; - // Pop the this parameter if the method has a this parameter. - if ((callingConvention & CallingConventions.HasThis) == CallingConventions.HasThis) + + // Pop the this parameter if the method has an implicit this parameter. + if ((callingConvention & CallingConventions.HasThis) == CallingConventions.HasThis && + (callingConvention & CallingConventions.ExplicitThis) == 0) stackchange--; + // Pop the native function pointer. stackchange--; + UpdateStackSize(OpCodes.Calli, stackchange); RecordTokenFixup(); diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/SignatureHelper.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/SignatureHelper.cs index c5bb88cf52503d..a117d29a3857c3 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/SignatureHelper.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/SignatureHelper.cs @@ -64,11 +64,12 @@ internal static SignatureHelper GetMethodSigHelper( intCall |= MdSigCallingConvention.Generic; } - if ((callingConvention & CallingConventions.HasThis) == CallingConventions.HasThis) - intCall |= MdSigCallingConvention.HasThis; + const byte Mask = (byte)(CallingConventions.HasThis | CallingConventions.ExplicitThis); + intCall = (MdSigCallingConvention)((byte)intCall | (unchecked((byte)callingConvention) & Mask)); sigHelp = new SignatureHelper(scope, intCall, cGenericParam, returnType, - requiredReturnTypeCustomModifiers, optionalReturnTypeCustomModifiers); + requiredReturnTypeCustomModifiers, optionalReturnTypeCustomModifiers); + sigHelp.AddArguments(parameterTypes, requiredParameterTypeCustomModifiers, optionalParameterTypeCustomModifiers); return sigHelp; @@ -151,11 +152,12 @@ public static SignatureHelper GetPropertySigHelper(Module? mod, CallingConventio MdSigCallingConvention intCall = MdSigCallingConvention.Property; - if ((callingConvention & CallingConventions.HasThis) == CallingConventions.HasThis) - intCall |= MdSigCallingConvention.HasThis; + const byte Mask = (byte)(CallingConventions.HasThis | CallingConventions.ExplicitThis); + intCall = (MdSigCallingConvention)((byte)intCall | (unchecked((byte)callingConvention) & Mask)); sigHelp = new SignatureHelper(mod, intCall, returnType, requiredReturnTypeCustomModifiers, optionalReturnTypeCustomModifiers); + sigHelp.AddArguments(parameterTypes, requiredParameterTypeCustomModifiers, optionalParameterTypeCustomModifiers); return sigHelp; diff --git a/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs b/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs index 4ad96dac7ed091..6e444db55a1932 100644 --- a/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs +++ b/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; -using System.Runtime.InteropServices; using Xunit; namespace System.Reflection.Emit.Tests @@ -695,6 +694,59 @@ public void GetDynamicILInfo_NotSameNotNull(bool skipVisibility) Assert.Equal(method, dynamicILInfo.DynamicMethod); } + [Theory] + [InlineData(true)] // todo: this is failing + [InlineData(false)] + public unsafe void InstanceBasedFunctionPointer(bool useExplicitThis) + { + Func generatedMethodToCall = GenerateDynamicMethod(useExplicitThis); + + // Call the property getter through the dynamic method. + IntPtr fn = typeof(MyClassWithGuidProperty).GetProperty(nameof(MyClassWithGuidProperty.MyGuid))!.GetGetMethod().MethodHandle.GetFunctionPointer(); + Guid guid = Guid.NewGuid(); + MyClassWithGuidProperty obj = new(guid); + Assert.Equal(guid, generatedMethodToCall(obj, fn)); + + static Func GenerateDynamicMethod(bool useExplicitThis) + { + DynamicMethod dynamicMethod = new DynamicMethod( + "GetGuid", + returnType: typeof(Guid), + parameterTypes: [typeof(MyClassWithGuidProperty), typeof(IntPtr)], + typeof(object).Module, + skipVisibility: false); + + ILGenerator il = dynamicMethod.GetILGenerator(); + il.Emit(OpCodes.Ldarg_0); // this + il.Emit(OpCodes.Ldarg_1); // fn + + if (useExplicitThis) + { + il.EmitCalli(OpCodes.Calli, CallingConventions.HasThis | CallingConventions.ExplicitThis, + returnType: typeof(Guid), parameterTypes: [typeof(MyClassWithGuidProperty)], null); + } + else + { + il.EmitCalli(OpCodes.Calli, CallingConventions.HasThis, + returnType: typeof(Guid), parameterTypes: null, null); + } + + il.Emit(OpCodes.Ret); + + return (Func)dynamicMethod.CreateDelegate(typeof(Func)); + } + } + + private class MyClassWithGuidProperty + { + public MyClassWithGuidProperty(Guid guid) + { + MyGuid = guid; + } + + public Guid MyGuid { get; init; } + } + private DynamicMethod GetDynamicMethod(bool skipVisibility) { return new DynamicMethod(nameof(DynamicMethod), typeof(void), diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs index 82416e72599516..d8f3445eb2434f 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs @@ -699,8 +699,10 @@ public override void EmitCalli(OpCode opcode, CallingConventions callingConventi { stackChange -= optionalParameterTypes.Length; } + // Pop the this parameter if the method has a this parameter. - if ((callingConvention & CallingConventions.HasThis) == CallingConventions.HasThis) + if ((callingConvention & CallingConventions.HasThis) == CallingConventions.HasThis && + (callingConvention & CallingConventions.ExplicitThis) == 0) { stackChange--; } diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs index 432cabfef0e707..8e7b05c08a814f 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs @@ -303,10 +303,13 @@ public void AssemblyWithDifferentTypes() } } - [Fact] - public unsafe void AssemblyWithInstanceBasedFunctionPointer() + [Theory] + [InlineData(true)] // todo: this is failing + [InlineData(false)] + public unsafe void AssemblyWithInstanceBasedFunctionPointer(bool useExplicitThis) { - byte[] assemblyData = GenerateMethodInPersistedAssembly(); + byte[] assemblyData = GenerateMethodInPersistedAssembly(useExplicitThis); + using MemoryStream stream = new MemoryStream(assemblyData); TestAssemblyLoadContext testAssemblyLoadContext = new(); @@ -318,7 +321,7 @@ public unsafe void AssemblyWithInstanceBasedFunctionPointer() Assert.NotNull(generatedType); MethodInfo generatedMethod = generatedType.GetMethod("GetGuid")!; Assert.NotNull(generatedMethod); - Func generatedMethodToCall = generatedMethod.CreateDelegate>(); + Func generatedMethodToCall = generatedMethod.CreateDelegate>(); // Call the property getter through the generated method. IntPtr fn = typeof(MyClassWithGuidProperty).GetProperty(nameof(MyClassWithGuidProperty.MyGuid))!.GetGetMethod().MethodHandle.GetFunctionPointer(); @@ -331,7 +334,7 @@ public unsafe void AssemblyWithInstanceBasedFunctionPointer() testAssemblyLoadContext.Unload(); } - static unsafe byte[] GenerateMethodInPersistedAssembly() + static unsafe byte[] GenerateMethodInPersistedAssembly(bool useExplicitThis) { AssemblyName assemblyName = new("MyAssembly"); PersistedAssemblyBuilder ab = new(assemblyName, typeof(object).Assembly); @@ -343,12 +346,24 @@ static unsafe byte[] GenerateMethodInPersistedAssembly() "GetGuid", MethodAttributes.Public | MethodAttributes.Static, typeof(Guid), - new Type[] { typeof(object), typeof(IntPtr) }); + [typeof(MyClassWithGuidProperty), typeof(IntPtr)]); + ILGenerator il = methodBuilder.GetILGenerator(); il.Emit(OpCodes.Ldarg_0); // this il.Emit(OpCodes.Ldarg_1); // fn - il.EmitCalli(OpCodes.Calli, CallingConventions.HasThis, typeof(Guid), null, null); + + if (useExplicitThis) + { + il.EmitCalli(OpCodes.Calli, CallingConventions.HasThis | CallingConventions.ExplicitThis, + returnType: typeof(Guid), parameterTypes: [typeof(MyClassWithGuidProperty)], null); + } + else + { + il.EmitCalli(OpCodes.Calli, CallingConventions.HasThis, + returnType: typeof(Guid), parameterTypes: null, null); + } + il.Emit(OpCodes.Ret); typeBuilder.CreateType(); From 1331ef5460fea5f87ce171b2bb8f3602d07421c1 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 21 Mar 2025 13:10:37 -0500 Subject: [PATCH 4/7] Fix issues with ExplicitThis --- src/coreclr/jit/importercalls.cpp | 5 +++++ .../tests/DynamicILInfoTests.cs | 4 ++-- .../AssemblySaveAssemblyBuilderTests.cs | 7 ++----- .../tests/TypeBuilder/TypeBuilderDefineConstructor.cs | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 23ceed3cde357d..f9aef1cd0064cd 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -6256,6 +6256,11 @@ void Compiler::impPopCallArgs(CORINFO_SIG_INFO* sig, GenTreeCall* call) else { arg = NewCallArg::Primitive(argNode, jitSigType); + + if (i == 1 && (sig->callConv & CORINFO_CALLCONV_EXPLICITTHIS)) + { + arg = arg.WellKnown(WellKnownArg::ThisPointer); + } } call->gtArgs.PushFront(this, arg); diff --git a/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs b/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs index 6e444db55a1932..76bd3fce65895d 100644 --- a/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs +++ b/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs @@ -695,7 +695,7 @@ public void GetDynamicILInfo_NotSameNotNull(bool skipVisibility) } [Theory] - [InlineData(true)] // todo: this is failing + [InlineData(true)] [InlineData(false)] public unsafe void InstanceBasedFunctionPointer(bool useExplicitThis) { @@ -733,7 +733,7 @@ static Func GenerateDynamicMethod(bool us il.Emit(OpCodes.Ret); - return (Func)dynamicMethod.CreateDelegate(typeof(Func)); + return dynamicMethod.CreateDelegate>(); } } diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs index 8e7b05c08a814f..7532987229a45a 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs @@ -304,7 +304,7 @@ public void AssemblyWithDifferentTypes() } [Theory] - [InlineData(true)] // todo: this is failing + [InlineData(true)] [InlineData(false)] public unsafe void AssemblyWithInstanceBasedFunctionPointer(bool useExplicitThis) { @@ -321,7 +321,7 @@ public unsafe void AssemblyWithInstanceBasedFunctionPointer(bool useExplicitThis Assert.NotNull(generatedType); MethodInfo generatedMethod = generatedType.GetMethod("GetGuid")!; Assert.NotNull(generatedMethod); - Func generatedMethodToCall = generatedMethod.CreateDelegate>(); + var generatedMethodToCall = generatedMethod.CreateDelegate>(); // Call the property getter through the generated method. IntPtr fn = typeof(MyClassWithGuidProperty).GetProperty(nameof(MyClassWithGuidProperty.MyGuid))!.GetGetMethod().MethodHandle.GetFunctionPointer(); @@ -348,7 +348,6 @@ static unsafe byte[] GenerateMethodInPersistedAssembly(bool useExplicitThis) typeof(Guid), [typeof(MyClassWithGuidProperty), typeof(IntPtr)]); - ILGenerator il = methodBuilder.GetILGenerator(); il.Emit(OpCodes.Ldarg_0); // this il.Emit(OpCodes.Ldarg_1); // fn @@ -365,9 +364,7 @@ static unsafe byte[] GenerateMethodInPersistedAssembly(bool useExplicitThis) } il.Emit(OpCodes.Ret); - typeBuilder.CreateType(); - MetadataBuilder metadataBuilder = ab.GenerateMetadata(out BlobBuilder ilStream, out BlobBuilder _); ManagedPEBuilder peBuilder = new ManagedPEBuilder(PEHeaderBuilder.CreateLibraryHeader(), new MetadataRootBuilder(metadataBuilder), ilStream); BlobBuilder blob = new BlobBuilder(); diff --git a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs index e58047416b3c75..f7b8489cee5109 100644 --- a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs +++ b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs @@ -27,7 +27,7 @@ public static IEnumerable TestData() // Ignores any CallingConventions, sets to CallingConventions.Standard yield return new object[] { MethodAttributes.Public, new Type[0], CallingConventions.Any }; - yield return new object[] { MethodAttributes.Public, new Type[0], CallingConventions.ExplicitThis }; + yield return new object[] { MethodAttributes.Public, new Type[0], CallingConventions.ExplicitThis | CallingConventions.HasThis }; yield return new object[] { MethodAttributes.Public, new Type[0], CallingConventions.HasThis }; yield return new object[] { MethodAttributes.Public, new Type[0], CallingConventions.Standard }; yield return new object[] { MethodAttributes.Public, new Type[0], CallingConventions.VarArgs }; From cb2f4577c0ce4d86e989920aac83f45626bcb8d2 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 21 Mar 2025 17:49:22 -0500 Subject: [PATCH 5/7] Use 'object' for type in tests; update ctor test to not support ExplicitThis --- .../tests/DynamicILInfoTests.cs | 10 +++++----- .../AssemblySaveAssemblyBuilderTests.cs | 6 +++--- .../TypeBuilderDefineConstructor.cs | 18 +++++++++++++++++- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs b/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs index 76bd3fce65895d..075d8a1eb82c5b 100644 --- a/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs +++ b/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs @@ -699,7 +699,7 @@ public void GetDynamicILInfo_NotSameNotNull(bool skipVisibility) [InlineData(false)] public unsafe void InstanceBasedFunctionPointer(bool useExplicitThis) { - Func generatedMethodToCall = GenerateDynamicMethod(useExplicitThis); + Func generatedMethodToCall = GenerateDynamicMethod(useExplicitThis); // Call the property getter through the dynamic method. IntPtr fn = typeof(MyClassWithGuidProperty).GetProperty(nameof(MyClassWithGuidProperty.MyGuid))!.GetGetMethod().MethodHandle.GetFunctionPointer(); @@ -707,12 +707,12 @@ public unsafe void InstanceBasedFunctionPointer(bool useExplicitThis) MyClassWithGuidProperty obj = new(guid); Assert.Equal(guid, generatedMethodToCall(obj, fn)); - static Func GenerateDynamicMethod(bool useExplicitThis) + static Func GenerateDynamicMethod(bool useExplicitThis) { DynamicMethod dynamicMethod = new DynamicMethod( "GetGuid", returnType: typeof(Guid), - parameterTypes: [typeof(MyClassWithGuidProperty), typeof(IntPtr)], + parameterTypes: [typeof(object), typeof(IntPtr)], typeof(object).Module, skipVisibility: false); @@ -723,7 +723,7 @@ static Func GenerateDynamicMethod(bool us if (useExplicitThis) { il.EmitCalli(OpCodes.Calli, CallingConventions.HasThis | CallingConventions.ExplicitThis, - returnType: typeof(Guid), parameterTypes: [typeof(MyClassWithGuidProperty)], null); + returnType: typeof(Guid), parameterTypes: [typeof(object)], null); } else { @@ -733,7 +733,7 @@ static Func GenerateDynamicMethod(bool us il.Emit(OpCodes.Ret); - return dynamicMethod.CreateDelegate>(); + return dynamicMethod.CreateDelegate>(); } } diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs index 7532987229a45a..9a689413197985 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs @@ -321,7 +321,7 @@ public unsafe void AssemblyWithInstanceBasedFunctionPointer(bool useExplicitThis Assert.NotNull(generatedType); MethodInfo generatedMethod = generatedType.GetMethod("GetGuid")!; Assert.NotNull(generatedMethod); - var generatedMethodToCall = generatedMethod.CreateDelegate>(); + var generatedMethodToCall = generatedMethod.CreateDelegate>(); // Call the property getter through the generated method. IntPtr fn = typeof(MyClassWithGuidProperty).GetProperty(nameof(MyClassWithGuidProperty.MyGuid))!.GetGetMethod().MethodHandle.GetFunctionPointer(); @@ -346,7 +346,7 @@ static unsafe byte[] GenerateMethodInPersistedAssembly(bool useExplicitThis) "GetGuid", MethodAttributes.Public | MethodAttributes.Static, typeof(Guid), - [typeof(MyClassWithGuidProperty), typeof(IntPtr)]); + [typeof(object), typeof(IntPtr)]); ILGenerator il = methodBuilder.GetILGenerator(); il.Emit(OpCodes.Ldarg_0); // this @@ -355,7 +355,7 @@ static unsafe byte[] GenerateMethodInPersistedAssembly(bool useExplicitThis) if (useExplicitThis) { il.EmitCalli(OpCodes.Calli, CallingConventions.HasThis | CallingConventions.ExplicitThis, - returnType: typeof(Guid), parameterTypes: [typeof(MyClassWithGuidProperty)], null); + returnType: typeof(Guid), parameterTypes: [typeof(object)], null); } else { diff --git a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs index f7b8489cee5109..2a02193aea863e 100644 --- a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs +++ b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs @@ -27,7 +27,6 @@ public static IEnumerable TestData() // Ignores any CallingConventions, sets to CallingConventions.Standard yield return new object[] { MethodAttributes.Public, new Type[0], CallingConventions.Any }; - yield return new object[] { MethodAttributes.Public, new Type[0], CallingConventions.ExplicitThis | CallingConventions.HasThis }; yield return new object[] { MethodAttributes.Public, new Type[0], CallingConventions.HasThis }; yield return new object[] { MethodAttributes.Public, new Type[0], CallingConventions.Standard }; yield return new object[] { MethodAttributes.Public, new Type[0], CallingConventions.VarArgs }; @@ -137,6 +136,23 @@ public void DefineConstructor_HasThisCallingConventionsForStaticMethod_ThrowsTyp Assert.Throws(() => type.CreateTypeInfo()); } + [Fact] + public void DefineConstructor_ExplicitThisCallingConventionsForInstanceMethod_ThrowsTypeLoadExceptionOnCreation() + { + TypeBuilder type = Helpers.DynamicType(TypeAttributes.Public); + + ConstructorBuilder constructor = type.DefineConstructor( + MethodAttributes.Public, + CallingConventions.HasThis | CallingConventions.ExplicitThis, + new Type[0]); + + constructor.GetILGenerator().Emit(OpCodes.Ret); + + // Dynamic types do not support ExplicitThis for constructors; a constructor however can be called + // as a method with the first parameter being the uninitialized object. + Assert.Throws(() => type.CreateTypeInfo()); + } + [Fact] public void GetConstructor_TypeNotCreated_ThrowsNotSupportedException() { From 8d36d3ce06c4e7328cd9ca9c9f222e22969f8c4e Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Sat, 22 Mar 2025 10:10:01 -0500 Subject: [PATCH 6/7] Add [ActiveIssue] for Mono; add some test comments --- .../tests/DynamicILInfoTests.cs | 5 +++++ .../AssemblySaveAssemblyBuilderTests.cs | 8 ++++++-- .../tests/TypeBuilder/TypeBuilderDefineConstructor.cs | 3 +-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs b/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs index 075d8a1eb82c5b..ee20b3ee5a802c 100644 --- a/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs +++ b/src/libraries/System.Reflection.Emit.Lightweight/tests/DynamicILInfoTests.cs @@ -695,6 +695,7 @@ public void GetDynamicILInfo_NotSameNotNull(bool skipVisibility) } [Theory] + [ActiveIssue("https://github.com/dotnet/runtime/issues/113789", TestRuntimes.Mono)] [InlineData(true)] [InlineData(false)] public unsafe void InstanceBasedFunctionPointer(bool useExplicitThis) @@ -712,7 +713,11 @@ static Func GenerateDynamicMethod(bool useExplicitThis) DynamicMethod dynamicMethod = new DynamicMethod( "GetGuid", returnType: typeof(Guid), + + // In this test, we use typeof(object) for the "this" pointer to ensure the IL could be re-used for other + // reference types, but normally this would be the appropriate type such as typeof(MyClassWithGuidProperty). parameterTypes: [typeof(object), typeof(IntPtr)], + typeof(object).Module, skipVisibility: false); diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs index 9a689413197985..256039267a529c 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveAssemblyBuilderTests.cs @@ -304,6 +304,7 @@ public void AssemblyWithDifferentTypes() } [Theory] + [ActiveIssue("https://github.com/dotnet/runtime/issues/113789", TestRuntimes.Mono)] [InlineData(true)] [InlineData(false)] public unsafe void AssemblyWithInstanceBasedFunctionPointer(bool useExplicitThis) @@ -345,8 +346,11 @@ static unsafe byte[] GenerateMethodInPersistedAssembly(bool useExplicitThis) MethodBuilder methodBuilder = typeBuilder.DefineMethod( "GetGuid", MethodAttributes.Public | MethodAttributes.Static, - typeof(Guid), - [typeof(object), typeof(IntPtr)]); + returnType: typeof(Guid), + + // In this test, we use typeof(object) for the "this" pointer to ensure the IL could be re-used for other + // reference types, but normally this would be the appropriate type such as typeof(MyClassWithGuidProperty). + parameterTypes: [typeof(object), typeof(IntPtr)]); ILGenerator il = methodBuilder.GetILGenerator(); il.Emit(OpCodes.Ldarg_0); // this diff --git a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs index 2a02193aea863e..5adeb7f44fb3fb 100644 --- a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs +++ b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs @@ -148,8 +148,7 @@ public void DefineConstructor_ExplicitThisCallingConventionsForInstanceMethod_Th constructor.GetILGenerator().Emit(OpCodes.Ret); - // Dynamic types do not support ExplicitThis for constructors; a constructor however can be called - // as a method with the first parameter being the uninitialized object. + // ExplicitThis is not valid on definitions; it is only used when calling. Assert.Throws(() => type.CreateTypeInfo()); } From 2a78d2864b8d9e9b5d73b0d45c35507632ed046a Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Mon, 24 Mar 2025 09:10:35 -0500 Subject: [PATCH 7/7] Add [ActiveIssue] for another Mono test --- .../tests/TypeBuilder/TypeBuilderDefineConstructor.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs index 5adeb7f44fb3fb..37ddbb9bd649d5 100644 --- a/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs +++ b/src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderDefineConstructor.cs @@ -137,6 +137,7 @@ public void DefineConstructor_HasThisCallingConventionsForStaticMethod_ThrowsTyp } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/113839", TestRuntimes.Mono)] public void DefineConstructor_ExplicitThisCallingConventionsForInstanceMethod_ThrowsTypeLoadExceptionOnCreation() { TypeBuilder type = Helpers.DynamicType(TypeAttributes.Public);