PE loader/image cleanups. No-copy mapping of R2R PEs on Windows. (#61938)

* removed IMAGE_MAPPED

* removed RawImageLayout

* delete MappedImageLayout

* use mem-mapping on Windows

* tweaks and touchups

* a few cleanups

* comments

* move EnsureLoaded into Assemby::Init

* A fix for IsDynamic() case.

* fix for preferred base, if used.

* disable failing scenario

* PR feedback

* Typo (CENTINEL --> SENTINEL)

* added a bug link to a disabled test scenario + couple comment tweaks

Author: VSadov

src/coreclr/debug/daccess/request.cpp

@@ -1344,11 +1344,7 @@ ClrDataAccess::GetMethodDescName(CLRDATA_ADDRESS methodDesc, unsigned int count,
             if (pMD->IsLCGMethod() || pMD->IsILStub())
             {
                 // In heap dumps, trying to format the signature can fail
-                // in certain cases because StoredSigMethodDesc::m_pSig points
-                // to the IMAGE_MAPPED layout (in the PEImage::m_pLayouts array).
-                // We save only the IMAGE_LOADED layout to the heap dump. Rather
-                // than bloat the dump, we just drop the signature in these
-                // cases.
+                // in certain cases.
 
                 str.Clear();
                 TypeString::AppendMethodInternal(str, pMD, TypeString::FormatNamespace|TypeString::FormatFullInst);

src/coreclr/inc/pedecoder.h

@@ -237,8 +237,6 @@ class PEDecoder
     BOOL IsILOnly() const;
     CHECK CheckILOnly() const;
 
-    void LayoutILOnly(void *base, bool enableExecution) const;
-
     // Strong name & hashing support
 
     BOOL HasStrongNameSignature() const;
@@ -348,6 +346,7 @@ class PEDecoder
     IMAGE_SECTION_HEADER *OffsetToSection(COUNT_T fileOffset) const;
 
     void SetRelocated();
+    IMAGE_NT_HEADERS* FindNTHeaders() const;
 
   private:
 
@@ -364,7 +363,6 @@ class PEDecoder
 
     static PTR_IMAGE_SECTION_HEADER FindFirstSection(IMAGE_NT_HEADERS * pNTHeaders);
 
-    IMAGE_NT_HEADERS *FindNTHeaders() const;
     IMAGE_COR20_HEADER *FindCorHeader() const;
     READYTORUN_HEADER *FindReadyToRunHeader() const;
 

src/coreclr/inc/pedecoder.inl

@@ -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;
@@ -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;

src/coreclr/inc/vptr_list.h

@@ -40,14 +40,12 @@ VPTR_CLASS(TailCallStubManager)
 #endif
 VPTR_CLASS(CallCountingStubManager)
 VPTR_CLASS(PEAssembly)
+
 VPTR_CLASS(PEImageLayout)
-VPTR_CLASS(RawImageLayout)
 VPTR_CLASS(ConvertedImageLayout)
-VPTR_CLASS(MappedImageLayout)
-#if !defined(TARGET_UNIX)
 VPTR_CLASS(LoadedImageLayout)
-#endif // !TARGET_UNIX
 VPTR_CLASS(FlatImageLayout)
+
 #ifdef FEATURE_COMINTEROP
 VPTR_CLASS(ComMethodFrame)
 VPTR_CLASS(ComPlusMethodFrame)

src/coreclr/pal/src/map/map.cpp

@@ -2043,9 +2043,9 @@ MAPmmapAndRecord(
 
         // Set the requested mapping with forced PROT_WRITE to ensure data from the file can be read there,
         // read the data in and finally remove the forced PROT_WRITE
-        if ((mprotect(pvBaseAddress, len + adjust, prot | PROT_WRITE) == -1) ||
+        if ((mprotect(pvBaseAddress, len + adjust, PROT_WRITE) == -1) ||
             (pread(fd, pvBaseAddress, len + adjust, offset - adjust) == -1) ||
-            (((prot & PROT_WRITE) == 0) && mprotect(pvBaseAddress, len + adjust, prot) == -1))
+            (mprotect(pvBaseAddress, len + adjust, prot) == -1))
         {
             palError = FILEGetLastErrorFromErrno();
         }

src/coreclr/tools/aot/ILCompiler.ReadyToRun/ObjectWriter/R2RPEBuilder.cs

@@ -683,17 +683,29 @@ public static PEHeaderBuilder Create(Subsystem subsystem, TargetDetails target)
             ulong imageBase = is64BitTarget ? PE64HeaderConstants.DllImageBase : PE32HeaderConstants.ImageBase;
 
             int fileAlignment = 0x200;
-            if (!target.IsWindows && !is64BitTarget)
+            bool isWindowsOr32bit = target.IsWindows || !is64BitTarget;
+            if (isWindowsOr32bit)
             {
-                // To minimize wasted VA space on 32-bit systems, align file to page boundaries (presumed to be 4K)
+                // To minimize wasted VA space on 32-bit systems (regardless of OS),
+                // align file to page boundaries (presumed to be 4K)
+                //
+                // On Windows we use 4K file alignment (regardless of ptr size),
+                // per requirements of memory mapping API (MapViewOfFile3, et al).
+                // The alternative could be using the same approach as on Unix, but that would result in PEs
+                // incompatible with OS loader. While that is not a problem on Unix, we do not want that on Windows.
                 fileAlignment = 0x1000;
             }
 
             int sectionAlignment = 0x1000;
-            if (!target.IsWindows && is64BitTarget)
+            if (!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;
             }
 

src/coreclr/utilcode/pedecoder.cpp

@@ -372,7 +372,13 @@ CHECK PEDecoder::CheckSection(COUNT_T previousAddressEnd, COUNT_T addressStart,
         CHECK(alignedSize >= VAL32(pNT->OptionalHeader.SizeOfImage));
 
     // Check expected alignments
+#if TARGET_WINDOWS
+    // On Windows we expect section starts to be a multiple of SectionAlignment.
+    // That is not an explicit requirement in the documentation, but it looks like OS loader expects it.
+    // In contrast to this, in Unix R2R files we keep lower 16bits of sections RVA and
+    // this condition does not hold.
     CHECK(CheckAligned(addressStart, VAL32(pNT->OptionalHeader.SectionAlignment)));
+#endif
     CHECK(CheckAligned(offsetStart, VAL32(pNT->OptionalHeader.FileAlignment)));
     CHECK(CheckAligned(offsetSize, VAL32(pNT->OptionalHeader.FileAlignment)));
 
@@ -1659,86 +1665,6 @@ CHECK PEDecoder::CheckILOnlyEntryPoint() const
 }
 #endif // TARGET_X86
 
-#ifndef DACCESS_COMPILE
-
-void PEDecoder::LayoutILOnly(void *base, bool enableExecution) const
-{
-    CONTRACT_VOID
-    {
-        INSTANCE_CHECK;
-        PRECONDITION(CheckZeroedMemory(base, VAL32(FindNTHeaders()->OptionalHeader.SizeOfImage)));
-        // Ideally we would require the layout address to honor the section alignment constraints.
-        // However, we do have 8K aligned IL only images which we load on 32 bit platforms. In this
-        // case, we can only guarantee OS page alignment (which after all, is good enough.)
-        PRECONDITION(CheckAligned((SIZE_T)base, GetOsPageSize()));
-        THROWS;
-        GC_NOTRIGGER;
-    }
-    CONTRACT_END;
-
-    // We're going to copy everything first, and write protect what we need to later.
-
-    // First, copy headers
-    CopyMemory(base, (void *)m_base, VAL32(FindNTHeaders()->OptionalHeader.SizeOfHeaders));
-
-    // Now, copy all sections to appropriate virtual address
-
-    IMAGE_SECTION_HEADER *sectionStart = IMAGE_FIRST_SECTION(FindNTHeaders());
-    IMAGE_SECTION_HEADER *sectionEnd = sectionStart + VAL16(FindNTHeaders()->FileHeader.NumberOfSections);
-
-    IMAGE_SECTION_HEADER *section = sectionStart;
-    while (section < sectionEnd)
-    {
-        // Raw data may be less than section size if tail is zero, but may be more since VirtualSize is
-        // not padded.
-        DWORD size = min(VAL32(section->SizeOfRawData), VAL32(section->Misc.VirtualSize));
-
-        CopyMemory((BYTE *) base + VAL32(section->VirtualAddress), (BYTE *) m_base + VAL32(section->PointerToRawData), size);
-
-        // Note that our memory is zeroed already, so no need to initialize any tail.
-
-        section++;
-    }
-
-    // Apply write protection to copied headers
-    DWORD oldProtection;
-    if (!ClrVirtualProtect((void *) base, VAL32(FindNTHeaders()->OptionalHeader.SizeOfHeaders),
-                           PAGE_READONLY, &oldProtection))
-        ThrowLastError();
-
-    // Finally, apply proper protection to copied sections
-    for (section = sectionStart; section < sectionEnd; section++)
-    {
-        // Add appropriate page protection.
-        DWORD newProtection;
-        if (!enableExecution)
-        {
-            if (section->Characteristics & IMAGE_SCN_MEM_WRITE)
-                continue;
-
-            newProtection = PAGE_READONLY;
-        }
-        else
-        {
-            newProtection = section->Characteristics & IMAGE_SCN_MEM_EXECUTE ?
-                PAGE_EXECUTE_READ :
-                section->Characteristics & IMAGE_SCN_MEM_WRITE ?
-                    PAGE_READWRITE :
-                    PAGE_READONLY;
-        }
-
-        if (!ClrVirtualProtect((void*)((BYTE*)base + VAL32(section->VirtualAddress)),
-            VAL32(section->Misc.VirtualSize),
-            newProtection, &oldProtection))
-        {
-            ThrowLastError();
-        }
-    }
-
-    RETURN;
-}
-
-#endif // #ifndef DACCESS_COMPILE
 
 bool ReadResourceDirectoryHeader(const PEDecoder *pDecoder, DWORD rvaOfResourceSection, DWORD rva, IMAGE_RESOURCE_DIRECTORY_ENTRY** ppDirectoryEntries, IMAGE_RESOURCE_DIRECTORY **ppResourceDirectory)
 {

src/coreclr/vm/assembly.cpp

@@ -189,11 +189,17 @@ void Assembly::Init(AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocat
     m_pClassLoader = new ClassLoader(this);
     m_pClassLoader->Init(pamTracker);
 
-    if (GetManifestFile()->IsDynamic())
+    PEAssembly* pPEAssembly = GetManifestFile();
+
+    // "Module::Create" will initialize R2R support, if there is an R2R header.
+    // make sure the PE is loaded or R2R will be disabled.
+    pPEAssembly->EnsureLoaded();
+
+    if (pPEAssembly->IsDynamic())
         // manifest modules of dynamic assemblies are always transient
-        m_pManifest = ReflectionModule::Create(this, GetManifestFile(), pamTracker, REFEMIT_MANIFEST_MODULE_NAME);
+        m_pManifest = ReflectionModule::Create(this, pPEAssembly, pamTracker, REFEMIT_MANIFEST_MODULE_NAME);
     else
-        m_pManifest = Module::Create(this, mdFileNil, GetManifestFile(), pamTracker);
+        m_pManifest = Module::Create(this, mdFileNil, pPEAssembly, pamTracker);
 
     FastInterlockIncrement((LONG*)&g_cAssemblies);
 
@@ -208,15 +214,16 @@ void Assembly::Init(AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocat
     //  loading it entirely.
     //CacheFriendAssemblyInfo();
 
-    if (IsCollectible())
+    if (IsCollectible() && !pPEAssembly->IsDynamic())
     {
         COUNT_T size;
-        BYTE *start = (BYTE*)m_pManifest->GetPEAssembly()->GetLoadedImageContents(&size);
-        if (start != NULL)
-        {
-            GCX_COOP();
-            LoaderAllocator::AssociateMemoryWithLoaderAllocator(start, start + size, m_pLoaderAllocator);
-        }
+        BYTE* start = (BYTE*)pPEAssembly->GetLoadedImageContents(&size);
+
+        // We should have the content loaded at this time. There will be no other attempt to associate memory.
+        _ASSERTE(start != NULL);
+
+        GCX_COOP();
+        LoaderAllocator::AssociateMemoryWithLoaderAllocator(start, start + size, m_pLoaderAllocator);
     }
 
     {

src/coreclr/vm/assemblyname.cpp

@@ -52,10 +52,9 @@ FCIMPL1(Object*, AssemblyNameNative::GetFileInformation, StringObject* filenameU
     SString sFileName(gc.filename->GetBuffer());
     PEImageHolder pImage = PEImage::OpenImage(sFileName, MDInternalImport_NoCache);
 
-    // Load the temporary image using a flat layout, instead of
-    // waiting for it to happen during HasNTHeaders. This allows us to
-    // get the assembly name for images that contain native code for a
-    // non-native platform.
+    // Ask for FLAT. We will only look at metadata and release the image shortly.
+    // Besides we may be getting the assembly name for images that contain native code for a
+    // non-native platform and would end up using flat anyways.
     PEImageLayout* pLayout = pImage->GetOrCreateLayout(PEImageLayout::LAYOUT_FLAT);
 
     pImage->VerifyIsAssembly();

src/coreclr/vm/assemblynative.cpp

@@ -138,20 +138,8 @@ Assembly* AssemblyNative::LoadFromPEImage(AssemblyBinder* pBinder, PEImage *pIma
     CONTRACT_END;
 
     Assembly *pLoadedAssembly = NULL;
-
     ReleaseHolder<BINDER_SPACE::Assembly> pAssembly;
 
-    // Force the image to be loaded and mapped so that subsequent loads do not
-    // map a duplicate copy.
-    if (pImage->IsFile())
-    {
-        pImage->Load();
-    }
-    else
-    {
-        pImage->LoadNoFile();
-    }
-
     DWORD dwMessageID = IDS_EE_FILELOAD_ERROR_GENERIC;
 
     // Set the caller's assembly to be CoreLib
@@ -251,11 +239,7 @@ extern "C" void QCALLTYPE AssemblyNative_LoadFromStream(INT_PTR ptrNativeAssembl
     _ASSERTE((ptrAssemblyArray != NULL) && (cbAssemblyArrayLength > 0));
     _ASSERTE((ptrSymbolArray == NULL) || (cbSymbolArrayLength > 0));
 
-    // We must have a flat image stashed away since we need a private
-    // copy of the data which we can verify before doing the mapping.
-    PVOID pAssemblyArray = reinterpret_cast<PVOID>(ptrAssemblyArray);
-
-    PEImageHolder pILImage(PEImage::LoadFlat(pAssemblyArray, (COUNT_T)cbAssemblyArrayLength));
+    PEImageHolder pILImage(PEImage::CreateFromByteArray((BYTE*)ptrAssemblyArray, (COUNT_T)cbAssemblyArrayLength));
 
     // Need to verify that this is a valid CLR assembly.
     if (!pILImage->CheckILFormat())
@@ -318,7 +302,7 @@ extern "C" void QCALLTYPE AssemblyNative_LoadFromInMemoryModule(INT_PTR ptrNativ
     _ASSERTE(ptrNativeAssemblyBinder != NULL);
     _ASSERTE(hModule != NULL);
 
-    PEImageHolder pILImage(PEImage::LoadImage((HMODULE)hModule));
+    PEImageHolder pILImage(PEImage::CreateFromHMODULE((HMODULE)hModule));
 
     // Need to verify that this is a valid CLR assembly.
     if (!pILImage->HasCorHeader())