Moved AssemblyName helpers to managed (part 2) (#63915)

* implement GetAssemblyName via dynamic call to MetadataReader * A few more file-locking tests. * fix #28153 * no need for version when getting MetadataReader * rename the argument to match AssemblyName * perf tweaks * use memory-mapped file to read metadata * adjust tests for the new implementation * use "bufferSize: 1" when stream is going to be mapped. * null-conditional operator. * do Dispose before re-throwing * get rid of the platform-specific/native stuff * remove assemblyname.hpp * remove `VerifyIsAssembly()` * PR feedback * put back gStdMngIEnumerableFuncs and the others
dotnet · Jan 26, 2022 · 2cf4826 · 2cf4826
1 parent cbc4cd8
commit 2cf4826
Show file tree

Hide file tree

Showing 29 changed files with 100 additions and 296 deletions.
diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/AssemblyName.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/AssemblyName.CoreCLR.cs
@@ -32,17 +32,6 @@ internal AssemblyName(string? name,
             _flags = flags;
         }
 
-        // This call opens and closes the file, but does not add the
-        // assembly to the domain.
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        internal static extern AssemblyName nGetFileInformation(string s);
-
-        internal static AssemblyName GetFileInformationCore(string assemblyFile)
-        {
-            string fullPath = Path.GetFullPath(assemblyFile);
-            return nGetFileInformation(fullPath);
-        }
-
         internal void SetProcArchIndex(PortableExecutableKinds pek, ImageFileMachine ifm)
         {
 #pragma warning disable SYSLIB0037 // AssemblyName.ProcessorArchitecture is obsolete

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj b/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj
@@ -129,7 +129,6 @@
     <Compile Include="System\Reflection\AssemblyRuntimeNameHelpers.cs" />
     <Compile Include="System\Reflection\Attribute.CoreRT.cs" />
     <Compile Include="System\Reflection\Assembly.CoreRT.cs" />
-    <Compile Include="System\Reflection\AssemblyName.CoreRT.cs" />
     <Compile Include="System\Reflection\BinderBundle.cs" />
     <Compile Include="System\Reflection\Emit\AssemblyBuilder.cs" />
     <Compile Include="System\Reflection\Emit\ConstructorBuilder.cs" />

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/AssemblyName.CoreRT.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/AssemblyName.CoreRT.cs
diff --git a/src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.cs b/src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.cs
@@ -157,7 +157,7 @@ public static unsafe PEReader OpenPEFile(string filePath, out MemoryMappedViewAc
             try
             {
                 // Create stream because CreateFromFile(string, ...) uses FileShare.None which is too strict
-                fileStream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read, 4096, false);
+                fileStream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize: 1);
                 mappedFile = MemoryMappedFile.CreateFromFile(
                     fileStream, null, fileStream.Length, MemoryMappedFileAccess.Read, HandleInheritability.None, true);
                 accessor = mappedFile.CreateViewAccessor(0, 0, MemoryMappedFileAccess.Read);
@@ -174,12 +174,9 @@ public static unsafe PEReader OpenPEFile(string filePath, out MemoryMappedViewAc
             }
             finally
             {
-                if (accessor != null)
-                    accessor.Dispose();
-                if (mappedFile != null)
-                    mappedFile.Dispose();
-                if (fileStream != null)
-                    fileStream.Dispose();
+                accessor?.Dispose();
+                mappedFile?.Dispose();
+                fileStream?.Dispose();
             }
         }
 

diff --git a/src/coreclr/tools/Common/TypeSystem/Ecma/SymbolReader/PortablePdbSymbolReader.cs b/src/coreclr/tools/Common/TypeSystem/Ecma/SymbolReader/PortablePdbSymbolReader.cs
@@ -26,7 +26,7 @@ private static unsafe MetadataReader TryOpenMetadataFile(string filePath, Metada
             try
             {
                 // Create stream because CreateFromFile(string, ...) uses FileShare.None which is too strict
-                fileStream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read, 4096, false);
+                fileStream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize: 1);
                 mappedFile = MemoryMappedFile.CreateFromFile(
                     fileStream, null, fileStream.Length, MemoryMappedFileAccess.Read, HandleInheritability.None, true);
 
@@ -56,12 +56,9 @@ private static unsafe MetadataReader TryOpenMetadataFile(string filePath, Metada
             }
             finally
             {
-                if (accessor != null)
-                    accessor.Dispose();
-                if (mappedFile != null)
-                    mappedFile.Dispose();
-                if (fileStream != null)
-                    fileStream.Dispose();
+                accessor?.Dispose();
+                mappedFile?.Dispose();
+                fileStream?.Dispose();
             }
         }
 

