Simplify AssemblyName marshalling between VM and CoreLib

src/coreclr/System.Private.CoreLib/src/System/Reflection/AssemblyName.CoreCLR.cs

@@ -9,22 +9,99 @@
 
 namespace System.Reflection
 {
+    //
+    // Unmanaged view of AssemblyNameParser.AssemblyNameParts used to interop with the VM
+    //
+    internal unsafe struct NativeAssemblyNameParts
+    {
+        public char* _pName;
+        public ushort _major, _minor, _build, _revision;
+        public char* _pCultureName;
+        public byte* _pPublicKeyOrToken;
+        public int _cbPublicKeyOrToken;
+        public AssemblyNameFlags _flags;
+
+        //
+        // Native AssemblySpec stores uint16 components for the version. Managed AssemblyName.Version stores int32.
+        // When the former are initialized from the latter, the components are truncated to uint16 size.
+        // When the latter are initialized from the former, they are zero-extended to int32 size.
+        // For uint16 components, the max value is used to indicate an unspecified component.
+        //
+
+        public void SetVersion(Version? version, ushort defaultValue)
+        {
+            if (version != null)
+            {
+                _major = (ushort)version.Major;
+                _minor = (ushort)version.Minor;
+                _build = (ushort)version.Build;
+                _revision = (ushort)version.Revision;
+            }
+            else
+            {
+                _major = defaultValue;
+                _minor = defaultValue;
+                _build = defaultValue;
+                _revision = defaultValue;
+            }
+        }
+
+        public Version? GetVersion()
+        {
+            if (_major == ushort.MaxValue || _minor == ushort.MaxValue)
+                return null;
+
+            if (_build == ushort.MaxValue)
+                return new Version(_major, _minor);
+
+            if (_revision == ushort.MaxValue)
+                return new Version(_major, _minor, _build);
+
+            return new Version(_major, _minor, _build, _revision);
+        }
+    }
+
     public sealed partial class AssemblyName : ICloneable, IDeserializationCallback, ISerializable
     {
-        internal AssemblyName(string? name,
-            byte[]? publicKey,
-            byte[]? publicKeyToken,
-            Version? version,
-            CultureInfo? cultureInfo,
-            AssemblyNameFlags flags)
+        internal unsafe AssemblyName(NativeAssemblyNameParts* pParts)
             : this()
         {
-            _name = name;
-            _publicKey = publicKey;
-            _publicKeyToken = publicKeyToken;
-            _version = version;
-            _cultureInfo = cultureInfo;
-            _flags = flags;
+            if (pParts->_pName != null)
+            {
+                _name = new string(pParts->_pName);
+            }
+
+            if (pParts->_pCultureName != null)
+            {
+                _cultureInfo = new CultureInfo(new string(pParts->_pCultureName));
+            }
+
+            if (pParts->_pPublicKeyOrToken != null)
+            {
+                byte[] publicKeyOrToken = new ReadOnlySpan<byte>(pParts->_pPublicKeyOrToken, pParts->_cbPublicKeyOrToken).ToArray();
+
+                if ((pParts->_flags & AssemblyNameFlags.PublicKey) != 0)
+                {
+                    _publicKey = publicKeyOrToken;
+                }
+                else
+                {
+                    _publicKeyToken = publicKeyOrToken;
+                }
+            }
+
+            _version = pParts->GetVersion();
+
+            _flags = pParts->_flags;
+        }
+
+        internal byte[]? RawPublicKey => _publicKey;
+        internal byte[]? RawPublicKeyToken => _publicKeyToken;
+
+        internal AssemblyNameFlags RawFlags
+        {
+            get => _flags;
+            set => _flags = value;
         }
 
         internal void SetProcArchIndex(PortableExecutableKinds pek, ImageFileMachine ifm)
@@ -34,7 +111,7 @@ internal void SetProcArchIndex(PortableExecutableKinds pek, ImageFileMachine ifm
 #pragma warning restore SYSLIB0037
         }
 
-        internal static ProcessorArchitecture CalculateProcArchIndex(PortableExecutableKinds pek, ImageFileMachine ifm, AssemblyNameFlags flags)
+        private static ProcessorArchitecture CalculateProcArchIndex(PortableExecutableKinds pek, ImageFileMachine ifm, AssemblyNameFlags flags)
         {
             if (((uint)flags & 0xF0) == 0x70)
                 return ProcessorArchitecture.None;

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/AssemblyBuilder.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Generic;
+using System.Configuration.Assemblies;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Diagnostics.SymbolStore;
@@ -54,23 +55,11 @@ internal AssemblyBuilder(AssemblyName name,
                 // know how to set the correct context of the new dynamic assembly.
                 throw new InvalidOperationException();
             }
-            if (assemblyLoadContext == null)
-            {
-                assemblyLoadContext = AssemblyLoadContext.GetLoadContext(callingAssembly);
-            }
-
-            // Clone the name in case the caller modifies it underneath us.
-            name = (AssemblyName)name.Clone();
-
-            RuntimeAssembly? retAssembly = null;
-            CreateDynamicAssembly(ObjectHandleOnStack.Create(ref name),
-                                  (int)access,
-                                  ObjectHandleOnStack.Create(ref assemblyLoadContext),
-                                  ObjectHandleOnStack.Create(ref retAssembly));
-            _internalAssembly = retAssembly!;
 
             _access = access;
 
+            _internalAssembly = CreateDynamicAssembly(assemblyLoadContext ?? AssemblyLoadContext.GetLoadContext(callingAssembly)!, name, access);
+
             // Make sure that ManifestModule is properly initialized
             // We need to do this before setting any CustomAttribute
             // Note that this ModuleBuilder cannot be used for RefEmit yet
@@ -117,10 +106,44 @@ public static AssemblyBuilder DefineDynamicAssembly(
         }
 
         [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "AppDomain_CreateDynamicAssembly")]
-        private static partial void CreateDynamicAssembly(ObjectHandleOnStack name,
-                                                         int access,
-                                                         ObjectHandleOnStack assemblyLoadContext,
-                                                         ObjectHandleOnStack retAssembly);
+        private static unsafe partial void CreateDynamicAssembly(ObjectHandleOnStack assemblyLoadContext,
+                                                                 NativeAssemblyNameParts* pAssemblyName,
+                                                                 AssemblyHashAlgorithm hashAlgId,
+                                                                 AssemblyBuilderAccess access,
+                                                                 ObjectHandleOnStack retAssembly);
+
+        private static unsafe RuntimeAssembly CreateDynamicAssembly(AssemblyLoadContext assemblyLoadContext, AssemblyName name, AssemblyBuilderAccess access)
+        {
+            RuntimeAssembly? retAssembly = null;
+
+            byte[]? publicKey = name.GetPublicKey();
+
+            fixed (char* pName = name.Name)
+            fixed (char* pCultureName = name.CultureName)
+            fixed (byte* pPublicKey = publicKey)
+            {
+                NativeAssemblyNameParts nameParts = default;
+
+                nameParts._flags = name.RawFlags;
+                nameParts._pName = pName;
+                nameParts._pCultureName = pCultureName;
+
+                nameParts._pPublicKeyOrToken = pPublicKey;
+                nameParts._cbPublicKeyOrToken = (publicKey != null) ? publicKey.Length : 0;
+
+                nameParts.SetVersion(name.Version, defaultValue: 0);
+
+#pragma warning disable SYSLIB0037 // AssemblyName.HashAlgorithm is obsolete
+                CreateDynamicAssembly(ObjectHandleOnStack.Create(ref assemblyLoadContext),
+                                  &nameParts,
+                                  name.HashAlgorithm,
+                                  access,
+                                  ObjectHandleOnStack.Create(ref retAssembly));
+#pragma warning restore SYSLIB0037
+            }
+
+            return retAssembly!;
+        }
 
         private static readonly object s_assemblyBuilderLock = new object();
 

src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs

@@ -119,17 +119,17 @@ public override string? CodeBase
         // is returned.
         public override AssemblyName GetName(bool copiedName)
         {
-            string? codeBase = GetCodeBase();
+            var an = new AssemblyName();
+            an.Name = GetSimpleName();
+            an.Version = GetVersion();
+            an.CultureInfo = GetLocale();
+
+            an.SetPublicKey(GetPublicKey());
 
-            var an = new AssemblyName(GetSimpleName(),
-                    GetPublicKey(),
-                    null, // public key token
-                    GetVersion(),
-                    GetLocale(),
-                    GetFlags() | AssemblyNameFlags.PublicKey);
+            an.RawFlags = GetFlags() | AssemblyNameFlags.PublicKey;
 
 #pragma warning disable IL3000 // System.Reflection.AssemblyName.CodeBase' always returns an empty string for assemblies embedded in a single-file app.
-            an.CodeBase = codeBase;
+            an.CodeBase = GetCodeBase();
 #pragma warning restore IL3000
 
 #pragma warning disable SYSLIB0037 // AssemblyName.HashAlgorithm is obsolete
@@ -341,27 +341,56 @@ public override IList<CustomAttributeData> GetCustomAttributesData()
         internal static RuntimeAssembly InternalLoad(string assemblyName, ref StackCrawlMark stackMark, AssemblyLoadContext? assemblyLoadContext = null)
             => InternalLoad(new AssemblyName(assemblyName), ref stackMark, assemblyLoadContext);
 
-        internal static RuntimeAssembly InternalLoad(AssemblyName assemblyName, ref StackCrawlMark stackMark, AssemblyLoadContext? assemblyLoadContext = null)
-            => InternalLoad(assemblyName, requestingAssembly: null, ref stackMark, throwOnFileNotFound: true, assemblyLoadContext);
-
-        internal static RuntimeAssembly InternalLoad(AssemblyName assemblyName,
-                                                     RuntimeAssembly? requestingAssembly,
-                                                     ref StackCrawlMark stackMark,
-                                                     bool throwOnFileNotFound,
-                                                     AssemblyLoadContext? assemblyLoadContext = null)
+        internal static unsafe RuntimeAssembly InternalLoad(AssemblyName assemblyName,
+                                                            ref StackCrawlMark stackMark,
+                                                            AssemblyLoadContext? assemblyLoadContext = null,
+                                                            RuntimeAssembly? requestingAssembly = null,
+                                                            bool throwOnFileNotFound = true)
         {
             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));
+
+            AssemblyNameFlags flags = assemblyName.RawFlags;
+
+            // Note that we prefer to take a public key token if present,
+            // even if flags indicate a full public key
+            byte[]? publicKeyOrToken;
+            if ((publicKeyOrToken = assemblyName.RawPublicKeyToken) != null)
+            {
+                flags |= ~AssemblyNameFlags.PublicKey;
+            }
+            else if ((publicKeyOrToken = assemblyName.RawPublicKey) != null)
+            {
+                flags |= AssemblyNameFlags.PublicKey;
+            }
+
+            fixed (char* pName = assemblyName.Name)
+            fixed (char* pCultureName = assemblyName.CultureName)
+            fixed (byte* pPublicKeyOrToken = publicKeyOrToken)
+            {
+                NativeAssemblyNameParts nameParts = default;
+
+                nameParts._flags = flags;
+                nameParts._pName = pName;
+                nameParts._pCultureName = pCultureName;
+
+                nameParts._pPublicKeyOrToken = pPublicKeyOrToken;
+                nameParts._cbPublicKeyOrToken = (publicKeyOrToken != null) ? publicKeyOrToken.Length : 0;
+
+                nameParts.SetVersion(assemblyName.Version, defaultValue: ushort.MaxValue);
+
+                InternalLoad(&nameParts,
+                             ObjectHandleOnStack.Create(ref requestingAssembly),
+                             new StackCrawlMarkHandle(ref stackMark),
+                             throwOnFileNotFound,
+                             ObjectHandleOnStack.Create(ref assemblyLoadContext),
+                             ObjectHandleOnStack.Create(ref retAssembly));
+            }
+
             return retAssembly!;
         }
 
         [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "AssemblyNative_InternalLoad")]
