Skip to content
Merged
Prev Previous commit
Next Next commit
use mem-mapping on Windows
  • Loading branch information
VSadov committed Dec 11, 2021
commit 519fe5c9c677eba65cab1f337ee60e59170bda9f
2 changes: 0 additions & 2 deletions src/coreclr/inc/pedecoder.inl
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ inline PEDecoder::PEDecoder(PTR_VOID mappedBase, bool fixedUp /*= FALSE*/)
{
CONSTRUCTOR_CHECK;
PRECONDITION(CheckPointer(mappedBase));
PRECONDITION(CheckAligned(mappedBase, GetOsPageSize()));
PRECONDITION(PEDecoder(mappedBase,fixedUp).CheckNTHeaders());
THROWS;
GC_NOTRIGGER;
Expand Down Expand Up @@ -172,7 +171,6 @@ inline HRESULT PEDecoder::Init(void *mappedBase, bool fixedUp /*= FALSE*/)
NOTHROW;
GC_NOTRIGGER;
PRECONDITION(CheckPointer(mappedBase));
PRECONDITION(CheckAligned(mappedBase, GetOsPageSize()));
PRECONDITION(!HasContents());
}
CONTRACTL_END;
Expand Down
7 changes: 1 addition & 6 deletions src/coreclr/pal/src/map/map.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2165,14 +2165,9 @@ void * MAPMapPEFile(HANDLE hFile, off_t offset)
goto done;
}

