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.

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

Jump to bottom

Return actual LoadLibrary failures for IJW scenarios. #45997

Merged
merged 2 commits into from
Jan 5, 2021
Merged

Return actual LoadLibrary failures for IJW scenarios. #45997

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2be0e16
Return actual LoadLibrary failures for IJW scenarios.
AaronRobinsonMSFT Dec 12, 2020
d12c4b1
Clarify comment.
AaronRobinsonMSFT Jan 5, 2021
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
Next Next commit
Return actual LoadLibrary failures for IJW scenarios.
  • Loading branch information
@AaronRobinsonMSFT
AaronRobinsonMSFT committed Dec 12, 2020
commit 2be0e165685cd3c2b79d34dc409a04df0be9fe6b
31 changes: 18 additions & 13 deletions src/coreclr/vm/peimage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1026,11 +1026,13 @@ PTR_PEImageLayout PEImage::CreateLayoutMapped()

PEImageLayout * pLoadLayout = NULL;

HRESULT loadFailure = S_OK;
if (m_bIsTrustedNativeImage || IsFile())
{
// For CoreCLR, try to load all files via LoadLibrary first. If LoadLibrary did not work, retry using
// regular mapping - but not for native images.
pLoadLayout = PEImageLayout::Load(this, FALSE /* bNTSafeLoad */, m_bIsTrustedNativeImage /* bThrowOnError */);
// Try to load all files via LoadLibrary first. If LoadLibrary did not work,
// retry using regular mapping.
HRESULT* returnDontThrow = m_bIsTrustedNativeImage ? NULL : &loadFailure;
pLoadLayout = PEImageLayout::Load(this, FALSE /* bNTSafeLoad */, returnDontThrow);
}

if (pLoadLayout != NULL)
Expand All @@ -1045,21 +1047,24 @@ PTR_PEImageLayout PEImage::CreateLayoutMapped()
PEImageLayoutHolder pLayout(PEImageLayout::Map(this));

bool fMarkAnyCpuImageAsLoaded = false;

// Avoid mapping another image if we can. We can only do this for IL-ONLY images
// since LoadLibrary is needed if we are to actually load code
// since LoadLibrary is needed if we are to actually load code (e.g. IJW).
if (pLayout->HasCorHeader())
{
if (pLayout->IsILOnly())
// IJW images must be successfully loaded by the OS to handle
// native dependencies, therefore they cannot be mapped.
if (!pLayout->IsILOnly())
{
// For CoreCLR, IL only images will always be mapped. We also dont bother doing the conversion of PE header on 64bit,
// as done below for the desktop case, as there is no appcompat burden for CoreCLR on 64bit to have that conversion done.
fMarkAnyCpuImageAsLoaded = true;
}
else
{
// IJW images must be loaded, not mapped
ThrowHR(COR_E_BADIMAGEFORMAT);
// For compat with older CoreCLR versions we will fallback to the
// COR_E_BADIMAGEFORMAT error code if a failure wasn't indicated.
loadFailure = FAILED(loadFailure) ? loadFailure : COR_E_BADIMAGEFORMAT;
EEFileLoadException::Throw(GetPath(), loadFailure);
}

// IL only images will always be mapped. We also dont bother doing the conversion of PE header on 64bit,
// as done below for the desktop case, as there is no appcompat burden for CoreCLR on 64bit to have that conversion done.
AaronRobinsonMSFT marked this conversation as resolved.
Show resolved Hide resolved
fMarkAnyCpuImageAsLoaded = true;
}

pLayout.SuppressRelease();
Expand Down
17 changes: 10 additions & 7 deletions src/coreclr/vm/peimagelayout.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ PEImageLayout* PEImageLayout::LoadConverted(PEImage* pOwner, BOOL isInBundle)
return new ConvertedImageLayout(pFlat, isInBundle);
}

PEImageLayout* PEImageLayout::Load(PEImage* pOwner, BOOL bNTSafeLoad, BOOL bThrowOnError)
PEImageLayout* PEImageLayout::Load(PEImage* pOwner, BOOL bNTSafeLoad, HRESULT* returnDontThrow)
{
STANDARD_VM_CONTRACT;

Expand All @@ -62,7 +62,7 @@ PEImageLayout* PEImageLayout::Load(PEImage* pOwner, BOOL bNTSafeLoad, BOOL bThro
return PEImageLayout::LoadConverted(pOwner, true);
}

PEImageLayoutHolder pAlloc(new LoadedImageLayout(pOwner,bNTSafeLoad,bThrowOnError));
PEImageLayoutHolder pAlloc(new LoadedImageLayout(pOwner,bNTSafeLoad,returnDontThrow));
if (pAlloc->GetBase()==NULL)
return NULL;
return pAlloc.Extract();
Expand Down Expand Up @@ -644,7 +644,7 @@ MappedImageLayout::MappedImageLayout(PEImage* pOwner)
}

#if !defined(CROSSGEN_COMPILE) && !defined(TARGET_UNIX)
LoadedImageLayout::LoadedImageLayout(PEImage* pOwner, BOOL bNTSafeLoad, BOOL bThrowOnError)
LoadedImageLayout::LoadedImageLayout(PEImage* pOwner, BOOL bNTSafeLoad, HRESULT* returnDontThrow)
{
CONTRACTL
{
Expand All @@ -664,12 +664,15 @@ LoadedImageLayout::LoadedImageLayout(PEImage* pOwner, BOOL bNTSafeLoad, BOOL bTh
m_Module = CLRLoadLibraryEx(pOwner->GetPath(), NULL, dwFlags);
if (m_Module == NULL)
{
if (!bThrowOnError)
return;

// Fetch the HRESULT upfront before anybody gets a chance to corrupt it
HRESULT hr = HRESULT_FROM_GetLastError();
EEFileLoadException::Throw(pOwner->GetPath(), hr, NULL);
if (returnDontThrow != NULL)
{
*returnDontThrow = hr;
return;
}

EEFileLoadException::Throw(pOwner->GetPath(), hr);
}
IfFailThrow(Init(m_Module,true));

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/peimagelayout.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class PEImageLayout : public PEDecoder
static PEImageLayout* CreateFlat(const void *flat, COUNT_T size,PEImage* pOwner);
static PEImageLayout* CreateFromHMODULE(HMODULE mappedbase,PEImage* pOwner, BOOL bTakeOwnership);
static PEImageLayout* LoadFromFlat(PEImageLayout* pflatimage);
static PEImageLayout* Load(PEImage* pOwner, BOOL bNTSafeLoad, BOOL bThrowOnError = TRUE);
static PEImageLayout* Load(PEImage* pOwner, BOOL bNTSafeLoad, HRESULT* returnDontThrow = NULL);
static PEImageLayout* LoadFlat(PEImage* pOwner);
static PEImageLayout* LoadConverted(PEImage* pOwner, BOOL isInBundle = FALSE);
static PEImageLayout* LoadNative(LPCWSTR fullPath);
Expand Down Expand Up @@ -142,7 +142,7 @@ class LoadedImageLayout: public PEImageLayout
HINSTANCE m_Module;
public:
#ifndef DACCESS_COMPILE
LoadedImageLayout(PEImage* pOwner, BOOL bNTSafeLoad, BOOL bThrowOnError);
LoadedImageLayout(PEImage* pOwner, BOOL bNTSafeLoad, HRESULT* returnDontThrow);
~LoadedImageLayout()
{
CONTRACTL
Expand Down