From f10af6beed0e9b0daf8887f2e92505af8c19f655 Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 17 Jan 2024 16:52:20 +0900 Subject: [PATCH 01/44] Add generic support for calli --- src/coreclr/vm/ceeload.cpp | 12 ++++-------- src/coreclr/vm/ceeload.h | 5 ++++- src/coreclr/vm/dllimport.cpp | 2 ++ src/coreclr/vm/dllimport.h | 7 +++++++ src/coreclr/vm/ilstubcache.cpp | 5 +++++ src/coreclr/vm/ilstubcache.h | 1 + src/coreclr/vm/jitinterface.cpp | 9 +++++++-- 7 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index 7b06b3d30aa2f..a795bf1956196 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4696,7 +4696,7 @@ PTR_VOID ReflectionModule::GetRvaField(RVA field) // virtual //========================================================================== // Enregisters a VASig. //========================================================================== -VASigCookie *Module::GetVASigCookie(Signature vaSignature) +VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* typeContext) { CONTRACT(VASigCookie*) { @@ -4734,13 +4734,7 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature) // Compute the size of args first, outside of the lock. - // @TODO GENERICS: We may be calling a varargs method from a - // generic type/method. Using an empty context will make such a - // case cause an unexpected exception. To make this work, - // we need to create a specialized signature for every instantiation - SigTypeContext typeContext; - - MetaSig metasig(vaSignature, this, &typeContext); + MetaSig metasig(vaSignature, this, typeContext); ArgIterator argit(&metasig); // Upper estimate of the vararg size @@ -4778,6 +4772,8 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature) pCookie->pNDirectILStub = NULL; pCookie->sizeOfArgs = sizeOfArgs; pCookie->signature = vaSignature; + pCookie->methInst = typeContext->m_methodInst.GetRawArgs(); + pCookie->methInstCount = typeContext->m_methodInst.GetNumArgs(); // Finally, now that it's safe for asynchronous readers to see it, // update the count. diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index aba0567c042c1..63f341cbdbc55 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -339,6 +339,9 @@ struct VASigCookie Volatile pNDirectILStub; // will be use if target is NDirect (tag == 0) PTR_Module pModule; Signature signature; + // classInst should always be NULL so we only need methInst here + TypeHandle* methInst; + DWORD methInstCount; }; // @@ -1361,7 +1364,7 @@ class Module : public ModuleBase void NotifyEtwLoadFinished(HRESULT hr); // Enregisters a VASig. - VASigCookie *GetVASigCookie(Signature vaSignature); + VASigCookie *GetVASigCookie(Signature vaSignature, const SigTypeContext* typeContext); public: #ifndef DACCESS_COMPILE diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 5dbea392d6b2a..6d58be251d885 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -4186,6 +4186,7 @@ namespace pHashParams, pParams->m_dwStubFlags, pParams->m_pModule, + pParams->m_pTypeContext, pParams->m_sig.GetRawSig(), pParams->m_sig.GetRawSigLen(), pamTracker, @@ -6059,6 +6060,7 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD) } StubSigDesc sigDesc(pMD, signature, pVASigCookie->pModule); + sigDesc.InitTypeContext(pVASigCookie->methInst, pVASigCookie->methInstCount); MethodDesc* pStubMD = NDirect::CreateCLRToNativeILStub(&sigDesc, nlType, diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h index 256b950799336..95bb03d353306 100644 --- a/src/coreclr/vm/dllimport.h +++ b/src/coreclr/vm/dllimport.h @@ -55,6 +55,13 @@ struct StubSigDesc m_pDebugClassName = NULL; } } + + void InitTypeContext(TypeHandle* methInst, DWORD methInstCount) + { + LIMITED_METHOD_CONTRACT; + + m_typeContext = SigTypeContext(Instantiation(), Instantiation(methInst, methInstCount)); + } #endif // _DEBUG }; diff --git a/src/coreclr/vm/ilstubcache.cpp b/src/coreclr/vm/ilstubcache.cpp index d0f55495c8290..8c9f6a70a1956 100644 --- a/src/coreclr/vm/ilstubcache.cpp +++ b/src/coreclr/vm/ilstubcache.cpp @@ -500,6 +500,7 @@ MethodDesc* ILStubCache::GetStubMethodDesc( ILStubHashBlob* pHashBlob, DWORD dwStubFlags, Module* pSigModule, + SigTypeContext* pTypeContext, PCCOR_SIGNATURE pSig, DWORD cbSig, AllocMemTracker* pamTracker, @@ -552,6 +553,10 @@ MethodDesc* ILStubCache::GetStubMethodDesc( { SigTypeContext::InitTypeContext(pTargetMD, &typeContext); } + else if (pTypeContext != NULL) + { + typeContext = *pTypeContext; + } pMD = ILStubCache::CreateNewMethodDesc(m_pAllocator->GetHighFrequencyHeap(), pStubMT, dwStubFlags, pSigModule, pSig, cbSig, &typeContext, pamTracker); diff --git a/src/coreclr/vm/ilstubcache.h b/src/coreclr/vm/ilstubcache.h index 6324bad28eebf..6df4c98aff3de 100644 --- a/src/coreclr/vm/ilstubcache.h +++ b/src/coreclr/vm/ilstubcache.h @@ -53,6 +53,7 @@ class ILStubCache final ILStubHashBlob* pHashBlob, DWORD dwStubFlags, // bitmask of NDirectStubFlags Module* pSigModule, + SigTypeContext* pTypeContext, PCCOR_SIGNATURE pSig, DWORD cbSig, AllocMemTracker* pamTracker, diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index ca1dd982e1cbb..0de867d5c6a44 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -6306,7 +6306,11 @@ CORINFO_VARARGS_HANDLE CEEInfo::getVarArgsHandle(CORINFO_SIG_INFO *sig, Module* module = GetModule(sig->scope); - result = CORINFO_VARARGS_HANDLE(module->GetVASigCookie(Signature(sig->pSig, sig->cbSig))); + Instantiation classInst = Instantiation((TypeHandle*) sig->sigInst.classInst, sig->sigInst.classInstCount); + Instantiation methodInst = Instantiation((TypeHandle*) sig->sigInst.methInst, sig->sigInst.methInstCount); + SigTypeContext typeContext = SigTypeContext(classInst, methodInst); + + result = CORINFO_VARARGS_HANDLE(module->GetVASigCookie(Signature(sig->pSig, sig->cbSig), &typeContext)); EE_TO_JIT_TRANSITION(); @@ -13425,7 +13429,8 @@ BOOL LoadDynamicInfoEntry(Module *currentModule, } { VarArgs: - result = (size_t) CORINFO_VARARGS_HANDLE(currentModule->GetVASigCookie(Signature(pSig, cSig))); + SigTypeContext typeContext = SigTypeContext(); + result = (size_t) CORINFO_VARARGS_HANDLE(currentModule->GetVASigCookie(Signature(pSig, cSig), &typeContext)); } break; From 345ef56ff87b38bc7dc115d92d64b4d5970e273b Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 17 Jan 2024 17:08:57 +0900 Subject: [PATCH 02/44] Consider classInst as well --- src/coreclr/vm/ceeload.cpp | 2 ++ src/coreclr/vm/ceeload.h | 3 ++- src/coreclr/vm/dllimport.cpp | 2 +- src/coreclr/vm/dllimport.h | 4 ++-- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index a795bf1956196..5fa6fbc0ff42b 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4772,6 +4772,8 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* pCookie->pNDirectILStub = NULL; pCookie->sizeOfArgs = sizeOfArgs; pCookie->signature = vaSignature; + pCookie->classInst = typeContext->m_classInst.GetRawArgs(); + pCookie->classInstCount = typeContext->m_classInst.GetNumArgs(); pCookie->methInst = typeContext->m_methodInst.GetRawArgs(); pCookie->methInstCount = typeContext->m_methodInst.GetNumArgs(); diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index 63f341cbdbc55..bbee2e279bdb2 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -339,7 +339,8 @@ struct VASigCookie Volatile pNDirectILStub; // will be use if target is NDirect (tag == 0) PTR_Module pModule; Signature signature; - // classInst should always be NULL so we only need methInst here + TypeHandle* classInst; + DWORD classInstCount; TypeHandle* methInst; DWORD methInstCount; }; diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 6d58be251d885..41847f45e14cc 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -6060,7 +6060,7 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD) } StubSigDesc sigDesc(pMD, signature, pVASigCookie->pModule); - sigDesc.InitTypeContext(pVASigCookie->methInst, pVASigCookie->methInstCount); + sigDesc.InitTypeContext(pVASigCookie->classInst, pVASigCookie->classInstCount, pVASigCookie->methInst, pVASigCookie->methInstCount); MethodDesc* pStubMD = NDirect::CreateCLRToNativeILStub(&sigDesc, nlType, diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h index 95bb03d353306..60e58d848081d 100644 --- a/src/coreclr/vm/dllimport.h +++ b/src/coreclr/vm/dllimport.h @@ -56,11 +56,11 @@ struct StubSigDesc } } - void InitTypeContext(TypeHandle* methInst, DWORD methInstCount) + void InitTypeContext(TypeHandle* classInst, DWORD classInstCount, TypeHandle* methInst, DWORD methInstCount) { LIMITED_METHOD_CONTRACT; - m_typeContext = SigTypeContext(Instantiation(), Instantiation(methInst, methInstCount)); + m_typeContext = SigTypeContext(Instantiation(classInst, classInstCount), Instantiation(methInst, methInstCount)); } #endif // _DEBUG }; From 42debae4cdb62c06cf5f3aca46f1f2176c0168f6 Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 17 Jan 2024 17:38:02 +0900 Subject: [PATCH 03/44] oops --- src/coreclr/vm/dllimport.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h index 60e58d848081d..da57306e862b9 100644 --- a/src/coreclr/vm/dllimport.h +++ b/src/coreclr/vm/dllimport.h @@ -55,6 +55,7 @@ struct StubSigDesc m_pDebugClassName = NULL; } } +#endif // _DEBUG void InitTypeContext(TypeHandle* classInst, DWORD classInstCount, TypeHandle* methInst, DWORD methInstCount) { @@ -62,7 +63,6 @@ struct StubSigDesc m_typeContext = SigTypeContext(Instantiation(classInst, classInstCount), Instantiation(methInst, methInstCount)); } -#endif // _DEBUG }; //======================================================================= From ab8839b03753dd92f07571e1e0268c63e013fd0f Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 17 Jan 2024 17:41:31 +0900 Subject: [PATCH 04/44] guarded by !DACCESS_COMPILE --- src/coreclr/vm/dllimport.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h index da57306e862b9..d28b8d210570b 100644 --- a/src/coreclr/vm/dllimport.h +++ b/src/coreclr/vm/dllimport.h @@ -56,13 +56,15 @@ struct StubSigDesc } } #endif // _DEBUG - + +#ifndef DACCESS_COMPILE void InitTypeContext(TypeHandle* classInst, DWORD classInstCount, TypeHandle* methInst, DWORD methInstCount) { LIMITED_METHOD_CONTRACT; m_typeContext = SigTypeContext(Instantiation(classInst, classInstCount), Instantiation(methInst, methInstCount)); } +#endif }; //======================================================================= From 0327254b22038b3f68c39c486cd4540615589d31 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jan 2024 20:57:47 +0900 Subject: [PATCH 05/44] Block runtime marshaling for calli --- src/coreclr/vm/dllimport.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 41847f45e14cc..865e32342b19b 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -3672,6 +3672,11 @@ static void CreateNDirectStubWorker(StubState* pss, COMPlusThrow(kMarshalDirectiveException, IDS_EE_NDIRECT_DISABLEDMARSHAL_PRESERVESIG); } + if (runtimeMarshallingEnabled && SF_IsCALLIStub(dwStubFlags) && NDirect::MarshalingRequired(pMD)) + { + COMPlusThrow(kMarshalDirectiveException, IDS_EE_NDIRECT_UNSUPPORTED_SIG); + } + int numArgs = msig.NumFixedArgs(); // thiscall must have at least one parameter (the "this") From 21c62027c1d6ab72fe628b9bd0ba5bcab9a95ce9 Mon Sep 17 00:00:00 2001 From: Steven He Date: Mon, 22 Jan 2024 11:06:38 +0900 Subject: [PATCH 06/44] Fix the error mode --- src/coreclr/vm/dllimport.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 865e32342b19b..f4c56d5baa69d 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -3672,11 +3672,6 @@ static void CreateNDirectStubWorker(StubState* pss, COMPlusThrow(kMarshalDirectiveException, IDS_EE_NDIRECT_DISABLEDMARSHAL_PRESERVESIG); } - if (runtimeMarshallingEnabled && SF_IsCALLIStub(dwStubFlags) && NDirect::MarshalingRequired(pMD)) - { - COMPlusThrow(kMarshalDirectiveException, IDS_EE_NDIRECT_UNSUPPORTED_SIG); - } - int numArgs = msig.NumFixedArgs(); // thiscall must have at least one parameter (the "this") @@ -5040,6 +5035,12 @@ namespace } else { + // For generic calli, we only support blittable types + if (!pSigDesc->m_typeContext.IsEmpty() && SF_IsCALLIStub(dwStubFlags) && NDirect::MarshalingRequired(pStubMD)) + { + COMPlusThrow(kMarshalDirectiveException, IDS_EE_NDIRECT_UNSUPPORTED_SIG); + } + CreateNDirectStubWorker(pss, pSigDesc, nlType, From a10142c6dadef2038af7e5f5501b155e35d751c5 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 23 Jan 2024 22:51:47 +0900 Subject: [PATCH 07/44] Adding tests --- .../FunctionPointer/FunctionPointer.cs | 50 +++++++++++++++++++ .../FunctionPointer/FunctionPointerNative.cpp | 5 ++ 2 files changed, 55 insertions(+) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index 766a37efd00cf..34a4b45632cae 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -1,6 +1,7 @@ // 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.Runtime.CompilerServices; using System.Runtime.InteropServices; using Xunit; @@ -111,4 +112,53 @@ public static void RunGetDelForOutIntTest() } Assert.Equal(expectedValue, outVar); } + + [UnmanagedCallersOnly(CallConvs = [typeof(CallConvCdecl)])] + static int UnmanagedExportedFunction(float arg) + { + return Convert.ToInt32(arg); + } + + class GenericCaller + { + internal static unsafe T GenericCalli(void* fnptr, U arg) + { + return ((delegate* unmanaged)fnptr)(arg); + } + } + + [Fact] + public static void RunGenericFunctionPointerTest() + { + Console.WriteLine($"Running {nameof(RunGenericFunctionPointerTest)}..."); + int outVar = 0; + int expectedValue = 42; + unsafe + { + outVar = GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])&UnmanagedExportedFunction, 42.0f); + } + Assert.Equal(expectedValue, outVar); + } + + [Fact] + public static void RunInvalidGenericFunctionPointerTest() + { + Console.WriteLine($"Running {nameof(RunInvalidGenericFunctionPointerTest)}..."); + unsafe + { + nint lib = nint.Zero; + try + { + lib = NativeLibrary.Load(nameof(FunctionPointerNative)); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])NativeLibrary.GetExport(lib, "ReturnParameter"), "test")); + } + finally + { + if (lib != nint.Zero) + { + NativeLibrary.Free(lib); + } + } + } + } } diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.cpp b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.cpp index e19d5f1182ad3..0dc14139fa80a 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.cpp +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.cpp @@ -41,3 +41,8 @@ extern "C" DLL_EXPORT void FillOutIntParameter(intptr_t *p) *p = 50; } +extern "C" DLL_EXPORT intptr_t* ReturnParameter(intptr_t *p) +{ + return p; +} + From 6f7eb0e2752c0588f503c92d4ac15385fbb82c5f Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 23 Jan 2024 23:22:35 +0900 Subject: [PATCH 08/44] Minor fixes to tests --- src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index 34a4b45632cae..ac53f85203b5a 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -123,7 +123,7 @@ class GenericCaller { internal static unsafe T GenericCalli(void* fnptr, U arg) { - return ((delegate* unmanaged)fnptr)(arg); + return ((delegate* unmanaged[Cdecl])fnptr)(arg); } } From 68d5a09f23fbb53c1346782d3f938f92cd6310c9 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 23 Jan 2024 23:23:10 +0900 Subject: [PATCH 09/44] Compare instantiations as well when lookup --- src/coreclr/vm/ceeload.cpp | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index 5fa6fbc0ff42b..23c31e3867351 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4722,8 +4722,37 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* { if (pBlock->m_cookies[i].signature.GetRawSig() == vaSignature.GetRawSig()) { - pCookie = &(pBlock->m_cookies[i]); - break; + _ASSERTE(pBlock->m_cookies[i].classInstCount == typeContext->m_classInst.GetNumArgs()); + _ASSERTE(pBlock->m_cookies[i].methInstCount == typeContext->m_methodInst.GetNumArgs()); + + BOOL instMatch = TRUE; + + for (UINT j = 0; j < pBlock->m_cookies[i].classInstCount; j++) + { + if (pBlock->m_cookies[i].classInst[j] != typeContext->m_classInst[j]) + { + instMatch = FALSE; + break; + } + } + + if (instMatch) + { + for (UINT j = 0; j < pBlock->m_cookies[i].methInstCount; j++) + { + if (pBlock->m_cookies[i].methInst[j] != typeContext->m_methodInst[j]) + { + instMatch = FALSE; + break; + } + } + } + + if (instMatch) + { + pCookie = &(pBlock->m_cookies[i]); + break; + } } } } From 2ea3dac321f0f548acdd84909715d8d92b8fbb22 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 23 Jan 2024 23:43:15 +0900 Subject: [PATCH 10/44] Adding more test cases --- .../FunctionPointer/FunctionPointer.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index ac53f85203b5a..514bececc39af 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -125,6 +125,16 @@ internal static unsafe T GenericCalli(void* fnptr, U arg) { return ((delegate* unmanaged[Cdecl])fnptr)(arg); } + + internal static unsafe BlittableGeneric WrappedGenericCalli(void* fnptr, U arg) + { + return ((delegate* unmanaged[Cdecl]>)fnptr)(arg); + } + } + + struct BlittableGeneric + { + public int X; } [Fact] @@ -133,11 +143,26 @@ public static void RunGenericFunctionPointerTest() Console.WriteLine($"Running {nameof(RunGenericFunctionPointerTest)}..."); int outVar = 0; int expectedValue = 42; + unsafe { outVar = GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])&UnmanagedExportedFunction, 42.0f); } Assert.Equal(expectedValue, outVar); + + outVar = 0; + unsafe + { + outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged[Cdecl])&UnmanagedExportedFunction, 42.0f).X; + } + Assert.Equal(expectedValue, outVar); + + outVar = 0; + unsafe + { + outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged[Cdecl])&UnmanagedExportedFunction, 42.0f).X; + } + Assert.Equal(expectedValue, outVar); } [Fact] @@ -150,6 +175,8 @@ public static void RunInvalidGenericFunctionPointerTest() try { lib = NativeLibrary.Load(nameof(FunctionPointerNative)); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])NativeLibrary.GetExport(lib, "ReturnParameter"), "test")); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])NativeLibrary.GetExport(lib, "ReturnParameter"), "test")); Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])NativeLibrary.GetExport(lib, "ReturnParameter"), "test")); } finally From 5ecc8888a0caa41508f7b4ab5c30765e0fc99dd6 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 23 Jan 2024 23:45:10 +0900 Subject: [PATCH 11/44] Avoid getting export multiple times --- .../Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index 514bececc39af..d8dd00a61d3b0 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -175,9 +175,10 @@ public static void RunInvalidGenericFunctionPointerTest() try { lib = NativeLibrary.Load(nameof(FunctionPointerNative)); - Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])NativeLibrary.GetExport(lib, "ReturnParameter"), "test")); - Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])NativeLibrary.GetExport(lib, "ReturnParameter"), "test")); - Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])NativeLibrary.GetExport(lib, "ReturnParameter"), "test")); + nint fnptr = NativeLibrary.GetExport(lib, "ReturnParameter"); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])fnptr, "test")); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])fnptr, "test")); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])fnptr, "test")); } finally { From 21d9862e276caed2195db5b4cd57d50d5bd833ff Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 24 Jan 2024 14:56:49 +0900 Subject: [PATCH 12/44] Improve tests --- .../FunctionPointer/FunctionPointer.cs | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index d8dd00a61d3b0..fb33f2c52d2bb 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -113,7 +113,7 @@ public static void RunGetDelForOutIntTest() Assert.Equal(expectedValue, outVar); } - [UnmanagedCallersOnly(CallConvs = [typeof(CallConvCdecl)])] + [UnmanagedCallersOnly] static int UnmanagedExportedFunction(float arg) { return Convert.ToInt32(arg); @@ -123,12 +123,12 @@ class GenericCaller { internal static unsafe T GenericCalli(void* fnptr, U arg) { - return ((delegate* unmanaged[Cdecl])fnptr)(arg); + return ((delegate* unmanaged)fnptr)(arg); } internal static unsafe BlittableGeneric WrappedGenericCalli(void* fnptr, U arg) { - return ((delegate* unmanaged[Cdecl]>)fnptr)(arg); + return ((delegate* unmanaged>)fnptr)(arg); } } @@ -144,23 +144,26 @@ public static void RunGenericFunctionPointerTest() int outVar = 0; int expectedValue = 42; + Console.WriteLine("Testing GenericCalli with int return type"); unsafe { - outVar = GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])&UnmanagedExportedFunction, 42.0f); + outVar = GenericCaller.GenericCalli((delegate* unmanaged)&UnmanagedExportedFunction, 42.0f); } Assert.Equal(expectedValue, outVar); outVar = 0; + Console.WriteLine("Testing GenericCalli with BlittableGeneric return type"); unsafe { - outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged[Cdecl])&UnmanagedExportedFunction, 42.0f).X; + outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged)&UnmanagedExportedFunction, 42.0f).X; } Assert.Equal(expectedValue, outVar); outVar = 0; + Console.WriteLine("Testing GenericCalli with BlittableGeneric return type"); unsafe { - outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged[Cdecl])&UnmanagedExportedFunction, 42.0f).X; + outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged)&UnmanagedExportedFunction, 42.0f).X; } Assert.Equal(expectedValue, outVar); } @@ -176,9 +179,12 @@ public static void RunInvalidGenericFunctionPointerTest() { lib = NativeLibrary.Load(nameof(FunctionPointerNative)); nint fnptr = NativeLibrary.GetExport(lib, "ReturnParameter"); - Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])fnptr, "test")); - Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])fnptr, "test")); - Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged[Cdecl])fnptr, "test")); + Console.WriteLine("Testing GenericCalli with string as parameter type"); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); + Console.WriteLine("Testing GenericCalli with string as return type"); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); + Console.WriteLine("Testing GenericCalli with string as both parameter and return type"); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); } finally { From 8939e320728fc6f20cd6052c11b3d14fa727e0c5 Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 24 Jan 2024 14:56:59 +0900 Subject: [PATCH 13/44] Use standard boolean --- src/coreclr/vm/ceeload.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index 23c31e3867351..d22b8d677348e 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4725,13 +4725,13 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* _ASSERTE(pBlock->m_cookies[i].classInstCount == typeContext->m_classInst.GetNumArgs()); _ASSERTE(pBlock->m_cookies[i].methInstCount == typeContext->m_methodInst.GetNumArgs()); - BOOL instMatch = TRUE; + bool instMatch = true; for (UINT j = 0; j < pBlock->m_cookies[i].classInstCount; j++) { if (pBlock->m_cookies[i].classInst[j] != typeContext->m_classInst[j]) { - instMatch = FALSE; + instMatch = false; break; } } @@ -4742,7 +4742,7 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* { if (pBlock->m_cookies[i].methInst[j] != typeContext->m_methodInst[j]) { - instMatch = FALSE; + instMatch = false; break; } } From ab165f155d26611f05c1f9ddbfe654ad8d06d395 Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 24 Jan 2024 15:06:34 +0900 Subject: [PATCH 14/44] Avoid casting to prevent ABI mismatch --- .../FunctionPointer/FunctionPointer.cs | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index fb33f2c52d2bb..14582fdb61ce9 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -118,6 +118,18 @@ static int UnmanagedExportedFunction(float arg) { return Convert.ToInt32(arg); } + + [UnmanagedCallersOnly] + static BlittableGeneric UnmanagedExportedFunctionBlittableGenericInt(float arg) + { + return new() { X = Convert.ToInt32(arg) }; + } + + [UnmanagedCallersOnly] + static BlittableGeneric UnmanagedExportedFunctionBlittableGenericString(float arg) + { + return new() { X = Convert.ToInt32(arg) }; + } class GenericCaller { @@ -144,7 +156,7 @@ public static void RunGenericFunctionPointerTest() int outVar = 0; int expectedValue = 42; - Console.WriteLine("Testing GenericCalli with int return type"); + Console.WriteLine("Testing GenericCalli with int as the return type"); unsafe { outVar = GenericCaller.GenericCalli((delegate* unmanaged)&UnmanagedExportedFunction, 42.0f); @@ -152,18 +164,18 @@ public static void RunGenericFunctionPointerTest() Assert.Equal(expectedValue, outVar); outVar = 0; - Console.WriteLine("Testing GenericCalli with BlittableGeneric return type"); + Console.WriteLine("Testing GenericCalli with BlittableGeneric as the return type"); unsafe { - outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged)&UnmanagedExportedFunction, 42.0f).X; + outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged>)&UnmanagedExportedFunctionBlittableGenericInt, 42.0f).X; } Assert.Equal(expectedValue, outVar); outVar = 0; - Console.WriteLine("Testing GenericCalli with BlittableGeneric return type"); + Console.WriteLine("Testing GenericCalli with BlittableGeneric as the return type"); unsafe { - outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged)&UnmanagedExportedFunction, 42.0f).X; + outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged>)&UnmanagedExportedFunctionBlittableGenericString, 42.0f).X; } Assert.Equal(expectedValue, outVar); } @@ -179,11 +191,11 @@ public static void RunInvalidGenericFunctionPointerTest() { lib = NativeLibrary.Load(nameof(FunctionPointerNative)); nint fnptr = NativeLibrary.GetExport(lib, "ReturnParameter"); - Console.WriteLine("Testing GenericCalli with string as parameter type"); + Console.WriteLine("Testing GenericCalli with string as the parameter type"); Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); - Console.WriteLine("Testing GenericCalli with string as return type"); + Console.WriteLine("Testing GenericCalli with string as the return type"); Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); - Console.WriteLine("Testing GenericCalli with string as both parameter and return type"); + Console.WriteLine("Testing GenericCalli with string as both the parameter and return type"); Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); } finally From e0e747821d9fbd1e54998cdcd90711e829b8b239 Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 25 Jan 2024 20:37:01 +0900 Subject: [PATCH 15/44] Handle collectible assemblies --- src/coreclr/vm/ceeload.cpp | 37 ++++++++++++++++++++++++++---------- src/coreclr/vm/ceeload.h | 16 ++++++++++++---- src/coreclr/vm/dllimport.cpp | 2 +- src/coreclr/vm/dllimport.h | 4 ++-- 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index d22b8d677348e..c3c50ce13f93a 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4722,15 +4722,28 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* { if (pBlock->m_cookies[i].signature.GetRawSig() == vaSignature.GetRawSig()) { - _ASSERTE(pBlock->m_cookies[i].classInstCount == typeContext->m_classInst.GetNumArgs()); - _ASSERTE(pBlock->m_cookies[i].methInstCount == typeContext->m_methodInst.GetNumArgs()); + if (pBlock->m_cookies[i].IsUnloaded()) + { + continue; + } + + const SigTypeContext& leftTypeContext = pBlock->m_cookies[i].typeContext; + _ASSERTE(leftTypeContext.m_classInst.GetNumArgs() == typeContext->m_classInst.GetNumArgs()); + _ASSERTE(leftTypeContext.m_methodInst.GetNumArgs() == typeContext->m_methodInst.GetNumArgs()); bool instMatch = true; - for (UINT j = 0; j < pBlock->m_cookies[i].classInstCount; j++) + for (UINT j = 0; j < leftTypeContext.m_classInst.GetNumArgs(); j++) { - if (pBlock->m_cookies[i].classInst[j] != typeContext->m_classInst[j]) + if (leftTypeContext.m_classInst[j] != typeContext->m_classInst[j]) + { + instMatch = false; + break; + } + else if (leftTypeContext.m_classInst[j].GetAssembly()->IsCollectible() + && leftTypeContext.m_classInst[j].GetLoaderAllocator()->IsUnloaded()) { + pBlock->m_cookies[i].SetUnloaded(); instMatch = false; break; } @@ -4738,10 +4751,17 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* if (instMatch) { - for (UINT j = 0; j < pBlock->m_cookies[i].methInstCount; j++) + for (UINT j = 0; j < leftTypeContext.m_methodInst.GetNumArgs(); j++) { - if (pBlock->m_cookies[i].methInst[j] != typeContext->m_methodInst[j]) + if (leftTypeContext.m_methodInst[j] != typeContext->m_methodInst[j]) + { + instMatch = false; + break; + } + else if (leftTypeContext.m_methodInst[j].GetAssembly()->IsCollectible() + && leftTypeContext.m_methodInst[j].GetLoaderAllocator()->IsUnloaded()) { + pBlock->m_cookies[i].SetUnloaded(); instMatch = false; break; } @@ -4801,10 +4821,7 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* pCookie->pNDirectILStub = NULL; pCookie->sizeOfArgs = sizeOfArgs; pCookie->signature = vaSignature; - pCookie->classInst = typeContext->m_classInst.GetRawArgs(); - pCookie->classInstCount = typeContext->m_classInst.GetNumArgs(); - pCookie->methInst = typeContext->m_methodInst.GetRawArgs(); - pCookie->methInstCount = typeContext->m_methodInst.GetNumArgs(); + pCookie->typeContext = *typeContext; // Finally, now that it's safe for asynchronous readers to see it, // update the count. diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index bbee2e279bdb2..0c2272f9dccb2 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -339,10 +339,18 @@ struct VASigCookie Volatile pNDirectILStub; // will be use if target is NDirect (tag == 0) PTR_Module pModule; Signature signature; - TypeHandle* classInst; - DWORD classInstCount; - TypeHandle* methInst; - DWORD methInstCount; + SigTypeContext typeContext; + bool isUnloaded; + + inline void SetUnloaded() + { + isUnloaded = true; + } + + inline bool IsUnloaded() const + { + return isUnloaded; + } }; // diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index f4c56d5baa69d..b0938a20c0d63 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -6066,7 +6066,7 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD) } StubSigDesc sigDesc(pMD, signature, pVASigCookie->pModule); - sigDesc.InitTypeContext(pVASigCookie->classInst, pVASigCookie->classInstCount, pVASigCookie->methInst, pVASigCookie->methInstCount); + sigDesc.InitTypeContext(pVASigCookie->typeContext); MethodDesc* pStubMD = NDirect::CreateCLRToNativeILStub(&sigDesc, nlType, diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h index d28b8d210570b..14d0e5c8cc798 100644 --- a/src/coreclr/vm/dllimport.h +++ b/src/coreclr/vm/dllimport.h @@ -58,11 +58,11 @@ struct StubSigDesc #endif // _DEBUG #ifndef DACCESS_COMPILE - void InitTypeContext(TypeHandle* classInst, DWORD classInstCount, TypeHandle* methInst, DWORD methInstCount) + void InitTypeContext(const SigTypeContext typeContext) { LIMITED_METHOD_CONTRACT; - m_typeContext = SigTypeContext(Instantiation(classInst, classInstCount), Instantiation(methInst, methInstCount)); + m_typeContext = typeContext; } #endif }; From 97a8bf60e5b315ff6b486784f536581143e9c75f Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 25 Jan 2024 20:39:14 +0900 Subject: [PATCH 16/44] Reverse the branch --- src/coreclr/vm/ceeload.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index c3c50ce13f93a..f4f0704af949f 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4735,15 +4735,16 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* for (UINT j = 0; j < leftTypeContext.m_classInst.GetNumArgs(); j++) { - if (leftTypeContext.m_classInst[j] != typeContext->m_classInst[j]) + if (leftTypeContext.m_classInst[j].GetAssembly()->IsCollectible() + && leftTypeContext.m_classInst[j].GetLoaderAllocator()->IsUnloaded()) { + pBlock->m_cookies[i].SetUnloaded(); instMatch = false; break; } - else if (leftTypeContext.m_classInst[j].GetAssembly()->IsCollectible() - && leftTypeContext.m_classInst[j].GetLoaderAllocator()->IsUnloaded()) + + if (leftTypeContext.m_classInst[j] != typeContext->m_classInst[j]) { - pBlock->m_cookies[i].SetUnloaded(); instMatch = false; break; } @@ -4753,15 +4754,16 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* { for (UINT j = 0; j < leftTypeContext.m_methodInst.GetNumArgs(); j++) { - if (leftTypeContext.m_methodInst[j] != typeContext->m_methodInst[j]) + if (leftTypeContext.m_methodInst[j].GetAssembly()->IsCollectible() + && leftTypeContext.m_methodInst[j].GetLoaderAllocator()->IsUnloaded()) { + pBlock->m_cookies[i].SetUnloaded(); instMatch = false; break; } - else if (leftTypeContext.m_methodInst[j].GetAssembly()->IsCollectible() - && leftTypeContext.m_methodInst[j].GetLoaderAllocator()->IsUnloaded()) + + if (leftTypeContext.m_methodInst[j] != typeContext->m_methodInst[j]) { - pBlock->m_cookies[i].SetUnloaded(); instMatch = false; break; } From fd43493799ef8c4a60bc829613dcf683a2deabec Mon Sep 17 00:00:00 2001 From: Steven He Date: Fri, 26 Jan 2024 21:31:00 +0900 Subject: [PATCH 17/44] Handle inlined P/Invoke transitions --- src/coreclr/vm/comdelegate.cpp | 2 +- src/coreclr/vm/dllimport.cpp | 5 ++--- src/coreclr/vm/dllimport.h | 1 + src/coreclr/vm/jitinterface.cpp | 5 ++++- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/comdelegate.cpp b/src/coreclr/vm/comdelegate.cpp index 5a784e15f6960..c79e4335e5590 100644 --- a/src/coreclr/vm/comdelegate.cpp +++ b/src/coreclr/vm/comdelegate.cpp @@ -2023,7 +2023,7 @@ void COMDelegate::ThrowIfInvalidUnmanagedCallersOnlyUsage(MethodDesc* pMD) // Arguments - Scenarios involving UnmanagedCallersOnly are handled during the jit. bool unmanagedCallersOnlyRequiresMarshalling = false; - if (NDirect::MarshalingRequired(pMD, NULL, NULL, unmanagedCallersOnlyRequiresMarshalling)) + if (NDirect::MarshalingRequired(pMD, NULL, NULL, NULL, unmanagedCallersOnlyRequiresMarshalling)) EX_THROW(EEResourceException, (kInvalidProgramException, W("InvalidProgram_NonBlittableTypes"))); } diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index b0938a20c0d63..4938f260b33cb 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -3174,6 +3174,7 @@ BOOL NDirect::MarshalingRequired( _In_opt_ MethodDesc* pMD, _In_opt_ PCCOR_SIGNATURE pSig, _In_opt_ Module* pModule, + _In_opt_ SigTypeContext* pTypeContext, _In_ bool unmanagedCallersOnlyRequiresMarshalling) { CONTRACTL @@ -3254,8 +3255,6 @@ BOOL NDirect::MarshalingRequired( mdParamDef *pParamTokenArray = (mdParamDef *)_alloca(numArgs * sizeof(mdParamDef)); IMDInternalImport *pMDImport = pModule->GetMDImport(); - SigTypeContext emptyTypeContext; - mdMethodDef methodToken = mdMethodDefNil; if (pMD != NULL) { @@ -3315,7 +3314,7 @@ BOOL NDirect::MarshalingRequired( case ELEMENT_TYPE_VALUETYPE: case ELEMENT_TYPE_GENERICINST: { - TypeHandle hndArgType = arg.GetTypeHandleThrowing(pModule, &emptyTypeContext); + TypeHandle hndArgType = arg.GetTypeHandleThrowing(pModule, pTypeContext); bool isValidGeneric = IsValidForGenericMarshalling(hndArgType.GetMethodTable(), false, runtimeMarshallingEnabled); if(!hndArgType.IsValueType() || !isValidGeneric) return true; diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h index 14d0e5c8cc798..2a3731bbcf0d4 100644 --- a/src/coreclr/vm/dllimport.h +++ b/src/coreclr/vm/dllimport.h @@ -101,6 +101,7 @@ class NDirect _In_opt_ MethodDesc* pMD, _In_opt_ PCCOR_SIGNATURE pSig = NULL, _In_opt_ Module* pModule = NULL, + _In_opt_ SigTypeContext* pTypeContext = NULL, _In_ bool unmanagedCallersOnlyRequiresMarshalling = true); static void PopulateNDirectMethodDesc(_Inout_ NDirectMethodDesc* pNMD); diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 0de867d5c6a44..58c34088092a6 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -9788,10 +9788,13 @@ bool CEEInfo::pInvokeMarshalingRequired(CORINFO_METHOD_HANDLE method, CORINFO_SI if (method == NULL) { // check the call site signature + SigTypeContext typeContext; + GetTypeContext(&callSiteSig->sigInst, &typeContext); result = NDirect::MarshalingRequired( NULL, callSiteSig->pSig, - GetModule(callSiteSig->scope)); + GetModule(callSiteSig->scope), + &typeContext); } else { From fad27011f1962ff964cdbaa64c7ae99e673cb8ca Mon Sep 17 00:00:00 2001 From: Steven He Date: Fri, 26 Jan 2024 22:58:35 +0900 Subject: [PATCH 18/44] Reimplement unloadability handling --- src/coreclr/vm/ceeload.cpp | 68 +++++++++++++++++++++--------------- src/coreclr/vm/ceeload.h | 17 +++------ src/coreclr/vm/dllimport.cpp | 2 +- src/coreclr/vm/dllimport.h | 4 +-- 4 files changed, 47 insertions(+), 44 deletions(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index f4f0704af949f..37e1cc885baa3 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4722,28 +4722,14 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* { if (pBlock->m_cookies[i].signature.GetRawSig() == vaSignature.GetRawSig()) { - if (pBlock->m_cookies[i].IsUnloaded()) - { - continue; - } - - const SigTypeContext& leftTypeContext = pBlock->m_cookies[i].typeContext; - _ASSERTE(leftTypeContext.m_classInst.GetNumArgs() == typeContext->m_classInst.GetNumArgs()); - _ASSERTE(leftTypeContext.m_methodInst.GetNumArgs() == typeContext->m_methodInst.GetNumArgs()); + _ASSERTE(pBlock->m_cookies[i].classInstCount == typeContext->m_classInst.GetNumArgs()); + _ASSERTE(pBlock->m_cookies[i].methodInstCount == typeContext->m_methodInst.GetNumArgs()); bool instMatch = true; - for (UINT j = 0; j < leftTypeContext.m_classInst.GetNumArgs(); j++) + for (DWORD j = 0; j < pBlock->m_cookies[i].classInstCount; j++) { - if (leftTypeContext.m_classInst[j].GetAssembly()->IsCollectible() - && leftTypeContext.m_classInst[j].GetLoaderAllocator()->IsUnloaded()) - { - pBlock->m_cookies[i].SetUnloaded(); - instMatch = false; - break; - } - - if (leftTypeContext.m_classInst[j] != typeContext->m_classInst[j]) + if (pBlock->m_cookies[i].classInst[j] != typeContext->m_classInst[j]) { instMatch = false; break; @@ -4752,17 +4738,9 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* if (instMatch) { - for (UINT j = 0; j < leftTypeContext.m_methodInst.GetNumArgs(); j++) + for (DWORD j = 0; j < pBlock->m_cookies[i].methodInstCount; j++) { - if (leftTypeContext.m_methodInst[j].GetAssembly()->IsCollectible() - && leftTypeContext.m_methodInst[j].GetLoaderAllocator()->IsUnloaded()) - { - pBlock->m_cookies[i].SetUnloaded(); - instMatch = false; - break; - } - - if (leftTypeContext.m_methodInst[j] != typeContext->m_methodInst[j]) + if (pBlock->m_cookies[i].methodInst[j] != typeContext->m_methodInst[j]) { instMatch = false; break; @@ -4791,6 +4769,15 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* // Upper estimate of the vararg size DWORD sizeOfArgs = argit.SizeOfArgStack(); + // Prepare instantiation + Module * pLoaderModule = ClassLoader::ComputeLoaderModuleWorker(this, mdTokenNil, typeContext->m_classInst, typeContext->m_methodInst); + LoaderAllocator * pAllocator = pLoaderModule->GetLoaderAllocator(); + + DWORD classInstCount = typeContext->m_classInst.GetNumArgs(); + DWORD methodInstCount = typeContext->m_methodInst.GetNumArgs(); + pAllocator->EnsureInstantiation(this, typeContext->m_classInst); + pAllocator->EnsureInstantiation(this, typeContext->m_methodInst); + // enable gc before taking lock { CrstHolder ch(&m_Crst); @@ -4823,7 +4810,30 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* pCookie->pNDirectILStub = NULL; pCookie->sizeOfArgs = sizeOfArgs; pCookie->signature = vaSignature; - pCookie->typeContext = *typeContext; + pCookie->classInstCount = classInstCount; + pCookie->methodInstCount = methodInstCount; + + if (classInstCount != 0) + { + DWORD infoSize; + DWORD allocSize = DictionaryLayout::GetDictionarySizeFromLayout(classInstCount, NULL, &infoSize); + pCookie->classInst = (TypeHandle*)(void*)pCookie->amt.Track(pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(allocSize))); + for (DWORD i = 0; i < classInstCount; i++) + { + pCookie->classInst[i] = typeContext->m_classInst[i]; + } + } + + if (methodInstCount != 0) + { + DWORD infoSize; + DWORD allocSize = DictionaryLayout::GetDictionarySizeFromLayout(methodInstCount, NULL, &infoSize); + pCookie->methodInst = (TypeHandle*)(void*)pCookie->amt.Track(pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(allocSize))); + for (DWORD i = 0; i < methodInstCount; i++) + { + pCookie->methodInst[i] = typeContext->m_methodInst[i]; + } + } // Finally, now that it's safe for asynchronous readers to see it, // update the count. diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index 0c2272f9dccb2..c3a554e98f665 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -339,18 +339,11 @@ struct VASigCookie Volatile pNDirectILStub; // will be use if target is NDirect (tag == 0) PTR_Module pModule; Signature signature; - SigTypeContext typeContext; - bool isUnloaded; - - inline void SetUnloaded() - { - isUnloaded = true; - } - - inline bool IsUnloaded() const - { - return isUnloaded; - } + AllocMemTracker amt; + DWORD classInstCount; + TypeHandle* classInst; + DWORD methodInstCount; + TypeHandle* methodInst; }; // diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 4938f260b33cb..43bb20e554bae 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -6065,7 +6065,7 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD) } StubSigDesc sigDesc(pMD, signature, pVASigCookie->pModule); - sigDesc.InitTypeContext(pVASigCookie->typeContext); + sigDesc.InitTypeContext(pVASigCookie->classInst, pVASigCookie->classInstCount, pVASigCookie->methodInst, pVASigCookie->methodInstCount); MethodDesc* pStubMD = NDirect::CreateCLRToNativeILStub(&sigDesc, nlType, diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h index 2a3731bbcf0d4..7e782df186b95 100644 --- a/src/coreclr/vm/dllimport.h +++ b/src/coreclr/vm/dllimport.h @@ -58,11 +58,11 @@ struct StubSigDesc #endif // _DEBUG #ifndef DACCESS_COMPILE - void InitTypeContext(const SigTypeContext typeContext) + void InitTypeContext(TypeHandle* classInst, DWORD classInstCount, TypeHandle* methodInst, DWORD methodInstCount) { LIMITED_METHOD_CONTRACT; - m_typeContext = typeContext; + m_typeContext = SigTypeContext(Instantiation(classInst, classInstCount), Instantiation(methodInst, methodInstCount)); } #endif }; From 4f67d66b0fc39dcd59316d33dc58e81c69b1167c Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 27 Jan 2024 13:23:16 +0900 Subject: [PATCH 19/44] Fix native library resolving in tests --- .../FunctionPointer/FunctionPointer.cs | 48 ++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index 14582fdb61ce9..8783643a8c142 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -1,6 +1,8 @@ // 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.IO; +using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using Xunit; @@ -189,7 +191,7 @@ public static void RunInvalidGenericFunctionPointerTest() nint lib = nint.Zero; try { - lib = NativeLibrary.Load(nameof(FunctionPointerNative)); + lib = NativeLibrary.Load(NativeLibraryToLoad.GetFullPath()); nint fnptr = NativeLibrary.GetExport(lib, "ReturnParameter"); Console.WriteLine("Testing GenericCalli with string as the parameter type"); Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); @@ -207,4 +209,48 @@ public static void RunInvalidGenericFunctionPointerTest() } } } + + class NativeLibraryToLoad + { + static string GetFileName() + { + return GetLibraryFileName(nameof(FunctionPointerNative)); + } + + static string GetLibraryFileName(string name) + { + if (OperatingSystem.IsWindows()) + return $"{name}.dll"; + + if (OperatingSystem.IsLinux()) + return $"lib{name}.so"; + + if (OperatingSystem.IsMacOS()) + return $"lib{name}.dylib"; + + throw new PlatformNotSupportedException(); + } + + internal static string GetFullPath() + { + return Path.Combine(GetDirectory(), GetFileName()); + } + + static string GetDirectory() + { + string directory; + if (TestLibrary.Utilities.IsNativeAot) + { + // NativeAOT test is put in a native/ subdirectory, so we want the parent + // directory that contains the native library to load + directory = new DirectoryInfo(AppContext.BaseDirectory).Parent.FullName; + } + else + { + directory = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); + } + + return directory; + } + } } From b050468cfd260b382d9da3b491eb329f6da6c815 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 27 Jan 2024 17:06:57 +0900 Subject: [PATCH 20/44] Use AllocMemTracker properly --- src/coreclr/vm/ceeload.cpp | 12 ++++++------ src/coreclr/vm/ceeload.h | 1 - 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index 37e1cc885baa3..09a7d6dd788e2 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4812,12 +4812,12 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* pCookie->signature = vaSignature; pCookie->classInstCount = classInstCount; pCookie->methodInstCount = methodInstCount; + + AllocMemTracker amt; if (classInstCount != 0) { - DWORD infoSize; - DWORD allocSize = DictionaryLayout::GetDictionarySizeFromLayout(classInstCount, NULL, &infoSize); - pCookie->classInst = (TypeHandle*)(void*)pCookie->amt.Track(pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(allocSize))); + pCookie->classInst = (TypeHandle*)(void*)amt.Track(pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(classInstCount * sizeof(TypeHandle)))); for (DWORD i = 0; i < classInstCount; i++) { pCookie->classInst[i] = typeContext->m_classInst[i]; @@ -4826,14 +4826,14 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* if (methodInstCount != 0) { - DWORD infoSize; - DWORD allocSize = DictionaryLayout::GetDictionarySizeFromLayout(methodInstCount, NULL, &infoSize); - pCookie->methodInst = (TypeHandle*)(void*)pCookie->amt.Track(pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(allocSize))); + pCookie->methodInst = (TypeHandle*)(void*)amt.Track(pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(methodInstCount * sizeof(TypeHandle)))); for (DWORD i = 0; i < methodInstCount; i++) { pCookie->methodInst[i] = typeContext->m_methodInst[i]; } } + + amt.SuppressRelease(); // Finally, now that it's safe for asynchronous readers to see it, // update the count. diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index c3a554e98f665..2e83572d60b11 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -339,7 +339,6 @@ struct VASigCookie Volatile pNDirectILStub; // will be use if target is NDirect (tag == 0) PTR_Module pModule; Signature signature; - AllocMemTracker amt; DWORD classInstCount; TypeHandle* classInst; DWORD methodInstCount; From 549c74647232416bec89b8a950b0392225b5895a Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 27 Jan 2024 17:07:21 +0900 Subject: [PATCH 21/44] Skip tests on windows x86 --- .../MarshalAPI/FunctionPointer/FunctionPointer.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index 8783643a8c142..3c602381fbf0b 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -3,7 +3,6 @@ using System; using System.IO; using System.Reflection; -using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using Xunit; @@ -22,8 +21,8 @@ static class FunctionPointerNative [DllImport(nameof(FunctionPointerNative))] static unsafe extern void FillOutPtr(IntPtr* p); - [DllImport(nameof(FunctionPointerNative))] - static unsafe extern void FillOutIntParameter(out IntPtr p); + [DllImport(nameof(FunctionPointerNative))] + static unsafe extern void FillOutIntParameter(out IntPtr p); } delegate void VoidDelegate(); @@ -185,6 +184,12 @@ public static void RunGenericFunctionPointerTest() [Fact] public static void RunInvalidGenericFunctionPointerTest() { + if (OperatingSystem.IsWindows() && RuntimeInformation.ProcessArchitecture == Architecture.X86) + { + // We have naming mangling issues on Windows x86 + return; + } + Console.WriteLine($"Running {nameof(RunInvalidGenericFunctionPointerTest)}..."); unsafe { From 4fcbbf84d18204672ef0553f1c77163eda578881 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 27 Jan 2024 19:39:37 +0900 Subject: [PATCH 22/44] Skip negative tests on Windows x86 and NativeAOT --- .../MarshalAPI/FunctionPointer/FunctionPointer.cs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index 3c602381fbf0b..65f7809a59c89 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -181,15 +181,11 @@ public static void RunGenericFunctionPointerTest() Assert.Equal(expectedValue, outVar); } - [Fact] + public static bool CanRunInvalidGenericFunctionPointerTest => !TestLibrary.Utilities.IsNativeAot && !(TestLibrary.Utilities.IsWindows && TestLibrary.Utilities.IsX86); + + [ConditionalFact(nameof(CanRunInvalidGenericFunctionPointerTest))] public static void RunInvalidGenericFunctionPointerTest() { - if (OperatingSystem.IsWindows() && RuntimeInformation.ProcessArchitecture == Architecture.X86) - { - // We have naming mangling issues on Windows x86 - return; - } - Console.WriteLine($"Running {nameof(RunInvalidGenericFunctionPointerTest)}..."); unsafe { From fec111fc3cac11c71bd1ff4257c493fca7fcca2a Mon Sep 17 00:00:00 2001 From: Steven He Date: Mon, 12 Feb 2024 17:52:58 +0900 Subject: [PATCH 23/44] Use better exception message --- src/coreclr/vm/dllimport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 43bb20e554bae..175189377cb81 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -5037,7 +5037,7 @@ namespace // For generic calli, we only support blittable types if (!pSigDesc->m_typeContext.IsEmpty() && SF_IsCALLIStub(dwStubFlags) && NDirect::MarshalingRequired(pStubMD)) { - COMPlusThrow(kMarshalDirectiveException, IDS_EE_NDIRECT_UNSUPPORTED_SIG); + COMPlusThrow(kMarshalDirectiveException, IDS_EE_BADMARSHAL_GENERICS_RESTRICTION); } CreateNDirectStubWorker(pss, From 83d9919a0b55da50fece762fb78b9f5ab515f016 Mon Sep 17 00:00:00 2001 From: Steven He Date: Mon, 12 Feb 2024 18:07:08 +0900 Subject: [PATCH 24/44] Enable Windows x86 tests again --- .../Interop/MarshalAPI/FunctionPointer/CMakeLists.txt | 2 +- .../Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs | 2 +- .../MarshalAPI/FunctionPointer/FunctionPointerNative.def | 7 +++++++ 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.def diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/CMakeLists.txt b/src/tests/Interop/MarshalAPI/FunctionPointer/CMakeLists.txt index 549f9dfcab606..cc9a924b1f29c 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/CMakeLists.txt +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/CMakeLists.txt @@ -1,6 +1,6 @@ project (FunctionPointerNative) include ("${CLR_INTEROP_TEST_ROOT}/Interop.cmake") -set(SOURCES FunctionPointerNative.cpp) +set(SOURCES FunctionPointerNative.cpp FunctionPointerNative.def) # add the executable add_library (FunctionPointerNative SHARED ${SOURCES}) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index 65f7809a59c89..35ea8ce8f7b6a 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -181,7 +181,7 @@ public static void RunGenericFunctionPointerTest() Assert.Equal(expectedValue, outVar); } - public static bool CanRunInvalidGenericFunctionPointerTest => !TestLibrary.Utilities.IsNativeAot && !(TestLibrary.Utilities.IsWindows && TestLibrary.Utilities.IsX86); + public static bool CanRunInvalidGenericFunctionPointerTest => !TestLibrary.Utilities.IsNativeAot; [ConditionalFact(nameof(CanRunInvalidGenericFunctionPointerTest))] public static void RunInvalidGenericFunctionPointerTest() diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.def b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.def new file mode 100644 index 0000000000000..d2e55788d1da2 --- /dev/null +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.def @@ -0,0 +1,7 @@ +LIBRARY + EXPORTS + GetVoidVoidFcnPtr + CheckFcnPtr + FillOutPtr + FillOutIntParameter + ReturnParameter From 13904551d586601de23d05397f1bd285ac5c0130 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 13 Feb 2024 14:56:59 +0900 Subject: [PATCH 25/44] Exclude tests for mono --- src/tests/issues.targets | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index f78799d8c7e27..856c7b1a8993c 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1221,6 +1221,9 @@ Crashes during LLVM AOT compilation. + + Generic calli support not implemented yet in mono. + Crashes during LLVM AOT compilation. From e37a0d9bc7b5bf274d34750655b1b8c2e10cfa25 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 13 Feb 2024 16:41:07 +0900 Subject: [PATCH 26/44] Use correct loader elsewhere --- src/coreclr/vm/ceeload.cpp | 1 + src/coreclr/vm/ceeload.h | 1 + src/coreclr/vm/dllimport.cpp | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index 6a2141f778552..f9781d5914e09 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4812,6 +4812,7 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* pCookie->signature = vaSignature; pCookie->classInstCount = classInstCount; pCookie->methodInstCount = methodInstCount; + pCookie->pLoaderModule = pLoaderModule; AllocMemTracker amt; diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index 2e83572d60b11..1d32db239cc03 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -338,6 +338,7 @@ struct VASigCookie unsigned sizeOfArgs; // size of argument list Volatile pNDirectILStub; // will be use if target is NDirect (tag == 0) PTR_Module pModule; + PTR_Module pLoaderModule; Signature signature; DWORD classInstCount; TypeHandle* classInst; diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 175189377cb81..42347b170a625 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -6026,7 +6026,7 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD) } } - LoaderHeap *pHeap = pVASigCookie->pModule->GetLoaderAllocator()->GetHighFrequencyHeap(); + LoaderHeap *pHeap = pVASigCookie->pLoaderModule->GetLoaderAllocator()->GetHighFrequencyHeap(); PCOR_SIGNATURE new_sig = (PCOR_SIGNATURE)(void *)pHeap->AllocMem(S_SIZE_T(signature.GetRawSigLen())); CopyMemory(new_sig, signature.GetRawSig(), signature.GetRawSigLen()); From 77216261fc108b6c4acb930c582789a7a8955fe1 Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 14 Feb 2024 12:31:59 +0900 Subject: [PATCH 27/44] Put VASigCookieBlock on loader module --- src/coreclr/vm/ceeload.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index f9781d5914e09..30b4959a19d11 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4714,9 +4714,11 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* pCookie = NULL; + Module * pLoaderModule = ClassLoader::ComputeLoaderModuleWorker(this, mdTokenNil, typeContext->m_classInst, typeContext->m_methodInst); + // First, see if we already enregistered this sig. // Note that we're outside the lock here, so be a bit careful with our logic - for (pBlock = m_pVASigCookieBlock; pBlock != NULL; pBlock = pBlock->m_Next) + for (pBlock = pLoaderModule->m_pVASigCookieBlock; pBlock != NULL; pBlock = pBlock->m_Next) { for (UINT i = 0; i < pBlock->m_numcookies; i++) { @@ -4770,7 +4772,6 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* DWORD sizeOfArgs = argit.SizeOfArgStack(); // Prepare instantiation - Module * pLoaderModule = ClassLoader::ComputeLoaderModuleWorker(this, mdTokenNil, typeContext->m_classInst, typeContext->m_methodInst); LoaderAllocator * pAllocator = pLoaderModule->GetLoaderAllocator(); DWORD classInstCount = typeContext->m_classInst.GetNumArgs(); @@ -4788,20 +4789,20 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* // occasional duplicate cookie instead. // Is the first block in the list full? - if (m_pVASigCookieBlock && m_pVASigCookieBlock->m_numcookies + if (pLoaderModule->m_pVASigCookieBlock && pLoaderModule->m_pVASigCookieBlock->m_numcookies < VASigCookieBlock::kVASigCookieBlockSize) { // Nope, reserve a new slot in the existing block. - pCookie = &(m_pVASigCookieBlock->m_cookies[m_pVASigCookieBlock->m_numcookies]); + pCookie = &(pLoaderModule->m_pVASigCookieBlock->m_cookies[pLoaderModule->m_pVASigCookieBlock->m_numcookies]); } else { // Yes, create a new block. VASigCookieBlock *pNewBlock = new VASigCookieBlock(); - pNewBlock->m_Next = m_pVASigCookieBlock; + pNewBlock->m_Next = pLoaderModule->m_pVASigCookieBlock; pNewBlock->m_numcookies = 0; - m_pVASigCookieBlock = pNewBlock; + pLoaderModule->m_pVASigCookieBlock = pNewBlock; pCookie = &(pNewBlock->m_cookies[0]); } @@ -4838,7 +4839,7 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* // Finally, now that it's safe for asynchronous readers to see it, // update the count. - m_pVASigCookieBlock->m_numcookies++; + pLoaderModule->m_pVASigCookieBlock->m_numcookies++; } } From d05b1cb8ed086e32968013d094c7e0c5488af24f Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 14 Feb 2024 13:16:32 +0900 Subject: [PATCH 28/44] Move VASigCookie creation to a new method --- src/coreclr/vm/ceeload.cpp | 144 +++++++++++++++++++++---------------- src/coreclr/vm/ceeload.h | 2 + 2 files changed, 83 insertions(+), 63 deletions(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index 30b4959a19d11..4503d56c1a0fa 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4762,85 +4762,103 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* if (!pCookie) { // If not, time to make a new one. + pCookie = MakeVASigCookieWorker(this, pLoaderModule, vaSignature, typeContext); + } - // Compute the size of args first, outside of the lock. + RETURN pCookie; +} - MetaSig metasig(vaSignature, this, typeContext); - ArgIterator argit(&metasig); +VASigCookie *Module::MakeVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, Signature vaSignature, const SigTypeContext* typeContext) +{ + CONTRACT(VASigCookie*) + { + THROWS; + GC_TRIGGERS; + MODE_ANY; + POSTCONDITION(CheckPointer(RETVAL)); + INJECT_FAULT(COMPlusThrowOM()); + } + CONTRACT_END; - // Upper estimate of the vararg size - DWORD sizeOfArgs = argit.SizeOfArgStack(); + VASigCookie *pCookie = NULL; + + // Compute the size of args first, outside of the lock. - // Prepare instantiation - LoaderAllocator * pAllocator = pLoaderModule->GetLoaderAllocator(); + MetaSig metasig(vaSignature, pDefiningModule, typeContext); + ArgIterator argit(&metasig); - DWORD classInstCount = typeContext->m_classInst.GetNumArgs(); - DWORD methodInstCount = typeContext->m_methodInst.GetNumArgs(); - pAllocator->EnsureInstantiation(this, typeContext->m_classInst); - pAllocator->EnsureInstantiation(this, typeContext->m_methodInst); + // Upper estimate of the vararg size + DWORD sizeOfArgs = argit.SizeOfArgStack(); - // enable gc before taking lock - { - CrstHolder ch(&m_Crst); + // Prepare instantiation + LoaderAllocator *pLoaderAllocator = pLoaderModule->GetLoaderAllocator(); - // Note that we were possibly racing to create the cookie, and another thread - // may have already created it. We could put another check - // here, but it's probably not worth the effort, so we'll just take an - // occasional duplicate cookie instead. + DWORD classInstCount = typeContext->m_classInst.GetNumArgs(); + DWORD methodInstCount = typeContext->m_methodInst.GetNumArgs(); + pLoaderAllocator->EnsureInstantiation(pDefiningModule, typeContext->m_classInst); + pLoaderAllocator->EnsureInstantiation(pDefiningModule, typeContext->m_methodInst); - // Is the first block in the list full? - if (pLoaderModule->m_pVASigCookieBlock && pLoaderModule->m_pVASigCookieBlock->m_numcookies - < VASigCookieBlock::kVASigCookieBlockSize) - { - // Nope, reserve a new slot in the existing block. - pCookie = &(pLoaderModule->m_pVASigCookieBlock->m_cookies[pLoaderModule->m_pVASigCookieBlock->m_numcookies]); - } - else - { - // Yes, create a new block. - VASigCookieBlock *pNewBlock = new VASigCookieBlock(); + // enable gc before taking lock + { + CrstHolder ch(&pLoaderModule->m_Crst); - pNewBlock->m_Next = pLoaderModule->m_pVASigCookieBlock; - pNewBlock->m_numcookies = 0; - pLoaderModule->m_pVASigCookieBlock = pNewBlock; - pCookie = &(pNewBlock->m_cookies[0]); - } + // Note that we were possibly racing to create the cookie, and another thread + // may have already created it. We could put another check + // here, but it's probably not worth the effort, so we'll just take an + // occasional duplicate cookie instead. + + // Is the first block in the list full? + if (pLoaderModule->m_pVASigCookieBlock && pLoaderModule->m_pVASigCookieBlock->m_numcookies + < VASigCookieBlock::kVASigCookieBlockSize) + { + // Nope, reserve a new slot in the existing block. + pCookie = &(pLoaderModule->m_pVASigCookieBlock->m_cookies[pLoaderModule->m_pVASigCookieBlock->m_numcookies]); + } + else + { + // Yes, create a new block. + VASigCookieBlock *pNewBlock = new VASigCookieBlock(); + + pNewBlock->m_Next = pLoaderModule->m_pVASigCookieBlock; + pNewBlock->m_numcookies = 0; + pLoaderModule->m_pVASigCookieBlock = pNewBlock; + pCookie = &(pNewBlock->m_cookies[0]); + } - // Now, fill in the new cookie (assuming we had enough memory to create one.) - pCookie->pModule = this; - pCookie->pNDirectILStub = NULL; - pCookie->sizeOfArgs = sizeOfArgs; - pCookie->signature = vaSignature; - pCookie->classInstCount = classInstCount; - pCookie->methodInstCount = methodInstCount; - pCookie->pLoaderModule = pLoaderModule; - - AllocMemTracker amt; - - if (classInstCount != 0) + // Now, fill in the new cookie (assuming we had enough memory to create one.) + pCookie->pModule = pDefiningModule; + pCookie->pNDirectILStub = NULL; + pCookie->sizeOfArgs = sizeOfArgs; + pCookie->signature = vaSignature; + pCookie->classInstCount = classInstCount; + pCookie->methodInstCount = methodInstCount; + pCookie->pLoaderModule = pLoaderModule; + + AllocMemTracker amt; + + if (classInstCount != 0) + { + pCookie->classInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(classInstCount * sizeof(TypeHandle)))); + for (DWORD i = 0; i < classInstCount; i++) { - pCookie->classInst = (TypeHandle*)(void*)amt.Track(pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(classInstCount * sizeof(TypeHandle)))); - for (DWORD i = 0; i < classInstCount; i++) - { - pCookie->classInst[i] = typeContext->m_classInst[i]; - } + pCookie->classInst[i] = typeContext->m_classInst[i]; } + } - if (methodInstCount != 0) + if (methodInstCount != 0) + { + pCookie->methodInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(methodInstCount * sizeof(TypeHandle)))); + for (DWORD i = 0; i < methodInstCount; i++) { - pCookie->methodInst = (TypeHandle*)(void*)amt.Track(pAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(methodInstCount * sizeof(TypeHandle)))); - for (DWORD i = 0; i < methodInstCount; i++) - { - pCookie->methodInst[i] = typeContext->m_methodInst[i]; - } + pCookie->methodInst[i] = typeContext->m_methodInst[i]; } - - amt.SuppressRelease(); - - // Finally, now that it's safe for asynchronous readers to see it, - // update the count. - pLoaderModule->m_pVASigCookieBlock->m_numcookies++; } + + amt.SuppressRelease(); + + // Finally, now that it's safe for asynchronous readers to see it, + // update the count. + pLoaderModule->m_pVASigCookieBlock->m_numcookies++; } RETURN pCookie; diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index 1d32db239cc03..e194deaf35258 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -1367,6 +1367,8 @@ class Module : public ModuleBase // Enregisters a VASig. VASigCookie *GetVASigCookie(Signature vaSignature, const SigTypeContext* typeContext); +private: + static VASigCookie *MakeVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, Signature vaSignature, const SigTypeContext* typeContext); public: #ifndef DACCESS_COMPILE From 909e7fa195adef94a31dacbb4a5d77d97d041710 Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 14 Feb 2024 13:24:49 +0900 Subject: [PATCH 29/44] Move more things --- src/coreclr/vm/ceeload.cpp | 164 ++++++++++++++++++------------------- src/coreclr/vm/ceeload.h | 2 +- 2 files changed, 82 insertions(+), 84 deletions(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index 4503d56c1a0fa..54286c5df497b 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4709,13 +4709,31 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* } CONTRACT_END; + VASigCookie *pCookie = GetVASigCookieWorker(this, + ClassLoader::ComputeLoaderModuleWorker(this, mdTokenNil, typeContext->m_classInst, typeContext->m_methodInst), + vaSignature, + typeContext); + + RETURN pCookie; +} + +VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, Signature vaSignature, const SigTypeContext* typeContext) +{ + CONTRACT(VASigCookie*) + { + THROWS; + GC_TRIGGERS; + MODE_ANY; + POSTCONDITION(CheckPointer(RETVAL)); + INJECT_FAULT(COMPlusThrowOM()); + } + CONTRACT_END; + VASigCookieBlock *pBlock; VASigCookie *pCookie; pCookie = NULL; - Module * pLoaderModule = ClassLoader::ComputeLoaderModuleWorker(this, mdTokenNil, typeContext->m_classInst, typeContext->m_methodInst); - // First, see if we already enregistered this sig. // Note that we're outside the lock here, so be a bit careful with our logic for (pBlock = pLoaderModule->m_pVASigCookieBlock; pBlock != NULL; pBlock = pBlock->m_Next) @@ -4758,107 +4776,87 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* } } } - + if (!pCookie) { - // If not, time to make a new one. - pCookie = MakeVASigCookieWorker(this, pLoaderModule, vaSignature, typeContext); - } - - RETURN pCookie; -} - -VASigCookie *Module::MakeVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, Signature vaSignature, const SigTypeContext* typeContext) -{ - CONTRACT(VASigCookie*) - { - THROWS; - GC_TRIGGERS; - MODE_ANY; - POSTCONDITION(CheckPointer(RETVAL)); - INJECT_FAULT(COMPlusThrowOM()); - } - CONTRACT_END; - - VASigCookie *pCookie = NULL; - - // Compute the size of args first, outside of the lock. + // Compute the size of args first, outside of the lock. - MetaSig metasig(vaSignature, pDefiningModule, typeContext); - ArgIterator argit(&metasig); + MetaSig metasig(vaSignature, pDefiningModule, typeContext); + ArgIterator argit(&metasig); - // Upper estimate of the vararg size - DWORD sizeOfArgs = argit.SizeOfArgStack(); + // Upper estimate of the vararg size + DWORD sizeOfArgs = argit.SizeOfArgStack(); - // Prepare instantiation - LoaderAllocator *pLoaderAllocator = pLoaderModule->GetLoaderAllocator(); + // Prepare instantiation + LoaderAllocator *pLoaderAllocator = pLoaderModule->GetLoaderAllocator(); - DWORD classInstCount = typeContext->m_classInst.GetNumArgs(); - DWORD methodInstCount = typeContext->m_methodInst.GetNumArgs(); - pLoaderAllocator->EnsureInstantiation(pDefiningModule, typeContext->m_classInst); - pLoaderAllocator->EnsureInstantiation(pDefiningModule, typeContext->m_methodInst); + DWORD classInstCount = typeContext->m_classInst.GetNumArgs(); + DWORD methodInstCount = typeContext->m_methodInst.GetNumArgs(); + pLoaderAllocator->EnsureInstantiation(pDefiningModule, typeContext->m_classInst); + pLoaderAllocator->EnsureInstantiation(pDefiningModule, typeContext->m_methodInst); - // enable gc before taking lock - { - CrstHolder ch(&pLoaderModule->m_Crst); + // enable gc before taking lock + { + CrstHolder ch(&pLoaderModule->m_Crst); - // Note that we were possibly racing to create the cookie, and another thread - // may have already created it. We could put another check - // here, but it's probably not worth the effort, so we'll just take an - // occasional duplicate cookie instead. + // Note that we were possibly racing to create the cookie, and another thread + // may have already created it. We could put another check + // here, but it's probably not worth the effort, so we'll just take an + // occasional duplicate cookie instead. - // Is the first block in the list full? - if (pLoaderModule->m_pVASigCookieBlock && pLoaderModule->m_pVASigCookieBlock->m_numcookies - < VASigCookieBlock::kVASigCookieBlockSize) - { - // Nope, reserve a new slot in the existing block. - pCookie = &(pLoaderModule->m_pVASigCookieBlock->m_cookies[pLoaderModule->m_pVASigCookieBlock->m_numcookies]); - } - else - { - // Yes, create a new block. - VASigCookieBlock *pNewBlock = new VASigCookieBlock(); + // Is the first block in the list full? + if (pLoaderModule->m_pVASigCookieBlock && pLoaderModule->m_pVASigCookieBlock->m_numcookies + < VASigCookieBlock::kVASigCookieBlockSize) + { + // Nope, reserve a new slot in the existing block. + pCookie = &(pLoaderModule->m_pVASigCookieBlock->m_cookies[pLoaderModule->m_pVASigCookieBlock->m_numcookies]); + } + else + { + // Yes, create a new block. + VASigCookieBlock *pNewBlock = new VASigCookieBlock(); - pNewBlock->m_Next = pLoaderModule->m_pVASigCookieBlock; - pNewBlock->m_numcookies = 0; - pLoaderModule->m_pVASigCookieBlock = pNewBlock; - pCookie = &(pNewBlock->m_cookies[0]); - } + pNewBlock->m_Next = pLoaderModule->m_pVASigCookieBlock; + pNewBlock->m_numcookies = 0; + pLoaderModule->m_pVASigCookieBlock = pNewBlock; + pCookie = &(pNewBlock->m_cookies[0]); + } - // Now, fill in the new cookie (assuming we had enough memory to create one.) - pCookie->pModule = pDefiningModule; - pCookie->pNDirectILStub = NULL; - pCookie->sizeOfArgs = sizeOfArgs; - pCookie->signature = vaSignature; - pCookie->classInstCount = classInstCount; - pCookie->methodInstCount = methodInstCount; - pCookie->pLoaderModule = pLoaderModule; + // Now, fill in the new cookie (assuming we had enough memory to create one.) + pCookie->pModule = pDefiningModule; + pCookie->pNDirectILStub = NULL; + pCookie->sizeOfArgs = sizeOfArgs; + pCookie->signature = vaSignature; + pCookie->classInstCount = classInstCount; + pCookie->methodInstCount = methodInstCount; + pCookie->pLoaderModule = pLoaderModule; - AllocMemTracker amt; + AllocMemTracker amt; - if (classInstCount != 0) - { - pCookie->classInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(classInstCount * sizeof(TypeHandle)))); - for (DWORD i = 0; i < classInstCount; i++) + if (classInstCount != 0) { - pCookie->classInst[i] = typeContext->m_classInst[i]; + pCookie->classInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(classInstCount * sizeof(TypeHandle)))); + for (DWORD i = 0; i < classInstCount; i++) + { + pCookie->classInst[i] = typeContext->m_classInst[i]; + } } - } - if (methodInstCount != 0) - { - pCookie->methodInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(methodInstCount * sizeof(TypeHandle)))); - for (DWORD i = 0; i < methodInstCount; i++) + if (methodInstCount != 0) { - pCookie->methodInst[i] = typeContext->m_methodInst[i]; + pCookie->methodInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(methodInstCount * sizeof(TypeHandle)))); + for (DWORD i = 0; i < methodInstCount; i++) + { + pCookie->methodInst[i] = typeContext->m_methodInst[i]; + } } - } - amt.SuppressRelease(); + amt.SuppressRelease(); - // Finally, now that it's safe for asynchronous readers to see it, - // update the count. - pLoaderModule->m_pVASigCookieBlock->m_numcookies++; + // Finally, now that it's safe for asynchronous readers to see it, + // update the count. + pLoaderModule->m_pVASigCookieBlock->m_numcookies++; + } } RETURN pCookie; diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index e194deaf35258..2814c9c8abdd7 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -1368,7 +1368,7 @@ class Module : public ModuleBase // Enregisters a VASig. VASigCookie *GetVASigCookie(Signature vaSignature, const SigTypeContext* typeContext); private: - static VASigCookie *MakeVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, Signature vaSignature, const SigTypeContext* typeContext); + static VASigCookie *GetVASigCookieWorker(Module* pDefiningModule, Module* pLoaderModule, Signature vaSignature, const SigTypeContext* typeContext); public: #ifndef DACCESS_COMPILE From f6833b9a017eac78f5bbcfd378d47825b0b72e4f Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 14 Feb 2024 13:26:56 +0900 Subject: [PATCH 30/44] oops --- src/coreclr/vm/ceeload.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index 54286c5df497b..7cfa1292d18c6 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4779,6 +4779,8 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad if (!pCookie) { + // If not, time to make a new one. + // Compute the size of args first, outside of the lock. MetaSig metasig(vaSignature, pDefiningModule, typeContext); From 35edb8d9102cce288d4b4efe82f7f46ee3b97cb5 Mon Sep 17 00:00:00 2001 From: Steve Date: Wed, 14 Feb 2024 15:12:39 +0900 Subject: [PATCH 31/44] Update issues.targets --- src/tests/issues.targets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 856c7b1a8993c..68ed725f4aa97 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1221,7 +1221,7 @@ Crashes during LLVM AOT compilation. - + Generic calli support not implemented yet in mono. From 40d5c35f508a8251fa9d4a4a1b791bba1942ec71 Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 14 Feb 2024 20:17:43 +0900 Subject: [PATCH 32/44] Allocate StubMD on loader module --- src/coreclr/vm/dllimport.cpp | 12 +++++++----- src/coreclr/vm/dllimport.h | 6 +++--- src/coreclr/vm/ilstubcache.cpp | 9 ++++++++- src/coreclr/vm/ilstubcache.h | 1 + 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 42347b170a625..c1bff8f75f250 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -113,7 +113,7 @@ StubSigDesc::StubSigDesc(MethodDesc *pMD) INDEBUG(InitDebugNames()); } -StubSigDesc::StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* pModule) +StubSigDesc::StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* pModule, Module* pLoaderModule) { CONTRACTL { @@ -135,13 +135,13 @@ StubSigDesc::StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* pModule) m_tkMethodDef = pMD->GetMemberDef(); SigTypeContext::InitTypeContext(pMD, &m_typeContext); m_pMetadataModule = pMD->GetModule(); - m_pLoaderModule = pMD->GetLoaderModule(); // Used for ILStubCache selection and MethodTable creation. + m_pLoaderModule = pLoaderModule == NULL ? pMD->GetLoaderModule() : pLoaderModule; // Used for ILStubCache selection and MethodTable creation. } else { m_tkMethodDef = mdMethodDefNil; m_pMetadataModule = m_pModule; - m_pLoaderModule = m_pModule; + m_pLoaderModule = pLoaderModule == NULL ? m_pModule : pLoaderModule; } INDEBUG(InitDebugNames()); @@ -4185,6 +4185,7 @@ namespace pHashParams, pParams->m_dwStubFlags, pParams->m_pModule, + pParams->m_pLoaderModule, pParams->m_pTypeContext, pParams->m_sig.GetRawSig(), pParams->m_sig.GetRawSigLen(), @@ -5035,7 +5036,8 @@ namespace else { // For generic calli, we only support blittable types - if (!pSigDesc->m_typeContext.IsEmpty() && SF_IsCALLIStub(dwStubFlags) && NDirect::MarshalingRequired(pStubMD)) + if (!pSigDesc->m_typeContext.IsEmpty() && SF_IsCALLIStub(dwStubFlags) + && NDirect::MarshalingRequired(NULL, pStubMD->GetSig(), pSigDesc->m_pModule, &pSigDesc->m_typeContext)) { COMPlusThrow(kMarshalDirectiveException, IDS_EE_BADMARSHAL_GENERICS_RESTRICTION); } @@ -6064,7 +6066,7 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD) nlType = nltAnsi; } - StubSigDesc sigDesc(pMD, signature, pVASigCookie->pModule); + StubSigDesc sigDesc(pMD, signature, pVASigCookie->pModule, pVASigCookie->pLoaderModule); sigDesc.InitTypeContext(pVASigCookie->classInst, pVASigCookie->classInstCount, pVASigCookie->methodInst, pVASigCookie->methodInstCount); MethodDesc* pStubMD = NDirect::CreateCLRToNativeILStub(&sigDesc, diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h index 7e782df186b95..f336f1d4080ba 100644 --- a/src/coreclr/vm/dllimport.h +++ b/src/coreclr/vm/dllimport.h @@ -16,9 +16,9 @@ struct StubSigDesc { public: StubSigDesc(MethodDesc* pMD); - StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* m_pModule); - StubSigDesc(MethodTable* pMT, const Signature& sig, Module* m_pModule); - StubSigDesc(const Signature& sig, Module* m_pModule); + StubSigDesc(MethodDesc* pMD, const Signature& sig, Module* pModule, Module* pLoaderModule = NULL); + StubSigDesc(MethodTable* pMT, const Signature& sig, Module* pModule); + StubSigDesc(const Signature& sig, Module* pModule); MethodDesc *m_pMD; MethodTable *m_pMT; diff --git a/src/coreclr/vm/ilstubcache.cpp b/src/coreclr/vm/ilstubcache.cpp index 8c9f6a70a1956..5be349a60c6eb 100644 --- a/src/coreclr/vm/ilstubcache.cpp +++ b/src/coreclr/vm/ilstubcache.cpp @@ -500,6 +500,7 @@ MethodDesc* ILStubCache::GetStubMethodDesc( ILStubHashBlob* pHashBlob, DWORD dwStubFlags, Module* pSigModule, + Module* pSigLoaderModule, SigTypeContext* pTypeContext, PCCOR_SIGNATURE pSig, DWORD cbSig, @@ -538,13 +539,18 @@ MethodDesc* ILStubCache::GetStubMethodDesc( // // Couldn't find it, let's make a new one. // - + Module *pContainingModule = pSigModule; if (pTargetMD != NULL) { // loader module may be different from signature module for generic targets pContainingModule = pTargetMD->GetLoaderModule(); } + else if (pTypeContext != NULL) + { + // for generic calli targets loader module is given directly + pContainingModule = pSigLoaderModule; + } MethodTable *pStubMT = GetOrCreateStubMethodTable(pContainingModule); @@ -555,6 +561,7 @@ MethodDesc* ILStubCache::GetStubMethodDesc( } else if (pTypeContext != NULL) { + // for generic calli targets typeContext is given directly typeContext = *pTypeContext; } diff --git a/src/coreclr/vm/ilstubcache.h b/src/coreclr/vm/ilstubcache.h index 6df4c98aff3de..74977fe0e00ed 100644 --- a/src/coreclr/vm/ilstubcache.h +++ b/src/coreclr/vm/ilstubcache.h @@ -53,6 +53,7 @@ class ILStubCache final ILStubHashBlob* pHashBlob, DWORD dwStubFlags, // bitmask of NDirectStubFlags Module* pSigModule, + Module* pSigLoaderModule, SigTypeContext* pTypeContext, PCCOR_SIGNATURE pSig, DWORD cbSig, From 0282d1a86e960eb147360fa3d1571d992faf2d57 Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 15 Feb 2024 00:07:43 +0900 Subject: [PATCH 33/44] Fixes for vararg cases --- src/coreclr/vm/ilstubcache.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/ilstubcache.cpp b/src/coreclr/vm/ilstubcache.cpp index 5be349a60c6eb..bb42d1af30016 100644 --- a/src/coreclr/vm/ilstubcache.cpp +++ b/src/coreclr/vm/ilstubcache.cpp @@ -546,7 +546,8 @@ MethodDesc* ILStubCache::GetStubMethodDesc( // loader module may be different from signature module for generic targets pContainingModule = pTargetMD->GetLoaderModule(); } - else if (pTypeContext != NULL) + + if (pTypeContext != NULL) { // for generic calli targets loader module is given directly pContainingModule = pSigLoaderModule; @@ -559,7 +560,8 @@ MethodDesc* ILStubCache::GetStubMethodDesc( { SigTypeContext::InitTypeContext(pTargetMD, &typeContext); } - else if (pTypeContext != NULL) + + if (pTypeContext != NULL) { // for generic calli targets typeContext is given directly typeContext = *pTypeContext; From 8fa2fd66c65f23663941cf34f18925cf75990e39 Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 15 Feb 2024 00:22:10 +0900 Subject: [PATCH 34/44] Handle varargs in error mode as well --- src/coreclr/vm/dllimport.cpp | 5 +++-- src/coreclr/vm/ilstubcache.cpp | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index c1bff8f75f250..656a9fd6d3d6d 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -5035,8 +5035,9 @@ namespace } else { - // For generic calli, we only support blittable types - if (!pSigDesc->m_typeContext.IsEmpty() && SF_IsCALLIStub(dwStubFlags) + // For generic calli/varargs, we only support blittable types + if (!pSigDesc->m_typeContext.IsEmpty() + && (SF_IsCALLIStub(dwStubFlags) || SF_IsVarArgStub(dwStubFlags)) && NDirect::MarshalingRequired(NULL, pStubMD->GetSig(), pSigDesc->m_pModule, &pSigDesc->m_typeContext)) { COMPlusThrow(kMarshalDirectiveException, IDS_EE_BADMARSHAL_GENERICS_RESTRICTION); diff --git a/src/coreclr/vm/ilstubcache.cpp b/src/coreclr/vm/ilstubcache.cpp index bb42d1af30016..17cc9566e0028 100644 --- a/src/coreclr/vm/ilstubcache.cpp +++ b/src/coreclr/vm/ilstubcache.cpp @@ -549,7 +549,7 @@ MethodDesc* ILStubCache::GetStubMethodDesc( if (pTypeContext != NULL) { - // for generic calli targets loader module is given directly + // for generic calli/varargs targets loader module is given directly pContainingModule = pSigLoaderModule; } @@ -563,7 +563,7 @@ MethodDesc* ILStubCache::GetStubMethodDesc( if (pTypeContext != NULL) { - // for generic calli targets typeContext is given directly + // for generic calli/varargs targets typeContext is given directly typeContext = *pTypeContext; } From c2cfc1b9817a956e2340b59b0520e1e326044c0f Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 15 Feb 2024 00:40:15 +0900 Subject: [PATCH 35/44] Add an error mode for generic varargs --- src/coreclr/vm/dllimport.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 656a9fd6d3d6d..0274473f20a27 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -5035,12 +5035,19 @@ namespace } else { - // For generic calli/varargs, we only support blittable types - if (!pSigDesc->m_typeContext.IsEmpty() - && (SF_IsCALLIStub(dwStubFlags) || SF_IsVarArgStub(dwStubFlags)) - && NDirect::MarshalingRequired(NULL, pStubMD->GetSig(), pSigDesc->m_pModule, &pSigDesc->m_typeContext)) + if (!pSigDesc->m_typeContext.IsEmpty()) { - COMPlusThrow(kMarshalDirectiveException, IDS_EE_BADMARSHAL_GENERICS_RESTRICTION); + // For generic calli, we only support blittable types + if (SF_IsCALLIStub(dwStubFlags) + && NDirect::MarshalingRequired(NULL, pStubMD->GetSig(), pSigDesc->m_pModule, &pSigDesc->m_typeContext)) + { + COMPlusThrow(kMarshalDirectiveException, IDS_EE_BADMARSHAL_GENERICS_RESTRICTION); + } + // We don't want to support generic varargs, so block it + else if (SF_IsVarArgStub(dwStubFlags)) + { + COMPlusThrow(kNotSupportedException, BFA_GENCODE_NOT_BE_VARARG); + } } CreateNDirectStubWorker(pss, From 3c1c25d9d7a228aa7380527a78ed772ae47f6645 Mon Sep 17 00:00:00 2001 From: Steve Date: Thu, 15 Feb 2024 00:49:44 +0900 Subject: [PATCH 36/44] Remove useless comments --- src/coreclr/vm/ilstubcache.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/ilstubcache.cpp b/src/coreclr/vm/ilstubcache.cpp index 17cc9566e0028..54d7642d863b7 100644 --- a/src/coreclr/vm/ilstubcache.cpp +++ b/src/coreclr/vm/ilstubcache.cpp @@ -539,7 +539,7 @@ MethodDesc* ILStubCache::GetStubMethodDesc( // // Couldn't find it, let's make a new one. // - + Module *pContainingModule = pSigModule; if (pTargetMD != NULL) { @@ -549,7 +549,7 @@ MethodDesc* ILStubCache::GetStubMethodDesc( if (pTypeContext != NULL) { - // for generic calli/varargs targets loader module is given directly + // for generic calli targets loader module is given directly pContainingModule = pSigLoaderModule; } @@ -563,7 +563,7 @@ MethodDesc* ILStubCache::GetStubMethodDesc( if (pTypeContext != NULL) { - // for generic calli/varargs targets typeContext is given directly + // for generic calli targets typeContext is given directly typeContext = *pTypeContext; } From e503dfa7131c25d96bf3b943ed725aecbfd90a8b Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 15 Feb 2024 22:52:00 +0900 Subject: [PATCH 37/44] FB --- src/coreclr/vm/ceeload.cpp | 26 ++++--- src/coreclr/vm/ceeload.h | 6 +- src/coreclr/vm/dllimport.cpp | 4 +- src/coreclr/vm/dllimport.h | 6 +- src/coreclr/vm/ilstubcache.cpp | 33 +++------ src/coreclr/vm/ilstubcache.h | 2 +- .../MarshalAPI/FunctionPointer/CMakeLists.txt | 2 +- .../FunctionPointer/FunctionPointer.cs | 70 +++---------------- .../FunctionPointer/FunctionPointerNative.cpp | 5 -- .../FunctionPointer/FunctionPointerNative.def | 7 -- 10 files changed, 43 insertions(+), 118 deletions(-) delete mode 100644 src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.def diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index 7cfa1292d18c6..3c07894ca380c 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4709,10 +4709,8 @@ VASigCookie *Module::GetVASigCookie(Signature vaSignature, const SigTypeContext* } CONTRACT_END; - VASigCookie *pCookie = GetVASigCookieWorker(this, - ClassLoader::ComputeLoaderModuleWorker(this, mdTokenNil, typeContext->m_classInst, typeContext->m_methodInst), - vaSignature, - typeContext); + Module* pLoaderModule = ClassLoader::ComputeLoaderModuleWorker(this, mdTokenNil, typeContext->m_classInst, typeContext->m_methodInst); + VASigCookie *pCookie = GetVASigCookieWorker(this, pLoaderModule, vaSignature, typeContext); RETURN pCookie; } @@ -4742,12 +4740,12 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad { if (pBlock->m_cookies[i].signature.GetRawSig() == vaSignature.GetRawSig()) { - _ASSERTE(pBlock->m_cookies[i].classInstCount == typeContext->m_classInst.GetNumArgs()); - _ASSERTE(pBlock->m_cookies[i].methodInstCount == typeContext->m_methodInst.GetNumArgs()); + _ASSERTE(pBlock->m_cookies[i].classInst.GetNumArgs() == typeContext->m_classInst.GetNumArgs()); + _ASSERTE(pBlock->m_cookies[i].methodInst.GetNumArgs() == typeContext->m_methodInst.GetNumArgs()); bool instMatch = true; - for (DWORD j = 0; j < pBlock->m_cookies[i].classInstCount; j++) + for (DWORD j = 0; j < pBlock->m_cookies[i].classInst.GetNumArgs(); j++) { if (pBlock->m_cookies[i].classInst[j] != typeContext->m_classInst[j]) { @@ -4758,7 +4756,7 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad if (instMatch) { - for (DWORD j = 0; j < pBlock->m_cookies[i].methodInstCount; j++) + for (DWORD j = 0; j < pBlock->m_cookies[i].methodInst.GetNumArgs(); j++) { if (pBlock->m_cookies[i].methodInst[j] != typeContext->m_methodInst[j]) { @@ -4829,28 +4827,28 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad pCookie->pNDirectILStub = NULL; pCookie->sizeOfArgs = sizeOfArgs; pCookie->signature = vaSignature; - pCookie->classInstCount = classInstCount; - pCookie->methodInstCount = methodInstCount; pCookie->pLoaderModule = pLoaderModule; AllocMemTracker amt; if (classInstCount != 0) { - pCookie->classInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(classInstCount * sizeof(TypeHandle)))); + TypeHandle* pClassInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(classInstCount * sizeof(TypeHandle)))); for (DWORD i = 0; i < classInstCount; i++) { - pCookie->classInst[i] = typeContext->m_classInst[i]; + pClassInst[i] = typeContext->m_classInst[i]; } + pCookie->classInst = Instantiation(pClassInst, classInstCount); } if (methodInstCount != 0) { - pCookie->methodInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(methodInstCount * sizeof(TypeHandle)))); + TypeHandle* pMethodInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(methodInstCount * sizeof(TypeHandle)))); for (DWORD i = 0; i < methodInstCount; i++) { - pCookie->methodInst[i] = typeContext->m_methodInst[i]; + pMethodInst[i] = typeContext->m_methodInst[i]; } + pCookie->methodInst = Instantiation(pMethodInst, methodInstCount); } amt.SuppressRelease(); diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index 2814c9c8abdd7..5f72c0be0b640 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -340,10 +340,8 @@ struct VASigCookie PTR_Module pModule; PTR_Module pLoaderModule; Signature signature; - DWORD classInstCount; - TypeHandle* classInst; - DWORD methodInstCount; - TypeHandle* methodInst; + Instantiation classInst; + Instantiation methodInst; }; // diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 0274473f20a27..ef0a5a5b1c39e 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -4186,9 +4186,9 @@ namespace pParams->m_dwStubFlags, pParams->m_pModule, pParams->m_pLoaderModule, - pParams->m_pTypeContext, pParams->m_sig.GetRawSig(), pParams->m_sig.GetRawSigLen(), + pParams->m_pTypeContext, pamTracker, bILStubCreator, pLastMD); @@ -6075,7 +6075,7 @@ PCODE GetILStubForCalli(VASigCookie *pVASigCookie, MethodDesc *pMD) } StubSigDesc sigDesc(pMD, signature, pVASigCookie->pModule, pVASigCookie->pLoaderModule); - sigDesc.InitTypeContext(pVASigCookie->classInst, pVASigCookie->classInstCount, pVASigCookie->methodInst, pVASigCookie->methodInstCount); + sigDesc.InitTypeContext(pVASigCookie->classInst, pVASigCookie->methodInst); MethodDesc* pStubMD = NDirect::CreateCLRToNativeILStub(&sigDesc, nlType, diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h index f336f1d4080ba..111c7436c9473 100644 --- a/src/coreclr/vm/dllimport.h +++ b/src/coreclr/vm/dllimport.h @@ -58,11 +58,13 @@ struct StubSigDesc #endif // _DEBUG #ifndef DACCESS_COMPILE - void InitTypeContext(TypeHandle* classInst, DWORD classInstCount, TypeHandle* methodInst, DWORD methodInstCount) + void InitTypeContext(Instantiation classInst, Instantiation methodInst) { LIMITED_METHOD_CONTRACT; - m_typeContext = SigTypeContext(Instantiation(classInst, classInstCount), Instantiation(methodInst, methodInstCount)); + _ASSERTE(m_typeContext.IsEmpty()); + + m_typeContext = SigTypeContext(classInst, methodInst); } #endif }; diff --git a/src/coreclr/vm/ilstubcache.cpp b/src/coreclr/vm/ilstubcache.cpp index 54d7642d863b7..1d8d14456a1f0 100644 --- a/src/coreclr/vm/ilstubcache.cpp +++ b/src/coreclr/vm/ilstubcache.cpp @@ -501,9 +501,9 @@ MethodDesc* ILStubCache::GetStubMethodDesc( DWORD dwStubFlags, Module* pSigModule, Module* pSigLoaderModule, - SigTypeContext* pTypeContext, PCCOR_SIGNATURE pSig, DWORD cbSig, + SigTypeContext* pTypeContext, AllocMemTracker* pamTracker, bool& bILStubCreator, MethodDesc *pLastMD) @@ -540,34 +540,23 @@ MethodDesc* ILStubCache::GetStubMethodDesc( // Couldn't find it, let's make a new one. // - Module *pContainingModule = pSigModule; - if (pTargetMD != NULL) - { - // loader module may be different from signature module for generic targets - pContainingModule = pTargetMD->GetLoaderModule(); - } - - if (pTypeContext != NULL) + if (pSigLoaderModule == NULL) { - // for generic calli targets loader module is given directly - pContainingModule = pSigLoaderModule; + pSigLoaderModule = (pTargetMD != NULL) ? pTargetMD->GetLoaderModule() : pSigModule; } - MethodTable *pStubMT = GetOrCreateStubMethodTable(pContainingModule); - SigTypeContext typeContext; - if (pTargetMD != NULL) + if (pTypeContext == NULL) { - SigTypeContext::InitTypeContext(pTargetMD, &typeContext); - } - - if (pTypeContext != NULL) - { - // for generic calli targets typeContext is given directly - typeContext = *pTypeContext; + if (pTargetMD != NULL) + { + SigTypeContext::InitTypeContext(pTargetMD, &typeContext); + } + pTypeContext = &typeContext; } - pMD = ILStubCache::CreateNewMethodDesc(m_pAllocator->GetHighFrequencyHeap(), pStubMT, dwStubFlags, pSigModule, pSig, cbSig, &typeContext, pamTracker); + MethodTable *pStubMT = GetOrCreateStubMethodTable(pSigLoaderModule); + pMD = ILStubCache::CreateNewMethodDesc(m_pAllocator->GetHighFrequencyHeap(), pStubMT, dwStubFlags, pSigModule, pSig, cbSig, pTypeContext, pamTracker); if (SF_IsSharedStub(dwStubFlags)) { diff --git a/src/coreclr/vm/ilstubcache.h b/src/coreclr/vm/ilstubcache.h index 74977fe0e00ed..c53fd7a1878c2 100644 --- a/src/coreclr/vm/ilstubcache.h +++ b/src/coreclr/vm/ilstubcache.h @@ -54,9 +54,9 @@ class ILStubCache final DWORD dwStubFlags, // bitmask of NDirectStubFlags Module* pSigModule, Module* pSigLoaderModule, - SigTypeContext* pTypeContext, PCCOR_SIGNATURE pSig, DWORD cbSig, + SigTypeContext* pTypeContext, AllocMemTracker* pamTracker, bool& bILStubCreator, MethodDesc* pLastMD); diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/CMakeLists.txt b/src/tests/Interop/MarshalAPI/FunctionPointer/CMakeLists.txt index cc9a924b1f29c..549f9dfcab606 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/CMakeLists.txt +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/CMakeLists.txt @@ -1,6 +1,6 @@ project (FunctionPointerNative) include ("${CLR_INTEROP_TEST_ROOT}/Interop.cmake") -set(SOURCES FunctionPointerNative.cpp FunctionPointerNative.def) +set(SOURCES FunctionPointerNative.cpp) # add the executable add_library (FunctionPointerNative SHARED ${SOURCES}) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index 35ea8ce8f7b6a..cf1d325d2dcb0 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -189,69 +189,19 @@ public static void RunInvalidGenericFunctionPointerTest() Console.WriteLine($"Running {nameof(RunInvalidGenericFunctionPointerTest)}..."); unsafe { - nint lib = nint.Zero; - try - { - lib = NativeLibrary.Load(NativeLibraryToLoad.GetFullPath()); - nint fnptr = NativeLibrary.GetExport(lib, "ReturnParameter"); - Console.WriteLine("Testing GenericCalli with string as the parameter type"); - Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); - Console.WriteLine("Testing GenericCalli with string as the return type"); - Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); - Console.WriteLine("Testing GenericCalli with string as both the parameter and return type"); - Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); - } - finally - { - if (lib != nint.Zero) - { - NativeLibrary.Free(lib); - } - } + nint fnptr = (nint)(delegate* unmanaged)&ReturnParameter; + Console.WriteLine("Testing GenericCalli with string as the parameter type"); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); + Console.WriteLine("Testing GenericCalli with string as the return type"); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); + Console.WriteLine("Testing GenericCalli with string as both the parameter and return type"); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); } } - class NativeLibraryToLoad + [UnmanagedCallersOnly] + static unsafe nint* ReturnParameter(nint* p) { - static string GetFileName() - { - return GetLibraryFileName(nameof(FunctionPointerNative)); - } - - static string GetLibraryFileName(string name) - { - if (OperatingSystem.IsWindows()) - return $"{name}.dll"; - - if (OperatingSystem.IsLinux()) - return $"lib{name}.so"; - - if (OperatingSystem.IsMacOS()) - return $"lib{name}.dylib"; - - throw new PlatformNotSupportedException(); - } - - internal static string GetFullPath() - { - return Path.Combine(GetDirectory(), GetFileName()); - } - - static string GetDirectory() - { - string directory; - if (TestLibrary.Utilities.IsNativeAot) - { - // NativeAOT test is put in a native/ subdirectory, so we want the parent - // directory that contains the native library to load - directory = new DirectoryInfo(AppContext.BaseDirectory).Parent.FullName; - } - else - { - directory = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); - } - - return directory; - } + return p; } } diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.cpp b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.cpp index 0dc14139fa80a..e19d5f1182ad3 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.cpp +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.cpp @@ -41,8 +41,3 @@ extern "C" DLL_EXPORT void FillOutIntParameter(intptr_t *p) *p = 50; } -extern "C" DLL_EXPORT intptr_t* ReturnParameter(intptr_t *p) -{ - return p; -} - diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.def b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.def deleted file mode 100644 index d2e55788d1da2..0000000000000 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointerNative.def +++ /dev/null @@ -1,7 +0,0 @@ -LIBRARY - EXPORTS - GetVoidVoidFcnPtr - CheckFcnPtr - FillOutPtr - FillOutIntParameter - ReturnParameter From fc79d98f859d03daa80ab4d12763a4d3c759b0a2 Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 15 Feb 2024 23:13:33 +0900 Subject: [PATCH 38/44] Update tests --- .../MarshalAPI/FunctionPointer/FunctionPointer.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index cf1d325d2dcb0..90d4146b48178 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -150,17 +150,18 @@ struct BlittableGeneric public int X; } - [Fact] - public static void RunGenericFunctionPointerTest() + [Theory] + [InlineData([-0f, 0f, 1f, -1f, 42f, 60f])] + public static void RunGenericFunctionPointerTest(float inVal) { Console.WriteLine($"Running {nameof(RunGenericFunctionPointerTest)}..."); int outVar = 0; - int expectedValue = 42; + int expectedValue = Convert.ToInt32(inVal); Console.WriteLine("Testing GenericCalli with int as the return type"); unsafe { - outVar = GenericCaller.GenericCalli((delegate* unmanaged)&UnmanagedExportedFunction, 42.0f); + outVar = GenericCaller.GenericCalli((delegate* unmanaged)&UnmanagedExportedFunction, inVal); } Assert.Equal(expectedValue, outVar); @@ -168,7 +169,7 @@ public static void RunGenericFunctionPointerTest() Console.WriteLine("Testing GenericCalli with BlittableGeneric as the return type"); unsafe { - outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged>)&UnmanagedExportedFunctionBlittableGenericInt, 42.0f).X; + outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged>)&UnmanagedExportedFunctionBlittableGenericInt, inVal).X; } Assert.Equal(expectedValue, outVar); @@ -176,7 +177,7 @@ public static void RunGenericFunctionPointerTest() Console.WriteLine("Testing GenericCalli with BlittableGeneric as the return type"); unsafe { - outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged>)&UnmanagedExportedFunctionBlittableGenericString, 42.0f).X; + outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged>)&UnmanagedExportedFunctionBlittableGenericString, inVal).X; } Assert.Equal(expectedValue, outVar); } From 22ecfb261eec8ac2c17bc69fc6528560ff370f1c Mon Sep 17 00:00:00 2001 From: Steven He Date: Fri, 16 Feb 2024 01:23:58 +0900 Subject: [PATCH 39/44] Fix tests --- .../Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index 90d4146b48178..abf54d211f6a6 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -151,7 +151,12 @@ struct BlittableGeneric } [Theory] - [InlineData([-0f, 0f, 1f, -1f, 42f, 60f])] + [InlineData(-0f)] + [InlineData(0f)] + [InlineData(1f)] + [InlineData(-1f)] + [InlineData(42f)] + [InlineData(60f)] public static void RunGenericFunctionPointerTest(float inVal) { Console.WriteLine($"Running {nameof(RunGenericFunctionPointerTest)}..."); From 9518dd93311f78631e08b39208d50465eea0fc10 Mon Sep 17 00:00:00 2001 From: Steve Date: Fri, 16 Feb 2024 02:38:35 +0900 Subject: [PATCH 40/44] Apply suggestions from code review Co-authored-by: Jan Kotas --- src/coreclr/vm/ceeload.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index 3c07894ca380c..42874997db519 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -4833,7 +4833,7 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad if (classInstCount != 0) { - TypeHandle* pClassInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(classInstCount * sizeof(TypeHandle)))); + TypeHandle* pClassInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(classInstCount) * S_SIZE_T(sizeof(TypeHandle)))); for (DWORD i = 0; i < classInstCount; i++) { pClassInst[i] = typeContext->m_classInst[i]; @@ -4843,7 +4843,7 @@ VASigCookie *Module::GetVASigCookieWorker(Module* pDefiningModule, Module* pLoad if (methodInstCount != 0) { - TypeHandle* pMethodInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(methodInstCount * sizeof(TypeHandle)))); + TypeHandle* pMethodInst = (TypeHandle*)(void*)amt.Track(pLoaderAllocator->GetHighFrequencyHeap()->AllocMem(S_SIZE_T(methodInstCount) * S_SIZE_T(sizeof(TypeHandle)))); for (DWORD i = 0; i < methodInstCount; i++) { pMethodInst[i] = typeContext->m_methodInst[i]; From c9c0881a9b19ddf5886e518192707a81f0faaa96 Mon Sep 17 00:00:00 2001 From: Steve Date: Fri, 16 Feb 2024 10:26:05 +0900 Subject: [PATCH 41/44] Make xUnit happy --- src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index abf54d211f6a6..e0a87c9e074b5 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -151,7 +151,6 @@ struct BlittableGeneric } [Theory] - [InlineData(-0f)] [InlineData(0f)] [InlineData(1f)] [InlineData(-1f)] From 3974945cfeda3c5e079b5869cac76d999ae10b7d Mon Sep 17 00:00:00 2001 From: Steven He Date: Fri, 16 Feb 2024 16:25:43 +0900 Subject: [PATCH 42/44] test mono ci --- .../Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs | 4 ++-- src/tests/issues.targets | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index e0a87c9e074b5..b3d7273701b45 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -185,7 +185,7 @@ public static void RunGenericFunctionPointerTest(float inVal) } Assert.Equal(expectedValue, outVar); } - +/* public static bool CanRunInvalidGenericFunctionPointerTest => !TestLibrary.Utilities.IsNativeAot; [ConditionalFact(nameof(CanRunInvalidGenericFunctionPointerTest))] @@ -203,7 +203,7 @@ public static void RunInvalidGenericFunctionPointerTest() Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); } } - +*/ [UnmanagedCallersOnly] static unsafe nint* ReturnParameter(nint* p) { diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 68ed725f4aa97..f78799d8c7e27 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1221,9 +1221,6 @@ Crashes during LLVM AOT compilation. - - Generic calli support not implemented yet in mono. - Crashes during LLVM AOT compilation. From a8d38b0e2b221cbf4d56838bb6905ba998d00d3e Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 21 Feb 2024 20:33:15 +0900 Subject: [PATCH 43/44] Tweak the tests --- src/tests/Interop/Interop.csproj | 2 + .../FunctionPointer/FunctionPointer.cs | 96 ---------------- .../FunctionPointer/GenericFunctionPointer.cs | 107 ++++++++++++++++++ 3 files changed, 109 insertions(+), 96 deletions(-) create mode 100644 src/tests/Interop/MarshalAPI/FunctionPointer/GenericFunctionPointer.cs diff --git a/src/tests/Interop/Interop.csproj b/src/tests/Interop/Interop.csproj index 6fc22f7bf4f27..b525cff832be6 100644 --- a/src/tests/Interop/Interop.csproj +++ b/src/tests/Interop/Interop.csproj @@ -21,6 +21,8 @@ + + diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs index b3d7273701b45..3dbcb6aaf3980 100644 --- a/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/FunctionPointer.cs @@ -113,100 +113,4 @@ public static void RunGetDelForOutIntTest() } Assert.Equal(expectedValue, outVar); } - - [UnmanagedCallersOnly] - static int UnmanagedExportedFunction(float arg) - { - return Convert.ToInt32(arg); - } - - [UnmanagedCallersOnly] - static BlittableGeneric UnmanagedExportedFunctionBlittableGenericInt(float arg) - { - return new() { X = Convert.ToInt32(arg) }; - } - - [UnmanagedCallersOnly] - static BlittableGeneric UnmanagedExportedFunctionBlittableGenericString(float arg) - { - return new() { X = Convert.ToInt32(arg) }; - } - - class GenericCaller - { - internal static unsafe T GenericCalli(void* fnptr, U arg) - { - return ((delegate* unmanaged)fnptr)(arg); - } - - internal static unsafe BlittableGeneric WrappedGenericCalli(void* fnptr, U arg) - { - return ((delegate* unmanaged>)fnptr)(arg); - } - } - - struct BlittableGeneric - { - public int X; - } - - [Theory] - [InlineData(0f)] - [InlineData(1f)] - [InlineData(-1f)] - [InlineData(42f)] - [InlineData(60f)] - public static void RunGenericFunctionPointerTest(float inVal) - { - Console.WriteLine($"Running {nameof(RunGenericFunctionPointerTest)}..."); - int outVar = 0; - int expectedValue = Convert.ToInt32(inVal); - - Console.WriteLine("Testing GenericCalli with int as the return type"); - unsafe - { - outVar = GenericCaller.GenericCalli((delegate* unmanaged)&UnmanagedExportedFunction, inVal); - } - Assert.Equal(expectedValue, outVar); - - outVar = 0; - Console.WriteLine("Testing GenericCalli with BlittableGeneric as the return type"); - unsafe - { - outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged>)&UnmanagedExportedFunctionBlittableGenericInt, inVal).X; - } - Assert.Equal(expectedValue, outVar); - - outVar = 0; - Console.WriteLine("Testing GenericCalli with BlittableGeneric as the return type"); - unsafe - { - outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged>)&UnmanagedExportedFunctionBlittableGenericString, inVal).X; - } - Assert.Equal(expectedValue, outVar); - } -/* - public static bool CanRunInvalidGenericFunctionPointerTest => !TestLibrary.Utilities.IsNativeAot; - - [ConditionalFact(nameof(CanRunInvalidGenericFunctionPointerTest))] - public static void RunInvalidGenericFunctionPointerTest() - { - Console.WriteLine($"Running {nameof(RunInvalidGenericFunctionPointerTest)}..."); - unsafe - { - nint fnptr = (nint)(delegate* unmanaged)&ReturnParameter; - Console.WriteLine("Testing GenericCalli with string as the parameter type"); - Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); - Console.WriteLine("Testing GenericCalli with string as the return type"); - Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); - Console.WriteLine("Testing GenericCalli with string as both the parameter and return type"); - Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); - } - } -*/ - [UnmanagedCallersOnly] - static unsafe nint* ReturnParameter(nint* p) - { - return p; - } } diff --git a/src/tests/Interop/MarshalAPI/FunctionPointer/GenericFunctionPointer.cs b/src/tests/Interop/MarshalAPI/FunctionPointer/GenericFunctionPointer.cs new file mode 100644 index 0000000000000..fe6d37f01a98f --- /dev/null +++ b/src/tests/Interop/MarshalAPI/FunctionPointer/GenericFunctionPointer.cs @@ -0,0 +1,107 @@ +// 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.IO; +using System.Reflection; +using System.Runtime.InteropServices; +using Xunit; + +public partial class FunctionPtr +{ + public static bool CanRunGenericFunctionPointerTest => !TestLibrary.Utilities.IsMonoRuntime; + public static bool CanRunInvalidGenericFunctionPointerTest => !TestLibrary.Utilities.IsNativeAot && !TestLibrary.Utilities.IsMonoRuntime; + + [UnmanagedCallersOnly] + static int UnmanagedExportedFunction(float arg) + { + return Convert.ToInt32(arg); + } + + [UnmanagedCallersOnly] + static BlittableGeneric UnmanagedExportedFunctionBlittableGenericInt(float arg) + { + return new() { X = Convert.ToInt32(arg) }; + } + + [UnmanagedCallersOnly] + static BlittableGeneric UnmanagedExportedFunctionBlittableGenericString(float arg) + { + return new() { X = Convert.ToInt32(arg) }; + } + + class GenericCaller + { + internal static unsafe T GenericCalli(void* fnptr, U arg) + { + return ((delegate* unmanaged)fnptr)(arg); + } + + internal static unsafe BlittableGeneric WrappedGenericCalli(void* fnptr, U arg) + { + return ((delegate* unmanaged>)fnptr)(arg); + } + } + + struct BlittableGeneric + { + public int X; + } + + [ConditionalTheory(nameof(CanRunGenericFunctionPointerTest))] + [InlineData(0f)] + [InlineData(1f)] + [InlineData(-1f)] + [InlineData(42f)] + [InlineData(60f)] + public static void RunGenericFunctionPointerTest(float inVal) + { + Console.WriteLine($"Running {nameof(RunGenericFunctionPointerTest)}..."); + int outVar = 0; + int expectedValue = Convert.ToInt32(inVal); + + Console.WriteLine("Testing GenericCalli with int as the return type"); + unsafe + { + outVar = GenericCaller.GenericCalli((delegate* unmanaged)&UnmanagedExportedFunction, inVal); + } + Assert.Equal(expectedValue, outVar); + + outVar = 0; + Console.WriteLine("Testing GenericCalli with BlittableGeneric as the return type"); + unsafe + { + outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged>)&UnmanagedExportedFunctionBlittableGenericInt, inVal).X; + } + Assert.Equal(expectedValue, outVar); + + outVar = 0; + Console.WriteLine("Testing GenericCalli with BlittableGeneric as the return type"); + unsafe + { + outVar = GenericCaller.WrappedGenericCalli((delegate* unmanaged>)&UnmanagedExportedFunctionBlittableGenericString, inVal).X; + } + Assert.Equal(expectedValue, outVar); + } + + [ConditionalFact(nameof(CanRunInvalidGenericFunctionPointerTest))] + public static void RunInvalidGenericFunctionPointerTest() + { + Console.WriteLine($"Running {nameof(RunInvalidGenericFunctionPointerTest)}..."); + unsafe + { + nint fnptr = (nint)(delegate* unmanaged)&ReturnParameter; + Console.WriteLine("Testing GenericCalli with string as the parameter type"); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); + Console.WriteLine("Testing GenericCalli with string as the return type"); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); + Console.WriteLine("Testing GenericCalli with string as both the parameter and return type"); + Assert.Throws(() => GenericCaller.GenericCalli((delegate* unmanaged)fnptr, "test")); + } + } + + [UnmanagedCallersOnly] + static unsafe nint* ReturnParameter(nint* p) + { + return p; + } +} From 151982af783f08ad18ddbca29f02f1dae550f928 Mon Sep 17 00:00:00 2001 From: Steve Date: Fri, 23 Feb 2024 01:13:31 +0900 Subject: [PATCH 44/44] Add link to an GH issue --- src/tests/Interop/Interop.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/Interop/Interop.csproj b/src/tests/Interop/Interop.csproj index b525cff832be6..0e25aba816182 100644 --- a/src/tests/Interop/Interop.csproj +++ b/src/tests/Interop/Interop.csproj @@ -21,7 +21,7 @@ - +