-        private static partial void InternalLoad(ObjectHandleOnStack assemblyName,
+        private static unsafe partial void InternalLoad(NativeAssemblyNameParts* pAssemblyNameParts,
                                                 ObjectHandleOnStack requestingAssembly,
                                                 StackCrawlMarkHandle stackMark,
                                                 [MarshalAs(UnmanagedType.Bool)] bool throwOnFileNotFound,
@@ -509,7 +538,7 @@ private static partial void GetVersion(QCallAssembly assembly,
                                               out int buildNum,
                                               out int revNum);
 
-        internal Version GetVersion()
+        private Version GetVersion()
         {
             RuntimeAssembly runtimeAssembly = this;
             GetVersion(new QCallAssembly(ref runtimeAssembly), out int majorVer, out int minorVer, out int build, out int revision);
@@ -519,7 +548,7 @@ internal Version GetVersion()
         [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "AssemblyNative_GetLocale")]
         private static partial void GetLocale(QCallAssembly assembly, StringHandleOnStack retString);
 
-        internal CultureInfo GetLocale()
+        private CultureInfo GetLocale()
         {
             string? locale = null;
 
@@ -605,7 +634,7 @@ public override Assembly GetSatelliteAssembly(CultureInfo culture, Version? vers
             // 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 = InternalLoad(an, this, ref unused, throwOnFileNotFound);
+            RuntimeAssembly? retAssembly = InternalLoad(an, ref unused, requestingAssembly: this, throwOnFileNotFound: throwOnFileNotFound);
 
             if (retAssembly == this)
             {

src/coreclr/vm/appdomainnative.cpp

@@ -12,40 +12,24 @@
 #include "eventtrace.h"
 #include "../binder/inc/defaultassemblybinder.h"
 
-#include "clr/fs/path.h"
-using namespace clr::fs;
-
 // static
-extern "C" void QCALLTYPE AppDomain_CreateDynamicAssembly(QCall::ObjectHandleOnStack assemblyName, INT32 access, QCall::ObjectHandleOnStack assemblyLoadContext, QCall::ObjectHandleOnStack retAssembly)
+extern "C" void QCALLTYPE AppDomain_CreateDynamicAssembly(QCall::ObjectHandleOnStack assemblyLoadContext, NativeAssemblyNameParts* pAssemblyNameParts, INT32 hashAlgorithm, INT32 access, QCall::ObjectHandleOnStack retAssembly)
 {
     QCALL_CONTRACT;
 
-    BEGIN_QCALL
+    BEGIN_QCALL;
 
     GCX_COOP();
 
-    //<TODO>
-    // @TODO: there MUST be a better way to do this...
-    //</TODO>
-    CreateDynamicAssemblyArgs   args;
-    ZeroMemory(&args, sizeof(args));
-
-    GCPROTECT_BEGIN((CreateDynamicAssemblyArgsGC&)args);
-
-    args.assemblyName           = (ASSEMBLYNAMEREF)assemblyName.Get();
-    args.loaderAllocator        = NULL;
-
-    args.access                 = access;
-
-    Assembly*       pAssembly = nullptr;
-    AssemblyBinder* pBinder = nullptr;
+    LOADERALLOCATORREF keepAlive = NULL;
+    GCPROTECT_BEGIN(keepAlive);
 
     _ASSERTE(assemblyLoadContext.Get() != NULL);
 
     INT_PTR nativeAssemblyBinder = ((ASSEMBLYLOADCONTEXTREF)assemblyLoadContext.Get())->GetNativeAssemblyBinder();
-    pBinder = reinterpret_cast<AssemblyBinder*>(nativeAssemblyBinder);
+    AssemblyBinder* pBinder = reinterpret_cast<AssemblyBinder*>(nativeAssemblyBinder);
 
-    pAssembly = Assembly::CreateDynamic(GetAppDomain(), pBinder, &args);
+    Assembly* pAssembly = Assembly::CreateDynamic(pBinder, pAssemblyNameParts, hashAlgorithm, access, &keepAlive);
 
     retAssembly.Set(pAssembly->GetExposedObject());
 

src/coreclr/vm/appdomainnative.hpp

@@ -23,6 +23,6 @@ class AppDomainNative
     static FCDECL1(Object*, GetOrInternString, StringObject* pStringUNSAFE);
     static FCDECL1(Object*, IsStringInterned, StringObject* pString);
 };
-extern "C" void QCALLTYPE AppDomain_CreateDynamicAssembly(QCall::ObjectHandleOnStack assemblyName, INT32 access, QCall::ObjectHandleOnStack assemblyLoadContext, QCall::ObjectHandleOnStack retAssembly);
+extern "C" void QCALLTYPE AppDomain_CreateDynamicAssembly(QCall::ObjectHandleOnStack assemblyLoadContext, NativeAssemblyNameParts* pAssemblyName, INT32 hashAlgorithm, INT32 access, QCall::ObjectHandleOnStack retAssembly);
 
 #endif