Skip to content

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

Merged
leculver merged 3 commits into
dotnet:mainfrom
leculver:cdac-fixes
Jun 2, 2026
Merged

cDac fixes: StressLog address and exceptions flowing out of NativeAOT#128845
leculver merged 3 commits into
dotnet:mainfrom
leculver:cdac-fixes

Conversation

@leculver
Copy link
Copy Markdown
Contributor

@leculver leculver commented Jun 1, 2026

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.

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates cDAC’s exported data descriptors and unmanaged entrypoints to (1) expose the correct StressLog address and (2) prevent managed exceptions from escaping NativeAOT-compiled unmanaged boundaries.

Changes:

  • Fix StressLog global pointer export to use &StressLog::theLog (CoreCLR + NativeAOT descriptors).
  • Wrap cDAC unmanaged entrypoints in try/catch and translate exceptions to failing HRESULTs to avoid hard crashes when exceptions cross the unmanaged boundary.
  • Refactor CLRDataCreateInstanceImpl to route work through a guarded core helper.
Show a summary per file
File Description
src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs Adds exception guards around unmanaged exports and refactors CLRDataCreateInstanceImpl to avoid exceptions crossing the boundary.
src/coreclr/vm/datadescriptor/datadescriptor.inc Updates StressLog global pointer export to point at the StressLog instance.
src/coreclr/nativeaot/Runtime/datadescriptor/datadescriptor.inc Same StressLog pointer correction for NativeAOT descriptor parity.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 2

Comment thread src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs Outdated
Comment thread src/native/managed/cdac/mscordaccore_universal/Entrypoints.cs
Init now rejects a null handle pointer with E_INVALIDARG and zeroes *handle
before doing work, so a TryCreate failure leaves the output deterministically
zeroed instead of uninitialized.

CreateSosInterface now validates obj before dereferencing and zeroes *obj up
front, so the bad/zero-handle failure path no longer leaves *obj uninitialized
for callers that don't check the return value. The failure code is also changed
from -1 to E_INVALIDARG for consistency with the other entrypoints.

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

@max-charlamb max-charlamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

Copy link
Copy Markdown
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@leculver leculver merged commit ea17bc5 into dotnet:main Jun 2, 2026
124 of 126 checks passed
@leculver leculver deleted the cdac-fixes branch June 2, 2026 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants