dbgshim: route live debugging through cDAC-capable DBI first

src/dbgshim/dbgshim.h

@@ -5,11 +5,25 @@
 //
 //*****************************************************************************
 
+#ifndef _DBG_SHIM_H_
+#define _DBG_SHIM_H_
+
 #include <windows.h>
 #include "metahost.h"
 
 typedef VOID (*PSTARTUP_CALLBACK)(IUnknown *pCordb, PVOID parameter, HRESULT hr);
 
+// Selects how dbgshim locates the data-access layer (DAC/cDAC) for a target.
+enum CDacLoadPolicy
+{
+    // Prefer the debugging tool bundled cDAC, falling back to the legacy DAC. This is the default.
+    CDacLoadPolicy_PreferCDac = 0,
+    // Use only the cDAC.
+    CDacLoadPolicy_CDacOnly = 1,
+    // Use only the legacy DAC.
+    CDacLoadPolicy_LegacyDacOnly = 2,
+};
+
 EXTERN_C HRESULT
 CreateProcessForLaunch(
     _In_ LPWSTR lpCommandLine,
@@ -115,3 +129,9 @@ RegisterForRuntimeStartupRemotePort(
     _In_ LPCWSTR szMscordbiPath,
     _In_ LPCWSTR szAssemblyBasePath,
     _Out_ IUnknown ** ppCordb);
+
+EXTERN_C HRESULT
+SetCDacLoadPolicy(
+    _In_ CDacLoadPolicy policy);
+
+#endif // _DBG_SHIM_H_

src/dbgshim/dbgshim.ntdef

@@ -19,3 +19,4 @@ EXPORTS
     CreateDebuggingInterfaceFromVersion3
     CLRCreateInstance
     RegisterForRuntimeStartupRemotePort
+    SetCDacLoadPolicy

src/dbgshim/dbgshim_unixexports.src

@@ -18,3 +18,4 @@ CreateDebuggingInterfaceFromVersion2
 CreateDebuggingInterfaceFromVersion3
 CLRCreateInstance
 RegisterForRuntimeStartupRemotePort
+SetCDacLoadPolicy

src/dbgshim/debugshim.cpp

@@ -64,10 +64,6 @@ static HRESULT OpenCorDebugProcessWithProvider(
     IUnknown** ppProcess,
     CLR_DEBUGGING_PROCESS_FLAGS* pFlags);
 static VOID RetargetDacIfNeeded(DWORD* pdwTimeStamp, DWORD* pdwSizeOfImage);
-static HRESULT SelectActivationResult(
-    HRESULT cdacHr,
-    HRESULT fallbackHr,
-    bool cdacEvaluated);
 static HRESULT TryGetContractDescriptorAddress(
     ULONG64 moduleBaseAddress,
     IUnknown* pDataTarget,
@@ -302,9 +298,10 @@ static HRESULT OpenCorDebugProcessWithCDac(
     CLR_DEBUGGING_PROCESS_FLAGS* pFlags)
 {
     SString dbiModulePath;
-    if (!GetDbiPath(dbiModulePath))
+    SString cdacModulePath;
+    if (!GetCDacAndDbiPaths(cdacModulePath, dbiModulePath))
     {
-        return E_FAIL;
+        return CORDBG_E_DEBUG_COMPONENT_MISSING;
     }
 
     HMODULE hDbi = LoadLibraryW(dbiModulePath);
@@ -320,19 +317,14 @@ static HRESULT OpenCorDebugProcessWithCDac(
         return CORDBG_E_MISSING_DEBUGGER_EXPORTS;
     }
 
-    SString cdacModulePath;
-    HRESULT hr = E_FAIL;
-    if (GetCDacPath(cdacModulePath))
-    {
-        hr = ovpFn(
-            moduleBaseAddress,
-            pDataTarget,
-            cdacModulePath,
-            pMaxDebuggerSupportedVersion,
-            riidProcess,
-            ppProcess,
-            pFlags);
-    }
+    HRESULT hr = ovpFn(
+        moduleBaseAddress,
+        pDataTarget,
+        cdacModulePath,
+        pMaxDebuggerSupportedVersion,
+        riidProcess,
+        ppProcess,
+        pFlags);
 
     if (SUCCEEDED(hr) && ppProcess != NULL && *ppProcess == NULL)
     {
@@ -712,13 +704,10 @@ static HRESULT LoadResolvedLibraries(
     return hr;
 }
 
-// Selects the final HRESULT after the cDAC and/or fallback activation attempts have run.
-//   - A cDAC success wins, then a fallback success.
-//   - On total failure, if the cDAC was evaluated (any policy other than LegacyDacOnly) its error
-//     is surfaced so tools can log why the cDAC rejected the target.
-//   - Otherwise (LegacyDacOnly) the fallback error is surfaced. Callers seed fallbackHr with
-//     E_POINTER so the LegacyDacOnly no-provider case preserves its historical HRESULT.
-static HRESULT SelectActivationResult(
+// Picks the final HRESULT after a cDAC attempt (cdacHr) and a legacy fallback attempt (fallbackHr).
+// On total failure the cDAC error is surfaced when it was attempted so tools can log why the cDAC
+// rejected the target; otherwise the fallback error (seeded by callers) is surfaced.
+HRESULT SelectActivationResult(
     HRESULT cdacHr,
     HRESULT fallbackHr,
     bool cdacEvaluated)
@@ -1215,6 +1204,30 @@ static bool GetCDacPath(SString& cdacPath)
         cdacPath);
 }
 
+bool GetCDacAndDbiPaths(SString& cdacPath, SString& dbiPath)
+{
+    SString resolvedDbiPath;
+    SString resolvedCDacPath;
+    if (!GetDbiPath(resolvedDbiPath) || !GetCDacPath(resolvedCDacPath))
+    {
+        return false;
+    }
+
+    DWORD dbiAttributes = GetFileAttributesW(resolvedDbiPath);
+    DWORD cdacAttributes = GetFileAttributesW(resolvedCDacPath);
+    if (dbiAttributes == INVALID_FILE_ATTRIBUTES ||
+        (dbiAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0 ||
+        cdacAttributes == INVALID_FILE_ATTRIBUTES ||
+        (cdacAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0)
+    {
+        return false;
+    }
+
+    cdacPath.Set(resolvedCDacPath);
+    dbiPath.Set(resolvedDbiPath);
+    return true;
+}
+
 // Loads the given DAC/cDAC module and creates the requested data-access interface from it by
 // calling its CLRDataCreateInstance export. The module is intentionally left resident.
 static HRESULT DacCreateInstance(
@@ -1464,23 +1477,23 @@ static bool IsDataAccessInterface(REFIID riid)
     return riid == IID_IXCLRDataProcess_Local || riid == IID_ISOSDacInterface_Local;
 }
 
-STDMETHODIMP CLRDebuggingImpl::SetCDacLoadPolicy(DWORD policy)
+STDMETHODIMP CLRDebuggingImpl::SetCDacLoadPolicy(CDacLoadPolicy policy)
 {
     if (policy > CDacLoadPolicy_LegacyDacOnly)
     {
         return E_INVALIDARG;
     }
-    m_cdacLoadPolicy = (CDacLoadPolicy)policy;
+    m_cdacLoadPolicy = policy;
     return S_OK;
 }
 
-STDMETHODIMP CLRDebuggingImpl::GetCDacLoadPolicy(DWORD* pPolicy)
+STDMETHODIMP CLRDebuggingImpl::GetCDacLoadPolicy(CDacLoadPolicy* pPolicy)
 {
     if (pPolicy == NULL)
     {
         return E_POINTER;
     }
-    *pPolicy = (DWORD)m_cdacLoadPolicy;
+    *pPolicy = m_cdacLoadPolicy;
     return S_OK;
 }
 

src/dbgshim/debugshim.h

@@ -17,6 +17,7 @@
 #include <metahost.h>
 #include <dn-u16.h>
 #include "runtimeinfo.h"
+#include "dbgshim.h"
 
 #if defined (HOST_WINDOWS) && defined(HOST_X86)
 #define CLRDEBUGINFO_RESOURCE_NAME W("CLRDEBUGINFOWINDOWSX86")
@@ -50,18 +51,6 @@
 // The cDAC (contract-based data access) is bundled next to dbgshim and is never downloaded.
 #define CORECLR_CDAC_MODULE_NAME_W W("mscordaccore_universal")
 
-// Controls how OpenVirtualProcess locates the data-access layer when a data-access interface
-// (for example IXCLRDataProcess) is requested. Mirrors the diagnostics CDacLoadPolicy.
-enum CDacLoadPolicy
-{
-    // Prefer the co-located cDAC; fall back to the legacy DAC via the library provider. (default)
-    CDacLoadPolicy_PreferCDac = 0,
-    // Use only the cDAC; do not fall back to the legacy DAC.
-    CDacLoadPolicy_CDacOnly = 1,
-    // Use only the legacy DAC; do not try the cDAC.
-    CDacLoadPolicy_LegacyDacOnly = 2,
-};
-
 // A small dbgshim-owned control interface, implemented by the same object as ICLRDebugging, that
 // lets a consumer attach a cDAC load policy to the debugging object before requesting a data-access
 // interface from OpenVirtualProcess. ICLRDebugging itself is a frozen published interface, so the
@@ -71,9 +60,8 @@ MIDL_INTERFACE("2D3B4F6A-1C7E-4B2A-9E5D-7F1A6C0B8D34")
 ICLRDebuggingPolicy : public IUnknown
 {
 public:
-    // policy is a CDacLoadPolicy value.
-    virtual HRESULT STDMETHODCALLTYPE SetCDacLoadPolicy(DWORD policy) = 0;
-    virtual HRESULT STDMETHODCALLTYPE GetCDacLoadPolicy(DWORD* pPolicy) = 0;
+    virtual HRESULT STDMETHODCALLTYPE SetCDacLoadPolicy(CDacLoadPolicy policy) = 0;
+    virtual HRESULT STDMETHODCALLTYPE GetCDacLoadPolicy(CDacLoadPolicy* pPolicy) = 0;
 };
 
 #define MAX_BUILDID_SIZE 24
@@ -222,8 +210,8 @@ class CLRDebuggingImpl : public ICLRDebugging, public ICLRDebuggingPolicy
     STDMETHOD(CanUnloadNow(HMODULE hModule));
 
     // ICLRDebuggingPolicy methods:
-    STDMETHOD(SetCDacLoadPolicy(DWORD policy));
-    STDMETHOD(GetCDacLoadPolicy(DWORD* pPolicy));
+    STDMETHOD(SetCDacLoadPolicy(CDacLoadPolicy policy));
+    STDMETHOD(GetCDacLoadPolicy(CDacLoadPolicy* pPolicy));
 
     // IUnknown methods:
     STDMETHOD(QueryInterface(REFIID riid, void **ppvObject));
@@ -281,4 +269,11 @@ class CLRDebuggingImpl : public ICLRDebugging, public ICLRDebuggingPolicy
 
 };  // class CLRDebuggingImpl
 
+// Resolves the paths to the cDAC and DBI bundled with dbgshim (or the DOTNET_CDAC_PATH/DOTNET_DBI_PATH
+// overrides), returning false if either is missing.
+bool GetCDacAndDbiPaths(SString& cdacPath, SString& dbiPath);
+
+// Picks the final HRESULT after the different activation attempts (cDAC vs. fallback) have completed.
+HRESULT SelectActivationResult(HRESULT cdacHr, HRESULT fallbackHr, bool cdacEvaluated);
+
 #endif

src/tests/DbgShim.UnitTests/DbgShimAPI.cs

@@ -34,6 +34,8 @@ public class DbgShimAPI
 
         private static CLRCreateInstanceDelegate _clrCreateInstance;
 
+        private static SetCDacLoadPolicyDelegate _setCDacLoadPolicy;
+
         private static IntPtr _dbgshimModuleHandle = IntPtr.Zero;
 
         public const int CorDebugVersion_2_0 = 3;
@@ -66,13 +68,16 @@ public static void Initialize(string dbgshimPath)
             _createDebuggingInterfaceFromVersion2 = GetDelegateFunction<CreateDebuggingInterfaceFromVersion2Delegate>("CreateDebuggingInterfaceFromVersion2");
             _createDebuggingInterfaceFromVersion3 = GetDelegateFunction<CreateDebuggingInterfaceFromVersion3Delegate>("CreateDebuggingInterfaceFromVersion3", optional: true);
             _clrCreateInstance = GetDelegateFunction<CLRCreateInstanceDelegate>("CLRCreateInstance");
+            _setCDacLoadPolicy = GetDelegateFunction<SetCDacLoadPolicyDelegate>("SetCDacLoadPolicy", optional: true);
             _initialized = true;
         }
 
         public static bool IsRegisterForRuntimeStartup3Supported => _registerForRuntimeStartup3 != default;
 
         public static bool IsCreateDebuggingInterfaceFromVersion3Supported => _createDebuggingInterfaceFromVersion3 != default;
 
+        public static bool IsSetCDacLoadPolicySupported => _setCDacLoadPolicy != default;
+
         public static HResult CreateProcessForLaunch(string commandLine, bool suspendProcess, string currentDirectory, out int processId, out IntPtr resumeHandle)
         {
             return _createProcessForLaunch(commandLine, suspendProcess, lpEnvironment: IntPtr.Zero, currentDirectory, out processId, out resumeHandle);
@@ -242,6 +247,15 @@ public static HResult CLRCreateInstance(out ICLRDebugging clrDebugging)
             return hr;
         }
 
+        public static HResult SetCDacLoadPolicy(DbgShimCDacLoadPolicy policy)
+        {
+            if (_setCDacLoadPolicy == default)
+            {
+                throw new NotSupportedException("SetCDacLoadPolicy not supported");
+            }
+            return _setCDacLoadPolicy(policy);
+        }
+
         private static T GetDelegateFunction<T>(string functionName, bool optional = false)
             where T : Delegate
         {
@@ -357,6 +371,10 @@ private unsafe delegate int CLRCreateInstanceDelegate(
             in Guid riid,
             out IntPtr pInterface);
 
+        [UnmanagedFunctionPointer(CallingConvention.StdCall)]
+        private delegate int SetCDacLoadPolicyDelegate(
+            DbgShimCDacLoadPolicy policy);
+
         #endregion
     }
 }