From 0622fa017488afe5c8262a012caad1404afbd75e Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 5 Jan 2021 11:19:02 -0800 Subject: [PATCH] Return actual LoadLibrary failures for IJW scenarios. (#45997) * Return actual LoadLibrary failures for IJW scenarios. --- src/coreclr/vm/peimage.cpp | 32 +++++++++++++++++++------------- src/coreclr/vm/peimagelayout.cpp | 17 ++++++++++------- src/coreclr/vm/peimagelayout.h | 4 ++-- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/coreclr/vm/peimage.cpp b/src/coreclr/vm/peimage.cpp index fc2c7de..9e83d8f 100644 --- a/src/coreclr/vm/peimage.cpp +++ b/src/coreclr/vm/peimage.cpp @@ -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) @@ -1045,21 +1047,25 @@ 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 don't bother doing a conversion + // of PE header on 64bit, as done for .NET Framework, since there is no + // appcompat burden for CoreCLR on 64bit. + fMarkAnyCpuImageAsLoaded = true; } pLayout.SuppressRelease(); diff --git a/src/coreclr/vm/peimagelayout.cpp b/src/coreclr/vm/peimagelayout.cpp index ad0114a..232b5bf 100644 --- a/src/coreclr/vm/peimagelayout.cpp +++ b/src/coreclr/vm/peimagelayout.cpp @@ -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; @@ -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(); @@ -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 { @@ -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)); diff --git a/src/coreclr/vm/peimagelayout.h b/src/coreclr/vm/peimagelayout.h index b5b0573..6116ca0 100644 --- a/src/coreclr/vm/peimagelayout.h +++ b/src/coreclr/vm/peimagelayout.h @@ -51,7 +51,7 @@ public: 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); @@ -142,7 +142,7 @@ protected: HINSTANCE m_Module; public: #ifndef DACCESS_COMPILE - LoadedImageLayout(PEImage* pOwner, BOOL bNTSafeLoad, BOOL bThrowOnError); + LoadedImageLayout(PEImage* pOwner, BOOL bNTSafeLoad, HRESULT* returnDontThrow); ~LoadedImageLayout() { CONTRACTL -- 2.7.4