From aba46196744626885dcc4c1d8415bf6faeb41af6 Mon Sep 17 00:00:00 2001 From: vsadov Date: Tue, 6 Apr 2021 10:55:11 -0700 Subject: [PATCH 1/9] enable compression of assemblies --- src/coreclr/hosts/inc/coreclrhost.h | 2 +- src/coreclr/inc/bundle.h | 4 +- src/coreclr/vm/CMakeLists.txt | 6 +- src/coreclr/vm/bundle.cpp | 16 ++++- src/coreclr/vm/peimage.h | 1 + src/coreclr/vm/peimage.inl | 6 ++ src/coreclr/vm/peimagelayout.cpp | 65 +++++++++++++++++-- src/coreclr/vm/peimagelayout.h | 1 - .../Microsoft.NET.HostModel/Bundle/Bundler.cs | 1 + .../TestProjects/BundleProbeTester/Program.cs | 12 +++- src/native/corehost/bundle/runner.cpp | 3 +- src/native/corehost/bundle/runner.h | 2 +- .../hostpolicy/hostpolicy_context.cpp | 4 +- 13 files changed, 104 insertions(+), 19 deletions(-) diff --git a/src/coreclr/hosts/inc/coreclrhost.h b/src/coreclr/hosts/inc/coreclrhost.h index f1d6c005a32f6..46a3119d628de 100644 --- a/src/coreclr/hosts/inc/coreclrhost.h +++ b/src/coreclr/hosts/inc/coreclrhost.h @@ -126,7 +126,7 @@ CORECLR_HOSTING_API(coreclr_execute_assembly, // // Callback types used by the hosts // -typedef bool(CORECLR_CALLING_CONVENTION BundleProbeFn)(const char* path, int64_t* offset, int64_t* size); +typedef bool(CORECLR_CALLING_CONVENTION BundleProbeFn)(const char* path, int64_t* offset, int64_t* size, int64_t* compressedSize); typedef const void* (CORECLR_CALLING_CONVENTION PInvokeOverrideFn)(const char* libraryName, const char* entrypointName); diff --git a/src/coreclr/inc/bundle.h b/src/coreclr/inc/bundle.h index ab3ece2d7f5cc..3d55a54b3515b 100644 --- a/src/coreclr/inc/bundle.h +++ b/src/coreclr/inc/bundle.h @@ -19,13 +19,15 @@ struct BundleFileLocation { INT64 Size; INT64 Offset; + INT64 UncompresedSize; BundleFileLocation() { LIMITED_METHOD_CONTRACT; - Size = 0; + Size = 0; Offset = 0; + UncompresedSize = 0; } static BundleFileLocation Invalid() { LIMITED_METHOD_CONTRACT; return BundleFileLocation(); } diff --git a/src/coreclr/vm/CMakeLists.txt b/src/coreclr/vm/CMakeLists.txt index 5bd32629f3d5b..2ad98076f8bdd 100644 --- a/src/coreclr/vm/CMakeLists.txt +++ b/src/coreclr/vm/CMakeLists.txt @@ -107,7 +107,6 @@ set(VM_SOURCES_DAC_AND_WKS_COMMON onstackreplacement.cpp pefile.cpp peimage.cpp - peimagelayout.cpp perfmap.cpp perfinfo.cpp pgo.cpp @@ -891,6 +890,11 @@ endif(CLR_CMAKE_TARGET_WIN32) set (VM_SOURCES_WKS_SPECIAL codeman.cpp ceemain.cpp + peimagelayout.cpp +) + +list(APPEND VM_SOURCES_DAC + peimagelayout.cpp ) # gcdecode.cpp is included by both JIT and VM. to avoid duplicate definitions we need to diff --git a/src/coreclr/vm/bundle.cpp b/src/coreclr/vm/bundle.cpp index 64994aae222b7..8a19b1b5b67f0 100644 --- a/src/coreclr/vm/bundle.cpp +++ b/src/coreclr/vm/bundle.cpp @@ -80,7 +80,21 @@ BundleFileLocation Bundle::Probe(const SString& path, bool pathIsBundleRelative) } } - m_probe(utf8Path, &loc.Offset, &loc.Size); + INT64 fileSize; + INT64 compressedSize; + + m_probe(utf8Path, &loc.Offset, &fileSize, &compressedSize); + + if (compressedSize) + { + loc.Size = compressedSize; + loc.UncompresedSize = fileSize; + } + else + { + loc.Size = fileSize; + loc.UncompresedSize = 0; + } return loc; } diff --git a/src/coreclr/vm/peimage.h b/src/coreclr/vm/peimage.h index 4b29a6cbc62f3..51b9dbc216b2a 100644 --- a/src/coreclr/vm/peimage.h +++ b/src/coreclr/vm/peimage.h @@ -148,6 +148,7 @@ class PEImage HANDLE GetFileHandle(); INT64 GetOffset() const; INT64 GetSize() const; + INT64 GetUncompressedSize() const; void SetFileHandle(HANDLE hFile); HRESULT TryOpenFile(); diff --git a/src/coreclr/vm/peimage.inl b/src/coreclr/vm/peimage.inl index 06d897efa8dbb..50cd4555c4ca6 100644 --- a/src/coreclr/vm/peimage.inl +++ b/src/coreclr/vm/peimage.inl @@ -59,6 +59,12 @@ inline INT64 PEImage::GetSize() const return m_bundleFileLocation.Size; } +inline INT64 PEImage::GetUncompressedSize() const +{ + LIMITED_METHOD_CONTRACT; + return m_bundleFileLocation.UncompresedSize; +} + inline void PEImage::SetModuleFileNameHintForDAC() { LIMITED_METHOD_DAC_CONTRACT; diff --git a/src/coreclr/vm/peimagelayout.cpp b/src/coreclr/vm/peimagelayout.cpp index 385d6efe1a23c..3cb79e2d70000 100644 --- a/src/coreclr/vm/peimagelayout.cpp +++ b/src/coreclr/vm/peimagelayout.cpp @@ -13,6 +13,13 @@ #include "amsi.h" #endif +#if defined(CORECLR_EMBEDDED) +extern "C" +{ +#include "../../../libraries/Native/AnyOS/zlib/pal_zlib.h" +} +#endif + #ifndef DACCESS_COMPILE PEImageLayout* PEImageLayout::CreateFlat(const void *flat, COUNT_T size,PEImage* pOwner) { @@ -397,7 +404,6 @@ ConvertedImageLayout::ConvertedImageLayout(PEImageLayout* source, BOOL isInBundl m_Layout=LAYOUT_LOADED; m_pOwner=source->m_pOwner; _ASSERTE(!source->IsMapped()); - m_isInBundle = isInBundle; m_pExceptionDir = NULL; @@ -441,7 +447,7 @@ ConvertedImageLayout::ConvertedImageLayout(PEImageLayout* source, BOOL isInBundl ApplyBaseRelocations(); } #elif !defined(TARGET_UNIX) - if (m_isInBundle && + if (isInBundle && HasCorHeader() && (HasNativeHeader() || HasReadyToRunHeader()) && g_fAllowNativeImages) @@ -704,6 +710,8 @@ FlatImageLayout::FlatImageLayout(PEImage* pOwner) } } + LPVOID addr = 0; + // It's okay if resource files are length zero if (size > 0) { @@ -721,14 +729,57 @@ FlatImageLayout::FlatImageLayout(PEImage* pOwner) _ASSERTE((offset - mapBegin) < mapSize); _ASSERTE(mapSize >= (UINT64)size); - char *addr = (char*)CLRMapViewOfFile(m_FileMap, FILE_MAP_READ, mapBegin >> 32, (DWORD)mapBegin, (DWORD)mapSize); - if (addr == NULL) + LPVOID view = CLRMapViewOfFile(m_FileMap, FILE_MAP_READ, mapBegin >> 32, (DWORD)mapBegin, (DWORD)mapSize); + if (view == NULL) ThrowLastError(); - addr += (offset - mapBegin); - m_FileView.Assign((LPVOID)addr); + m_FileView.Assign(view); + addr = (LPVOID)((size_t)view + offset - mapBegin); + + INT64 uncompressedSize = pOwner->GetUncompressedSize(); + if (uncompressedSize > 0) + { +#if defined(CORECLR_EMBEDDED) + HandleHolder anonMap = WszCreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, uncompressedSize >> 32, (DWORD)uncompressedSize, NULL); + if (anonMap == NULL) + ThrowLastError(); + + LPVOID anonView = CLRMapViewOfFile(anonMap, FILE_MAP_READ | FILE_MAP_WRITE, 0, 0, 0); + if (anonView == NULL) + ThrowLastError(); + + //NB: PE cannot be larger than 4GB, conversions are ok + PAL_ZStream zStream; + zStream.nextIn = (uint8_t*)addr; + zStream.availIn = (uint32_t)size; + zStream.nextOut = (uint8_t*)anonView; + zStream.availOut = (uint32_t)uncompressedSize; + + // Legal values are 8..15 and -8..-15. 15 is the window size, + // negative val causes deflate to produce raw deflate data (no zlib header). + const int Deflate_DefaultWindowBits = -15; + if (CompressionNative_InflateInit2_(&zStream, Deflate_DefaultWindowBits) != PAL_Z_OK) + ThrowLastError(); + + int ret = CompressionNative_Inflate(&zStream, PAL_Z_NOFLUSH); + CompressionNative_InflateEnd(&zStream); + if (ret < 0) + ThrowLastError(); + + addr = anonView; + size = uncompressedSize; + // Replace file handles with handles to anonymous map. This will release the handles to the source view and map. + m_FileView.Assign(anonView); + m_FileMap.Assign(anonMap); + +#else + _ASSERTE(!"Failure extracting contents of the application bundle. Compressed files used with a standalone (not singlefile) apphost."); + ThrowLastError(); +#endif + } } - Init(m_FileView, (COUNT_T)size); + + Init(addr, (COUNT_T)size); } NativeImageLayout::NativeImageLayout(LPCWSTR fullPath) diff --git a/src/coreclr/vm/peimagelayout.h b/src/coreclr/vm/peimagelayout.h index 6116ca0c52620..02dae065ac72a 100644 --- a/src/coreclr/vm/peimagelayout.h +++ b/src/coreclr/vm/peimagelayout.h @@ -113,7 +113,6 @@ class ConvertedImageLayout: public PEImageLayout virtual ~ConvertedImageLayout(); #endif private: - bool m_isInBundle; PT_RUNTIME_FUNCTION m_pExceptionDir; }; diff --git a/src/installer/managed/Microsoft.NET.HostModel/Bundle/Bundler.cs b/src/installer/managed/Microsoft.NET.HostModel/Bundle/Bundler.cs index bee21ddc065db..6c905cbcbd809 100644 --- a/src/installer/managed/Microsoft.NET.HostModel/Bundle/Bundler.cs +++ b/src/installer/managed/Microsoft.NET.HostModel/Bundle/Bundler.cs @@ -74,6 +74,7 @@ private bool ShouldCompress(FileType type) { case FileType.Symbols: case FileType.NativeBinary: + case FileType.Assembly: return true; default: diff --git a/src/installer/tests/Assets/TestProjects/BundleProbeTester/Program.cs b/src/installer/tests/Assets/TestProjects/BundleProbeTester/Program.cs index ac963679797cc..fdd1fdeb32f24 100644 --- a/src/installer/tests/Assets/TestProjects/BundleProbeTester/Program.cs +++ b/src/installer/tests/Assets/TestProjects/BundleProbeTester/Program.cs @@ -13,16 +13,22 @@ public static class Program // The bundle-probe callback is only called from native code in the product // Therefore the type on this test is adjusted to circumvent the failure. [UnmanagedFunctionPointer(CallingConvention.StdCall)] - public delegate byte BundleProbeDelegate([MarshalAs(UnmanagedType.LPUTF8Str)] string path, IntPtr size, IntPtr offset); + public delegate byte BundleProbeDelegate([MarshalAs(UnmanagedType.LPUTF8Str)] string path, IntPtr offset, IntPtr size, IntPtr compressedSize); unsafe static bool Probe(BundleProbeDelegate bundleProbe, string path, bool isExpected) { - Int64 size, offset; - bool exists = bundleProbe(path, (IntPtr)(&offset), (IntPtr)(&size)) != 0; + Int64 size, offset, compressedSize; + bool exists = bundleProbe(path, (IntPtr)(&offset), (IntPtr)(&size), (IntPtr)(&compressedSize)) != 0; switch (exists, isExpected) { case (true, true): + if (compressedSize < 0 || compressedSize > size) + { + Console.WriteLine($"Invalid compressedSize obtained for {path} within bundle."); + return false; + } + if (size > 0 && offset > 0) { return true; diff --git a/src/native/corehost/bundle/runner.cpp b/src/native/corehost/bundle/runner.cpp index 36edd791fa44d..449f6935c999a 100644 --- a/src/native/corehost/bundle/runner.cpp +++ b/src/native/corehost/bundle/runner.cpp @@ -61,7 +61,7 @@ const file_entry_t* runner_t::probe(const pal::string_t &relative_path) const return nullptr; } -bool runner_t::probe(const pal::string_t& relative_path, int64_t* offset, int64_t* size) const +bool runner_t::probe(const pal::string_t& relative_path, int64_t* offset, int64_t* size, int64_t* compressedSize) const { const bundle::file_entry_t* entry = probe(relative_path); @@ -76,6 +76,7 @@ bool runner_t::probe(const pal::string_t& relative_path, int64_t* offset, int64_ *offset = entry->offset(); *size = entry->size(); + *compressedSize = entry->compressedSize(); return true; } diff --git a/src/native/corehost/bundle/runner.h b/src/native/corehost/bundle/runner.h index 1cc6d3b03991a..0291df4b1729f 100644 --- a/src/native/corehost/bundle/runner.h +++ b/src/native/corehost/bundle/runner.h @@ -27,7 +27,7 @@ namespace bundle const pal::string_t& extraction_path() const { return m_extraction_path; } bool has_base(const pal::string_t& base) const { return base.compare(base_path()) == 0; } - bool probe(const pal::string_t& relative_path, int64_t* offset, int64_t* size) const; + bool probe(const pal::string_t& relative_path, int64_t* offset, int64_t* size, int64_t* compressedSize) const; const file_entry_t* probe(const pal::string_t& relative_path) const; bool locate(const pal::string_t& relative_path, pal::string_t& full_path, bool& extracted_to_disk) const; bool locate(const pal::string_t& relative_path, pal::string_t& full_path) const diff --git a/src/native/corehost/hostpolicy/hostpolicy_context.cpp b/src/native/corehost/hostpolicy/hostpolicy_context.cpp index 4c7ad15f2cb2b..2b9a953e28d53 100644 --- a/src/native/corehost/hostpolicy/hostpolicy_context.cpp +++ b/src/native/corehost/hostpolicy/hostpolicy_context.cpp @@ -23,7 +23,7 @@ namespace // This function is an API exported to the runtime via the BUNDLE_PROBE property. // This function used by the runtime to probe for bundled assemblies // This function assumes that the currently executing app is a single-file bundle. - bool STDMETHODCALLTYPE bundle_probe(const char* path, int64_t* offset, int64_t* size) + bool STDMETHODCALLTYPE bundle_probe(const char* path, int64_t* offset, int64_t* size, int64_t* compressedSize) { if (path == nullptr) { @@ -40,7 +40,7 @@ namespace return false; } - return bundle::runner_t::app()->probe(file_path, offset, size); + return bundle::runner_t::app()->probe(file_path, offset, size, compressedSize); } #if defined(NATIVE_LIBS_EMBEDDED) From 04a1791cc31abeee1d1e177666c353d9dbf86c23 Mon Sep 17 00:00:00 2001 From: vsadov Date: Tue, 6 Apr 2021 15:39:19 -0700 Subject: [PATCH 2/9] fix Unix build --- src/coreclr/vm/CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/vm/CMakeLists.txt b/src/coreclr/vm/CMakeLists.txt index 2ad98076f8bdd..92d2e2b59d02b 100644 --- a/src/coreclr/vm/CMakeLists.txt +++ b/src/coreclr/vm/CMakeLists.txt @@ -5,6 +5,11 @@ include_directories(BEFORE ${CMAKE_CURRENT_SOURCE_DIR}) include_directories(${ARCH_SOURCES_DIR}) include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../interop/inc) +# needed when zLib compression is used +if(NOT CLR_CMAKE_TARGET_WIN32) + include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../libraries/Native/Unix/Common) +endif() + add_definitions(-DUNICODE) add_definitions(-D_UNICODE) From 7cf28fbb7a32d026b41f8165943cb9d9baa8fdca Mon Sep 17 00:00:00 2001 From: vsadov Date: Thu, 8 Apr 2021 13:21:07 -0700 Subject: [PATCH 3/9] map should use converted layout for compressed --- src/coreclr/inc/pedecoder.h | 2 +- src/coreclr/utilcode/pedecoder.cpp | 3 +- src/coreclr/vm/peimagelayout.cpp | 44 ++++++++++++++++++------------ 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/coreclr/inc/pedecoder.h b/src/coreclr/inc/pedecoder.h index da5643108acc8..56997526c005d 100644 --- a/src/coreclr/inc/pedecoder.h +++ b/src/coreclr/inc/pedecoder.h @@ -236,7 +236,7 @@ class PEDecoder BOOL IsILOnly() const; CHECK CheckILOnly() const; - void LayoutILOnly(void *base, BOOL allowFullPE = FALSE) const; + void LayoutILOnly(void *base) const; // Strong name & hashing support diff --git a/src/coreclr/utilcode/pedecoder.cpp b/src/coreclr/utilcode/pedecoder.cpp index 91fde64d297c7..b1ee4861a4b88 100644 --- a/src/coreclr/utilcode/pedecoder.cpp +++ b/src/coreclr/utilcode/pedecoder.cpp @@ -1724,12 +1724,11 @@ CHECK PEDecoder::CheckILOnlyEntryPoint() const #ifndef DACCESS_COMPILE -void PEDecoder::LayoutILOnly(void *base, BOOL allowFullPE) const +void PEDecoder::LayoutILOnly(void *base) const { CONTRACT_VOID { INSTANCE_CHECK; - PRECONDITION(allowFullPE || CheckILOnlyFormat()); 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 diff --git a/src/coreclr/vm/peimagelayout.cpp b/src/coreclr/vm/peimagelayout.cpp index 3cb79e2d70000..10ce792d596ec 100644 --- a/src/coreclr/vm/peimagelayout.cpp +++ b/src/coreclr/vm/peimagelayout.cpp @@ -101,15 +101,21 @@ PEImageLayout* PEImageLayout::Map(PEImage* pOwner) } CONTRACT_END; - PEImageLayoutHolder pAlloc(new MappedImageLayout(pOwner)); + PEImageLayoutHolder pAlloc = pOwner->GetUncompressedSize() ? + LoadConverted(pOwner, /* isInBundle */ true): + new MappedImageLayout(pOwner); + if (pAlloc->GetBase()==NULL) { //cross-platform or a bad image pAlloc = LoadConverted(pOwner); } else - if(!pAlloc->CheckFormat()) + { + if (!pAlloc->CheckFormat()) ThrowHR(COR_E_BADIMAGEFORMAT); + } + RETURN pAlloc.Extract(); } @@ -411,34 +417,37 @@ ConvertedImageLayout::ConvertedImageLayout(PEImageLayout* source, BOOL isInBundl EEFileLoadException::Throw(GetPath(), COR_E_BADIMAGEFORMAT); LOG((LF_LOADER, LL_INFO100, "PEImage: Opening manually mapped stream\n")); -#if !defined(CROSSGEN_COMPILE) && !defined(TARGET_UNIX) - // on Windows we may want to enable execution if the image contains R2R sections - // so must ensure the mapping is compatible with that - m_FileMap.Assign(WszCreateFileMapping(INVALID_HANDLE_VALUE, NULL, - PAGE_EXECUTE_READWRITE, 0, - source->GetVirtualSize(), NULL)); + DWORD mapAccess; + DWORD viewAccess; + if (isInBundle && (HasNativeHeader() || HasReadyToRunHeader())) + { + // in bundle we may want to enable execution if the image contains R2R sections + // so must ensure the mapping is compatible with that + mapAccess = PAGE_EXECUTE_READWRITE; + viewAccess = FILE_MAP_EXECUTE | FILE_MAP_WRITE; + } + else + { + mapAccess = PAGE_READWRITE; + viewAccess = FILE_MAP_ALL_ACCESS; + } - DWORD allAccess = FILE_MAP_EXECUTE | FILE_MAP_WRITE; -#else m_FileMap.Assign(WszCreateFileMapping(INVALID_HANDLE_VALUE, NULL, - PAGE_READWRITE, 0, + mapAccess, 0, source->GetVirtualSize(), NULL)); - DWORD allAccess = FILE_MAP_ALL_ACCESS; -#endif - if (m_FileMap == NULL) ThrowLastError(); - m_FileView.Assign(CLRMapViewOfFile(m_FileMap, allAccess, 0, 0, 0, + m_FileView.Assign(CLRMapViewOfFile(m_FileMap, viewAccess, 0, 0, 0, (void *) source->GetPreferredBase())); if (m_FileView == NULL) - m_FileView.Assign(CLRMapViewOfFile(m_FileMap, allAccess, 0, 0, 0)); + m_FileView.Assign(CLRMapViewOfFile(m_FileMap, viewAccess, 0, 0, 0)); if (m_FileView == NULL) ThrowLastError(); - source->LayoutILOnly(m_FileView, TRUE); //@TODO should be false for streams + source->LayoutILOnly(m_FileView); IfFailThrow(Init(m_FileView)); #if defined(CROSSGEN_COMPILE) @@ -508,6 +517,7 @@ MappedImageLayout::MappedImageLayout(PEImage* pOwner) HANDLE hFile = pOwner->GetFileHandle(); INT64 offset = pOwner->GetOffset(); + _ASSERTE(!pOwner->GetUncompressedSize()); // If mapping was requested, try to do SEC_IMAGE mapping LOG((LF_LOADER, LL_INFO100, "PEImage: Opening OS mapped %S (hFile %p)\n", (LPCWSTR) GetPath(), hFile)); From 937e10d7868a918b644ae51539223e517d9ea45e Mon Sep 17 00:00:00 2001 From: vsadov Date: Thu, 8 Apr 2021 13:46:52 -0700 Subject: [PATCH 4/9] enable execution for R2R --- src/coreclr/inc/pedecoder.h | 2 +- src/coreclr/utilcode/pedecoder.cpp | 28 ++++++++++++++++------------ src/coreclr/vm/peimagelayout.cpp | 30 +++++++++++++++++++----------- 3 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/coreclr/inc/pedecoder.h b/src/coreclr/inc/pedecoder.h index 56997526c005d..d637835036dee 100644 --- a/src/coreclr/inc/pedecoder.h +++ b/src/coreclr/inc/pedecoder.h @@ -236,7 +236,7 @@ class PEDecoder BOOL IsILOnly() const; CHECK CheckILOnly() const; - void LayoutILOnly(void *base) const; + void LayoutILOnly(void *base, bool enableExecution) const; // Strong name & hashing support diff --git a/src/coreclr/utilcode/pedecoder.cpp b/src/coreclr/utilcode/pedecoder.cpp index b1ee4861a4b88..5ec34fa070000 100644 --- a/src/coreclr/utilcode/pedecoder.cpp +++ b/src/coreclr/utilcode/pedecoder.cpp @@ -1724,7 +1724,7 @@ CHECK PEDecoder::CheckILOnlyEntryPoint() const #ifndef DACCESS_COMPILE -void PEDecoder::LayoutILOnly(void *base) const +void PEDecoder::LayoutILOnly(void *base, bool enableExecution) const { CONTRACT_VOID { @@ -1773,18 +1773,22 @@ void PEDecoder::LayoutILOnly(void *base) const for (section = sectionStart; section < sectionEnd; section++) { // Add appropriate page protection. -#if defined(CROSSGEN_COMPILE) || defined(TARGET_UNIX) - if (section->Characteristics & IMAGE_SCN_MEM_WRITE) - continue; + DWORD newProtection; + if (!enableExecution) + { + if (section->Characteristics & IMAGE_SCN_MEM_WRITE) + continue; - DWORD newProtection = PAGE_READONLY; -#else - DWORD newProtection = section->Characteristics & IMAGE_SCN_MEM_EXECUTE ? - PAGE_EXECUTE_READ : - section->Characteristics & IMAGE_SCN_MEM_WRITE ? - PAGE_READWRITE : - PAGE_READONLY; -#endif + 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), diff --git a/src/coreclr/vm/peimagelayout.cpp b/src/coreclr/vm/peimagelayout.cpp index 10ce792d596ec..c42da063441a3 100644 --- a/src/coreclr/vm/peimagelayout.cpp +++ b/src/coreclr/vm/peimagelayout.cpp @@ -417,14 +417,24 @@ ConvertedImageLayout::ConvertedImageLayout(PEImageLayout* source, BOOL isInBundl EEFileLoadException::Throw(GetPath(), COR_E_BADIMAGEFORMAT); LOG((LF_LOADER, LL_INFO100, "PEImage: Opening manually mapped stream\n")); + // in bundle we may want to enable execution if the image contains R2R sections + // so must ensure the mapping is compatible with that + bool enableExecution = isInBundle && + source->HasCorHeader() && + (source->HasNativeHeader() || source->HasReadyToRunHeader()) && + g_fAllowNativeImages; + DWORD mapAccess; DWORD viewAccess; - if (isInBundle && (HasNativeHeader() || HasReadyToRunHeader())) + if (enableExecution) { - // in bundle we may want to enable execution if the image contains R2R sections - // so must ensure the mapping is compatible with that mapAccess = PAGE_EXECUTE_READWRITE; + +#if defined(CROSSGEN_COMPILE) || defined(TARGET_UNIX) + viewAccess = FILE_MAP_ALL_ACCESS; +#else viewAccess = FILE_MAP_EXECUTE | FILE_MAP_WRITE; +#endif } else { @@ -447,7 +457,7 @@ ConvertedImageLayout::ConvertedImageLayout(PEImageLayout* source, BOOL isInBundl if (m_FileView == NULL) ThrowLastError(); - source->LayoutILOnly(m_FileView); + source->LayoutILOnly(m_FileView, enableExecution); IfFailThrow(Init(m_FileView)); #if defined(CROSSGEN_COMPILE) @@ -455,11 +465,9 @@ ConvertedImageLayout::ConvertedImageLayout(PEImageLayout* source, BOOL isInBundl { ApplyBaseRelocations(); } -#elif !defined(TARGET_UNIX) - if (isInBundle && - HasCorHeader() && - (HasNativeHeader() || HasReadyToRunHeader()) && - g_fAllowNativeImages) + +#else + if (enableExecution) { if (!IsNativeMachineFormat()) ThrowHR(COR_E_BADIMAGEFORMAT); @@ -468,8 +476,8 @@ ConvertedImageLayout::ConvertedImageLayout(PEImageLayout* source, BOOL isInBundl // otherwise R2R will be disabled for this image. ApplyBaseRelocations(); - // Check if there is a static function table and install it. (except x86) -#if !defined(TARGET_X86) + // Check if there is a static function table and install it. (Windows only, except x86) +#if !defined(TARGET_UNIX) && !defined(TARGET_X86) COUNT_T cbSize = 0; PT_RUNTIME_FUNCTION pExceptionDir = (PT_RUNTIME_FUNCTION)GetDirectoryEntryData(IMAGE_DIRECTORY_ENTRY_EXCEPTION, &cbSize); DWORD tableSize = cbSize / sizeof(T_RUNTIME_FUNCTION); From 7b5d43e3715bd28f002a322e9f446c46db246e19 Mon Sep 17 00:00:00 2001 From: vsadov Date: Fri, 9 Apr 2021 11:13:58 -0700 Subject: [PATCH 5/9] fixes for OSX --- src/coreclr/vm/bundle.cpp | 4 ++-- src/coreclr/vm/peimage.cpp | 2 ++ src/coreclr/vm/peimagelayout.cpp | 18 ++++++------------ src/native/corehost/bundle/file_entry.h | 1 + 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/coreclr/vm/bundle.cpp b/src/coreclr/vm/bundle.cpp index 8a19b1b5b67f0..a51ed906b710d 100644 --- a/src/coreclr/vm/bundle.cpp +++ b/src/coreclr/vm/bundle.cpp @@ -80,8 +80,8 @@ BundleFileLocation Bundle::Probe(const SString& path, bool pathIsBundleRelative) } } - INT64 fileSize; - INT64 compressedSize; + INT64 fileSize = 0; + INT64 compressedSize = 0; m_probe(utf8Path, &loc.Offset, &fileSize, &compressedSize); diff --git a/src/coreclr/vm/peimage.cpp b/src/coreclr/vm/peimage.cpp index 9e83d8ff2f593..7a38a5549fd26 100644 --- a/src/coreclr/vm/peimage.cpp +++ b/src/coreclr/vm/peimage.cpp @@ -887,7 +887,9 @@ void PEImage::EnumMemoryRegions(CLRDataEnumMemoryFlags flags) PEImage::PEImage(): + m_path(), m_refCount(1), + m_bundleFileLocation(), m_bIsTrustedNativeImage(FALSE), m_bInHashMap(FALSE), #ifdef METADATATRACKER_DATA diff --git a/src/coreclr/vm/peimagelayout.cpp b/src/coreclr/vm/peimagelayout.cpp index c42da063441a3..1d437506322a3 100644 --- a/src/coreclr/vm/peimagelayout.cpp +++ b/src/coreclr/vm/peimagelayout.cpp @@ -424,23 +424,17 @@ ConvertedImageLayout::ConvertedImageLayout(PEImageLayout* source, BOOL isInBundl (source->HasNativeHeader() || source->HasReadyToRunHeader()) && g_fAllowNativeImages; - DWORD mapAccess; - DWORD viewAccess; + DWORD mapAccess = PAGE_READWRITE; + DWORD viewAccess = FILE_MAP_ALL_ACCESS; + +#if !defined(CROSSGEN_COMPILE) && !defined(TARGET_UNIX) if (enableExecution) { + // to make sections executable on Windows the view must have EXECUTE permissions mapAccess = PAGE_EXECUTE_READWRITE; - -#if defined(CROSSGEN_COMPILE) || defined(TARGET_UNIX) - viewAccess = FILE_MAP_ALL_ACCESS; -#else viewAccess = FILE_MAP_EXECUTE | FILE_MAP_WRITE; -#endif - } - else - { - mapAccess = PAGE_READWRITE; - viewAccess = FILE_MAP_ALL_ACCESS; } +#endif m_FileMap.Assign(WszCreateFileMapping(INVALID_HANDLE_VALUE, NULL, mapAccess, 0, diff --git a/src/native/corehost/bundle/file_entry.h b/src/native/corehost/bundle/file_entry.h index 225cf225bcde4..a342cbcd86262 100644 --- a/src/native/corehost/bundle/file_entry.h +++ b/src/native/corehost/bundle/file_entry.h @@ -37,6 +37,7 @@ namespace bundle file_entry_t() : m_offset(0) , m_size(0) + , m_compressedSize(0) , m_type(file_type_t::__last) , m_relative_path() , m_disabled(false) From f218f1904cbe4150fd5640bbc98ca2c224e6f288 Mon Sep 17 00:00:00 2001 From: vsadov Date: Fri, 9 Apr 2021 16:24:15 -0700 Subject: [PATCH 6/9] PR feedback (comments) --- src/coreclr/vm/peimagelayout.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/peimagelayout.cpp b/src/coreclr/vm/peimagelayout.cpp index 1d437506322a3..786071a691110 100644 --- a/src/coreclr/vm/peimagelayout.cpp +++ b/src/coreclr/vm/peimagelayout.cpp @@ -752,6 +752,9 @@ FlatImageLayout::FlatImageLayout(PEImage* pOwner) if (uncompressedSize > 0) { #if defined(CORECLR_EMBEDDED) + // The mapping we have just created refers to the region in the bundle that contains compressed data. + // We will create another anonymous memory-only mapping and uncompress file there. + // The flat image will refer to the anonimous mapping instead and we will release the original mapping. HandleHolder anonMap = WszCreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, uncompressedSize >> 32, (DWORD)uncompressedSize, NULL); if (anonMap == NULL) ThrowLastError(); @@ -760,15 +763,15 @@ FlatImageLayout::FlatImageLayout(PEImage* pOwner) if (anonView == NULL) ThrowLastError(); - //NB: PE cannot be larger than 4GB, conversions are ok + //NB: PE cannot be larger than 4GB and we are decompressing a managed assembly, which is a PE image, + // thus converting sizes to uint32 is ok. PAL_ZStream zStream; zStream.nextIn = (uint8_t*)addr; zStream.availIn = (uint32_t)size; zStream.nextOut = (uint8_t*)anonView; zStream.availOut = (uint32_t)uncompressedSize; - // Legal values are 8..15 and -8..-15. 15 is the window size, - // negative val causes deflate to produce raw deflate data (no zlib header). + // we match the compression side here. 15 is the window sise, negative means no zlib header. const int Deflate_DefaultWindowBits = -15; if (CompressionNative_InflateInit2_(&zStream, Deflate_DefaultWindowBits) != PAL_Z_OK) ThrowLastError(); @@ -780,7 +783,7 @@ FlatImageLayout::FlatImageLayout(PEImage* pOwner) addr = anonView; size = uncompressedSize; - // Replace file handles with handles to anonymous map. This will release the handles to the source view and map. + // Replace file handles with the handles to anonymous map. This will release the handles to the original view and map. m_FileView.Assign(anonView); m_FileMap.Assign(anonMap); From f522313dcbdd5b0af65a8ad226086d55c681771d Mon Sep 17 00:00:00 2001 From: vsadov Date: Mon, 12 Apr 2021 15:09:44 -0700 Subject: [PATCH 7/9] more PR feedback --- src/coreclr/vm/peimagelayout.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/peimagelayout.cpp b/src/coreclr/vm/peimagelayout.cpp index 786071a691110..5f121e873c661 100644 --- a/src/coreclr/vm/peimagelayout.cpp +++ b/src/coreclr/vm/peimagelayout.cpp @@ -519,7 +519,7 @@ MappedImageLayout::MappedImageLayout(PEImage* pOwner) HANDLE hFile = pOwner->GetFileHandle(); INT64 offset = pOwner->GetOffset(); - _ASSERTE(!pOwner->GetUncompressedSize()); + _ASSERTE(pOwner->GetUncompressedSize() == 0); // If mapping was requested, try to do SEC_IMAGE mapping LOG((LF_LOADER, LL_INFO100, "PEImage: Opening OS mapped %S (hFile %p)\n", (LPCWSTR) GetPath(), hFile)); @@ -774,12 +774,20 @@ FlatImageLayout::FlatImageLayout(PEImage* pOwner) // we match the compression side here. 15 is the window sise, negative means no zlib header. const int Deflate_DefaultWindowBits = -15; if (CompressionNative_InflateInit2_(&zStream, Deflate_DefaultWindowBits) != PAL_Z_OK) - ThrowLastError(); + ThrowHR(COR_E_BADIMAGEFORMAT); int ret = CompressionNative_Inflate(&zStream, PAL_Z_NOFLUSH); + + // decompression should have consumed the entire input + // and the entire output budgets + if ((ret < 0) || + !(zStream.availIn == 0 && zStream.availOut == 0)) + { + CompressionNative_InflateEnd(&zStream); + ThrowHR(COR_E_BADIMAGEFORMAT); + } + CompressionNative_InflateEnd(&zStream); - if (ret < 0) - ThrowLastError(); addr = anonView; size = uncompressedSize; @@ -789,7 +797,7 @@ FlatImageLayout::FlatImageLayout(PEImage* pOwner) #else _ASSERTE(!"Failure extracting contents of the application bundle. Compressed files used with a standalone (not singlefile) apphost."); - ThrowLastError(); + ThrowHR(E_FAIL); // we don't have any indication of what kind of failure. Possibly a corrupt image. #endif } } From 9b3f8d4a2bfe87399d7b06a0c9a3040fd27802c2 Mon Sep 17 00:00:00 2001 From: vsadov Date: Mon, 12 Apr 2021 20:45:49 -0700 Subject: [PATCH 8/9] shorter include path to pal_zlib.h --- eng/native/configurepaths.cmake | 1 + src/coreclr/vm/CMakeLists.txt | 3 ++- src/coreclr/vm/peimagelayout.cpp | 2 +- src/native/corehost/apphost/static/CMakeLists.txt | 3 ++- src/native/corehost/bundle/extractor.cpp | 2 +- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/eng/native/configurepaths.cmake b/eng/native/configurepaths.cmake index b53553391c81e..d59e1aafc5d1a 100644 --- a/eng/native/configurepaths.cmake +++ b/eng/native/configurepaths.cmake @@ -1,3 +1,4 @@ get_filename_component(CLR_REPO_ROOT_DIR ${CMAKE_CURRENT_LIST_DIR}/../.. ABSOLUTE) set(CLR_ENG_NATIVE_DIR ${CMAKE_CURRENT_LIST_DIR}) get_filename_component(CLR_SRC_NATIVE_DIR ${CMAKE_CURRENT_LIST_DIR}/../../src/native ABSOLUTE) +get_filename_component(CLR_SRC_LIBS_NATIVE_DIR ${CMAKE_CURRENT_LIST_DIR}/../../src/libraries/Native ABSOLUTE) diff --git a/src/coreclr/vm/CMakeLists.txt b/src/coreclr/vm/CMakeLists.txt index 92d2e2b59d02b..22de51cbd83b9 100644 --- a/src/coreclr/vm/CMakeLists.txt +++ b/src/coreclr/vm/CMakeLists.txt @@ -6,8 +6,9 @@ include_directories(${ARCH_SOURCES_DIR}) include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../interop/inc) # needed when zLib compression is used +include_directories(${CLR_SRC_LIBS_NATIVE_DIR}/AnyOS/zlib) if(NOT CLR_CMAKE_TARGET_WIN32) - include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../libraries/Native/Unix/Common) + include_directories(${CLR_SRC_LIBS_NATIVE_DIR}/Unix/Common) endif() add_definitions(-DUNICODE) diff --git a/src/coreclr/vm/peimagelayout.cpp b/src/coreclr/vm/peimagelayout.cpp index 5f121e873c661..e84951dc866d4 100644 --- a/src/coreclr/vm/peimagelayout.cpp +++ b/src/coreclr/vm/peimagelayout.cpp @@ -16,7 +16,7 @@ #if defined(CORECLR_EMBEDDED) extern "C" { -#include "../../../libraries/Native/AnyOS/zlib/pal_zlib.h" +#include "pal_zlib.h" } #endif diff --git a/src/native/corehost/apphost/static/CMakeLists.txt b/src/native/corehost/apphost/static/CMakeLists.txt index d7e129c5d2060..1ede7829767f9 100644 --- a/src/native/corehost/apphost/static/CMakeLists.txt +++ b/src/native/corehost/apphost/static/CMakeLists.txt @@ -19,8 +19,9 @@ set(SKIP_VERSIONING 1) include_directories(..) include_directories(../../json) +include_directories(${CLR_SRC_LIBS_NATIVE_DIR}/AnyOS/zlib) if(NOT CLR_CMAKE_TARGET_WIN32) - include_directories(../../../../libraries/Native/Unix/Common) + include_directories(${CLR_SRC_LIBS_NATIVE_DIR}/Unix/Common) endif() set(SOURCES diff --git a/src/native/corehost/bundle/extractor.cpp b/src/native/corehost/bundle/extractor.cpp index e33daf8e70ce0..da4449e1c25ff 100644 --- a/src/native/corehost/bundle/extractor.cpp +++ b/src/native/corehost/bundle/extractor.cpp @@ -10,7 +10,7 @@ #if defined(NATIVE_LIBS_EMBEDDED) extern "C" { -#include "../../../libraries/Native/AnyOS/zlib/pal_zlib.h" +#include "pal_zlib.h" } #endif From 113bd6b097e35cb80c535ba73b1e9ecdebffa2ea Mon Sep 17 00:00:00 2001 From: Vladimir Sadov Date: Wed, 14 Apr 2021 15:14:06 -0700 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: Vitek Karas --- src/coreclr/vm/peimagelayout.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/peimagelayout.cpp b/src/coreclr/vm/peimagelayout.cpp index e84951dc866d4..76a4f52d6edd3 100644 --- a/src/coreclr/vm/peimagelayout.cpp +++ b/src/coreclr/vm/peimagelayout.cpp @@ -101,7 +101,7 @@ PEImageLayout* PEImageLayout::Map(PEImage* pOwner) } CONTRACT_END; - PEImageLayoutHolder pAlloc = pOwner->GetUncompressedSize() ? + PEImageLayoutHolder pAlloc = pOwner->GetUncompressedSize() != 0 ? LoadConverted(pOwner, /* isInBundle */ true): new MappedImageLayout(pOwner); @@ -754,7 +754,7 @@ FlatImageLayout::FlatImageLayout(PEImage* pOwner) #if defined(CORECLR_EMBEDDED) // The mapping we have just created refers to the region in the bundle that contains compressed data. // We will create another anonymous memory-only mapping and uncompress file there. - // The flat image will refer to the anonimous mapping instead and we will release the original mapping. + // The flat image will refer to the anonymous mapping instead and we will release the original mapping. HandleHolder anonMap = WszCreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, uncompressedSize >> 32, (DWORD)uncompressedSize, NULL); if (anonMap == NULL) ThrowLastError();