Skip to content
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.

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

Singlefile: enabling compression for managed assemblies. #50817

Merged
merged 9 commits into from
Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
2 changes: 1 addition & 1 deletion src/coreclr/hosts/inc/coreclrhost.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);


Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/inc/bundle.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/inc/pedecoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ class PEDecoder
BOOL IsILOnly() const;
CHECK CheckILOnly() const;

void LayoutILOnly(void *base, BOOL allowFullPE = FALSE) const;
void LayoutILOnly(void *base, bool enableExecution) const;

// Strong name & hashing support

Expand Down
29 changes: 16 additions & 13 deletions src/coreclr/utilcode/pedecoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1724,12 +1724,11 @@ CHECK PEDecoder::CheckILOnlyEntryPoint() const

#ifndef DACCESS_COMPILE

void PEDecoder::LayoutILOnly(void *base, BOOL allowFullPE) const
void PEDecoder::LayoutILOnly(void *base, bool enableExecution) 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
Expand Down Expand Up @@ -1774,18 +1773,22 @@ void PEDecoder::LayoutILOnly(void *base, BOOL allowFullPE) 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;
janvorli marked this conversation as resolved.
Show resolved Hide resolved
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),
Expand Down
11 changes: 10 additions & 1 deletion src/coreclr/vm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -107,7 +112,6 @@ set(VM_SOURCES_DAC_AND_WKS_COMMON
onstackreplacement.cpp
pefile.cpp
peimage.cpp
peimagelayout.cpp
perfmap.cpp
perfinfo.cpp
pgo.cpp
Expand Down Expand Up @@ -891,6 +895,11 @@ endif(CLR_CMAKE_TARGET_WIN32)
set (VM_SOURCES_WKS_SPECIAL
codeman.cpp
ceemain.cpp
peimagelayout.cpp
Copy link
Member

Choose a reason for hiding this comment

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

Why couldn't it stay in the common list? I have originally structured the lists so that no file needed to be duplicated in multiple lists.

Copy link
Member Author

@VSadov VSadov Apr 13, 2021

Choose a reason for hiding this comment

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

The common list is compiled by the build only once. That includes almost every file. And we reuse objects for singlefile and regular coreclr.

Very few files need to know about singlefile and have parts conditionally compiled for CORECLR_EMBEDDED. peimagelayout.cpp cannot and does not need to support decompression in regular coreclr mode, so it needs to use #ifdef CORECLR_EMBEDDED

)

list(APPEND VM_SOURCES_DAC
peimagelayout.cpp
)

# gcdecode.cpp is included by both JIT and VM. to avoid duplicate definitions we need to
Expand Down
16 changes: 15 additions & 1 deletion src/coreclr/vm/bundle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,21 @@ BundleFileLocation Bundle::Probe(const SString& path, bool pathIsBundleRelative)
}
}

m_probe(utf8Path, &loc.Offset, &loc.Size);
INT64 fileSize = 0;
INT64 compressedSize = 0;

m_probe(utf8Path, &loc.Offset, &fileSize, &compressedSize);

if (compressedSize)
{
loc.Size = compressedSize;
loc.UncompresedSize = fileSize;
}
else
{
loc.Size = fileSize;
loc.UncompresedSize = 0;
}

return loc;
}
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/peimage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,9 @@ void PEImage::EnumMemoryRegions(CLRDataEnumMemoryFlags flags)


PEImage::PEImage():
m_path(),
Copy link
Member

Choose a reason for hiding this comment

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

Are these additions really needed? Isn't the parameter-less constructor used by default?

Copy link
Member Author

@VSadov VSadov Apr 13, 2021

Choose a reason for hiding this comment

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

Only if you do not have a constructor, compiler may generate one. Once you have a constructor, compiler just calls it.

new PEImage() would leave these fields uninitialized, but typically we were lucky except for OSX with optimizations (i.e. does not fail in Debug, but started failing in Checked/Release)

m_refCount(1),
m_bundleFileLocation(),
m_bIsTrustedNativeImage(FALSE),
m_bInHashMap(FALSE),
#ifdef METADATATRACKER_DATA
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 @@ -148,6 +148,7 @@ class PEImage
HANDLE GetFileHandle();
INT64 GetOffset() const;
INT64 GetSize() const;
INT64 GetUncompressedSize() const;

void SetFileHandle(HANDLE hFile);
HRESULT TryOpenFile();
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/vm/peimage.inl
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
132 changes: 103 additions & 29 deletions src/coreclr/vm/peimagelayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@
#include "amsi.h"
#endif

