Skip to content

Moved AssemblyName helpers to managed (part 2) #63915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
VSadov merged 16 commits into from
Jan 26, 2022
Merged

Moved AssemblyName helpers to managed (part 2) #63915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
573f27e
implement GetAssemblyName via dynamic call to MetadataReader
VSadov Jan 18, 2022
5118749
A few more file-locking tests.
VSadov Jan 18, 2022
b20e281
fix #28153
VSadov Jan 18, 2022
8edc23c
no need for version when getting MetadataReader
VSadov Jan 18, 2022
e7c37c5
rename the argument to match AssemblyName
VSadov Jan 18, 2022
bfe66ce
perf tweaks
VSadov Jan 20, 2022
d616812
use memory-mapped file to read metadata
VSadov Jan 26, 2022
e28dbd0
adjust tests for the new implementation
VSadov Jan 26, 2022
2cb85b1
use "bufferSize: 1" when stream is going to be mapped.
VSadov Jan 26, 2022
b3e720a
null-conditional operator.
VSadov Jan 26, 2022
23de380
do Dispose before re-throwing
VSadov Jan 26, 2022
15aa53a
get rid of the platform-specific/native stuff
VSadov Jan 26, 2022
f0b78e9
remove assemblyname.hpp
VSadov Jan 26, 2022
f56b90d
remove `VerifyIsAssembly()`
VSadov Jan 26, 2022
b5f5346
PR feedback
VSadov Jan 26, 2022
f07d4f8
put back gStdMngIEnumerableFuncs and the others
VSadov Jan 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/AssemblyName.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
15 changes: 0 additions & 15 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/AssemblyName.CoreRT.cs
View file
Open in desktop

This file was deleted.

11 changes: 4 additions & 7 deletions src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
}
}

Expand Down
11 changes: 4 additions & 7 deletions src/coreclr/tools/Common/TypeSystem/Ecma/SymbolReader/PortablePdbSymbolReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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();
}
}

Expand Down
11 changes: 4 additions & 7 deletions src/coreclr/tools/dotnet-pgo/TraceTypeSystemContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
}
}

Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/vm/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/appdomain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions src/coreclr/vm/assembly.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@

#include "assembly.hpp"
#include "appdomain.hpp"
#include "assemblyname.hpp"



#include "eeprofinterfaces.h"
#include "reflectclasswriter.h"
Expand Down
69 changes: 0 additions & 69 deletions src/coreclr/vm/assemblyname.cpp
View file
Open in desktop

This file was deleted.

25 changes: 0 additions & 25 deletions src/coreclr/vm/assemblyname.hpp
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion src/coreclr/vm/assemblynative.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/corelib.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 0 additions & 5 deletions src/coreclr/vm/ecalllist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Copy link
Copy Markdown
Member

@am11 am11 Jan 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - these three are also dead/unused:

FCClassElement("IEnumerable", "System.Collections", gStdMngIEnumerableFuncs)
FCClassElement("IEnumerator", "System.Collections", gStdMngIEnumeratorFuncs)
FCClassElement("IReflect", "System.Reflection", gStdMngIReflectFuncs)

Copy link
Copy Markdown
Member

@jkotas jkotas Jan 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They come from #include "mngstditflist.h"

Copy link
Copy Markdown
Member

@am11 am11 Jan 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah string concatenation. I was searching for the whole word (gStdMngIEnumerableFuncs).

Copy link
Copy Markdown
Member Author

@VSadov VSadov Jan 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also could not find these and removed them. But everything is passing. Do we have tests for this?

Copy link
Copy Markdown
Member

@jkotas jkotas Jan 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very specialized COM interop that we tried to avoid touching. It is possible that the tests are in the outer runs only, or does not exist at all.

Copy link
Copy Markdown
Member Author

@VSadov VSadov Jan 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some interop tests started failing eventually. I will put these back.
It could be nicer if removal caused some kind of build break

FCClassElement("Buffer", "System", gBufferFuncs)
FCClassElement("CastHelpers", "System.Runtime.CompilerServices", gCastHelpers)
FCClassElement("CompatibilitySwitch", "System.Runtime.Versioning", gCompatibilitySwitchFuncs)
Expand Down
28 changes: 0 additions & 28 deletions src/coreclr/vm/peimage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/vm/peimage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,6 @@ class PEImage final
CHECK CheckILFormat();
CHECK CheckUniqueInstance();

void VerifyIsAssembly();

void SetModuleFileNameHintForDAC();
#ifdef DACCESS_COMPILE
void EnumMemoryRegions(CLRDataEnumMemoryFlags flags);
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/qcallentrypoints.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion src/installer/managed/Microsoft.NET.HostModel/AppHost/HostWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Loading