Skip to content

Commit

Permalink
Some more cleanups in Assembly/Binder/Loader area (#59590)
Browse files Browse the repository at this point in the history 
* moved default binder allocation to AppDomain::Create

* allowNativeSkip parameter is unused.

* Removed PEFile::IsResource and PEFile::IsIStream - these do not exist in CoreClr

* make GCC happy

* folded impl

* Folded PEFile into PEAssembly

* renamed pefile files

* reorder PEAssembly members

* removed meaningless LAYOUT_CREATEIFNEEDED - we always require that

* r2r bundled image should Open via cache - no reasons not to.

* Some cleanup in PEImage

* PEAssembly needs only one reference to PEImage

* a few tweaks

* some more

* We never update published hosted assemblies, no need to support the scenario.

* Done with PEAssembly for now

* couple fixes, mostly in comments afected by renames

* more cleanups for  PEImage

* PR review suggestion

Co-authored-by: Elinor Fung <[email protected]>

* PR review feedback

* this line should be deleted, not commented.

* Apply suggestions from code review

Co-authored-by: Elinor Fung <[email protected]>

* More PR feedback

Co-authored-by: Elinor Fung <[email protected]>
  • Loading branch information
@VSadov @elinor-fung
VSadov and elinor-fung authored Oct 11, 2021
1 parent bda8727 commit 7088332
Show file tree
Hide file tree
Showing 82 changed files with 1,384 additions and 2,784 deletions.
6 changes: 2 additions & 4 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ internal RuntimeType[] GetDefinedTypes()
{
return GetTypes(this);
}

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern bool IsResource(RuntimeModule module);
#endregion

#region Module overrides
Expand Down Expand Up @@ -472,7 +469,8 @@ public override Guid ModuleVersionId

public override bool IsResource()
{
return IsResource(this);
// CoreClr does not support resource-only modules.
return false;
}

[RequiresUnreferencedCode("Fields might be removed")]
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/binder/bindertracing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,12 @@ namespace
DomainAssembly *parentAssembly = spec->GetParentAssembly();
if (parentAssembly != nullptr)
{
PEAssembly *peAssembly = parentAssembly->GetFile();
_ASSERTE(peAssembly != nullptr);
peAssembly->GetDisplayName(request.RequestingAssembly);
PEAssembly *pPEAssembly = parentAssembly->GetPEAssembly();
_ASSERTE(pPEAssembly != nullptr);
pPEAssembly->GetDisplayName(request.RequestingAssembly);

AppDomain *domain = parentAssembly->GetAppDomain();
AssemblyBinder *binder = peAssembly->GetAssemblyBinder();
AssemblyBinder *binder = pPEAssembly->GetAssemblyBinder();

GetAssemblyLoadContextNameFromBinder(binder, domain, request.RequestingAssemblyLoadContext);
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/debug/createdump/crashinfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ CrashInfo::EnumerateManagedModules(IXCLRDataProcess* pClrDataProcess)
if (SUCCEEDED(hr = moduleData.Request(pClrDataModule.GetPtr())))
{
TRACE("MODULE: %" PRIA PRIx64 " dyn %d inmem %d file %d pe %" PRIA PRIx64 " pdb %" PRIA PRIx64, (uint64_t)moduleData.LoadedPEAddress, moduleData.IsDynamic,
moduleData.IsInMemory, moduleData.IsFileLayout, (uint64_t)moduleData.PEFile, (uint64_t)moduleData.InMemoryPdbAddress);
moduleData.IsInMemory, moduleData.IsFileLayout, (uint64_t)moduleData.PEAssembly, (uint64_t)moduleData.InMemoryPdbAddress);

if (!moduleData.IsDynamic && moduleData.LoadedPEAddress != 0)
{
Expand Down
57 changes: 26 additions & 31 deletions src/coreclr/debug/daccess/daccess.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -679,11 +679,6 @@ MetaEnum::New(Module* mod,
*handle = TO_CDENUM(NULL);
}

if (!mod->GetFile()->HasMetadata())
{
return S_FALSE;
}

metaEnum = new (nothrow) MetaEnum;
if (!metaEnum)
{
Expand Down Expand Up @@ -4079,9 +4074,9 @@ ClrDataAccess::GetModuleByAddress(
{
TADDR base;
ULONG32 length;
PEFile* file = modDef->GetFile();
PEAssembly* pPEAssembly = modDef->GetPEAssembly();

if ((base = PTR_TO_TADDR(file->GetLoadedImageContents(&length))))
if ((base = PTR_TO_TADDR(pPEAssembly->GetLoadedImageContents(&length))))
{
if (TO_CDADDR(base) <= address &&
TO_CDADDR(base + length) > address)
Expand Down Expand Up @@ -4133,9 +4128,9 @@ ClrDataAccess::StartEnumMethodDefinitionsByAddress(
{
TADDR base;
ULONG32 length;
PEFile* file = modDef->GetFile();
PEAssembly* assembly = modDef->GetPEAssembly();

if ((base = PTR_TO_TADDR(file->GetLoadedImageContents(&length))))
if ((base = PTR_TO_TADDR(assembly->GetLoadedImageContents(&length))))
{
if (TO_CDADDR(base) <= address &&
TO_CDADDR(base + length) > address)
Expand Down Expand Up @@ -6459,7 +6454,7 @@ ClrDataAccess::GetHostGcNotificationTable()
}

/* static */ bool
ClrDataAccess::GetMetaDataFileInfoFromPEFile(PEFile *pPEFile,
ClrDataAccess::GetMetaDataFileInfoFromPEFile(PEAssembly *pPEAssembly,
DWORD &dwTimeStamp,
DWORD &dwSize,
DWORD &dwDataSize,
Expand All @@ -6477,7 +6472,7 @@ ClrDataAccess::GetMetaDataFileInfoFromPEFile(PEFile *pPEFile,
isNGEN = false;
if (pDir == NULL || pDir->Size == 0)
{
mdImage = pPEFile->GetILimage();
mdImage = pPEAssembly->GetPEImage();
if (mdImage != NULL)
{
layout = mdImage->GetLoadedLayout();
Expand Down Expand Up @@ -6526,7 +6521,7 @@ ClrDataAccess::GetMetaDataFileInfoFromPEFile(PEFile *pPEFile,
}

/* static */
bool ClrDataAccess::GetILImageInfoFromNgenPEFile(PEFile *peFile,
bool ClrDataAccess::GetILImageInfoFromNgenPEFile(PEAssembly *pPEAssembly,
DWORD &dwTimeStamp,
DWORD &dwSize,
__out_ecount(cchFilePath) LPWSTR wszFilePath,
Expand All @@ -6536,10 +6531,10 @@ bool ClrDataAccess::GetILImageInfoFromNgenPEFile(PEFile *peFile,
DWORD dwWritten = 0;

// use the IL File name
if (!peFile->GetPath().DacGetUnicode(cchFilePath, wszFilePath, (COUNT_T *)(&dwWritten)))
if (!pPEAssembly->GetPath().DacGetUnicode(cchFilePath, wszFilePath, (COUNT_T *)(&dwWritten)))
{
// Use DAC hint to retrieve the IL name.
peFile->GetModuleFileNameHint().DacGetUnicode(cchFilePath, wszFilePath, (COUNT_T *)(&dwWritten));
pPEAssembly->GetModuleFileNameHint().DacGetUnicode(cchFilePath, wszFilePath, (COUNT_T *)(&dwWritten));
}
dwTimeStamp = 0;
dwSize = 0;
Expand Down Expand Up @@ -6603,7 +6598,7 @@ bool ClrDataAccess::GetILImageNameFromNgenImage( LPCWSTR ilExtension,
#endif // FEATURE_CORESYSTEM

void *
ClrDataAccess::GetMetaDataFromHost(PEFile* peFile,
ClrDataAccess::GetMetaDataFromHost(PEAssembly* pPEAssembly,
bool* isAlternate)
{
DWORD imageTimestamp, imageSize, dataSize;
Expand Down Expand Up @@ -6634,7 +6629,7 @@ ClrDataAccess::GetMetaDataFromHost(PEFile* peFile,
// so the field remains for now.

if (!ClrDataAccess::GetMetaDataFileInfoFromPEFile(
peFile,
pPEAssembly,
imageTimestamp,
imageSize,
dataSize,
Expand All @@ -6647,7 +6642,7 @@ ClrDataAccess::GetMetaDataFromHost(PEFile* peFile,
}

// try direct match for the image that is loaded into the managed process
peFile->GetLoadedMetadata((COUNT_T *)(&dataSize));
pPEAssembly->GetLoadedMetadata((COUNT_T *)(&dataSize));

DWORD allocSize = 0;
if (!ClrSafeInt<DWORD>::addition(dataSize, sizeof(DAC_INSTANCE), allocSize))
Expand All @@ -6665,7 +6660,7 @@ ClrDataAccess::GetMetaDataFromHost(PEFile* peFile,
buffer = (void*)(inst + 1);

// APIs implemented by hosting debugger. It can use the path/filename, timestamp, and
// file size to find an exact match for the image. If that fails for an ngen'ed image,
// pPEAssembly size to find an exact match for the image. If that fails for an ngen'ed image,
// we can request the IL image which it came from.
if (m_legacyMetaDataLocator)
{
Expand Down Expand Up @@ -6701,7 +6696,7 @@ ClrDataAccess::GetMetaDataFromHost(PEFile* peFile,
//
isAlt = true;
if (!ClrDataAccess::GetILImageInfoFromNgenPEFile(
peFile,
pPEAssembly,
imageTimestamp,
imageSize,
uniPath,
Expand Down Expand Up @@ -6781,13 +6776,13 @@ ClrDataAccess::GetMetaDataFromHost(PEFile* peFile,

//++++++++++++++++++++++++++++++++++++++++++++++++++++++++
//
// Given a PEFile or a ReflectionModule try to find the corresponding metadata
// Given a PEAssembly or a ReflectionModule try to find the corresponding metadata
// We will first ask debugger to locate it. If fail, we will try
// to get it from the target process
//
//++++++++++++++++++++++++++++++++++++++++++++++++++++++++
IMDInternalImport*
ClrDataAccess::GetMDImport(const PEFile* peFile, const ReflectionModule* reflectionModule, bool throwEx)
ClrDataAccess::GetMDImport(const PEAssembly* pPEAssembly, const ReflectionModule* reflectionModule, bool throwEx)
{
HRESULT status;
PTR_CVOID mdBaseTarget = NULL;
Expand All @@ -6796,22 +6791,22 @@ ClrDataAccess::GetMDImport(const PEFile* peFile, const ReflectionModule* reflect
PVOID mdBaseHost = NULL;
bool isAlternate = false;

_ASSERTE((peFile == NULL && reflectionModule != NULL) || (peFile != NULL && reflectionModule == NULL));
TADDR peFileAddr = (peFile != NULL) ? dac_cast<TADDR>(peFile) : dac_cast<TADDR>(reflectionModule);
_ASSERTE((pPEAssembly == NULL && reflectionModule != NULL) || (pPEAssembly != NULL && reflectionModule == NULL));
TADDR peAssemblyAddr = (pPEAssembly != NULL) ? dac_cast<TADDR>(pPEAssembly) : dac_cast<TADDR>(reflectionModule);

//
// Look for one we've already created.
//
mdImport = m_mdImports.Get(peFileAddr);
mdImport = m_mdImports.Get(peAssemblyAddr);
if (mdImport != NULL)
{
return mdImport;
}

if (peFile != NULL)
if (pPEAssembly != NULL)
{
// Get the metadata size
mdBaseTarget = ((PEFile*)peFile)->GetLoadedMetadata(&mdSize);
mdBaseTarget = const_cast<PEAssembly*>(pPEAssembly)->GetLoadedMetadata(&mdSize);
}
else if (reflectionModule != NULL)
{
Expand Down Expand Up @@ -6862,12 +6857,12 @@ ClrDataAccess::GetMDImport(const PEFile* peFile, const ReflectionModule* reflect
}

// Try to see if debugger can locate it
if (peFile != NULL && mdBaseHost == NULL && (m_target3 || m_legacyMetaDataLocator))
if (pPEAssembly != NULL && mdBaseHost == NULL && (m_target3 || m_legacyMetaDataLocator))
{
// We couldn't read the metadata from memory. Ask
// the target for metadata as it may be able to
// provide it from some alternate means.
mdBaseHost = GetMetaDataFromHost(const_cast<PEFile *>(peFile), &isAlternate);
mdBaseHost = GetMetaDataFromHost(const_cast<PEAssembly *>(pPEAssembly), &isAlternate);
}

if (mdBaseHost == NULL)
Expand Down Expand Up @@ -6902,7 +6897,7 @@ ClrDataAccess::GetMDImport(const PEFile* peFile, const ReflectionModule* reflect
// The m_mdImports list does get cleaned up by calls to ClrDataAccess::Flush,
// i.e. every time the process changes state.

if (m_mdImports.Add(peFileAddr, mdImport, isAlternate) == NULL)
if (m_mdImports.Add(peAssemblyAddr, mdImport, isAlternate) == NULL)
{
mdImport->Release();
DacError(E_OUTOFMEMORY);
Expand Down Expand Up @@ -6970,9 +6965,9 @@ HRESULT ClrDataAccess::VerifyDlls()
}

// Read the debug directory timestamp from the target mscorwks image using DAC
// Note that we don't use the PE timestamp because the PE file might be changed in ways
// Note that we don't use the PE timestamp because the PE pPEAssembly might be changed in ways
// that don't effect the PDB (and therefore don't effect DAC). Specifically, we rebase
// our DLLs at the end of a build, that changes the PE file, but not the PDB.
// our DLLs at the end of a build, that changes the PE pPEAssembly, but not the PDB.
// Note that if we wanted to be extra careful, we could read the CV contents (which includes
// the GUID signature) and verify it matches. Using the timestamp is useful for helpful error
// messages, and should be sufficient in any real scenario.
Expand Down
Loading

0 comments on commit 7088332

Please sign in to comment.