diff --git a/src/coreclr/tools/dotnet-pgo/TraceTypeSystemContext.cs b/src/coreclr/tools/dotnet-pgo/TraceTypeSystemContext.cs
@@ -260,7 +260,7 @@ public static unsafe PEReader OpenPEFile(string filePath, byte[] moduleBytes, ou
             try
             {
                 // Create stream because CreateFromFile(string, ...) uses FileShare.None which is too strict
-                fileStream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read, 4096, false);
+                fileStream = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize: 1);
                 mappedFile = MemoryMappedFile.CreateFromFile(
                     fileStream, null, fileStream.Length, MemoryMappedFileAccess.Read, HandleInheritability.None, true);
                 accessor = mappedFile.CreateViewAccessor(0, 0, MemoryMappedFileAccess.Read);
@@ -278,12 +278,9 @@ public static unsafe PEReader OpenPEFile(string filePath, byte[] moduleBytes, ou
             }
             finally
             {
-                if (accessor != null)
-                    accessor.Dispose();
-                if (mappedFile != null)
-                    mappedFile.Dispose();
-                if (fileStream != null)
-                    fileStream.Dispose();
+                accessor?.Dispose();
+                mappedFile?.Dispose();
+                fileStream?.Dispose();
             }
         }
 

diff --git a/src/coreclr/vm/CMakeLists.txt b/src/coreclr/vm/CMakeLists.txt
@@ -289,7 +289,6 @@ set(GC_HEADERS_DAC
 set(VM_SOURCES_WKS
     ${VM_SOURCES_DAC_AND_WKS_COMMON}
     appdomainnative.cpp
-    assemblyname.cpp
     assemblynative.cpp
     assemblyspec.cpp
     baseassemblyspec.cpp
@@ -392,7 +391,6 @@ set(VM_HEADERS_WKS
     ${VM_HEADERS_DAC_AND_WKS_COMMON}
     ../inc/jithelpers.h
     appdomainnative.hpp
-    assemblyname.hpp
     assemblynative.hpp
     assemblyspec.hpp
     assemblyspecbase.h

diff --git a/src/coreclr/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp
@@ -11,7 +11,6 @@
 #include "eeconfig.h"
 #include "gcheaputilities.h"
 #include "eventtrace.h"
-#include "assemblyname.hpp"
 #include "eeprofinterfaces.h"
 #include "dbginterface.h"
 #ifndef DACCESS_COMPILE

diff --git a/src/coreclr/vm/assembly.cpp b/src/coreclr/vm/assembly.cpp
@@ -17,9 +17,6 @@
 
 #include "assembly.hpp"
 #include "appdomain.hpp"
-#include "assemblyname.hpp"
-
-
 
 #include "eeprofinterfaces.h"
 #include "reflectclasswriter.h"

diff --git a/src/coreclr/vm/assemblyname.cpp b/src/coreclr/vm/assemblyname.cpp
diff --git a/src/coreclr/vm/assemblyname.hpp b/src/coreclr/vm/assemblyname.hpp
diff --git a/src/coreclr/vm/assemblynative.cpp b/src/coreclr/vm/assemblynative.cpp
@@ -20,7 +20,6 @@
 #include "assemblynative.hpp"
 #include "dllimport.h"
 #include "field.h"
-#include "assemblyname.hpp"
 #include "eeconfig.h"
 #include "interoputil.h"
 #include "frames.h"

diff --git a/src/coreclr/vm/corelib.cpp b/src/coreclr/vm/corelib.cpp
@@ -32,7 +32,6 @@
 #include "comdatetime.h"
 #include "compatibilityswitch.h"
 #include "debugdebugger.h"
-#include "assemblyname.hpp"
 #include "assemblynative.hpp"
 #include "comthreadpool.h"
 #include "comwaithandle.h"

diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h
@@ -291,10 +291,6 @@ FCFuncStart(gAssemblyLoadContextFuncs)
     FCFuncElement("IsTracingEnabled", AssemblyNative::IsTracingEnabled)
 FCFuncEnd()
 
-FCFuncStart(gAssemblyNameFuncs)
-    FCFuncElement("nGetFileInformation", AssemblyNameNative::GetFileInformation)
-FCFuncEnd()
-
 FCFuncStart(gAssemblyBuilderFuncs)
     FCFuncElement("GetInMemoryAssemblyModule", AssemblyNative::GetInMemoryAssemblyModule)
 FCFuncEnd()
@@ -768,7 +764,6 @@ FCClassElement("ArgIterator", "System", gVarArgFuncs)
 FCClassElement("Array", "System", gArrayFuncs)
 FCClassElement("AssemblyBuilder", "System.Reflection.Emit", gAssemblyBuilderFuncs)
 FCClassElement("AssemblyLoadContext", "System.Runtime.Loader", gAssemblyLoadContextFuncs)
-FCClassElement("AssemblyName", "System.Reflection", gAssemblyNameFuncs)
 FCClassElement("Buffer", "System", gBufferFuncs)
 FCClassElement("CastHelpers", "System.Runtime.CompilerServices", gCastHelpers)
 FCClassElement("CompatibilitySwitch", "System.Runtime.Versioning", gCompatibilitySwitchFuncs)

diff --git a/src/coreclr/vm/peimage.cpp b/src/coreclr/vm/peimage.cpp
@@ -456,34 +456,6 @@ void PEImage::GetMVID(GUID *pMvid)
 #endif // _DEBUG
 }
 
-void PEImage::VerifyIsAssembly()
-{
-    CONTRACTL
-    {
-        THROWS;
-        GC_TRIGGERS;
-        MODE_ANY;
-    }
-    CONTRACTL_END;
-
-    // buch of legacy stuff here wrt the error codes...
-
-    if (!HasNTHeaders())
-        ThrowFormat(COR_E_BADIMAGEFORMAT);
-
-    if(!HasCorHeader())
-        ThrowFormat(COR_E_ASSEMBLYEXPECTED);
-
-    CHECK checkGoodFormat;
-    checkGoodFormat = CheckILFormat();
-    if (!checkGoodFormat)
-        ThrowFormat(COR_E_BADIMAGEFORMAT);
-
-    mdAssembly a;
-    if (FAILED(GetMDImport()->GetAssemblyFromScope(&a)))
-        ThrowFormat(COR_E_ASSEMBLYEXPECTED);
-}
-
 void DECLSPEC_NORETURN PEImage::ThrowFormat(HRESULT hrError)
 {
     CONTRACTL

diff --git a/src/coreclr/vm/peimage.h b/src/coreclr/vm/peimage.h
@@ -182,8 +182,6 @@ class PEImage final
     CHECK CheckILFormat();
     CHECK CheckUniqueInstance();
 
-    void VerifyIsAssembly();
-
     void SetModuleFileNameHintForDAC();
 #ifdef DACCESS_COMPILE
     void EnumMemoryRegions(CLRDataEnumMemoryFlags flags);

diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp
@@ -27,7 +27,6 @@
 #include "comdatetime.h"
 #include "compatibilityswitch.h"
 #include "debugdebugger.h"
-#include "assemblyname.hpp"
 #include "assemblynative.hpp"
 #include "comthreadpool.h"
 #include "comwaithandle.h"

diff --git a/src/installer/managed/Microsoft.NET.HostModel/AppHost/HostWriter.cs b/src/installer/managed/Microsoft.NET.HostModel/AppHost/HostWriter.cs
@@ -94,7 +94,7 @@ void UpdateResources()
                     try
                     {
                         // Open the source host file.
-                        appHostSourceStream = new FileStream(appHostSourceFilePath, FileMode.Open, FileAccess.Read, FileShare.Read);
+                        appHostSourceStream = new FileStream(appHostSourceFilePath, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize: 1);
                         memoryMappedFile = MemoryMappedFile.CreateFromFile(appHostSourceStream, null, 0, MemoryMappedFileAccess.Read, HandleInheritability.None, true);
                         memoryMappedViewAccessor = memoryMappedFile.CreateViewAccessor(0, 0, MemoryMappedFileAccess.CopyOnWrite);