cDac fixes: StressLog address and exceptions flowing out of NativeAOT (#128845)

Fix two simple cDac bugs:

1. StressLogs were passing &g_pStressLog, but the original dac actually
hands out StressLog::theLog, so we get the wrong address on the consumer
side. I chose to just export &StressLog::theLog, but I could wrap this
as (&g_pStressLog) and dereference in cDac if there's a preference for
that.
2. The entrypoint for clrmd had no try/catch. Since cDac is nativeAOT
compiled throwing an exception past the boundary would hard crash the
process. I converted it to E_FAIL for now. The diff shows a lot of
changes, but it's just adding a try/catches to entrypoints, and argument
validation per copilot review.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: leculver

src/coreclr/nativeaot/Runtime/datadescriptor/datadescriptor.inc

@@ -154,7 +154,7 @@ CDAC_GLOBAL(ObjectToMethodTableUnmask, T_UINT8, 3)
 
 // StressLog globals (no module table in NativeAOT)
 CDAC_GLOBAL(StressLogEnabled, T_UINT8, 1)
-CDAC_GLOBAL_POINTER(StressLog, &g_pStressLog)
+CDAC_GLOBAL_POINTER(StressLog, &StressLog::theLog)
 CDAC_GLOBAL(StressLogHasModuleTable, T_UINT8, 0)
 CDAC_GLOBAL(StressLogChunkSize, T_UINT32, STRESSLOG_CHUNK_SIZE)
 CDAC_GLOBAL(StressLogValidChunkSig, T_UINT32, 0xCFCFCFCF)

src/coreclr/vm/datadescriptor/datadescriptor.inc

@@ -1592,7 +1592,7 @@ CDAC_GLOBAL_POINTER(TlsIndexBase, &::_tls_index)
 #endif // TARGET_WINDOWS
 #ifdef STRESS_LOG
 CDAC_GLOBAL(StressLogEnabled, T_UINT8, 1)
-CDAC_GLOBAL_POINTER(StressLog, &g_pStressLog)
+CDAC_GLOBAL_POINTER(StressLog, &StressLog::theLog)
 CDAC_GLOBAL(StressLogHasModuleTable, T_UINT8, 1)
 CDAC_GLOBAL(StressLogMaxModules, T_UINT64, cdac_offsets<StressLog>::MAX_MODULES)
 CDAC_GLOBAL(StressLogChunkSize, T_UINT32, STRESSLOG_CHUNK_SIZE)

src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs

@@ -22,70 +22,90 @@ private static unsafe int Init(
         void* delegateContext,
         IntPtr* handle)
     {
-        // Build the allocVirtual delegate if the caller provided a callback
-        ContractDescriptorTarget.AllocVirtualDelegate allocDelegate = (ulong size, out ulong allocatedAddress) =>
+        try
         {
-            allocatedAddress = 0;
-            return HResults.E_NOTIMPL;
-        };
+            if (handle == null)
+                return HResults.E_INVALIDARG;
+            *handle = IntPtr.Zero;
 
-        if (allocVirtual != null)
-        {
-            allocDelegate = (ulong size, out ulong allocatedAddress) =>
+            // Build the allocVirtual delegate if the caller provided a callback
+            ContractDescriptorTarget.AllocVirtualDelegate allocDelegate = (ulong size, out ulong allocatedAddress) =>
             {
-                if (size > uint.MaxValue)
-                {
-                    allocatedAddress = 0;
-                    return HResults.E_INVALIDARG;
-                }
-
-                fixed (ulong* addrPtr = &allocatedAddress)
-                {
-                    return allocVirtual((uint)size, addrPtr, delegateContext);
-                }
+                allocatedAddress = 0;
+                return HResults.E_NOTIMPL;
             };
-        }
 
-        // TODO: [cdac] Better error code/details
-        if (!ContractDescriptorTarget.TryCreate(
-            descriptor,
-            (address, buffer) =>
+            if (allocVirtual != null)
             {
-                fixed (byte* bufferPtr = buffer)
+                allocDelegate = (ulong size, out ulong allocatedAddress) =>
                 {
-                    return readFromTarget(address, bufferPtr, (uint)buffer.Length, delegateContext);
-                }
-            },
-            (address, buffer) =>
-            {
-                fixed (byte* bufferPtr = buffer)
+                    if (size > uint.MaxValue)
+                    {
+                        allocatedAddress = 0;
+                        return HResults.E_INVALIDARG;
+                    }
+
+                    fixed (ulong* addrPtr = &allocatedAddress)
+                    {
+                        return allocVirtual((uint)size, addrPtr, delegateContext);
+                    }
+                };
+            }
+
+            // TODO: [cdac] Better error code/details
+            if (!ContractDescriptorTarget.TryCreate(
+                descriptor,
+                (address, buffer) =>
                 {
-                    return writeToTarget(address, bufferPtr, (uint)buffer.Length, delegateContext);
-                }
-            },
-            (threadId, contextFlags, buffer) =>
-            {
-                fixed (byte* bufferPtr = buffer)
+                    fixed (byte* bufferPtr = buffer)
+                    {
+                        return readFromTarget(address, bufferPtr, (uint)buffer.Length, delegateContext);
+                    }
+                },
+                (address, buffer) =>
                 {
-                    return readThreadContext(threadId, contextFlags, (uint)buffer.Length, bufferPtr, delegateContext);
-                }
-            },
-            allocDelegate,
-            [Contracts.CoreCLRContracts.Register],
-            out ContractDescriptorTarget? target))
-            return -1;
+                    fixed (byte* bufferPtr = buffer)
+                    {
+                        return writeToTarget(address, bufferPtr, (uint)buffer.Length, delegateContext);
+                    }
+                },
+                (threadId, contextFlags, buffer) =>
+                {
+                    fixed (byte* bufferPtr = buffer)
+                    {
+                        return readThreadContext(threadId, contextFlags, (uint)buffer.Length, bufferPtr, delegateContext);
+                    }
+                },
+                allocDelegate,
+                [Contracts.CoreCLRContracts.Register],
+                out ContractDescriptorTarget? target))
+                return -1;
 
