Skip to content

Some cleanups in Assembly/Loader area #57023

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

Merged
23 commits merged into from
Aug 18, 2021
Merged

Some cleanups in Assembly/Loader area #57023

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
be13383
rename GAC -> TPA
VSadov Aug 6, 2021
440bd6e
remove GetAvailableImageTypes
VSadov Aug 7, 2021
081f08c
Removed ICLRPrivAssembly
VSadov Aug 7, 2021
27d414c
Move Assembly:: Add/Release to .inl
VSadov Aug 7, 2021
767ef95
ApplicationContext is not implementing IUnknown
VSadov Aug 13, 2021
a984bd5
ApplicationContext does not need AppDomainId
VSadov Aug 13, 2021
5b2218c
ICLRPrivBinder is not a COM object
VSadov Aug 13, 2021
68a9b4c
Assembly is not a binder and should not have BindAssemblyByName
VSadov Aug 14, 2021
24ca380
ApplicationContext is always an embedded value and does not need refe…
VSadov Aug 14, 2021
6ed1653
Simplified GetBinderID and GetLoaderAllocator, since not COM
VSadov Aug 14, 2021
d435108
Moved AppContext to up to ICLRPrivBinder
VSadov Aug 16, 2021
eb3bc8e
Removed GetBinderHash
VSadov Aug 16, 2021
331eccb
Removed a couple now pointless AddRef/Release
VSadov Aug 16, 2021
a186ca4
virtualized BindUsingAssemblyName
VSadov Aug 16, 2021
a616b18
renamed ICLRPrivBinder --> AssemblyBinder
VSadov Aug 16, 2021
63ef26b
renamed BINDER_SPACE::AssemblyBinder --> BINDER_SPACE::AssemblyBinder…
VSadov Aug 16, 2021
8d61a2d
Merge CCoreCLRBinderHelper into AssemblyBinderCommon
VSadov Aug 16, 2021
61068b0
Rename CLRPrivBinderCoreCLR -->DefaultAssemblyBinder
VSadov Aug 16, 2021
f1ee77f
Renamed CLRPrivBinderAssemblyLoadContext --> CustomAssemblyBinder
VSadov Aug 16, 2021
9920bc9
Renamed PTR_ICLRPrivBinder --> PTR_AssemblyBinder
VSadov Aug 16, 2021
6cc9cef
Remove clrprivbinding_i.cpp
VSadov Aug 16, 2021
babbcd2
A few touch ups
VSadov Aug 16, 2021
b241939
fix Linux build
VSadov Aug 16, 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
Prev Previous commit
Next Next commit
Merge CCoreCLRBinderHelper into AssemblyBinderCommon
  • Loading branch information