#if defined(CORECLR_EMBEDDED)
extern "C"
{
#include "../../../libraries/Native/AnyOS/zlib/pal_zlib.h"
Copy link
Member

Choose a reason for hiding this comment

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

I don't like long relative paths in general, they are a burden when structure of the source tree changes. It seems it would be nicer to add the ../../../libraries/Native/AnyOS/zlib to the include paths in cmake scripts and have just

#include "pal_zlib.h"

here. It would be also nice to define the libraries path at one place in the cmake files and use it as a basis for this and the path in src/coreclr/vm/CMakeLists.txt.

Copy link
Member Author

Choose a reason for hiding this comment

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

There was another long pall_zlib.h include in apphost/static. I have changed that too.

}
#endif

#ifndef DACCESS_COMPILE
PEImageLayout* PEImageLayout::CreateFlat(const void *flat, COUNT_T size,PEImage* pOwner)
{
Expand Down Expand Up @@ -94,15 +101,21 @@ PEImageLayout* PEImageLayout::Map(PEImage* pOwner)
}
CONTRACT_END;

PEImageLayoutHolder pAlloc(new MappedImageLayout(pOwner));
PEImageLayoutHolder pAlloc = pOwner->GetUncompressedSize() ?
Copy link
Member

Choose a reason for hiding this comment

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

Similar as the other place - I would prefer pOwner->GetUncompressedSize() != 0 - using numbers as booleans is confusing.

VSadov marked this conversation as resolved.
Show resolved Hide resolved
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();
}

Expand Down Expand Up @@ -397,54 +410,58 @@ ConvertedImageLayout::ConvertedImageLayout(PEImageLayout* source, BOOL isInBundl
m_Layout=LAYOUT_LOADED;
m_pOwner=source->m_pOwner;
_ASSERTE(!source->IsMapped());
m_isInBundle = isInBundle;

m_pExceptionDir = NULL;

if (!source->HasNTHeaders())
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
// in bundle 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));
bool enableExecution = isInBundle &&
source->HasCorHeader() &&
(source->HasNativeHeader() || source->HasReadyToRunHeader()) &&
g_fAllowNativeImages;

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;
viewAccess = FILE_MAP_EXECUTE | FILE_MAP_WRITE;
vitek-karas marked this conversation as resolved.
Show resolved Hide resolved
}
#endif

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, enableExecution);
IfFailThrow(Init(m_FileView));

#if defined(CROSSGEN_COMPILE)
if (HasNativeHeader())
{
ApplyBaseRelocations();
}
#elif !defined(TARGET_UNIX)
if (m_isInBundle &&
HasCorHeader() &&
(HasNativeHeader() || HasReadyToRunHeader()) &&
g_fAllowNativeImages)

#else
if (enableExecution)
{
if (!IsNativeMachineFormat())
ThrowHR(COR_E_BADIMAGEFORMAT);
Expand All @@ -453,8 +470,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);
Expand Down Expand Up @@ -502,6 +519,7 @@ MappedImageLayout::MappedImageLayout(PEImage* pOwner)

HANDLE hFile = pOwner->GetFileHandle();
INT64 offset = pOwner->GetOffset();
_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));
Expand Down Expand Up @@ -704,6 +722,8 @@ FlatImageLayout::FlatImageLayout(PEImage* pOwner)
}
}

LPVOID addr = 0;

// It's okay if resource files are length zero
if (size > 0)
{
Expand All @@ -721,14 +741,68 @@ 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);
vitek-karas marked this conversation as resolved.
Show resolved Hide resolved

INT64 uncompressedSize = pOwner->GetUncompressedSize();
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.
VSadov marked this conversation as resolved.
Show resolved Hide resolved
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();
vitek-karas marked this conversation as resolved.
Show resolved Hide resolved

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

// 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)
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);

addr = anonView;
size = uncompressedSize;
// 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);

#else
_ASSERTE(!"Failure extracting contents of the application bundle. Compressed files used with a standalone (not singlefile) apphost.");
ThrowHR(E_FAIL); // we don't have any indication of what kind of failure. Possibly a corrupt image.
#endif
}
}
Init(m_FileView, (COUNT_T)size);

Init(addr, (COUNT_T)size);
}

NativeImageLayout::NativeImageLayout(LPCWSTR fullPath)
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/peimagelayout.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ class ConvertedImageLayout: public PEImageLayout
virtual ~ConvertedImageLayout();
#endif
private:
bool m_isInBundle;
PT_RUNTIME_FUNCTION m_pExceptionDir;
};

Expand Down
Loading