From 075aa15c96c4457e4e350995600576bf69a26bb3 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Fri, 21 Mar 2025 11:49:23 -0700 Subject: [PATCH 1/2] =?UTF-8?q?Reapply=20"Refactor=20`external=5Fassembly?= =?UTF-8?q?=5Fprobe`=20to=20be=20separate=20from=20single-file=20bu?= =?UTF-8?q?=E2=80=A6"=20(#113738)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 15118117f2c9273ab545157225dd0c8b3bdb5824. --- src/coreclr/binder/assemblybindercommon.cpp | 53 +++++++-------- src/coreclr/binder/inc/assembly.hpp | 1 - .../binder/inc/assemblybindercommon.hpp | 6 +- src/coreclr/dlls/mscoree/exports.cpp | 8 +-- src/coreclr/inc/assemblyprobeextension.h | 68 +++++++++++++++++++ src/coreclr/inc/bundle.h | 13 ++-- src/coreclr/inc/hostinformation.h | 3 + src/coreclr/vm/CMakeLists.txt | 2 + src/coreclr/vm/assemblynative.cpp | 4 +- src/coreclr/vm/assemblyprobeextension.cpp | 42 ++++++++++++ src/coreclr/vm/assemblyspec.cpp | 2 +- src/coreclr/vm/bundle.cpp | 56 +++++---------- src/coreclr/vm/coreassemblyspec.cpp | 10 +-- src/coreclr/vm/hostinformation.cpp | 15 ++++ src/coreclr/vm/nativeimage.cpp | 6 +- src/coreclr/vm/peassembly.cpp | 4 +- src/coreclr/vm/peassembly.inl | 2 +- src/coreclr/vm/peimage.cpp | 9 +-- src/coreclr/vm/peimage.h | 29 ++++---- src/coreclr/vm/peimage.inl | 59 +++++++++++----- src/coreclr/vm/peimagelayout.cpp | 13 ++-- 21 files changed, 264 insertions(+), 141 deletions(-) create mode 100644 src/coreclr/inc/assemblyprobeextension.h create mode 100644 src/coreclr/vm/assemblyprobeextension.cpp diff --git a/src/coreclr/binder/assemblybindercommon.cpp b/src/coreclr/binder/assemblybindercommon.cpp index 6bd3d1653a8b07..fecd7538d1a407 100644 --- a/src/coreclr/binder/assemblybindercommon.cpp +++ b/src/coreclr/binder/assemblybindercommon.cpp @@ -37,7 +37,7 @@ extern HRESULT RuntimeInvokeHostAssemblyResolver(INT_PTR pManagedAssemblyLoadCon STDAPI BinderAcquirePEImage(LPCTSTR szAssemblyPath, PEImage** ppPEImage, - BundleFileLocation bundleFileLocation); + ProbeExtensionResult probeExtensionResult); namespace BINDER_SPACE { @@ -271,8 +271,8 @@ namespace BINDER_SPACE StackSString sCoreLibName(CoreLibName_IL_W); StackSString sCoreLib; BinderTracing::PathSource pathSource = BinderTracing::PathSource::Bundle; - BundleFileLocation bundleFileLocation = Bundle::ProbeAppBundle(sCoreLibName, /*pathIsBundleRelative */ true); - if (!bundleFileLocation.IsValid()) + ProbeExtensionResult probeExtensionResult = AssemblyProbeExtension::Probe(sCoreLibName, /*pathIsBundleRelative */ true); + if (!probeExtensionResult.IsValid()) { pathSource = BinderTracing::PathSource::ApplicationAssemblies; } @@ -282,7 +282,7 @@ namespace BINDER_SPACE hr = AssemblyBinderCommon::GetAssembly(sCoreLib, TRUE /* fIsInTPA */, &pSystemAssembly, - bundleFileLocation); + probeExtensionResult); BinderTracing::PathProbed(sCoreLib, pathSource, hr); @@ -322,7 +322,7 @@ namespace BINDER_SPACE hr = AssemblyBinderCommon::GetAssembly(sCoreLib, TRUE /* fIsInTPA */, &pSystemAssembly, - bundleFileLocation); + probeExtensionResult); BinderTracing::PathProbed(sCoreLib, BinderTracing::PathSource::ApplicationAssemblies, hr); } @@ -367,8 +367,8 @@ namespace BINDER_SPACE StackSString sCoreLibSatellite; BinderTracing::PathSource pathSource = BinderTracing::PathSource::Bundle; - BundleFileLocation bundleFileLocation = Bundle::ProbeAppBundle(relativePath, /*pathIsBundleRelative */ true); - if (!bundleFileLocation.IsValid()) + ProbeExtensionResult probeExtensionResult = AssemblyProbeExtension::Probe(relativePath, /*pathIsBundleRelative */ true); + if (!probeExtensionResult.IsValid()) { sCoreLibSatellite.Set(systemDirectory); pathSource = BinderTracing::PathSource::ApplicationAssemblies; @@ -379,7 +379,7 @@ namespace BINDER_SPACE IF_FAIL_GO(AssemblyBinderCommon::GetAssembly(sCoreLibSatellite, TRUE /* fIsInTPA */, &pSystemAssembly, - bundleFileLocation)); + probeExtensionResult)); BinderTracing::PathProbed(sCoreLibSatellite, pathSource, hr); *ppSystemAssembly = pSystemAssembly.Extract(); @@ -590,15 +590,15 @@ namespace BINDER_SPACE namespace { - HRESULT BindSatelliteResourceFromBundle( + HRESULT BindSatelliteResourceByProbeExtension( AssemblyName* pRequestedAssemblyName, SString &relativePath, BindResult* pBindResult) { HRESULT hr = S_OK; - BundleFileLocation bundleFileLocation = Bundle::ProbeAppBundle(relativePath, /* pathIsBundleRelative */ true); - if (!bundleFileLocation.IsValid()) + ProbeExtensionResult probeExtensionResult = AssemblyProbeExtension::Probe(relativePath, /* pathIsBundleRelative */ true); + if (!probeExtensionResult.IsValid()) { return hr; } @@ -607,7 +607,7 @@ namespace BINDER_SPACE hr = AssemblyBinderCommon::GetAssembly(relativePath, FALSE /* fIsInTPA */, &pAssembly, - bundleFileLocation); + probeExtensionResult); BinderTracing::PathProbed(relativePath, BinderTracing::PathSource::Bundle, hr); @@ -692,7 +692,7 @@ namespace BINDER_SPACE BindResult* pBindResult) { // Satellite resource probing strategy is to look: - // * First within the single-file bundle + // * First via probe extensions (single-file bundle, external data) // * Then under each of the Platform Resource Roots // * Then under each of the App Paths. // @@ -712,7 +712,7 @@ namespace BINDER_SPACE CombinePath(fileName, simpleNameRef, fileName); fileName.Append(W(".dll")); - hr = BindSatelliteResourceFromBundle(pRequestedAssemblyName, fileName, pBindResult); + hr = BindSatelliteResourceByProbeExtension(pRequestedAssemblyName, fileName, pBindResult); if (pBindResult->HaveResult() || FAILED(hr)) { @@ -841,12 +841,9 @@ namespace BINDER_SPACE ReleaseHolder pTPAAssembly; const SString& simpleName = pRequestedAssemblyName->GetSimpleName(); - // Is assembly in the bundle? - // Single-file bundle contents take precedence over TPA. - // The list of bundled assemblies is contained in the bundle manifest, and NOT in the TPA. - // Therefore the bundle is first probed using the assembly's simple name. - // If found, the assembly is loaded from the bundle. - if (Bundle::AppIsBundle()) + // Probing extensions (single-file, external probe) take precedence over TPA. + // For single-file, bundled assemblies should only be in the bundle manifest, not in the TPA. + if (AssemblyProbeExtension::IsEnabled()) { // Search Assembly.ni.dll, then Assembly.dll // The Assembly.ni.dll paths are rare, and intended for supporting managed C++ R2R assemblies. @@ -858,16 +855,16 @@ namespace BINDER_SPACE SString assemblyFileName(simpleName); assemblyFileName.Append(candidates[i]); - SString assemblyFilePath(Bundle::AppBundle->BasePath()); - assemblyFilePath.Append(assemblyFileName); - - BundleFileLocation bundleFileLocation = Bundle::ProbeAppBundle(assemblyFileName, /* pathIsBundleRelative */ true); - if (bundleFileLocation.IsValid()) + ProbeExtensionResult probeExtensionResult = AssemblyProbeExtension::Probe(assemblyFileName, /* pathIsBundleRelative */ true); + if (probeExtensionResult.IsValid()) { + SString assemblyFilePath(Bundle::AppIsBundle() ? Bundle::AppBundle->BasePath() : SString::Empty()); + assemblyFilePath.Append(assemblyFileName); + hr = GetAssembly(assemblyFilePath, TRUE, // fIsInTPA &pTPAAssembly, - bundleFileLocation); + probeExtensionResult); BinderTracing::PathProbed(assemblyFilePath, BinderTracing::PathSource::Bundle, hr); @@ -996,7 +993,7 @@ namespace BINDER_SPACE HRESULT AssemblyBinderCommon::GetAssembly(SString &assemblyPath, BOOL fIsInTPA, Assembly **ppAssembly, - BundleFileLocation bundleFileLocation) + ProbeExtensionResult probeExtensionResult) { HRESULT hr = S_OK; @@ -1012,7 +1009,7 @@ namespace BINDER_SPACE { LPCTSTR szAssemblyPath = const_cast(assemblyPath.GetUnicode()); - hr = BinderAcquirePEImage(szAssemblyPath, &pPEImage, bundleFileLocation); + hr = BinderAcquirePEImage(szAssemblyPath, &pPEImage, probeExtensionResult); IF_FAIL_GO(hr); } diff --git a/src/coreclr/binder/inc/assembly.hpp b/src/coreclr/binder/inc/assembly.hpp index 650877cafd888a..ebaf045207fe9e 100644 --- a/src/coreclr/binder/inc/assembly.hpp +++ b/src/coreclr/binder/inc/assembly.hpp @@ -25,7 +25,6 @@ #include "customassemblybinder.h" #endif // !defined(DACCESS_COMPILE) -#include "bundle.h" #include namespace BINDER_SPACE diff --git a/src/coreclr/binder/inc/assemblybindercommon.hpp b/src/coreclr/binder/inc/assemblybindercommon.hpp index cdc091b9802441..184d2cd3b8fb86 100644 --- a/src/coreclr/binder/inc/assemblybindercommon.hpp +++ b/src/coreclr/binder/inc/assemblybindercommon.hpp @@ -16,7 +16,7 @@ #include "bindertypes.hpp" #include "bindresult.hpp" -#include "bundle.h" +#include class AssemblyBinder; class DefaultAssemblyBinder; @@ -28,7 +28,7 @@ namespace BINDER_SPACE class AssemblyBinderCommon { public: - static HRESULT BindAssembly(/* in */ AssemblyBinder *pBinder, + static HRESULT BindAssembly(/* in */ AssemblyBinder *pBinder, /* in */ AssemblyName *pAssemblyName, /* in */ bool excludeAppPaths, /* out */ Assembly **ppAssembly); @@ -44,7 +44,7 @@ namespace BINDER_SPACE static HRESULT GetAssembly(/* in */ SString &assemblyPath, /* in */ BOOL fIsInTPA, /* out */ Assembly **ppAssembly, - /* in */ BundleFileLocation bundleFileLocation = BundleFileLocation::Invalid()); + /* in */ ProbeExtensionResult probeExtensionResult = ProbeExtensionResult::Invalid()); #if !defined(DACCESS_COMPILE) static HRESULT BindUsingHostAssemblyResolver (/* in */ INT_PTR pManagedAssemblyLoadContextToBindWithin, diff --git a/src/coreclr/dlls/mscoree/exports.cpp b/src/coreclr/dlls/mscoree/exports.cpp index 29be23657eb483..1515423445bb32 100644 --- a/src/coreclr/dlls/mscoree/exports.cpp +++ b/src/coreclr/dlls/mscoree/exports.cpp @@ -288,12 +288,9 @@ int coreclr_initialize( hr = CorHost2::CreateObject(IID_ICLRRuntimeHost4, (void**)&host); IfFailRet(hr); - ConstWStringHolder appDomainFriendlyNameW = StringToUnicode(appDomainFriendlyName); - - ExternalAssemblyProbeFn* externalAssemblyProbe = hostContract != nullptr ? hostContract->external_assembly_probe : nullptr; - if (bundleProbe != nullptr || externalAssemblyProbe != nullptr) + if (bundleProbe != nullptr) { - static Bundle bundle(exePath, bundleProbe, externalAssemblyProbe); + static Bundle bundle(exePath, bundleProbe); Bundle::AppBundle = &bundle; } @@ -309,6 +306,7 @@ int coreclr_initialize( hr = host->Start(); IfFailRet(hr); + ConstWStringHolder appDomainFriendlyNameW = StringToUnicode(appDomainFriendlyName); hr = host->CreateAppDomainWithManager( appDomainFriendlyNameW, 0, diff --git a/src/coreclr/inc/assemblyprobeextension.h b/src/coreclr/inc/assemblyprobeextension.h new file mode 100644 index 00000000000000..b745ac829de241 --- /dev/null +++ b/src/coreclr/inc/assemblyprobeextension.h @@ -0,0 +1,68 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#ifndef HAVE_ASSEMBLY_PROBE_EXTENSIONS_H +#define HAVE_ASSEMBLY_PROBE_EXTENSIONS_H + +#include +#include "bundle.h" + +class ProbeExtensionResult +{ +public: + enum class Type + { + Invalid, + Bundle, + External, + }; + + Type Type; + union + { + BundleFileLocation BundleLocation; + struct + { + void* Data; + int64_t Size; + } ExternalData; + }; + + ProbeExtensionResult() + : Type{Type::Invalid} + { } + + static ProbeExtensionResult Bundle(BundleFileLocation location) + { + return ProbeExtensionResult(location); + } + + static ProbeExtensionResult External(void* data, int64_t size) + { + return ProbeExtensionResult(data, size); + } + + static ProbeExtensionResult Invalid() { LIMITED_METHOD_CONTRACT; return ProbeExtensionResult(); } + + bool IsValid() const { return Type != Type::Invalid; } + +private: + ProbeExtensionResult(BundleFileLocation location) + : Type{Type::Bundle} + , BundleLocation{location} + { } + + ProbeExtensionResult(void* data, int64_t size) + : Type{Type::External} + , ExternalData{data, size} + { } +}; + +class AssemblyProbeExtension +{ +public: + static bool IsEnabled(); + static ProbeExtensionResult Probe(const SString& path, bool pathIsBundleRelative = false); +}; + +#endif // HAVE_ASSEMBLY_PROBE_EXTENSIONS_H diff --git a/src/coreclr/inc/bundle.h b/src/coreclr/inc/bundle.h index 6384ec6c1ec7e1..3cc887423b85fd 100644 --- a/src/coreclr/inc/bundle.h +++ b/src/coreclr/inc/bundle.h @@ -18,31 +18,29 @@ class Bundle; struct BundleFileLocation { INT64 Size; - void* DataStart; INT64 Offset; - INT64 UncompresedSize; + INT64 UncompressedSize; BundleFileLocation() { LIMITED_METHOD_CONTRACT; Size = 0; - DataStart = nullptr; Offset = 0; - UncompresedSize = 0; + UncompressedSize = 0; } static BundleFileLocation Invalid() { LIMITED_METHOD_CONTRACT; return BundleFileLocation(); } const SString &Path() const; - bool IsValid() const { LIMITED_METHOD_CONTRACT; return DataStart != nullptr || Offset != 0; } + bool IsValid() const { LIMITED_METHOD_CONTRACT; return Offset != 0; } }; class Bundle { public: - Bundle(LPCSTR bundlePath, BundleProbeFn *probe, ExternalAssemblyProbeFn* externalAssemblyProbe = nullptr); + Bundle(LPCSTR bundlePath, BundleProbeFn *probe); BundleFileLocation Probe(const SString& path, bool pathIsBundleRelative = false) const; const SString &Path() const { LIMITED_METHOD_CONTRACT; return m_path; } @@ -53,9 +51,8 @@ class Bundle static BundleFileLocation ProbeAppBundle(const SString& path, bool pathIsBundleRelative = false); private: - SString m_path; // The path to single-file executable or package name/id on Android + SString m_path; // The path to single-file executable BundleProbeFn *m_probe; - ExternalAssemblyProbeFn *m_externalAssemblyProbe; SString m_basePath; // The prefix to denote a path within the bundle COUNT_T m_basePathLength; diff --git a/src/coreclr/inc/hostinformation.h b/src/coreclr/inc/hostinformation.h index d57b4729d30e68..5a9a62fec48ab0 100644 --- a/src/coreclr/inc/hostinformation.h +++ b/src/coreclr/inc/hostinformation.h @@ -11,6 +11,9 @@ class HostInformation public: static void SetContract(_In_ host_runtime_contract* hostContract); static bool GetProperty(_In_z_ const char* name, SString& value); + + static bool HasExternalProbe(); + static bool ExternalAssemblyProbe(_In_ const SString& path, _Out_ void** data, _Out_ int64_t* size); }; #endif // _HOSTINFORMATION_H_ diff --git a/src/coreclr/vm/CMakeLists.txt b/src/coreclr/vm/CMakeLists.txt index 157edb931583a0..de4b671d4dc400 100644 --- a/src/coreclr/vm/CMakeLists.txt +++ b/src/coreclr/vm/CMakeLists.txt @@ -293,6 +293,7 @@ set(VM_SOURCES_WKS ${VM_SOURCES_DAC_AND_WKS_COMMON} appdomainnative.cpp assemblynative.cpp + assemblyprobeextension.cpp assemblyspec.cpp baseassemblyspec.cpp ${RUNTIME_DIR}/CachedInterfaceDispatch.cpp @@ -390,6 +391,7 @@ set(VM_HEADERS_WKS ../inc/jithelpers.h appdomainnative.hpp assemblynative.hpp + ../inc/assemblyprobeextension.h assemblyspec.hpp assemblyspecbase.h baseassemblyspec.h diff --git a/src/coreclr/vm/assemblynative.cpp b/src/coreclr/vm/assemblynative.cpp index c2d9d8706f25ed..507a723cd880c1 100644 --- a/src/coreclr/vm/assemblynative.cpp +++ b/src/coreclr/vm/assemblynative.cpp @@ -191,9 +191,7 @@ extern "C" void QCALLTYPE AssemblyNative_LoadFromPath(INT_PTR ptrNativeAssemblyB if (pwzILPath != NULL) { - pILImage = PEImage::OpenImage(pwzILPath, - MDInternalImport_Default, - BundleFileLocation::Invalid()); + pILImage = PEImage::OpenImage(pwzILPath); // Need to verify that this is a valid CLR assembly. if (!pILImage->CheckILFormat()) diff --git a/src/coreclr/vm/assemblyprobeextension.cpp b/src/coreclr/vm/assemblyprobeextension.cpp new file mode 100644 index 00000000000000..87d29716e79311 --- /dev/null +++ b/src/coreclr/vm/assemblyprobeextension.cpp @@ -0,0 +1,42 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include "common.h" +#include +#include +#include + +/*static*/ +bool AssemblyProbeExtension::IsEnabled() +{ + LIMITED_METHOD_CONTRACT; + + return Bundle::AppIsBundle() || HostInformation::HasExternalProbe(); +} + +/*static*/ +ProbeExtensionResult AssemblyProbeExtension::Probe(const SString& path, bool pathIsBundleRelative) +{ + STANDARD_VM_CONTRACT; + + if (!Bundle::AppIsBundle() && !HostInformation::HasExternalProbe()) + return ProbeExtensionResult::Invalid(); + + if (Bundle::AppIsBundle()) + { + BundleFileLocation bundleLocation = Bundle::AppBundle->Probe(path, pathIsBundleRelative); + if (bundleLocation.IsValid()) + { + return ProbeExtensionResult::Bundle(bundleLocation); + } + } + + void* data; + int64_t size; + if (HostInformation::ExternalAssemblyProbe(path, &data, &size)) + { + return ProbeExtensionResult::External(data, size); + } + + return ProbeExtensionResult::Invalid(); +} diff --git a/src/coreclr/vm/assemblyspec.cpp b/src/coreclr/vm/assemblyspec.cpp index 92a9942fc99716..87c30dbdc10f24 100644 --- a/src/coreclr/vm/assemblyspec.cpp +++ b/src/coreclr/vm/assemblyspec.cpp @@ -433,7 +433,7 @@ Assembly *AssemblySpec::LoadAssembly(LPCWSTR pFilePath) pILImage = PEImage::OpenImage(pFilePath, MDInternalImport_Default, - Bundle::ProbeAppBundle(SString{ SString::Literal, pFilePath })); + AssemblyProbeExtension::Probe(SString{ SString::Literal, pFilePath })); // Need to verify that this is a valid CLR assembly. if (!pILImage->CheckILFormat()) diff --git a/src/coreclr/vm/bundle.cpp b/src/coreclr/vm/bundle.cpp index 5e79d527ed5bc6..8e6c08482ad3de 100644 --- a/src/coreclr/vm/bundle.cpp +++ b/src/coreclr/vm/bundle.cpp @@ -30,27 +30,23 @@ const SString &BundleFileLocation::Path() const return Bundle::AppBundle->Path(); } -Bundle::Bundle(LPCSTR bundlePath, BundleProbeFn *probe, ExternalAssemblyProbeFn* externalAssemblyProbe) +Bundle::Bundle(LPCSTR bundlePath, BundleProbeFn *probe) : m_probe(probe) - , m_externalAssemblyProbe(externalAssemblyProbe) , m_basePathLength(0) { STANDARD_VM_CONTRACT; - _ASSERTE(m_probe != nullptr || m_externalAssemblyProbe != nullptr); + _ASSERTE(m_probe != nullptr); - // On Android this is not a real path, but rather the application's package name m_path.SetUTF8(bundlePath); -#if !defined(TARGET_ANDROID) + // The bundle-base path is the directory containing the single-file bundle. // When the Probe() function searches within the bundle, it masks out the basePath from the assembly-path (if found). - LPCSTR pos = strrchr(bundlePath, DIRECTORY_SEPARATOR_CHAR_A); _ASSERTE(pos != nullptr); size_t baseLen = pos - bundlePath + 1; // Include DIRECTORY_SEPARATOR_CHAR_A in m_basePath m_basePath.SetUTF8(bundlePath, (COUNT_T)baseLen); m_basePathLength = (COUNT_T)baseLen; -#endif // !TARGET_ANDROID } BundleFileLocation Bundle::Probe(const SString& path, bool pathIsBundleRelative) const @@ -83,45 +79,25 @@ BundleFileLocation Bundle::Probe(const SString& path, bool pathIsBundleRelative) } } - if (m_probe != nullptr) + BundleFileLocation loc; + INT64 fileSize = 0; + INT64 compressedSize = 0; + if (m_probe(utf8Path, &loc.Offset, &fileSize, &compressedSize)) { - BundleFileLocation loc; - INT64 fileSize = 0; - INT64 compressedSize = 0; - if (m_probe(utf8Path, &loc.Offset, &fileSize, &compressedSize)) + // Found assembly in bundle + if (compressedSize) { - // Found assembly in bundle - if (compressedSize) - { - loc.Size = compressedSize; - loc.UncompresedSize = fileSize; - } - else - { - loc.Size = fileSize; - loc.UncompresedSize = 0; - } - - return loc; + loc.Size = compressedSize; + loc.UncompressedSize = fileSize; } - } - - if (m_externalAssemblyProbe != nullptr) - { - BundleFileLocation loc; - if (m_externalAssemblyProbe(utf8Path, &loc.DataStart, &loc.Size)) + else { - // Found via external assembly probe - return loc; + loc.Size = fileSize; + loc.UncompressedSize = 0; } + + return loc; } return BundleFileLocation::Invalid(); } - -BundleFileLocation Bundle::ProbeAppBundle(const SString& path, bool pathIsBundleRelative) -{ - STANDARD_VM_CONTRACT; - - return AppIsBundle() ? AppBundle->Probe(path, pathIsBundleRelative) : BundleFileLocation::Invalid(); -} diff --git a/src/coreclr/vm/coreassemblyspec.cpp b/src/coreclr/vm/coreassemblyspec.cpp index a6b5f9752374b0..58a5d1ab56789a 100644 --- a/src/coreclr/vm/coreassemblyspec.cpp +++ b/src/coreclr/vm/coreassemblyspec.cpp @@ -17,7 +17,7 @@ #include "peimagelayout.inl" #include "domainassembly.h" #include "holder.h" -#include "bundle.h" +#include #include "strongnameinternal.h" #include "../binder/inc/assemblyidentity.hpp" @@ -76,9 +76,9 @@ HRESULT AssemblySpec::Bind(AppDomain *pAppDomain, BINDER_SPACE::Assembly** ppAs } -STDAPI BinderAcquirePEImage(LPCWSTR wszAssemblyPath, - PEImage **ppPEImage, - BundleFileLocation bundleFileLocation) +STDAPI BinderAcquirePEImage(LPCWSTR wszAssemblyPath, + PEImage **ppPEImage, + ProbeExtensionResult probeExtensionResult) { HRESULT hr = S_OK; @@ -86,7 +86,7 @@ STDAPI BinderAcquirePEImage(LPCWSTR wszAssemblyPath, EX_TRY { - PEImageHolder pImage = PEImage::OpenImage(wszAssemblyPath, MDInternalImport_Default, bundleFileLocation); + PEImageHolder pImage = PEImage::OpenImage(wszAssemblyPath, MDInternalImport_Default, probeExtensionResult); // Make sure that the IL image can be opened. if (pImage->IsFile()) diff --git a/src/coreclr/vm/hostinformation.cpp b/src/coreclr/vm/hostinformation.cpp index b440f6f2168ab7..44bd91d8662a8b 100644 --- a/src/coreclr/vm/hostinformation.cpp +++ b/src/coreclr/vm/hostinformation.cpp @@ -40,3 +40,18 @@ bool HostInformation::GetProperty(_In_z_ const char* name, SString& value) return lenActual > 0 && lenActual <= len; } + +bool HostInformation::HasExternalProbe() +{ + return s_hostContract != nullptr && s_hostContract->external_assembly_probe != nullptr; +} + +bool HostInformation::ExternalAssemblyProbe(_In_ const SString& path, _Out_ void** data, _Out_ int64_t* size) +{ + if (!HasExternalProbe()) + return false; + + StackSString utf8Path; + utf8Path.SetAndConvertToUTF8(path.GetUnicode()); + return s_hostContract->external_assembly_probe(utf8Path.GetUTF8(), data, size); +} diff --git a/src/coreclr/vm/nativeimage.cpp b/src/coreclr/vm/nativeimage.cpp index d68d031673f4e3..66e57b5e2a0004 100644 --- a/src/coreclr/vm/nativeimage.cpp +++ b/src/coreclr/vm/nativeimage.cpp @@ -148,13 +148,13 @@ NativeImage *NativeImage::Open( PEImageLayoutHolder peLoadedImage; - BundleFileLocation bundleFileLocation = Bundle::ProbeAppBundle(fullPath, /*pathIsBundleRelative */ true); - if (bundleFileLocation.IsValid()) + ProbeExtensionResult probeExtensionResult = AssemblyProbeExtension::Probe(fullPath, /*pathIsBundleRelative */ true); + if (probeExtensionResult.IsValid()) { // No need to use cache for this PE image. // Composite r2r PE image is not a part of anyone's identity. // We only need it to obtain the native image, which will be cached at AppDomain level. - PEImageHolder pImage = PEImage::OpenImage(fullPath, MDInternalImport_NoCache, bundleFileLocation); + PEImageHolder pImage = PEImage::OpenImage(fullPath, MDInternalImport_NoCache, probeExtensionResult); PEImageLayout* loaded = pImage->GetOrCreateLayout(PEImageLayout::LAYOUT_LOADED); // We will let pImage instance be freed after exiting this scope, but we will keep the layout, // thus the layout needs an AddRef, or it will be gone together with pImage. diff --git a/src/coreclr/vm/peassembly.cpp b/src/coreclr/vm/peassembly.cpp index a9349831732700..2d6db638c7734d 100644 --- a/src/coreclr/vm/peassembly.cpp +++ b/src/coreclr/vm/peassembly.cpp @@ -850,7 +850,7 @@ PEAssembly *PEAssembly::Create(IMetaDataAssemblyEmit *pAssemblyEmit) #ifndef DACCESS_COMPILE // Supports implementation of the legacy Assembly.CodeBase property. -// Returns false if the assembly was loaded from a bundle, true otherwise +// Returns false if the assembly was loaded via reflection emit or from a probe extension, true otherwise BOOL PEAssembly::GetCodeBase(SString &result) { CONTRACTL @@ -864,7 +864,7 @@ BOOL PEAssembly::GetCodeBase(SString &result) CONTRACTL_END; PEImage* ilImage = GetPEImage(); - if (ilImage != NULL && !ilImage->IsInBundle()) + if (ilImage != NULL && !ilImage->IsInBundle() && !ilImage->IsExternalData()) { // All other cases use the file path. result.Set(ilImage->GetPath()); diff --git a/src/coreclr/vm/peassembly.inl b/src/coreclr/vm/peassembly.inl index 124c3bd6787b26..eb485215f14cf2 100644 --- a/src/coreclr/vm/peassembly.inl +++ b/src/coreclr/vm/peassembly.inl @@ -150,7 +150,7 @@ inline const SString& PEAssembly::GetPath() } CONTRACTL_END; - if (IsReflectionEmit() || m_PEImage->IsInBundle ()) + if (IsReflectionEmit() || m_PEImage->IsInBundle() || m_PEImage->IsExternalData()) { return SString::Empty(); } diff --git a/src/coreclr/vm/peimage.cpp b/src/coreclr/vm/peimage.cpp index 19d9949327c295..45d281e25ee455 100644 --- a/src/coreclr/vm/peimage.cpp +++ b/src/coreclr/vm/peimage.cpp @@ -240,9 +240,10 @@ BOOL PEImage::CompareImage(UPTR u1, UPTR u2) PEImage *pImage = (PEImage *) u2; if (pLocator->m_bIsInBundle != pImage->IsInBundle()) - { return FALSE; - } + + if (pLocator->m_bIsExternalData != pImage->IsExternalData()) + return FALSE; BOOL ret = FALSE; HRESULT hr; @@ -547,7 +548,7 @@ PEImage::PEImage(const WCHAR* path): m_pathHash(0), m_refCount(1), m_bInHashMap(FALSE), - m_bundleFileLocation(), + m_probeExtensionResult(), m_hFile(INVALID_HANDLE_VALUE), m_dwPEKind(0), m_dwMachine(0), @@ -627,7 +628,7 @@ PTR_PEImageLayout PEImage::GetOrCreateLayoutInternal(DWORD imageLayoutMask) #ifdef TARGET_WINDOWS // on Windows we prefer to just load the file using OS loader - if (!IsInBundle() && bIsLoadedLayoutSuitable) + if (!IsInBundle() && IsFile() && bIsLoadedLayoutSuitable) { bIsLoadedLayoutPreferred = TRUE; } diff --git a/src/coreclr/vm/peimage.h b/src/coreclr/vm/peimage.h index c2d412e791b4c8..8df3a64494b69d 100644 --- a/src/coreclr/vm/peimage.h +++ b/src/coreclr/vm/peimage.h @@ -18,7 +18,7 @@ #include "peimagelayout.h" #include "sstring.h" #include "holder.h" -#include +#include class SimpleRWLock; // -------------------------------------------------------------------------------- @@ -119,9 +119,9 @@ class PEImage final static PTR_PEImage OpenImage( LPCWSTR pPath, MDInternalImportFlags flags = MDInternalImport_Default, - BundleFileLocation bundleFileLocation = BundleFileLocation::Invalid()); + ProbeExtensionResult probeExtensionResult = ProbeExtensionResult::Invalid()); - static PTR_PEImage FindByPath(LPCWSTR pPath, BOOL isInBundle); + static PTR_PEImage FindByPath(LPCWSTR pPath, BOOL isInBundle, BOOL isExternalData); void AddToHashMap(); #endif @@ -138,10 +138,11 @@ class PEImage final BOOL IsFile(); BOOL IsInBundle() const; + BOOL IsExternalData() const; void* GetExternalData(INT64* size); INT64 GetOffset() const; INT64 GetSize() const; - INT64 GetUncompressedSize() const; + BOOL IsCompressed(INT64* uncompressedSize = NULL) const; HANDLE GetFileHandle(); HRESULT TryOpenFile(bool takeLock = false); @@ -207,24 +208,26 @@ class PEImage final // Private routines // ------------------------------------------------------------ - void Init(BundleFileLocation bundleFileLocation); + void Init(ProbeExtensionResult probeExtensionResult); struct PEImageLocator { - LPCWSTR m_pPath; BOOL m_bIsInBundle; + BOOL m_bIsExternalData; - PEImageLocator(LPCWSTR pPath, BOOL bIsInBundle) - : m_pPath(pPath), - m_bIsInBundle(bIsInBundle) + PEImageLocator(LPCWSTR pPath, BOOL bIsInBundle, BOOL bIsExternalData) + : m_pPath(pPath) + , m_bIsInBundle(bIsInBundle) + , m_bIsExternalData(bIsExternalData) { } PEImageLocator(PEImage * pImage) : m_pPath(pImage->m_path.GetUnicode()) + , m_bIsInBundle(pImage->IsInBundle()) + , m_bIsExternalData(pImage->IsExternalData()) { - m_bIsInBundle = pImage->IsInBundle(); } }; @@ -288,9 +291,9 @@ class PEImage final // means this is a unique (deduped) instance. BOOL m_bInHashMap; - // If this image is located within a single-file bundle, the location within the bundle. - // If m_bundleFileLocation is valid, it takes precedence over m_path for loading. - BundleFileLocation m_bundleFileLocation; + // Valid if this image is from a probe extension (single-file bundle, external data). + // If m_probeExtensionResult is valid, it takes precedence over m_path for loading. + ProbeExtensionResult m_probeExtensionResult; // valid handle if we tried to open the file/path and succeeded. HANDLE m_hFile; diff --git a/src/coreclr/vm/peimage.inl b/src/coreclr/vm/peimage.inl index 57196222215cf1..70306304a23fd9 100644 --- a/src/coreclr/vm/peimage.inl +++ b/src/coreclr/vm/peimage.inl @@ -36,7 +36,7 @@ inline const SString& PEImage::GetPathToLoad() { LIMITED_METHOD_DAC_CONTRACT; - return IsInBundle() ? m_bundleFileLocation.Path() : m_path; + return IsInBundle() ? m_probeExtensionResult.BundleLocation.Path() : m_path; } inline void* PEImage::GetExternalData(INT64* size) @@ -44,35 +44,57 @@ inline void* PEImage::GetExternalData(INT64* size) LIMITED_METHOD_CONTRACT; _ASSERTE(size != nullptr); + if (!IsExternalData()) + { + *size = 0; + return nullptr; + } - *size = m_bundleFileLocation.Size; - return m_bundleFileLocation.DataStart; + *size = m_probeExtensionResult.ExternalData.Size; + return m_probeExtensionResult.ExternalData.Data; } inline INT64 PEImage::GetOffset() const { LIMITED_METHOD_CONTRACT; - return m_bundleFileLocation.Offset; + return IsInBundle() ? m_probeExtensionResult.BundleLocation.Offset : 0; } inline BOOL PEImage::IsInBundle() const { LIMITED_METHOD_CONTRACT; - return m_bundleFileLocation.IsValid(); + return m_probeExtensionResult.Type == ProbeExtensionResult::Type::Bundle; } -inline INT64 PEImage::GetSize() const +inline BOOL PEImage::IsExternalData() const { LIMITED_METHOD_CONTRACT; - return m_bundleFileLocation.Size; + + return m_probeExtensionResult.Type == ProbeExtensionResult::Type::External; +} + +inline INT64 PEImage::GetSize() const +{ + if (IsInBundle()) + return m_probeExtensionResult.BundleLocation.Size; + + if (IsExternalData()) + return m_probeExtensionResult.ExternalData.Size; + + // Size is not specified + return 0; } -inline INT64 PEImage::GetUncompressedSize() const +inline BOOL PEImage::IsCompressed(INT64* uncompressedSize) const { LIMITED_METHOD_CONTRACT; - return m_bundleFileLocation.UncompresedSize; + + if (uncompressedSize != NULL) + *uncompressedSize = IsInBundle() ? m_probeExtensionResult.BundleLocation.UncompressedSize : 0; + + return IsInBundle() && m_probeExtensionResult.BundleLocation.UncompressedSize != 0; } inline void PEImage::SetModuleFileNameHintForDAC() @@ -108,7 +130,7 @@ inline const SString &PEImage::GetModuleFileNameHintForDAC() inline BOOL PEImage::IsFile() { WRAPPER_NO_CONTRACT; - return m_bundleFileLocation.DataStart == nullptr && !GetPathToLoad().IsEmpty(); + return !IsExternalData() && !GetPathToLoad().IsEmpty(); } // @@ -281,7 +303,7 @@ inline CHECK PEImage::CheckFormat() CHECK_OK; } -inline void PEImage::Init(BundleFileLocation bundleFileLocation) +inline void PEImage::Init(ProbeExtensionResult probeExtensionResult) { CONTRACTL { @@ -292,14 +314,14 @@ inline void PEImage::Init(BundleFileLocation bundleFileLocation) CONTRACTL_END; m_pathHash = m_path.HashCaseInsensitive(); - m_bundleFileLocation = bundleFileLocation; + m_probeExtensionResult = probeExtensionResult; SetModuleFileNameHintForDAC(); } #ifndef DACCESS_COMPILE /*static*/ -inline PTR_PEImage PEImage::FindByPath(LPCWSTR pPath, BOOL isInBundle) +inline PTR_PEImage PEImage::FindByPath(LPCWSTR pPath, BOOL isInBundle, BOOL isExternalData) { CONTRACTL { @@ -307,31 +329,32 @@ inline PTR_PEImage PEImage::FindByPath(LPCWSTR pPath, BOOL isInBundle) GC_TRIGGERS; MODE_ANY; PRECONDITION(CheckPointer(pPath)); + PRECONDITION(!(isInBundle && isExternalData)); PRECONDITION(s_hashLock.OwnedByCurrentThread()); } CONTRACTL_END; int CaseHashHelper(const WCHAR *buffer, COUNT_T count); - PEImageLocator locator(pPath, isInBundle); + PEImageLocator locator(pPath, isInBundle, isExternalData); DWORD dwHash = CaseHashHelper(pPath, (COUNT_T) u16_strlen(pPath)); return (PEImage *) s_Images->LookupValue(dwHash, &locator); } /* static */ -inline PTR_PEImage PEImage::OpenImage(LPCWSTR pPath, MDInternalImportFlags flags /* = MDInternalImport_Default */, BundleFileLocation bundleFileLocation /* = BundleFileLocation::Invalid() */) +inline PTR_PEImage PEImage::OpenImage(LPCWSTR pPath, MDInternalImportFlags flags /* = MDInternalImport_Default */, ProbeExtensionResult probeExtensionResult /* = ProbeExtensionResult::Invalid() */) { BOOL forbidCache = (flags & MDInternalImport_NoCache); if (forbidCache) { PEImageHolder pImage(new PEImage{pPath}); - pImage->Init(bundleFileLocation); + pImage->Init(probeExtensionResult); return dac_cast(pImage.Extract()); } CrstHolder holder(&s_hashLock); - PEImage* found = FindByPath(pPath, bundleFileLocation.IsValid()); + PEImage* found = FindByPath(pPath, probeExtensionResult.Type == ProbeExtensionResult::Type::Bundle, probeExtensionResult.Type == ProbeExtensionResult::Type::External); if (found == (PEImage*) INVALIDENTRY) { // We did not find the entry in the Cache, and we've been asked to only use the cache. @@ -341,7 +364,7 @@ inline PTR_PEImage PEImage::OpenImage(LPCWSTR pPath, MDInternalImportFlags flags } PEImageHolder pImage(new PEImage{pPath}); - pImage->Init(bundleFileLocation); + pImage->Init(probeExtensionResult); pImage->AddToHashMap(); return dac_cast(pImage.Extract()); diff --git a/src/coreclr/vm/peimagelayout.cpp b/src/coreclr/vm/peimagelayout.cpp index 89486101c88aac..c96285f377c3ce 100644 --- a/src/coreclr/vm/peimagelayout.cpp +++ b/src/coreclr/vm/peimagelayout.cpp @@ -124,7 +124,7 @@ PEImageLayout* PEImageLayout::Load(PEImage* pOwner, HRESULT* loadFailure) { if (!pOwner->IsInBundle() #if defined(TARGET_UNIX) - || (pOwner->GetUncompressedSize() == 0) + || !pOwner->IsCompressed() #endif ) { @@ -537,7 +537,7 @@ LoadedImageLayout::LoadedImageLayout(PEImage* pOwner, HRESULT* loadFailure) CONTRACTL_END; m_pOwner = pOwner; - _ASSERTE(pOwner->GetUncompressedSize() == 0); + _ASSERTE(!pOwner->IsCompressed()); #ifndef TARGET_UNIX _ASSERTE(!pOwner->IsInBundle()); @@ -670,12 +670,13 @@ FlatImageLayout::FlatImageLayout(PEImage* pOwner) // It's okay if resource files are length zero if (size > 0) { - INT64 uncompressedSize = pOwner->GetUncompressedSize(); + INT64 uncompressedSize; + BOOL isCompressed = pOwner->IsCompressed(&uncompressedSize); DWORD mapAccess = PAGE_READONLY; #if !defined(TARGET_UNIX) // to map sections into executable views on Windows the mapping must have EXECUTE permissions - if (uncompressedSize == 0) + if (!isCompressed) { mapAccess = PAGE_EXECUTE_READ; } @@ -701,7 +702,7 @@ FlatImageLayout::FlatImageLayout(PEImage* pOwner) m_FileView.Assign(view); addr = (LPVOID)((size_t)view + offset - mapBegin); - if (uncompressedSize > 0) + if (isCompressed) { #if defined(CORECLR_EMBEDDED) // The mapping we have just created refers to the region in the bundle that contains compressed data. @@ -1031,7 +1032,7 @@ void* FlatImageLayout::LoadImageByMappingParts(SIZE_T* m_imageParts) const } CONTRACTL_END; - if (!HavePlaceholderAPI() || m_pOwner->GetUncompressedSize() != 0) + if (!HavePlaceholderAPI() || m_pOwner->IsCompressed()) { return NULL; } From 0d3c85285b463efb34ac24cfa2a0ab119df2a7cd Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Fri, 21 Mar 2025 16:35:33 -0700 Subject: [PATCH 2/2] Copy host contract values instead of relying on the pointer directly --- src/coreclr/vm/hostinformation.cpp | 18 ++++++++++-------- src/native/corehost/host_runtime_contract.h | 1 + 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/coreclr/vm/hostinformation.cpp b/src/coreclr/vm/hostinformation.cpp index 44bd91d8662a8b..4d85f74671d7dd 100644 --- a/src/coreclr/vm/hostinformation.cpp +++ b/src/coreclr/vm/hostinformation.cpp @@ -6,23 +6,25 @@ namespace { - host_runtime_contract* s_hostContract = nullptr; + host_runtime_contract s_hostContract = {}; } void HostInformation::SetContract(_In_ host_runtime_contract* hostContract) { - _ASSERTE(s_hostContract == nullptr); - s_hostContract = hostContract; + _ASSERTE(s_hostContract.size == 0 && hostContract != nullptr); + + // Copy the contract values + s_hostContract = *hostContract; } bool HostInformation::GetProperty(_In_z_ const char* name, SString& value) { - if (s_hostContract == nullptr || s_hostContract->get_runtime_property == nullptr) + if (s_hostContract.get_runtime_property == nullptr) return false; size_t len = MAX_PATH + 1; char* dest = value.OpenUTF8Buffer(static_cast(len)); - size_t lenActual = s_hostContract->get_runtime_property(name, dest, len, s_hostContract->context); + size_t lenActual = s_hostContract.get_runtime_property(name, dest, len, s_hostContract.context); value.CloseBuffer(); // Doesn't exist or failed to get property @@ -35,7 +37,7 @@ bool HostInformation::GetProperty(_In_z_ const char* name, SString& value) // Buffer was not large enough len = lenActual; dest = value.OpenUTF8Buffer(static_cast(len)); - lenActual = s_hostContract->get_runtime_property(name, dest, len, s_hostContract->context); + lenActual = s_hostContract.get_runtime_property(name, dest, len, s_hostContract.context); value.CloseBuffer(); return lenActual > 0 && lenActual <= len; @@ -43,7 +45,7 @@ bool HostInformation::GetProperty(_In_z_ const char* name, SString& value) bool HostInformation::HasExternalProbe() { - return s_hostContract != nullptr && s_hostContract->external_assembly_probe != nullptr; + return s_hostContract.external_assembly_probe != nullptr; } bool HostInformation::ExternalAssemblyProbe(_In_ const SString& path, _Out_ void** data, _Out_ int64_t* size) @@ -53,5 +55,5 @@ bool HostInformation::ExternalAssemblyProbe(_In_ const SString& path, _Out_ void StackSString utf8Path; utf8Path.SetAndConvertToUTF8(path.GetUnicode()); - return s_hostContract->external_assembly_probe(utf8Path.GetUTF8(), data, size); + return s_hostContract.external_assembly_probe(utf8Path.GetUTF8(), data, size); } diff --git a/src/native/corehost/host_runtime_contract.h b/src/native/corehost/host_runtime_contract.h index 9d03c52cf24bf0..fa7d27fce49717 100644 --- a/src/native/corehost/host_runtime_contract.h +++ b/src/native/corehost/host_runtime_contract.h @@ -23,6 +23,7 @@ #define HOST_PROPERTY_PLATFORM_RESOURCE_ROOTS "PLATFORM_RESOURCE_ROOTS" #define HOST_PROPERTY_TRUSTED_PLATFORM_ASSEMBLIES "TRUSTED_PLATFORM_ASSEMBLIES" +// Any callbacks set on this contract are expected to be valid for the lifetime of the process struct host_runtime_contract { size_t size;