Convert AssemblyNative::Load to QCall (#1929)

* Convert AssemblyNative::Load to QCall

- FCall uses libunwind to find Method description.
- Get rid of the libunwind overhead in AssemblyNative::Load
  by converting to QCall

* Add workaround for https://github.com/dotnet/runtime/issues/2240

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs
index 6d8f2dc..8fbf876 100644 (file)
--- a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs
+++ b/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs
@@ -313,14 +313,31 @@ namespace System.Reflection
             => InternalLoad(new AssemblyName(assemblyName), ref stackMark, assemblyLoadContext);
 
         internal static RuntimeAssembly InternalLoad(AssemblyName assemblyName, ref StackCrawlMark stackMark, AssemblyLoadContext? assemblyLoadContext = null)
-            => nLoad(assemblyName, requestingAssembly: null, ref stackMark, throwOnFileNotFound: true, assemblyLoadContext);
+            => InternalLoad(assemblyName, requestingAssembly: null, ref stackMark, throwOnFileNotFound: true, assemblyLoadContext);
 
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        private static extern RuntimeAssembly nLoad(AssemblyName assemblyName,
-                                                    RuntimeAssembly? requestingAssembly,
-                                                    ref StackCrawlMark stackMark,
-                                                    bool throwOnFileNotFound,
-                                                    AssemblyLoadContext? assemblyLoadContext = null);
+        internal static RuntimeAssembly InternalLoad(AssemblyName assemblyName,
+                                                     RuntimeAssembly? requestingAssembly,
+                                                     ref StackCrawlMark stackMark,
+                                                     bool throwOnFileNotFound,
+                                                     AssemblyLoadContext? assemblyLoadContext = null)
+        {
+            RuntimeAssembly? retAssembly = null;
+            InternalLoad(ObjectHandleOnStack.Create(ref assemblyName),
+                         ObjectHandleOnStack.Create(ref requestingAssembly),
+                         new StackCrawlMarkHandle(ref stackMark),
+                         throwOnFileNotFound,
+                         ObjectHandleOnStack.Create(ref assemblyLoadContext),
+                         ObjectHandleOnStack.Create(ref retAssembly));
+            return retAssembly;
+        }
+
+        [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)]
+        private static extern void InternalLoad(ObjectHandleOnStack assemblyName,
+                                                ObjectHandleOnStack requestingAssembly,
+                                                StackCrawlMarkHandle stackMark,
+                                                bool throwOnFileNotFound,
+                                                ObjectHandleOnStack assemblyLoadContext,
+                                                ObjectHandleOnStack retAssembly);
 
         public override bool ReflectionOnly => false;
 
@@ -545,7 +562,7 @@ namespace System.Reflection
             // This stack crawl mark is never used because the requesting assembly is explicitly specified,
             // so the value could be anything.
             StackCrawlMark unused = default;