-        GCHandle gcHandle = GCHandle.Alloc(target);
-        *handle = GCHandle.ToIntPtr(gcHandle);
-        return 0;
+            GCHandle gcHandle = GCHandle.Alloc(target);
+            *handle = GCHandle.ToIntPtr(gcHandle);
+            return 0;
+        }
+        catch (Exception ex)
+        {
+            int hr = ex.HResult;
+            return hr < 0 ? hr : HResults.E_FAIL;
+        }
     }
 
     [UnmanagedCallersOnly(EntryPoint = $"{CDAC}free")]
     private static unsafe int Free(IntPtr handle)
     {
-        GCHandle h = GCHandle.FromIntPtr(handle);
-        h.Free();
-        return 0;
+        try
+        {
+            GCHandle h = GCHandle.FromIntPtr(handle);
+            h.Free();
+            return 0;
+        }
+        catch (Exception ex)
+        {
+            int hr = ex.HResult;
+            return hr < 0 ? hr : HResults.E_FAIL;
+        }
     }
 
     /// <summary>
@@ -98,17 +118,31 @@ private static unsafe int Free(IntPtr handle)
     [UnmanagedCallersOnly(EntryPoint = $"{CDAC}create_sos_interface")]
     private static unsafe int CreateSosInterface(IntPtr handle, IntPtr legacyImplPtr, nint* obj)
     {
-        Target? target = GCHandle.FromIntPtr(handle).Target as Target;
-        if (target == null)
-            return -1;
+        try
+        {
+            if (obj == null)
+                return HResults.E_INVALIDARG;
+            *obj = IntPtr.Zero;
 
-        object? legacyImpl = legacyImplPtr != IntPtr.Zero
-            ? ComInterfaceMarshaller<ISOSDacInterface>.ConvertToManaged((void*)legacyImplPtr)
-            : null;
-        Legacy.SOSDacImpl impl = new(target, legacyImpl);
-        nint ptr = (nint)ComInterfaceMarshaller<ISOSDacInterface>.ConvertToUnmanaged(impl);
-        *obj = ptr;
-        return 0;
+            Target? target = GCHandle.FromIntPtr(handle).Target as Target;
+            if (target == null)
+                return HResults.E_INVALIDARG;
+
+            object? legacyImpl = legacyImplPtr != IntPtr.Zero
+                ? ComInterfaceMarshaller<ISOSDacInterface>.ConvertToManaged((void*)legacyImplPtr)
+                : null;
+            Legacy.SOSDacImpl impl = new(target, legacyImpl);
+            nint ptr = (nint)ComInterfaceMarshaller<ISOSDacInterface>.ConvertToUnmanaged(impl);
+            *obj = ptr;
+            return 0;
+        }
+        catch (Exception ex)
+        {
+            if (obj != null)
+                *obj = IntPtr.Zero;
+            int hr = ex.HResult;
+            return hr < 0 ? hr : HResults.E_FAIL;
+        }
     }
 
     /// <summary>
