Skip to content

Commit

Permalink
Fix loading of cDAC in DAC (#106289)
Browse files Browse the repository at this point in the history
When loading the cDAC, we were relying on `GetClrModuleDirectory` in the DAC actually giving us the DAC directory. With #106217, it explicitly stores/uses the CLR module base. When we first try to load the cDAC in the DAC, that has not yet been set. The current intent is to actually load from next to the DAC, so switch the logic to look next to the current module for the cDAC instead of using the CLR module directory.

Also renamed `GetCurrentModuleFileName` in utilcode to `GetCurrentExecutableFileName` so that it accurately represents what it does (I tried to use it and was sad).
  • Loading branch information
elinor-fung authored Aug 12, 2024
1 parent 8dc239b commit fd304de
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 11 deletions.
10 changes: 8 additions & 2 deletions src/coreclr/debug/daccess/cdac.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,17 @@ namespace
{
bool TryLoadCDACLibrary(HMODULE *phCDAC)
{
// Load cdacreader from next to DAC binary
// Load cdacreader from next to current module (DAC binary)
PathString path;
if (FAILED(GetClrModuleDirectory(path)))
if (WszGetModuleFileName((HMODULE)GetCurrentModuleBase(), path) == 0)
return false;

SString::Iterator iter = path.End();
if (!path.FindBack(iter, DIRECTORY_SEPARATOR_CHAR_W))
return false;

iter++;
path.Truncate(iter);
path.Append(CDAC_LIB_NAME);
*phCDAC = CLRLoadLibrary(path.GetUnicode());
if (*phCDAC == NULL)
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/inc/utilcode.h
Original file line number Diff line number Diff line change
Expand Up @@ -3241,7 +3241,7 @@ BOOL GetRegistryLongValue(HKEY hKeyParent, // Parent key.
long *pValue, // Put value here, if found.
BOOL fReadNonVirtualizedKey); // Whether to read 64-bit hive on WOW64

HRESULT GetCurrentModuleFileName(SString& pBuffer);
HRESULT GetCurrentExecutableFileName(SString& pBuffer);

//*****************************************************************************
// Retrieve information regarding what registered default debugger
Expand Down Expand Up @@ -3867,6 +3867,7 @@ inline T* InterlockedCompareExchangeT(
// Returns the directory for clr module. So, if path was for "C:\Dir1\Dir2\Filename.DLL",
// then this would return "C:\Dir1\Dir2\" (note the trailing backslash).
HRESULT GetClrModuleDirectory(SString& wszPath);
void* GetCurrentModuleBase();

namespace Clr { namespace Util
{
Expand Down
9 changes: 7 additions & 2 deletions src/coreclr/utilcode/clrhost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ void* GetClrModuleBase()
#else // DACCESS_COMPILE

void* GetClrModuleBase()
{
return GetCurrentModuleBase();
}

#endif // DACCESS_COMPILE

void* GetCurrentModuleBase()
{
LIMITED_METHOD_CONTRACT;

Expand All @@ -48,8 +55,6 @@ void* GetClrModuleBase()
#endif // HOST_WINDOWS
}

#endif // DACCESS_COMPILE

thread_local int t_CantAllocCount;

DWORD GetClrModulePathName(SString& buffer)
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/utilcode/util_nodependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ BOOL GetRegistryLongValue(HKEY hKeyParent,

//----------------------------------------------------------------------------
//
// GetCurrentModuleFileName - Retrieve the current module's filename
// GetCurrentExecutableFileName - Retrieve the current executable's filename
//
// Arguments:
// pBuffer - output string buffer
Expand All @@ -155,7 +155,7 @@ BOOL GetRegistryLongValue(HKEY hKeyParent,
// Note:
//
//----------------------------------------------------------------------------
HRESULT GetCurrentModuleFileName(SString& pBuffer)
HRESULT GetCurrentExecutableFileName(SString& pBuffer)
{
LIMITED_METHOD_CONTRACT;

Expand Down Expand Up @@ -211,7 +211,7 @@ BOOL IsCurrentModuleFileNameInAutoExclusionList()
PathString wszAppName;

// Get the appname to look up in the exclusion or inclusion list.
if (GetCurrentModuleFileName(wszAppName) != S_OK)
if (GetCurrentExecutableFileName(wszAppName) != S_OK)
{
// Assume it is not on the exclusion list if we cannot find the module's filename.
return FALSE;
Expand Down Expand Up @@ -380,7 +380,7 @@ HRESULT GetDebuggerSettingInfoWorker(_Out_writes_to_opt_(*pcchDebuggerString, *p
long iValue;

// Check DebugApplications setting
if ((SUCCEEDED(GetCurrentModuleFileName(wzAppName))) &&
if ((SUCCEEDED(GetCurrentExecutableFileName(wzAppName))) &&
(
GetRegistryLongValue(HKEY_LOCAL_MACHINE, kDebugApplicationsPoliciesKey, wzAppName, &iValue, TRUE) ||
GetRegistryLongValue(HKEY_LOCAL_MACHINE, kDebugApplicationsKey, wzAppName, &iValue, TRUE) ||
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/dwbucketmanager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ void BaseBucketParamsManager::GetAppName(_Out_writes_(maxLength) WCHAR* targetPa
CONTRACTL_END;

PathString appPath;
if (GetCurrentModuleFileName(appPath) == S_OK)
if (GetCurrentExecutableFileName(appPath) == S_OK)
{
// Get just the module name; remove the path
const WCHAR* appName = u16_strrchr(appPath, DIRECTORY_SEPARATOR_CHAR_W);
Expand Down Expand Up @@ -479,7 +479,7 @@ void BaseBucketParamsManager::GetAppVersion(_Out_writes_(maxLength) WCHAR* targe
WCHAR verBuf[23] = {0};
USHORT major, minor, build, revision;

if ((GetCurrentModuleFileName(appPath) == S_OK) && SUCCEEDED(DwGetFileVersionInfo(appPath, major, minor, build, revision)))
if ((GetCurrentExecutableFileName(appPath) == S_OK) && SUCCEEDED(DwGetFileVersionInfo(appPath, major, minor, build, revision)))
{
_snwprintf_s(targetParam,
maxLength,
Expand Down

0 comments on commit fd304de

Please sign in to comment.