-            RuntimeAssembly? retAssembly = nLoad(an, this, ref unused, throwOnFileNotFound);
+            RuntimeAssembly? retAssembly = InternalLoad(an, this, ref unused, throwOnFileNotFound);
 
             if (retAssembly == this)
             {

diff --git a/src/coreclr/src/vm/assemblynative.cpp b/src/coreclr/src/vm/assemblynative.cpp
index 98b15c4..acb6c4d 100644 (file)
--- a/src/coreclr/src/vm/assemblynative.cpp
+++ b/src/coreclr/src/vm/assemblynative.cpp
@@ -31,66 +31,68 @@
 #include "../binder/inc/bindertracing.h"
 #include "../binder/inc/clrprivbindercoreclr.h"
 
-FCIMPL5(Object*, AssemblyNative::Load, AssemblyNameBaseObject* assemblyNameUNSAFE,
-        AssemblyBaseObject* requestingAssemblyUNSAFE,
-        StackCrawlMark* stackMark,
-        CLR_BOOL fThrowOnFileNotFound,
-        AssemblyLoadContextBaseObject *assemblyLoadContextUNSAFE)
+/* static */
+void QCALLTYPE AssemblyNative::InternalLoad(QCall::ObjectHandleOnStack assemblyName,
+                                            QCall::ObjectHandleOnStack requestingAssembly,
+                                            QCall::StackCrawlMarkHandle stackMark,
+                                            BOOL fThrowOnFileNotFound,
+                                            QCall::ObjectHandleOnStack assemblyLoadContext,
+                                            QCall::ObjectHandleOnStack retAssembly)
 {
-    FCALL_CONTRACT;
+    QCALL_CONTRACT;
 
-    struct _gc
-    {
-        ASSEMBLYNAMEREF        assemblyName;
-        ASSEMBLYREF            requestingAssembly;
-        ASSEMBLYREF            rv;
-        ASSEMBLYLOADCONTEXTREF assemblyLoadContext;
-    } gc;
+    BEGIN_QCALL;
 
-    gc.assemblyName        = (ASSEMBLYNAMEREF)        assemblyNameUNSAFE;
-    gc.requestingAssembly  = (ASSEMBLYREF)            requestingAssemblyUNSAFE;
-    gc.rv                  = NULL;
-    gc.assemblyLoadContext = (ASSEMBLYLOADCONTEXTREF) assemblyLoadContextUNSAFE;
+    GCX_COOP();
 
-    HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc);
+    // Workaround for https://github.com/dotnet/runtime/issues/2240
+    FrameWithCookie<ProtectValueClassFrame> workaround;
 
-    if (gc.assemblyName == NULL)
+    if (assemblyName.Get() == NULL)
+    {
         COMPlusThrow(kArgumentNullException, W("ArgumentNull_AssemblyName"));
-
+    }
     ACQUIRE_STACKING_ALLOCATOR(pStackingAllocator);
 
     DomainAssembly * pParentAssembly = NULL;
     Assembly * pRefAssembly = NULL;
+    ICLRPrivBinder *pBinderContext = NULL;
 
-    INT_PTR ptrLoadContextBinder = (gc.assemblyLoadContext != NULL) ? gc.assemblyLoadContext->GetNativeAssemblyLoadContext() : NULL;
+    if (assemblyLoadContext.Get() != NULL)
+    {
+        INT_PTR nativeAssemblyLoadContext = ((ASSEMBLYLOADCONTEXTREF)assemblyLoadContext.Get())->GetNativeAssemblyLoadContext();
+        pBinderContext = reinterpret_cast<ICLRPrivBinder*>(nativeAssemblyLoadContext);
+    }
+
+    AssemblySpec spec;
+    ASSEMBLYNAMEREF assemblyNameRef = NULL;
 
-    if(gc.assemblyName->GetSimpleName() == NULL)
+    GCPROTECT_BEGIN(assemblyNameRef);
+    assemblyNameRef = (ASSEMBLYNAMEREF)assemblyName.Get();
+    if (assemblyNameRef->GetSimpleName() == NULL)
     {
         COMPlusThrow(kArgumentException, W("Format_StringZeroLength"));
     }
-    else
-    {
-        // Compute parent assembly
-        if (gc.requestingAssembly != NULL)
-        {
-            pRefAssembly = gc.requestingAssembly->GetAssembly();
-        }
-        else if (ptrLoadContextBinder == NULL)
-        {
-            pRefAssembly = SystemDomain::GetCallersAssembly(stackMark);
-        }
 
-        if (pRefAssembly)
-        {
-            pParentAssembly = pRefAssembly->GetDomainAssembly();
-        }
+    // Compute parent assembly
+    if (requestingAssembly.Get() != NULL)
+    {
+        pRefAssembly = ((ASSEMBLYREF)requestingAssembly.Get())->GetAssembly();
+    }
+    else if (pBinderContext == NULL)
+    {
+        pRefAssembly = SystemDomain::GetCallersAssembly(stackMark);
+    }
+    if (pRefAssembly)
+    {
+        pParentAssembly = pRefAssembly->GetDomainAssembly();
     }
 
     // Initialize spec
-    AssemblySpec spec;
     spec.InitializeSpec(pStackingAllocator,
-                        &gc.assemblyName,
+                        &assemblyNameRef,
                         FALSE);
+    GCPROTECT_END();
 
     spec.SetCodeBase(NULL);
 
@@ -104,9 +106,9 @@ FCIMPL5(Object*, AssemblyNative::Load, AssemblyNameBaseObject* assemblyNameUNSAF
 
     // Have we been passed the reference to the binder against which this load should be triggered?
     // If so, then use it to set the fallback load context binder.
-    if (ptrLoadContextBinder != NULL)
+    if (pBinderContext != NULL)
     {
-        spec.SetFallbackLoadContextBinderForRequestingAssembly(reinterpret_cast<ICLRPrivBinder *>(ptrLoadContextBinder));
+        spec.SetFallbackLoadContextBinderForRequestingAssembly(pBinderContext);
         spec.SetPreferFallbackLoadContextBinder();
     }
     else if (pRefAssembly != NULL)
@@ -118,20 +120,20 @@ FCIMPL5(Object*, AssemblyNative::Load, AssemblyNameBaseObject* assemblyNameUNSAF
     }
 
     Assembly *pAssembly;
-
     {
         GCX_PREEMP();
         pAssembly = spec.LoadAssembly(FILE_LOADED, fThrowOnFileNotFound);
     }
 
     if (pAssembly != NULL)
-        gc.rv = (ASSEMBLYREF) pAssembly->GetExposedObject();
+    {
+        retAssembly.Set(pAssembly->GetExposedObject());
+    }
 
-    HELPER_METHOD_FRAME_END();
+    workaround.Pop();
 
-    return OBJECTREFToObject(gc.rv);
+    END_QCALL;
 }
-FCIMPLEND
 
 /* static */
 Assembly* AssemblyNative::LoadFromPEImage(ICLRPrivBinder* pBinderContext, PEImage *pILImage, PEImage *pNIImage)

diff --git a/src/coreclr/src/vm/assemblynative.hpp b/src/coreclr/src/vm/assemblynative.hpp
index 7a5d571..6cc131e 100644 (file)
--- a/src/coreclr/src/vm/assemblynative.hpp
+++ b/src/coreclr/src/vm/assemblynative.hpp
@@ -32,12 +32,6 @@ public:
     static
     void QCALLTYPE GetExecutingAssembly(QCall::StackCrawlMarkHandle stackMark, QCall::ObjectHandleOnStack retAssembly);
 
-    static FCDECL5(Object*,         Load,                       AssemblyNameBaseObject* assemblyNameUNSAFE,
-                                                                AssemblyBaseObject* requestingAssemblyUNSAFE,
-                                                                StackCrawlMark* stackMark,
-                                                                CLR_BOOL fThrowOnFileNotFound,
-                                                                AssemblyLoadContextBaseObject *assemblyLoadContextUNSAFE);
-
     static FCDECL0(FC_BOOL_RET, IsTracingEnabled);
 
     //
@@ -120,6 +114,7 @@ public:
 
     static INT_PTR QCALLTYPE InitializeAssemblyLoadContext(INT_PTR ptrManagedAssemblyLoadContext, BOOL fRepresentsTPALoadContext, BOOL fIsCollectible);
     static void QCALLTYPE PrepareForAssemblyLoadContextRelease(INT_PTR ptrNativeAssemblyLoadContext, INT_PTR ptrManagedStrongAssemblyLoadContext);
+    static void QCALLTYPE InternalLoad(QCall::ObjectHandleOnStack assemblyName, QCall::ObjectHandleOnStack requestingAssembly, QCall::StackCrawlMarkHandle stackMark,BOOL fThrowOnFileNotFound, QCall::ObjectHandleOnStack assemblyLoadContext, QCall::ObjectHandleOnStack retAssembly);
     static void QCALLTYPE LoadFromPath(INT_PTR ptrNativeAssemblyLoadContext, LPCWSTR pwzILPath, LPCWSTR pwzNIPath, QCall::ObjectHandleOnStack retLoadedAssembly);
     static void QCALLTYPE LoadFromStream(INT_PTR ptrNativeAssemblyLoadContext, INT_PTR ptrAssemblyArray, INT32 cbAssemblyArrayLength, INT_PTR ptrSymbolArray, INT32 cbSymbolArrayLength, QCall::ObjectHandleOnStack retLoadedAssembly);
 #ifndef FEATURE_PAL

diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h
index 79039e9..44f977d 100644 (file)
--- a/src/coreclr/src/vm/ecalllist.h
+++ b/src/coreclr/src/vm/ecalllist.h
@@ -475,7 +475,7 @@ FCFuncStart(gRuntimeAssemblyFuncs)
     QCFuncElement("GetSimpleName", AssemblyNative::GetSimpleName)
     QCFuncElement("GetVersion", AssemblyNative::GetVersion)
     FCFuncElement("FCallIsDynamic", AssemblyNative::IsDynamic)
-    FCFuncElement("nLoad", AssemblyNative::Load)
+    QCFuncElement("InternalLoad", AssemblyNative::InternalLoad)
     QCFuncElement("GetType", AssemblyNative::GetType)
     QCFuncElement("GetForwardedType", AssemblyNative::GetForwardedType)
     QCFuncElement("GetManifestResourceInfo", AssemblyNative::GetManifestResourceInfo)

diff --git a/src/coreclr/src/vm/qcall.h b/src/coreclr/src/vm/qcall.h
index fbcc1dc..9753592 100644 (file)
--- a/src/coreclr/src/vm/qcall.h
+++ b/src/coreclr/src/vm/qcall.h
@@ -197,6 +197,12 @@ public:
     {
         Object ** m_ppObject;
 
+        OBJECTREF Get()
+        {
+            LIMITED_METHOD_CONTRACT;
+            return ObjectToOBJECTREF(*m_ppObject);
+        }
+
 #ifndef DACCESS_COMPILE
         //
         // Helpers for returning common managed types from QCall