@VSadov
VSadov committed Aug 18, 2021
commit 8d61a2da5267305b95e5252a7cd1f407937831aa
2 changes: 0 additions & 2 deletions src/coreclr/binder/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ set(BINDER_COMMON_SOURCES
assemblyname.cpp
bindertracing.cpp
clrprivbindercoreclr.cpp
coreclrbindercommon.cpp
failurecache.cpp
stringlexer.cpp
textualidentityparser.cpp
Expand All @@ -37,7 +36,6 @@ set(BINDER_COMMON_HEADERS
inc/bindresult.hpp
inc/bindresult.inl
inc/clrprivbindercoreclr.h
inc/coreclrbindercommon.h
inc/failurecache.hpp
inc/failurecachehashtraits.hpp
inc/loadcontext.hpp
Expand Down
94 changes: 92 additions & 2 deletions src/coreclr/binder/assemblybindercommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//
// ============================================================

#include "common.h"
#include "assemblybindercommon.hpp"
#include "assemblyname.hpp"
#include "assembly.hpp"
Expand Down Expand Up @@ -347,6 +348,30 @@ namespace BINDER_SPACE
return hr;
}

#if !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
/* static */
HRESULT AssemblyBinderCommon::BindToSystem(BINDER_SPACE::Assembly** ppSystemAssembly, bool fBindToNativeImage)
{
HRESULT hr = S_OK;
_ASSERTE(ppSystemAssembly != NULL);

EX_TRY
{
ReleaseHolder<BINDER_SPACE::Assembly> pAsm;
StackSString systemPath(SystemDomain::System()->SystemDirectory());
hr = AssemblyBinderCommon::BindToSystem(systemPath, &pAsm, fBindToNativeImage);
if (SUCCEEDED(hr))
{
_ASSERTE(pAsm != NULL);
*ppSystemAssembly = pAsm.Extract();
}
}
EX_CATCH_HRESULT(hr);

return hr;
}
#endif // !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)

/* static */
HRESULT AssemblyBinderCommon::BindToSystem(SString &systemDirectory,
Assembly **ppSystemAssembly,
Expand Down Expand Up @@ -715,8 +740,6 @@ namespace BINDER_SPACE
AssemblyName *pAssemblyName,
ContextEntry **ppContextEntry)
{
HRESULT hr = S_OK;

_ASSERTE(pApplicationContext != NULL);
_ASSERTE(pAssemblyName != NULL);
_ASSERTE(ppContextEntry != NULL);
Expand Down Expand Up @@ -1542,6 +1565,73 @@ HRESULT AssemblyBinderCommon::BindUsingPEImage(/* in */ ::AssemblyBinder* pBind
tracer.TraceBindResult(bindResult, mvidMismatch);
return hr;
}

HRESULT AssemblyBinderCommon::DefaultBinderSetupContext(CLRPrivBinderCoreCLR** ppTPABinder)
{
HRESULT hr = S_OK;
EX_TRY
{
if (ppTPABinder != NULL)
{
NewHolder<CLRPrivBinderCoreCLR> pBinder;
SAFE_NEW(pBinder, CLRPrivBinderCoreCLR);

BINDER_SPACE::ApplicationContext* pApplicationContext = pBinder->GetAppContext();
hr = pApplicationContext->Init();
if (SUCCEEDED(hr))
{
pBinder->SetManagedAssemblyLoadContext(NULL);
*ppTPABinder = pBinder.Extract();
}
}
}
EX_CATCH_HRESULT(hr);

Exit:
return hr;
}

HRESULT AssemblyBinderCommon::GetAssemblyIdentity(LPCSTR szTextualIdentity,
BINDER_SPACE::ApplicationContext* pApplicationContext,
NewHolder<AssemblyIdentityUTF8>& assemblyIdentityHolder)
{
HRESULT hr = S_OK;
_ASSERTE(szTextualIdentity != NULL);

EX_TRY
{
AssemblyIdentityUTF8 * pAssemblyIdentity = NULL;
if (pApplicationContext != NULL)
{
// This returns a cached copy owned by application context
hr = pApplicationContext->GetAssemblyIdentity(szTextualIdentity, &pAssemblyIdentity);
if (SUCCEEDED(hr))
{
assemblyIdentityHolder = pAssemblyIdentity;
assemblyIdentityHolder.SuppressRelease();
}
}
else
{
SString sTextualIdentity;

sTextualIdentity.SetUTF8(szTextualIdentity);

// This is a private copy
pAssemblyIdentity = new AssemblyIdentityUTF8();
hr = TextualIdentityParser::Parse(sTextualIdentity, pAssemblyIdentity);
if (SUCCEEDED(hr))
{
pAssemblyIdentity->PopulateUTF8Fields();
assemblyIdentityHolder = pAssemblyIdentity;
}
}
}
EX_CATCH_HRESULT(hr);

return hr;
}

#endif // !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
};

Expand Down
125 changes: 0 additions & 125 deletions src/coreclr/binder/coreclrbindercommon.cpp
View file
Open in desktop

This file was deleted.

13 changes: 13 additions & 0 deletions src/coreclr/binder/inc/assemblybindercommon.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class PEImage;

namespace BINDER_SPACE
{
class AssemblyIdentityUTF8;

class AssemblyBinderCommon
{
public:
Expand All @@ -40,6 +42,8 @@ namespace BINDER_SPACE
/* in */ bool excludeAppPaths,
/* out */ Assembly **ppAssembly);

static HRESULT BindToSystem(BINDER_SPACE::Assembly** ppSystemAssembly, bool fBindToNativeImage);

static HRESULT BindToSystem(/* in */ SString &systemDirectory,
/* out */ Assembly **ppSystemAssembly,
/* in */ bool fBindToNativeImage);
Expand Down Expand Up @@ -72,6 +76,15 @@ namespace BINDER_SPACE

static HRESULT TranslatePEToArchitectureType(DWORD *pdwPAFlags, PEKIND *PeKind);

static HRESULT DefaultBinderSetupContext(CLRPrivBinderCoreCLR** ppTPABinder);

// TODO: The call indicates that this can come from a case where
Copy link
Member

@janvorli janvorli Sep 2, 2021

Choose a reason for hiding this comment

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

@VSadov is this TODO obsolete? If not, it would be nice to have an issue for it and reference it here.

Copy link
Member Author

@VSadov VSadov Sep 2, 2021

Choose a reason for hiding this comment

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

Possibly obsolete. CorLib may have NULL binder, but I am not sure if it goes through here.

I will check on this TODO in later changes.
I also think the statics on AssemblyBinderCommon may be moved to AssemblyBinder, but have not decided on that yet.

Copy link
Member Author

@VSadov VSadov Sep 2, 2021

Choose a reason for hiding this comment

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

I think some of this code will go to managed eventually, just want to clean it up a bit as-is beforehand. It seems like a good thing to do regardless.

// pDomain->GetFusionContext() is null, hence this is static function
// which handles a null binder. See if this actually happens
static HRESULT GetAssemblyIdentity(LPCSTR szTextualIdentity,
BINDER_SPACE::ApplicationContext* pApplicationContext,
NewHolder<BINDER_SPACE::AssemblyIdentityUTF8>& assemblyIdentityHolder);

private:
static HRESULT BindByName(/* in */ ApplicationContext *pApplicationContext,
/* in */ AssemblyName *pAssemblyName,
Expand Down
41 changes: 0 additions & 41 deletions src/coreclr/binder/inc/coreclrbindercommon.h
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions src/coreclr/vm/appdomain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@

#include "../binder/inc/bindertracing.h"
#include "../binder/inc/clrprivbindercoreclr.h"
#include "../binder/inc/coreclrbindercommon.h"
#include "../binder/inc/assemblybindercommon.hpp"

// this file handles string conversion errors for itself
#undef MAKE_TRANSLATIONFAILED
Expand Down Expand Up @@ -4336,7 +4336,7 @@ CLRPrivBinderCoreCLR *AppDomain::CreateBinderContext()
GCX_PREEMP();

// Initialize the assembly binder for the default context loads for CoreCLR.
IfFailThrow(CCoreCLRBinderHelper::DefaultBinderSetupContext(&m_pTPABinderContext));
IfFailThrow(BINDER_SPACE::AssemblyBinderCommon::DefaultBinderSetupContext(&m_pTPABinderContext));
}

RETURN m_pTPABinderContext;
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/vm/coreassemblyspec.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#include "../binder/inc/assembly.hpp"
#include "../binder/inc/assemblyname.hpp"

#include "../binder/inc/coreclrbindercommon.h"
#include "../binder/inc/assemblybindercommon.hpp"
#include "../binder/inc/applicationcontext.hpp"

STDAPI BinderAddRefPEImage(PEImage *pPEImage)
Expand Down Expand Up @@ -118,7 +118,7 @@ VOID AssemblySpec::Bind(AppDomain *pAppDomain,
tmpString.ConvertToUnicode(sCultureName);
}

hr = CCoreCLRBinderHelper::BindToSystemSatellite(sSystemDirectory, sSimpleName, sCultureName, &pPrivAsm);
hr = BINDER_SPACE::AssemblyBinderCommon::BindToSystemSatellite(sSystemDirectory, sSimpleName, sCultureName, &pPrivAsm);
}
else if (m_wszCodeBase == NULL)
{
Expand Down Expand Up @@ -314,7 +314,7 @@ HRESULT BaseAssemblySpec::ParseName()
pAppContext = pBinder->GetAppContext();
}

hr = CCoreCLRBinderHelper::GetAssemblyIdentity(m_pAssemblyName, pAppContext, pAssemblyIdentity);
hr = BINDER_SPACE::AssemblyBinderCommon::GetAssemblyIdentity(m_pAssemblyName, pAppContext, pAssemblyIdentity);

if (FAILED(hr))
{
Expand Down
Loading