// For compat reasons we always allow 32bit header.
// We may not get correct preferred ImageBase, but otherwise 32bit header is the same for our uses here.
// We will not patch the header into platform bitness though, since coreclr does not require that.
// NOTE: actual checks whether the PE is platform-specific or contains native code will happen later as needed.
if ((VAL16(IMAGE_DOS_SIGNATURE) != VAL16(dosHeader.e_magic))
|| (VAL32(IMAGE_NT_SIGNATURE) != VAL32(ntHeader.Signature))
|| ((VAL16(IMAGE_NT_OPTIONAL_HDR_MAGIC) != VAL16(ntHeader.OptionalHeader.Magic)) &&
(VAL16(IMAGE_NT_OPTIONAL_HDR32_MAGIC) != VAL16(ntHeader.OptionalHeader.Magic))))
|| (VAL16(IMAGE_NT_OPTIONAL_HDR_MAGIC) != VAL16(ntHeader.OptionalHeader.Magic) ) )
{
ERROR_(LOADER)( "Magic number mismatch\n" );
palError = ERROR_INVALID_PARAMETER;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -683,17 +683,25 @@ public static PEHeaderBuilder Create(Subsystem subsystem, TargetDetails target)
ulong imageBase = is64BitTarget ? PE64HeaderConstants.DllImageBase : PE32HeaderConstants.ImageBase;

int fileAlignment = 0x200;
if (!target.IsWindows && !is64BitTarget)
if (target.IsWindows || !is64BitTarget)
{
// To minimize wasted VA space on 32-bit systems, align file to page boundaries (presumed to be 4K)
//
// On Windows we use 4K file alignment, so that we could load the PE manually, if needed, using
// placeholdr APIs (MapViewOfFile3, etc), which have 4K granularity.
fileAlignment = 0x1000;
}

int sectionAlignment = 0x1000;
if (!target.IsWindows && is64BitTarget)
Copy link
Member

Choose a reason for hiding this comment

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

The condition !target.IsWindows && is64BitTarget is apparently a negation of the condition target.IsWindows || !is64BitTarget but it takes a bit of Boolean algebra to double-check that. Could we use an if-else construct or a temporary boolean variable with a sufficiently descriptive name, e.g. isWindowsOr32Bit?

{
// On Linux, we must match the bottom 12 bits of section RVA's to their file offsets. For this reason
// On 64bit Linux, we must match the bottom 12 bits of section RVA's to their file offsets. For this reason
// we need the same alignment for both.
//
// In addition to that we specify section RVAs to be at least 64K apart, which is > page on most systems.
// It ensures that the sections will not overlap when mapped from a singlefile bundle, which introduces a sub-page skew.
//
// Such format would not be accepted by OS loader on Windows, but it is not a problem on Unix.
sectionAlignment = fileAlignment;
}

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/utilcode/pedecoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ CHECK PEDecoder::CheckSection(COUNT_T previousAddressEnd, COUNT_T addressStart,
CHECK(alignedSize >= VAL32(pNT->OptionalHeader.SizeOfImage));

// Check expected alignments
CHECK(CheckAligned(addressStart, VAL32(pNT->OptionalHeader.SectionAlignment)));
// CHECK(CheckAligned(addressStart, VAL32(pNT->OptionalHeader.SectionAlignment)));
CHECK(CheckAligned(offsetStart, VAL32(pNT->OptionalHeader.FileAlignment)));
CHECK(CheckAligned(offsetSize, VAL32(pNT->OptionalHeader.FileAlignment)));

Expand Down
25 changes: 14 additions & 11 deletions src/coreclr/vm/assembly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,17 +208,6 @@ void Assembly::Init(AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocat
// loading it entirely.
//CacheFriendAssemblyInfo();

if (IsCollectible())
{
COUNT_T size;
BYTE *start = (BYTE*)m_pManifest->GetPEAssembly()->GetLoadedImageContents(&size);
if (start != NULL)
{
GCX_COOP();
LoaderAllocator::AssociateMemoryWithLoaderAllocator(start, start + size, m_pLoaderAllocator);
}
}

{
CANNOTTHROWCOMPLUSEXCEPTION();
FAULT_FORBID();
Expand All @@ -230,6 +219,20 @@ void Assembly::Init(AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocat
}
}

void Assembly::AssociateMemoryIfCollectible()
{
if (IsCollectible())
{
COUNT_T size;
BYTE* start = (BYTE*)m_pManifest->GetPEAssembly()->GetLoadedImageContents(&size);
if (start != NULL)
{
GCX_COOP();
LoaderAllocator::AssociateMemoryWithLoaderAllocator(start, start + size, m_pLoaderAllocator);
}
}
}

Assembly::~Assembly()
{
CONTRACTL
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/assembly.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class Assembly
public:
Assembly(BaseDomain *pDomain, PEAssembly *pPEAssembly, DebuggerAssemblyControlFlags debuggerFlags, BOOL fIsCollectible);
void Init(AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocator);
void AssociateMemoryIfCollectible();

void StartUnload();
void Terminate( BOOL signalProfiler = TRUE );
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/domainfile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,7 @@ void DomainFile::LoadLibrary()
CONTRACTL_END;

GetPEAssembly()->EnsureLoaded();
((DomainAssembly*)this)->GetAssembly()->AssociateMemoryIfCollectible();
}

void DomainFile::PostLoadLibrary()
Expand Down
43 changes: 12 additions & 31 deletions src/coreclr/vm/peimage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -814,13 +814,6 @@ PTR_PEImageLayout PEImage::GetOrCreateLayoutInternal(DWORD imageLayoutMask)

if (pRetVal==NULL)
{
// no-path layouts are filled up at creation, if image format permits.
if (!HasPath())
{
// TODO: VS we should manually map flat layouts for R2R here.
ThrowHR(COR_E_BADIMAGEFORMAT);
}

BOOL bIsLoadedLayoutSuitable = ((imageLayoutMask & PEImageLayout::LAYOUT_LOADED) != 0);
BOOL bIsFlatLayoutSuitable = ((imageLayoutMask & PEImageLayout::LAYOUT_FLAT) != 0);

Expand Down Expand Up @@ -862,7 +855,6 @@ PTR_PEImageLayout PEImage::CreateLoadedLayout(bool throwOnFailure)
GC_TRIGGERS;
MODE_ANY;
PRECONDITION(m_pLayoutLock->IsWriterLock());
PRECONDITION(IsFile());
}
CONTRACTL_END;

Expand Down Expand Up @@ -901,17 +893,6 @@ PTR_PEImageLayout PEImage::CreateFlatLayout()

PTR_PEImageLayout pFlatLayout = PEImageLayout::LoadFlat(this);
SetLayout(IMAGE_FLAT, pFlatLayout);

if (m_pLayouts[IMAGE_LOADED] == NULL &&
pFlatLayout->CheckNTHeaders() &&
pFlatLayout->CheckILOnly() &&
!pFlatLayout->HasWriteableSections() &&
!pFlatLayout->HasReadyToRunHeader())
{
pFlatLayout->AddRef();
SetLayout(IMAGE_LOADED, pFlatLayout);
}

return pFlatLayout;
}

Expand All @@ -930,16 +911,6 @@ PTR_PEImage PEImage::CreateFromByteArray(const BYTE* array, COUNT_T size)

SimpleWriteLockHolder lock(pImage->m_pLayoutLock);
pImage->SetLayout(IMAGE_FLAT,pLayout);

// TODO: VS allow R2R for now, meaning it will run as IL-only
if (pLayout->CheckNTHeaders() &&
pLayout->CheckILOnly() &&
!pLayout->HasWriteableSections())
{
pLayout->AddRef();
pImage->SetLayout(IMAGE_LOADED, pLayout);
}

RETURN dac_cast<PTR_PEImage>(pImage.Extract());
}

Expand Down Expand Up @@ -996,7 +967,12 @@ HANDLE PEImage::GetFileHandle()
{
ErrorModeHolder mode(SEM_NOOPENFILEERRORBOX|SEM_FAILCRITICALERRORS);
m_hFile=WszCreateFile((LPCWSTR) GetPathToLoad(),
GENERIC_READ,
GENERIC_READ
#if TARGET_WINDOWS
// the file may have native code sections, make sure we have execute permissions
| GENERIC_EXECUTE
#endif
,
FILE_SHARE_READ|FILE_SHARE_DELETE,
NULL,
OPEN_EXISTING,
Expand Down Expand Up @@ -1027,7 +1003,12 @@ HRESULT PEImage::TryOpenFile()
{
ErrorModeHolder mode(SEM_NOOPENFILEERRORBOX | SEM_FAILCRITICALERRORS);
m_hFile=WszCreateFile((LPCWSTR)GetPathToLoad(),
GENERIC_READ,
GENERIC_READ
Copy link
Member

Choose a reason for hiding this comment

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

A nit - this function shares 90% with the GetFileHandle. Since you are touching this code, it seems it would make sense to call TryOpenFile from the GetFileHandle to get rid of the code duplication.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. The difference is in taking a lock and in hadling the failure. I will just make one call another.

#if TARGET_WINDOWS
// the file may have native code sections, make sure we have execute permissions
| GENERIC_EXECUTE
#endif
,
FILE_SHARE_READ|FILE_SHARE_DELETE,
NULL,
OPEN_EXISTING,
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/peimage.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class PEImage final

BOOL HasLoadedLayout();
PTR_PEImageLayout GetLoadedLayout();
PTR_PEImageLayout GetFlatLayout();

BOOL HasPath();
ULONG GetPathHash();
Expand Down
9 changes: 9 additions & 0 deletions src/coreclr/vm/peimage.inl
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,15 @@ inline PTR_PEImageLayout PEImage::GetLoadedLayout()
return m_pLayouts[IMAGE_LOADED];
}

inline PTR_PEImageLayout PEImage::GetFlatLayout()
{
LIMITED_METHOD_CONTRACT;
SUPPORTS_DAC;

_ASSERTE(m_pLayouts[IMAGE_FLAT] != NULL);
return m_pLayouts[IMAGE_FLAT];
}

inline BOOL PEImage::IsOpened()
{
LIMITED_METHOD_CONTRACT;
Expand Down
Loading