From 26cbc99f9e9156a4ce720f3bdfcdc7b565af4bd3 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Mon, 15 Apr 2024 07:34:30 -0700 Subject: [PATCH] [NativeAOT] Fix Activator.CreateInstance for shared generic structs (#101021) The default constructor has to be invoked using fat pointer in shared generic structs. --- .../src/System/Activator.NativeAot.cs | 2 +- .../src/System/Array.NativeAot.cs | 4 +- .../src/System/Runtime/TypeLoaderExports.cs | 17 ++++++++- .../TypeLoader/GenericDictionaryCell.cs | 14 ++++++- .../Compiler/Compilation.cs | 2 +- .../DependencyAnalysis/GenericLookupResult.cs | 2 +- .../IL/ILImporter.Scanner.cs | 5 ++- .../System/ActivatorTests.Generic.cs | 38 +++++++++++++++++++ .../TestStructs/System.TestStructs.il | 12 ------ .../SmokeTests/DynamicGenerics/activation.cs | 14 +++++++ 10 files changed, 87 insertions(+), 23 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Activator.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Activator.NativeAot.cs index c15fd13580e1bb..a3b69bf6f52f69 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Activator.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Activator.NativeAot.cs @@ -63,7 +63,7 @@ public static partial class Activator else { t = default!; - RawCalliHelper.Call(defaultConstructor, ref Unsafe.As(ref t)); + RawCalliHelper.CallDefaultStructConstructor(defaultConstructor, ref Unsafe.As(ref t)); // Debugger goo so that stepping in works. Only affects debug info generation. // The call gets optimized away. diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Array.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Array.NativeAot.cs index 61f70e212483c1..bb44ad49d5f616 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Array.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Array.NativeAot.cs @@ -143,14 +143,14 @@ public unsafe void Initialize() if (constructorEntryPoint == IntPtr.Zero) return; - var constructorFtn = (delegate*)RuntimeAugments.TypeLoaderCallbacks.ConvertUnboxingFunctionPointerToUnderlyingNonUnboxingPointer(constructorEntryPoint, new RuntimeTypeHandle(pElementEEType)); + IntPtr constructorFtn = RuntimeAugments.TypeLoaderCallbacks.ConvertUnboxingFunctionPointerToUnderlyingNonUnboxingPointer(constructorEntryPoint, new RuntimeTypeHandle(pElementEEType)); ref byte arrayRef = ref MemoryMarshal.GetArrayDataReference(this); nuint elementSize = ElementSize; for (int i = 0; i < Length; i++) { - constructorFtn(ref arrayRef); + RawCalliHelper.CallDefaultStructConstructor(constructorFtn, ref arrayRef); arrayRef = ref Unsafe.Add(ref arrayRef, elementSize); } } diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/TypeLoaderExports.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/TypeLoaderExports.cs index 4388566b0cf0c0..82065e8c510fce 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/TypeLoaderExports.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/TypeLoaderExports.cs @@ -10,6 +10,7 @@ using Internal.Runtime; using Internal.Runtime.Augments; +using Internal.Runtime.CompilerServices; namespace System.Runtime { @@ -162,8 +163,20 @@ private static unsafe Value CacheMiss(IntPtr context, IntPtr signature, RuntimeO internal static unsafe class RawCalliHelper { [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Call(System.IntPtr pfn, ref byte data) - => ((delegate*)pfn)(ref data); + public static void CallDefaultStructConstructor(System.IntPtr pfn, ref byte data) + { + // Manually expand call of the instance method fat pointer. We cannot use a regular static C# function + // pointer call since it would not work for shared generic instance method. + if (FunctionPointerOps.IsGenericMethodPointer(pfn)) + { + GenericMethodDescriptor* gmd = FunctionPointerOps.ConvertToGenericDescriptor(pfn); + ((delegate*)gmd->MethodFunctionPointer)(ref data, gmd->InstantiationArgument); + } + else + { + ((delegate*)pfn)(ref data); + } + } [MethodImpl(MethodImplOptions.AggressiveInlining)] public static T Call(System.IntPtr pfn, IntPtr arg) diff --git a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/GenericDictionaryCell.cs b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/GenericDictionaryCell.cs index d973b0a02a70c6..7883e22a5e2332 100644 --- a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/GenericDictionaryCell.cs +++ b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/GenericDictionaryCell.cs @@ -337,9 +337,19 @@ internal override IntPtr Create(TypeBuilder builder) { IntPtr result = TypeLoaderEnvironment.TryGetDefaultConstructorForType(Type); - - if (result == IntPtr.Zero) + if (result != IntPtr.Zero) + { + if (Type.IsValueType) + { + result = TypeLoaderEnvironment.ConvertUnboxingFunctionPointerToUnderlyingNonUnboxingPointer(result, + builder.GetRuntimeTypeHandle(Type)); + } + } + else + { result = RuntimeAugments.GetFallbackDefaultConstructor(); + } + return result; } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs index 31c868a77f00b7..bf0ea7662fec0b 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs @@ -326,7 +326,7 @@ public ISymbolNode ComputeConstantLookup(ReadyToRunHelperId lookupKind, object t { var type = (TypeDesc)targetOfLookup; MethodDesc ctor = GetConstructorForCreateInstanceIntrinsic(type); - return NodeFactory.CanonicalEntrypoint(ctor); + return type.IsValueType ? NodeFactory.ExactCallableAddress(ctor) : NodeFactory.CanonicalEntrypoint(ctor); } case ReadyToRunHelperId.ObjectAllocator: { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs index 17f717d49bb5a4..5b37ad7e0acc26 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs @@ -781,7 +781,7 @@ public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultCo { TypeDesc instantiatedType = _type.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); MethodDesc defaultCtor = Compilation.GetConstructorForCreateInstanceIntrinsic(instantiatedType); - return factory.CanonicalEntrypoint(defaultCtor); + return instantiatedType.IsValueType ? factory.ExactCallableAddress(defaultCtor) : factory.CanonicalEntrypoint(defaultCtor); } public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs index 524261def1a05b..41869a9e2f18c9 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs @@ -360,8 +360,9 @@ private void ImportCall(ILOpcode opcode, int token) } else { - MethodDesc ctor = Compilation.GetConstructorForCreateInstanceIntrinsic(method.Instantiation[0]); - _dependencies.Add(_factory.CanonicalEntrypoint(ctor), reason); + TypeDesc type = method.Instantiation[0]; + MethodDesc ctor = Compilation.GetConstructorForCreateInstanceIntrinsic(type); + _dependencies.Add(type.IsValueType ? _factory.ExactCallableAddress(ctor) : _factory.CanonicalEntrypoint(ctor), reason); } return; diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ActivatorTests.Generic.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ActivatorTests.Generic.cs index 616cfd6b6d5d97..e6ac3da7f45900 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ActivatorTests.Generic.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ActivatorTests.Generic.cs @@ -61,6 +61,22 @@ public void CreateInstanceT_StructWithoutDefaultConstructor_InvokesConstructor() public void CreateInstanceT_StructWithDefaultConstructorThatThrows_ThrowsTargetInvocationException() => Assert.Throws(() => Activator.CreateInstance()); + [Fact] + public void CreateInstanceT_GenericTypes() + { + TestGenericClassWithDefaultConstructor(); + TestGenericClassWithDefaultConstructor(); + + TestGenericStructWithDefaultConstructor(); + TestGenericStructWithDefaultConstructor(); + + void TestGenericClassWithDefaultConstructor() + => Assert.Equal(typeof(T), Activator.CreateInstance>().TypeOfT); + + void TestGenericStructWithDefaultConstructor() + => Assert.Equal(typeof(T), Activator.CreateInstance>().TypeOfT); + } + private interface IInterface { } @@ -96,5 +112,27 @@ private class ClassWithDefaultConstructorThatThrows public ClassWithDefaultConstructorThatThrows() => throw new Exception(); } + + public class GenericClassWithDefaultConstructor + { + public GenericClassWithDefaultConstructor() => + TypeOfT = typeof(T); + + public Type TypeOfT { get; } + } + + public struct StructWithDefaultConstructorThatThrows + { + public StructWithDefaultConstructorThatThrows() => + throw new Exception(); + } + + public struct GenericStructWithDefaultConstructor + { + public GenericStructWithDefaultConstructor() => + TypeOfT = typeof(T); + + public Type TypeOfT { get; } + } } } diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/TestStructs/System.TestStructs.il b/src/libraries/System.Runtime/tests/System.Runtime.Tests/TestStructs/System.TestStructs.il index a7573c0116d651..2be57ece0cc327 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/TestStructs/System.TestStructs.il +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/TestStructs/System.TestStructs.il @@ -57,18 +57,6 @@ ret } } - - .class public sequential sealed StructWithDefaultConstructorThatThrows - extends [System.Runtime]System.ValueType - { - .size 1 - - .method public hidebysig specialname rtspecialname instance void .ctor () cil managed - { - newobj instance void [System.Runtime]System.Exception::.ctor() - throw - } - } } .class public sequential sealed '.GlobalStructStartingWithDot' diff --git a/src/tests/nativeaot/SmokeTests/DynamicGenerics/activation.cs b/src/tests/nativeaot/SmokeTests/DynamicGenerics/activation.cs index 2e358c53eb59b3..4e645560d6a0b8 100644 --- a/src/tests/nativeaot/SmokeTests/DynamicGenerics/activation.cs +++ b/src/tests/nativeaot/SmokeTests/DynamicGenerics/activation.cs @@ -131,6 +131,18 @@ public override string ToString() return memberVar; } } + struct StructToString + { + string memberVar; + public StructToString() + { + memberVar = typeof(T).Name; + } + public override string ToString() + { + return memberVar; + } + } class SomeUnrealtedType { string memberVar; @@ -195,5 +207,7 @@ public static void TestDefaultCtorInLazyGenerics() AllocViaGVMBase typeWithGVM = AllocViaGVMDerived.Alloc(); Assert.AreEqual("ToStringIsInteresting1", typeWithGVM.Alloc().ToString()); Assert.AreEqual("ToStringIsInteresting2", typeWithGVM.Alloc().ToString()); + Assert.AreEqual("ToStringIsInteresting1", typeWithGVM.Alloc>().ToString()); + Assert.AreEqual("ToStringIsInteresting2", typeWithGVM.Alloc>().ToString()); } }