Skip to content

Remove logic around checking for and determining NGEN paths from dac-dbi interface #108853

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
merged 7 commits into from
Oct 15, 2024
Merged

Remove logic around checking for and determining NGEN paths from dac-dbi interface #108853

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7b98b54
Remove isNGEN parameter from GetMetaDataFileInfoFromPEFile - always s…
elinor-fung Oct 11, 2024
84fdf95
Remove IDacDbiInterface::GetModuleNGenPath - always returns FALSE and…
elinor-fung Oct 11, 2024
7b36ef4
Remove CordbModule::GetNGenImagePath - always null
elinor-fung Oct 11, 2024
b86e172
Remove GetILImageInfoFromNgenPEFile
elinor-fung Oct 11, 2024
6034080
Remove isILMetaDataForNGENImage parameter - always false
elinor-fung Oct 14, 2024
1f0f207
Remove isAlternate parameter/field - always false
elinor-fung Oct 14, 2024
4c7649a
Merge branch 'main' into remove-isNGEN-param
elinor-fung Oct 15, 2024
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
113 changes: 3 additions & 110 deletions src/coreclr/debug/daccess/daccess.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6414,7 +6414,6 @@ ClrDataAccess::GetMetaDataFileInfoFromPEFile(PEAssembly *pPEAssembly,
DWORD &dwSize,
DWORD &dwDataSize,
DWORD &dwRvaHint,
bool &isNGEN,
_Out_writes_(cchFilePath) LPWSTR wszFilePath,
const DWORD cchFilePath)
{
Expand All @@ -6424,7 +6423,6 @@ ClrDataAccess::GetMetaDataFileInfoFromPEFile(PEAssembly *pPEAssembly,
IMAGE_DATA_DIRECTORY *pDir = NULL;
COUNT_T uniPathChars = 0;

isNGEN = false;
if (pDir == NULL || pDir->Size == 0)
{
mdImage = pPEAssembly->GetPEImage();
Expand Down Expand Up @@ -6475,66 +6473,22 @@ ClrDataAccess::GetMetaDataFileInfoFromPEFile(PEAssembly *pPEAssembly,
return true;
}

/* static */
bool ClrDataAccess::GetILImageInfoFromNgenPEFile(PEAssembly *pPEAssembly,
DWORD &dwTimeStamp,
DWORD &dwSize,
_Out_writes_(cchFilePath) LPWSTR wszFilePath,
const DWORD cchFilePath)
{
SUPPORTS_DAC_HOST_ONLY;
DWORD dwWritten = 0;

// use the IL File name
if (!pPEAssembly->GetPath().DacGetUnicode(cchFilePath, wszFilePath, (COUNT_T *)(&dwWritten)))
{
// Use DAC hint to retrieve the IL name.
pPEAssembly->GetModuleFileNameHint().DacGetUnicode(cchFilePath, wszFilePath, (COUNT_T *)(&dwWritten));
}
dwTimeStamp = 0;
dwSize = 0;

return true;
}

void *
ClrDataAccess::GetMetaDataFromHost(PEAssembly* pPEAssembly,
bool* isAlternate)
ClrDataAccess::GetMetaDataFromHost(PEAssembly* pPEAssembly)
{
DWORD imageTimestamp, imageSize, dataSize;
void* buffer = NULL;
WCHAR uniPath[MAX_LONGPATH] = {0};
bool isAlt = false;
bool isNGEN = false;
DAC_INSTANCE* inst = NULL;
HRESULT hr = S_OK;
DWORD ulRvaHint;
//
// We always ask for the IL image metadata,
// as we expect that to be more
// available than others. The drawback is that
// there may be differences between the IL image
// metadata and native image metadata, so we
// have to mark such alternate metadata so that
// we can fail unsupported usage of it.
//

// Microsoft - above comment seems to be an unimplemented thing.
// The DAC_MD_IMPORT.isAlternate field gets ultimately set, but
// on the searching I did, I cannot find any usage of it
// other than in the ctor. Should we be doing something, or should
// we remove this comment and the isAlternate field?
// It's possible that test will want us to track whether we have
// an IL image's metadata loaded against an NGEN'ed image
// so the field remains for now.

if (!ClrDataAccess::GetMetaDataFileInfoFromPEFile(
pPEAssembly,
imageTimestamp,
imageSize,
dataSize,
ulRvaHint,
isNGEN,
uniPath,
ARRAY_SIZE(uniPath)))
{
Expand Down Expand Up @@ -6589,72 +6543,12 @@ ClrDataAccess::GetMetaDataFromHost(PEAssembly* pPEAssembly,
(BYTE*)buffer,
NULL);
}
if (FAILED(hr) && isNGEN)
{
// We failed to locate the ngen'ed image. We should try to
// find the matching IL image
//
isAlt = true;
if (!ClrDataAccess::GetILImageInfoFromNgenPEFile(
pPEAssembly,
imageTimestamp,
imageSize,
uniPath,
ARRAY_SIZE(uniPath)))
{
goto ErrExit;
}

const WCHAR* ilExtension = W("dll");
WCHAR ngenImageName[MAX_LONGPATH] = {0};
if (wcscpy_s(ngenImageName, ARRAY_SIZE(ngenImageName), uniPath) != 0)
{
goto ErrExit;
}
if (wcscpy_s(uniPath, ARRAY_SIZE(uniPath), ngenImageName) != 0)
{
goto ErrExit;
}

// RVA size in ngen image and IL image is the same. Because the only
// different is in RVA. That is 4 bytes column fixed.
//

// try again
if (m_legacyMetaDataLocator)
{
hr = m_legacyMetaDataLocator->GetMetadata(
uniPath,
imageTimestamp,
imageSize,
NULL, // MVID - not used yet
0, // pass zero hint here... important
0, // flags - reserved for future.
dataSize,
(BYTE*)buffer,
NULL);
}
else
{
hr = m_target3->GetMetaData(
uniPath,
imageTimestamp,
imageSize,
NULL, // MVID - not used yet
0, // pass zero hint here... important
0, // flags - reserved for future.
dataSize,
(BYTE*)buffer,
NULL);
}
}

if (FAILED(hr))
{
goto ErrExit;
}

*isAlternate = isAlt;
m_instances.AddSuperseded(inst);
return buffer;

Expand Down Expand Up @@ -6682,7 +6576,6 @@ ClrDataAccess::GetMDImport(const PEAssembly* pPEAssembly, const ReflectionModule
COUNT_T mdSize;
IMDInternalImport* mdImport = NULL;
PVOID mdBaseHost = NULL;
bool isAlternate = false;

_ASSERTE((pPEAssembly == NULL && reflectionModule != NULL) || (pPEAssembly != NULL && reflectionModule == NULL));
TADDR peAssemblyAddr = (pPEAssembly != NULL) ? dac_cast<TADDR>(pPEAssembly) : dac_cast<TADDR>(reflectionModule);
Expand Down Expand Up @@ -6755,7 +6648,7 @@ ClrDataAccess::GetMDImport(const PEAssembly* pPEAssembly, const ReflectionModule
// 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<PEAssembly *>(pPEAssembly), &isAlternate);
mdBaseHost = GetMetaDataFromHost(const_cast<PEAssembly *>(pPEAssembly));
}

if (mdBaseHost == NULL)
Expand Down Expand Up @@ -6790,7 +6683,7 @@ ClrDataAccess::GetMDImport(const PEAssembly* pPEAssembly, const ReflectionModule
// 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(peAssemblyAddr, mdImport, isAlternate) == NULL)
if (m_mdImports.Add(peAssemblyAddr, mdImport) == NULL)
{
mdImport->Release();
DacError(E_OUTOFMEMORY);
Expand Down
32 changes: 1 addition & 31 deletions src/coreclr/debug/daccess/dacdbiimpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,19 +392,13 @@ interface IMDInternalImport* DacDbiInterfaceImpl::GetMDImport(

// Go to DBI to find the metadata.
IMDInternalImport * pInternal = NULL;
bool isILMetaDataForNI = false;
EX_TRY
{
// If test needs it in the future, prop isILMetaDataForNI back up to
// ClrDataAccess.m_mdImports.Add() call.
// example in code:ClrDataAccess::GetMDImport
// CordbModule::GetMetaDataInterface also looks up MetaData and would need attention.

// This is the new codepath that uses ICorDebugMetaDataLookup.
// To get the old codepath that uses the v2 metadata lookup methods,
// you'd have to load DAC only and then you'll get ClrDataAccess's implementation
// of this function.
pInternal = pLookup->LookupMetaData(vmPEAssembly, isILMetaDataForNI);
pInternal = pLookup->LookupMetaData(vmPEAssembly);
}
EX_CATCH
{
Expand Down Expand Up @@ -1225,7 +1219,6 @@ mdSignature DacDbiInterfaceImpl::GetILCodeAndSigHelper(Module * pModule,
bool DacDbiInterfaceImpl::GetMetaDataFileInfoFromPEFile(VMPTR_PEAssembly vmPEAssembly,
DWORD &dwTimeStamp,
DWORD &dwSize,
bool &isNGEN,
IStringHolder* pStrFilename)
{
DD_ENTER_MAY_THROW;
Expand All @@ -1244,25 +1237,13 @@ bool DacDbiInterfaceImpl::GetMetaDataFileInfoFromPEFile(VMPTR_PEAssembly vmPEAss
dwSize,
dwDataSize,
dwRvaHint,
isNGEN,
wszFilePath,
cchFilePath);

pStrFilename->AssignCopy(wszFilePath);
return ret;
}


bool DacDbiInterfaceImpl::GetILImageInfoFromNgenPEFile(VMPTR_PEAssembly vmPEAssembly,
DWORD &dwTimeStamp,
DWORD &dwSize,
IStringHolder* pStrFilename)
{

return false;

}

// Get start addresses and sizes for hot and cold regions for a native code blob.
// Arguments:
// Input:
Expand Down Expand Up @@ -4123,17 +4104,6 @@ BOOL DacDbiInterfaceImpl::GetModulePath(VMPTR_Module vmModule,
return FALSE;
}

// Get the full path and file name to the ngen image for the module (if any).
BOOL DacDbiInterfaceImpl::GetModuleNGenPath(VMPTR_Module vmModule,
IStringHolder * pStrFilename)
{
DD_ENTER_MAY_THROW;

// no ngen filename
IfFailThrow(pStrFilename->AssignCopy(W("")));
return FALSE;
}

// Implementation of IDacDbiInterface::GetModuleSimpleName
void DacDbiInterfaceImpl::GetModuleSimpleName(VMPTR_Module vmModule, IStringHolder * pStrFilename)
{
Expand Down
14 changes: 1 addition & 13 deletions src/coreclr/debug/daccess/dacdbiimpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,10 +715,6 @@ class DacDbiInterfaceImpl :
BOOL GetModulePath(VMPTR_Module vmModule,
IStringHolder * pStrFilename);

// Get the full path and file name to the ngen image for the module (if any).
BOOL GetModuleNGenPath(VMPTR_Module vmModule,
IStringHolder * pStrFilename);

// Implementation of IDacDbiInterface::GetModuleSimpleName
void GetModuleSimpleName(VMPTR_Module vmModule, IStringHolder * pStrFilename);

Expand Down Expand Up @@ -1103,19 +1099,11 @@ class DacDbiInterfaceImpl :
TargetBuffer * pIL);

public:
// APIs for picking up the info needed for a debugger to look up an ngen image or IL image
// from it's search path.
// API for picking up the info needed for a debugger to look up an image from its search path.
bool GetMetaDataFileInfoFromPEFile(VMPTR_PEAssembly vmPEAssembly,
DWORD &dwTimeStamp,
DWORD &dwSize,
bool &isNGEN,
IStringHolder* pStrFilename);

bool GetILImageInfoFromNgenPEFile(VMPTR_PEAssembly vmPEAssembly,
DWORD &dwTimeStamp,
DWORD &dwSize,
IStringHolder* pStrFilename);

};


Expand Down
20 changes: 4 additions & 16 deletions src/coreclr/debug/daccess/dacimpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,16 +131,13 @@ struct DAC_MD_IMPORT
DAC_MD_IMPORT* next; // list link field
TADDR peFile; // a TADDR for a PEAssembly* or a ReflectionModule*
IMDInternalImport* impl; // Associated metadata interface
bool isAlternate; // for NGEN images set to true if the metadata corresponds to the IL image

DAC_MD_IMPORT(TADDR peFile_,
IMDInternalImport* impl_,
bool isAlt_ = false,
DAC_MD_IMPORT* next_ = NULL)
: next(next_)
, peFile(peFile_)
, impl(impl_)
, isAlternate(isAlt_)
{
SUPPORTS_DAC_HOST_ONLY;
}
Expand Down Expand Up @@ -179,10 +176,10 @@ class MDImportsCache
}

FORCEINLINE
DAC_MD_IMPORT* Add(TADDR peFile, IMDInternalImport* impl, bool isAlt)
DAC_MD_IMPORT* Add(TADDR peFile, IMDInternalImport* impl)
{
SUPPORTS_DAC;
DAC_MD_IMPORT* importList = new (nothrow) DAC_MD_IMPORT(peFile, impl, isAlt, m_head);
DAC_MD_IMPORT* importList = new (nothrow) DAC_MD_IMPORT(peFile, impl, m_head);
if (!importList)
{
return NULL;
Expand Down Expand Up @@ -1356,8 +1353,7 @@ class ClrDataAccess
JITNotification* GetHostJitNotificationTable();
GcNotification* GetHostGcNotificationTable();

void* GetMetaDataFromHost(PEAssembly* pPEAssembly,
bool* isAlternate);
void* GetMetaDataFromHost(PEAssembly* pPEAssembly);

virtual
interface IMDInternalImport* GetMDImport(const PEAssembly* pPEAssembly,
Expand Down Expand Up @@ -1512,22 +1508,14 @@ class ClrDataAccess
#endif

public:
// APIs for picking up the info needed for a debugger to look up an ngen image or IL image
// from it's search path.
// API for picking up the info needed for a debugger to look up an image from its search path.
static bool GetMetaDataFileInfoFromPEFile(PEAssembly *pPEAssembly,
DWORD &dwImageTimestamp,
DWORD &dwImageSize,
DWORD &dwDataSize,
DWORD &dwRvaHint,
bool &isNGEN,
_Out_writes_(cchFilePath) LPWSTR wszFilePath,
DWORD cchFilePath);

static bool GetILImageInfoFromNgenPEFile(PEAssembly *pPEAssembly,
DWORD &dwTimeStamp,
DWORD &dwSize,
_Out_writes_(cchPath) LPWSTR wszPath,
const DWORD cchPath);
};

extern ClrDataAccess* g_dacImpl;
Expand Down
Loading
Loading