Skip to content

Commit

Permalink
Fix OpenVirtualProcess SIGSEGV on Linux. (dotnet#17444)
Browse files Browse the repository at this point in the history 
Add DBI OpenVirtualProcessImpl2 that takes a module path instead of handle.

Fix assert on Windows debug.

Issue #17446
  • Loading branch information
@mikem8361
mikem8361 authored Apr 7, 2018
1 parent 8499136 commit 0f0320e
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 46 deletions.
33 changes: 33 additions & 0 deletions src/debug/di/process.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,39 @@ STDAPI OpenVirtualProcessImpl(
return hr;
};

//---------------------------------------------------------------------------------------
//
// OpenVirtualProcessImpl2 method called by the dbgshim to get an ICorDebugProcess4 instance
//
// Arguments:
// clrInstanceId - target pointer identifying which CLR in the Target to debug.
// pDataTarget - data target abstraction.
// pDacModulePath - the module path of the appropriate DAC dll for this runtime
// riid - interface ID to query for.
// ppProcessOut - new object for target, interface ID matches riid.
// ppFlagsOut - currently only has 1 bit to indicate whether or not this runtime
// instance will send a managed event after attach
//
// Return Value:
// S_OK on success. Else failure
//---------------------------------------------------------------------------------------
STDAPI OpenVirtualProcessImpl2(
ULONG64 clrInstanceId,
IUnknown * pDataTarget,
LPCWSTR pDacModulePath,
CLR_DEBUGGING_VERSION * pMaxDebuggerSupportedVersion,
REFIID riid,
IUnknown ** ppInstance,
CLR_DEBUGGING_PROCESS_FLAGS* pFlagsOut)
{
HMODULE hDac = LoadLibraryW(pDacModulePath);
if (hDac == NULL)
{
return HRESULT_FROM_WIN32(GetLastError());
}
return OpenVirtualProcessImpl(clrInstanceId, pDataTarget, hDac, pMaxDebuggerSupportedVersion, riid, ppInstance, pFlagsOut);
}

//---------------------------------------------------------------------------------------
// DEPRECATED - use OpenVirtualProcessImpl
// OpenVirtualProcess method used by the shim in CLR v4 Beta1
Expand Down
143 changes: 100 additions & 43 deletions src/debug/shim/debugshim.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@
// CLRDebuggingImpl implementation (ICLRDebugging)
//*****************************************************************************

typedef HRESULT (STDAPICALLTYPE *OpenVirtualProcessImpl2FnPtr)(ULONG64 clrInstanceId,
IUnknown * pDataTarget,
LPCWSTR pDacModulePath,
CLR_DEBUGGING_VERSION * pMaxDebuggerSupportedVersion,
REFIID riid,
IUnknown ** ppInstance,
CLR_DEBUGGING_PROCESS_FLAGS * pdwFlags);

typedef HRESULT (STDAPICALLTYPE *OpenVirtualProcessImplFnPtr)(ULONG64 clrInstanceId,
IUnknown * pDataTarget,
HMODULE hDacDll,
Expand All @@ -53,6 +61,8 @@ typedef HRESULT (STDAPICALLTYPE *OpenVirtualProcess2FnPtr)(ULONG64 clrInstanceI
IUnknown ** ppInstance,
CLR_DEBUGGING_PROCESS_FLAGS * pdwFlags);

typedef HMODULE (STDAPICALLTYPE *LoadLibraryWFnPtr)(LPCWSTR lpLibFileName);

// Implementation of ICLRDebugging::OpenVirtualProcess
//
// Arguments:
Expand Down Expand Up @@ -81,63 +91,64 @@ STDMETHODIMP CLRDebuggingImpl::OpenVirtualProcess(
ICorDebugDataTarget * pDt = NULL;
HMODULE hDbi = NULL;
HMODULE hDac = NULL;
LPWSTR pDacModulePath = NULL;
LPWSTR pDbiModulePath = NULL;
DWORD dbiTimestamp;
DWORD dbiSizeOfImage;
WCHAR dbiName[MAX_PATH_FNAME] = {0};
WCHAR dbiName[MAX_PATH_FNAME] = { 0 };
DWORD dacTimestamp;
DWORD dacSizeOfImage;
WCHAR dacName[MAX_PATH_FNAME] = {0};
WCHAR dacName[MAX_PATH_FNAME] = { 0 };
CLR_DEBUGGING_VERSION version;
BOOL versionSupportedByCaller = FALSE;

// argument checking
if( (ppProcess != NULL || pFlags != NULL) && pLibraryProvider == NULL)
if ((ppProcess != NULL || pFlags != NULL) && pLibraryProvider == NULL)
{
hr = E_POINTER; // the library provider must be specified if either
// ppProcess or pFlags is non-NULL
}
else if( (ppProcess != NULL || pFlags != NULL) && pMaxDebuggerSupportedVersion == NULL)
else if ((ppProcess != NULL || pFlags != NULL) && pMaxDebuggerSupportedVersion == NULL)
{
hr = E_POINTER; // the max supported version must be specified if either
// ppProcess or pFlags is non-NULL
}
else if(pVersion != NULL && pVersion->wStructVersion != 0)
else if (pVersion != NULL && pVersion->wStructVersion != 0)
{
hr = CORDBG_E_UNSUPPORTED_VERSION_STRUCT;
}
else if(FAILED(pDataTarget->QueryInterface(__uuidof(ICorDebugDataTarget), (void**) &pDt)))
else if (FAILED(pDataTarget->QueryInterface(__uuidof(ICorDebugDataTarget), (void**)&pDt)))
{
hr = CORDBG_E_MISSING_DATA_TARGET_INTERFACE;
}

if(SUCCEEDED(hr))
if (SUCCEEDED(hr))
{
// get CLR version
// The expectation is that new versions of the CLR will continue to use the same GUID
// (unless there's a reason to hide them from older shims), but debuggers will tell us the
// CLR version they're designed for and mscordbi.dll can decide whether or not to accept it.
version.wStructVersion = 0;
hr = GetCLRInfo(pDt,
moduleBaseAddress,
&version,
&dbiTimestamp,
&dbiSizeOfImage,
dbiName,
MAX_PATH_FNAME,
&dacTimestamp,
&dacSizeOfImage,
dacName,
MAX_PATH_FNAME);
hr = GetCLRInfo(pDt,
moduleBaseAddress,
&version,
&dbiTimestamp,
&dbiSizeOfImage,
dbiName,
MAX_PATH_FNAME,
&dacTimestamp,
&dacSizeOfImage,
dacName,
MAX_PATH_FNAME);
}

// If we need to fetch either the process info or the flags info then we need to find
// mscordbi and DAC and do the version specific OVP work
if(SUCCEEDED(hr) && (ppProcess != NULL || pFlags != NULL))
if (SUCCEEDED(hr) && (ppProcess != NULL || pFlags != NULL))
{
ICLRDebuggingLibraryProvider2* pLibraryProvider2;
if (SUCCEEDED(pLibraryProvider->QueryInterface(__uuidof(ICLRDebuggingLibraryProvider2), (void**)&pLibraryProvider2)))
{
LPWSTR pDbiModulePath;
if (FAILED(pLibraryProvider2->ProvideLibrary2(dbiName, dbiTimestamp, dbiSizeOfImage, &pDbiModulePath)) ||
pDbiModulePath == NULL)
{
Expand All @@ -151,20 +162,14 @@ STDMETHODIMP CLRDebuggingImpl::OpenVirtualProcess(
{
hr = HRESULT_FROM_WIN32(GetLastError());
}
#ifdef FEATURE_PAL
free(pDbiModulePath);
#else
CoTaskMemFree(pDbiModulePath);
#endif
}

if (SUCCEEDED(hr))
{
// Adjust the timestamp and size of image if this DAC is a known buggy version and needs to be retargeted
RetargetDacIfNeeded(&dacTimestamp, &dacSizeOfImage);

// ask library provider for dac
LPWSTR pDacModulePath;
// Ask library provider for dac
if (FAILED(pLibraryProvider2->ProvideLibrary2(dacName, dacTimestamp, dacSizeOfImage, &pDacModulePath)) ||
pDacModulePath == NULL)
{
Expand All @@ -178,18 +183,13 @@ STDMETHODIMP CLRDebuggingImpl::OpenVirtualProcess(
{
hr = HRESULT_FROM_WIN32(GetLastError());
}
#ifdef FEATURE_PAL
free(pDacModulePath);
#else
CoTaskMemFree(pDacModulePath);
#endif
}
}

pLibraryProvider2->Release();
}
else {
// ask library provider for dbi
// Ask library provider for dbi
if (FAILED(pLibraryProvider->ProvideLibrary(dbiName, dbiTimestamp, dbiSizeOfImage, &hDbi)) ||
hDbi == NULL)
{
Expand All @@ -210,14 +210,53 @@ STDMETHODIMP CLRDebuggingImpl::OpenVirtualProcess(
}
}

if(SUCCEEDED(hr))
*ppProcess = NULL;

if (SUCCEEDED(hr) && pDacModulePath != NULL)
{
// Get access to the latest OVP implementation and call it
OpenVirtualProcessImpl2FnPtr ovpFn = (OpenVirtualProcessImpl2FnPtr)GetProcAddress(hDbi, "OpenVirtualProcessImpl2");
if (ovpFn != NULL)
{
hr = ovpFn(moduleBaseAddress, pDataTarget, pDacModulePath, pMaxDebuggerSupportedVersion, riidProcess, ppProcess, pFlags);
if (FAILED(hr))
{
_ASSERTE(ppProcess == NULL || *ppProcess == NULL);
_ASSERTE(pFlags == NULL || *pFlags == 0);
}
}
#ifdef FEATURE_PAL
else
{
// On Linux/MacOS the DAC module handle needs to be re-created using the DAC PAL instance
// before being passed to DBI's OpenVirtualProcess* implementation. The DBI and DAC share
// the same PAL where dbgshim has it's own.
LoadLibraryWFnPtr loadLibraryWFn = (LoadLibraryWFnPtr)GetProcAddress(hDac, "LoadLibraryW");
if (loadLibraryWFn != NULL)
{
hDac = loadLibraryWFn(pDacModulePath);
if (hDac == NULL)
{
hr = E_HANDLE;
}
}
else
{
hr = E_HANDLE;
}
}
#endif // FEATURE_PAL
}

// If no errors so far and "OpenVirtualProcessImpl2" doesn't exist
if (SUCCEEDED(hr) && *ppProcess == NULL)
{
// get access to OVP and call it
OpenVirtualProcessImplFnPtr ovpFn = (OpenVirtualProcessImplFnPtr) GetProcAddress(hDbi, "OpenVirtualProcessImpl");
if(ovpFn == NULL)
// Get access to OVP and call it
OpenVirtualProcessImplFnPtr ovpFn = (OpenVirtualProcessImplFnPtr)GetProcAddress(hDbi, "OpenVirtualProcessImpl");
if (ovpFn == NULL)
{
// Fallback to CLR v4 Beta1 path, but skip some of the checking we'd normally do (maxSupportedVersion, etc.)
OpenVirtualProcess2FnPtr ovp2Fn = (OpenVirtualProcess2FnPtr) GetProcAddress(hDbi, "OpenVirtualProcess2");
OpenVirtualProcess2FnPtr ovp2Fn = (OpenVirtualProcess2FnPtr)GetProcAddress(hDbi, "OpenVirtualProcess2");
if (ovp2Fn == NULL)
{
hr = CORDBG_E_LIBRARY_PROVIDER_ERROR;
Expand All @@ -231,7 +270,7 @@ STDMETHODIMP CLRDebuggingImpl::OpenVirtualProcess(
{
// Have a CLR v4 Beta2+ DBI, call it and let it do the version check
hr = ovpFn(moduleBaseAddress, pDataTarget, hDac, pMaxDebuggerSupportedVersion, riidProcess, ppProcess, pFlags);
if(FAILED(hr))
if (FAILED(hr))
{
_ASSERTE(ppProcess == NULL || *ppProcess == NULL);
_ASSERTE(pFlags == NULL || *pFlags == 0);
Expand All @@ -241,16 +280,34 @@ STDMETHODIMP CLRDebuggingImpl::OpenVirtualProcess(
}

//version is still valid in some failure cases
if(pVersion != NULL &&
if (pVersion != NULL &&
(SUCCEEDED(hr) ||
(hr == CORDBG_E_UNSUPPORTED_DEBUGGING_MODEL) ||
(hr == CORDBG_E_UNSUPPORTED_FORWARD_COMPAT)))
(hr == CORDBG_E_UNSUPPORTED_FORWARD_COMPAT)))
{
memcpy(pVersion, &version, sizeof(CLR_DEBUGGING_VERSION));
}

if (pDacModulePath != NULL)
{
#ifdef FEATURE_PAL
free(pDacModulePath);
#else
CoTaskMemFree(pDacModulePath);
#endif
}

if (pDbiModulePath != NULL)
{
#ifdef FEATURE_PAL
free(pDbiModulePath);
#else
CoTaskMemFree(pDbiModulePath);
#endif
}

// free the data target we QI'ed earlier
if(pDt != NULL)
if (pDt != NULL)
{
pDt->Release();
}
Expand Down
9 changes: 7 additions & 2 deletions src/dlls/dbgshim/dbgshim.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1808,6 +1808,8 @@ CLRCreateInstance(
REFIID riid,
LPVOID *ppInterface)
{
PUBLIC_CONTRACT;

#if defined(FEATURE_CORESYSTEM)

if (ppInterface == NULL)
Expand All @@ -1822,9 +1824,12 @@ CLRCreateInstance(
GUID skuId = CLR_ID_CORECLR;
#endif

CLRDebuggingImpl *pDebuggingImpl = new CLRDebuggingImpl(skuId);
CLRDebuggingImpl *pDebuggingImpl = new (nothrow) CLRDebuggingImpl(skuId);
if (NULL == pDebuggingImpl)
return E_OUTOFMEMORY;

return pDebuggingImpl->QueryInterface(riid, ppInterface);
#else
return E_NOTIMPL;
#endif
}
}
1 change: 1 addition & 0 deletions src/dlls/mscordbi/mscordbi.src
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ EXPORTS

// Out-of-proc creation path from the shim - ICLRDebugging
OpenVirtualProcessImpl
OpenVirtualProcessImpl2

// DEPRECATED - use OpenVirtualProcessImpl
OpenVirtualProcess private
Expand Down
3 changes: 2 additions & 1 deletion src/dlls/mscordbi/mscordbi_unixexports.src
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ CoreCLRCreateCordbObject

; Out-of-proc creation path from the shim - ICLRDebugging
OpenVirtualProcessImpl
OpenVirtualProcessImpl2

; PAL module registration
DllMain
PAL_RegisterModule
PAL_UnregisterModule
PAL_UnregisterModule

0 comments on commit 0f0320e

Please sign in to comment.