@@ -120,35 +154,45 @@ private static unsafe int CreateSosInterface(IntPtr handle, IntPtr legacyImplPtr
     [UnmanagedCallersOnly(EntryPoint = $"{CDAC}create_dacdbi_interface")]
     private static unsafe int CreateDacDbiInterface(IntPtr handle, IntPtr legacyImplPtr, nint* obj)
     {
-        if (obj == null)
-            return HResults.E_INVALIDARG;
-        if (handle == IntPtr.Zero)
-        {
-            *obj = IntPtr.Zero;
-            return HResults.E_NOTIMPL;
-        }
-
-        Target? target = GCHandle.FromIntPtr(handle).Target as Target;
-        if (target is null)
+        try
         {
-            *obj = IntPtr.Zero;
-            return HResults.E_INVALIDARG;
-        }
+            if (obj == null)
+                return HResults.E_INVALIDARG;
+            if (handle == IntPtr.Zero)
+            {
+                *obj = IntPtr.Zero;
+                return HResults.E_NOTIMPL;
+            }
 
-        object? legacyObj = null;
-        if (legacyImplPtr != IntPtr.Zero)
-        {
-            legacyObj = ComInterfaceMarshaller<IDacDbiInterface>.ConvertToManaged((void*)legacyImplPtr);
-            if (legacyObj is not Legacy.IDacDbiInterface)
+            Target? target = GCHandle.FromIntPtr(handle).Target as Target;
+            if (target is null)
             {
                 *obj = IntPtr.Zero;
-                return HResults.COR_E_INVALIDCAST; // E_NOINTERFACE
+                return HResults.E_INVALIDARG;
+            }
+
+            object? legacyObj = null;
+            if (legacyImplPtr != IntPtr.Zero)
+            {
+                legacyObj = ComInterfaceMarshaller<IDacDbiInterface>.ConvertToManaged((void*)legacyImplPtr);
+                if (legacyObj is not Legacy.IDacDbiInterface)
+                {
+                    *obj = IntPtr.Zero;
+                    return HResults.COR_E_INVALIDCAST; // E_NOINTERFACE
+                }
             }
-        }
 
-        Legacy.DacDbiImpl impl = new(target, legacyObj);
-        *obj = (nint)ComInterfaceMarshaller<IDacDbiInterface>.ConvertToUnmanaged(impl);
-        return HResults.S_OK;
+            Legacy.DacDbiImpl impl = new(target, legacyObj);
+            *obj = (nint)ComInterfaceMarshaller<IDacDbiInterface>.ConvertToUnmanaged(impl);
+            return HResults.S_OK;
+        }
+        catch (Exception ex)
+        {
+            if (obj != null)
+                *obj = IntPtr.Zero;
+            int hr = ex.HResult;
+            return hr < 0 ? hr : HResults.E_FAIL;
+        }
     }
 
     [UnmanagedCallersOnly(EntryPoint = "CLRDataCreateInstanceWithFallback")]
@@ -170,6 +214,19 @@ private static unsafe int CLRDataCreateInstanceImpl(Guid* pIID, IntPtr /*ICLRDat
             return HResults.E_INVALIDARG;
         *iface = null;
 
+        try
+        {
+            return CLRDataCreateInstanceCore(pIID, pLegacyTarget, pLegacyImpl, iface);
+        }
+        catch (Exception ex)
+        {
+            int hr = ex.HResult;
+            return hr < 0 ? hr : HResults.E_FAIL;
+        }
+    }
+
+    private static unsafe int CLRDataCreateInstanceCore(Guid* pIID, IntPtr /*ICLRDataTarget*/ pLegacyTarget, IntPtr pLegacyImpl, void** iface)
+    {
         object legacyTarget = ComInterfaceMarshaller<ICLRDataTarget>.ConvertToManaged((void*)pLegacyTarget)!;
         object? legacyImpl = pLegacyImpl != IntPtr.Zero ?
             ComInterfaceMarshaller<ISOSDacInterface>.ConvertToManaged((void*)pLegacyImpl) : null;