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

Reapply "Refactor external_assembly_probe to be separate from single-file bundle probing" #113776

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
53 changes: 25 additions & 28 deletions src/coreclr/binder/assemblybindercommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ extern HRESULT RuntimeInvokeHostAssemblyResolver(INT_PTR pManagedAssemblyLoadCon

STDAPI BinderAcquirePEImage(LPCTSTR szAssemblyPath,
PEImage** ppPEImage,
BundleFileLocation bundleFileLocation);
ProbeExtensionResult probeExtensionResult);

namespace BINDER_SPACE
{
Expand Down Expand Up @@ -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;
}
Expand All @@ -282,7 +282,7 @@ namespace BINDER_SPACE
hr = AssemblyBinderCommon::GetAssembly(sCoreLib,
TRUE /* fIsInTPA */,
&pSystemAssembly,
bundleFileLocation);
probeExtensionResult);

BinderTracing::PathProbed(sCoreLib, pathSource, hr);

Expand Down Expand Up @@ -322,7 +322,7 @@ namespace BINDER_SPACE
hr = AssemblyBinderCommon::GetAssembly(sCoreLib,
TRUE /* fIsInTPA */,
&pSystemAssembly,
bundleFileLocation);
probeExtensionResult);

BinderTracing::PathProbed(sCoreLib, BinderTracing::PathSource::ApplicationAssemblies, hr);
}
Expand Down Expand Up @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -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;
}
Expand All @@ -607,7 +607,7 @@ namespace BINDER_SPACE
hr = AssemblyBinderCommon::GetAssembly(relativePath,
FALSE /* fIsInTPA */,
&pAssembly,
bundleFileLocation);
probeExtensionResult);

BinderTracing::PathProbed(relativePath, BinderTracing::PathSource::Bundle, hr);

Expand Down Expand Up @@ -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.
//
Expand All @@ -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))
{
Expand Down Expand Up @@ -841,12 +841,9 @@ namespace BINDER_SPACE
ReleaseHolder<Assembly> 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.
Expand All @@ -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);

Expand Down Expand Up @@ -996,7 +993,7 @@ namespace BINDER_SPACE
HRESULT AssemblyBinderCommon::GetAssembly(SString &assemblyPath,
BOOL fIsInTPA,
Assembly **ppAssembly,
BundleFileLocation bundleFileLocation)
ProbeExtensionResult probeExtensionResult)
{
HRESULT hr = S_OK;

Expand All @@ -1012,7 +1009,7 @@ namespace BINDER_SPACE
{
LPCTSTR szAssemblyPath = const_cast<LPCTSTR>(assemblyPath.GetUnicode());

hr = BinderAcquirePEImage(szAssemblyPath, &pPEImage, bundleFileLocation);
hr = BinderAcquirePEImage(szAssemblyPath, &pPEImage, probeExtensionResult);
IF_FAIL_GO(hr);
}

Expand Down
1 change: 0 additions & 1 deletion src/coreclr/binder/inc/assembly.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include "customassemblybinder.h"
#endif // !defined(DACCESS_COMPILE)

#include "bundle.h"
#include <assemblybinderutil.h>

namespace BINDER_SPACE
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/binder/inc/assemblybindercommon.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

#include "bindertypes.hpp"
#include "bindresult.hpp"
#include "bundle.h"
#include <assemblyprobeextension.h>

class AssemblyBinder;
class DefaultAssemblyBinder;
Expand All @@ -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);
Expand All @@ -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,
Expand Down
8 changes: 3 additions & 5 deletions src/coreclr/dlls/mscoree/exports.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -309,6 +306,7 @@ int coreclr_initialize(
hr = host->Start();
IfFailRet(hr);

ConstWStringHolder appDomainFriendlyNameW = StringToUnicode(appDomainFriendlyName);
hr = host->CreateAppDomainWithManager(
appDomainFriendlyNameW,
0,
Expand Down
68 changes: 68 additions & 0 deletions src/coreclr/inc/assemblyprobeextension.h
Original file line number Diff line number Diff line change
@@ -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 <sstring.h>
#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
13 changes: 5 additions & 8 deletions src/coreclr/inc/bundle.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/inc/hostinformation.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_
2 changes: 2 additions & 0 deletions src/coreclr/vm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -390,6 +391,7 @@ set(VM_HEADERS_WKS
../inc/jithelpers.h
appdomainnative.hpp
assemblynative.hpp
../inc/assemblyprobeextension.h
assemblyspec.hpp
assemblyspecbase.h
baseassemblyspec.h
Expand Down
4 changes: 1 addition & 3 deletions src/coreclr/vm/assemblynative.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
Loading
Loading