Skip to content

Adding GetClrNotification cDAC API #117737

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

rcj1
Copy link
Contributor

@rcj1 rcj1 commented Jul 16, 2025

No description provided.

@Copilot Copilot AI review requested due to automatic review settings July 16, 2025 23:00
Copy link
Contributor

@Copilot 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

Adds the new GetClrNotification cDAC API and its supporting global constants.

  • Implements ISOSDacInterface4.GetClrNotification in SOSDacImpl with direct reads from target memory and optional legacy cross-checks in DEBUG.
  • Defines two new global names (MaxClrNotificationArgs and ClrNotificationArguments) in the managed constants and registers them in the native descriptor header.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs Added full implementation of GetClrNotification
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs Introduced MaxClrNotificationArgs and ClrNotificationArguments
src/coreclr/debug/runtimeinfo/datadescriptor.h Registered the two new globals with CDAC_GLOBAL/CDAC_GLOBAL_POINTER
Comments suppressed due to low confidence (2)

src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs:1776

  • This new GetClrNotification implementation lacks accompanying unit tests to verify behavior under different scenarios (e.g., when there are no notifications, at maximum argument count, and during exception paths). Adding tests in the existing test suite will help ensure correctness and prevent regressions.
    int ISOSDacInterface4.GetClrNotification(ClrDataAddress[] arguments, int count, int* pNeeded)

src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs:1792

  • The pointer arithmetic uses sizeof(ClrDataAddress), which may not match the actual pointer size on all architectures. Consider using the target pointer size (e.g., (ulong)IntPtr.Size) or a defined address-size constant to compute the correct offset.
                    arguments[i] = _target.Read<ulong>(basePtr.Value + (ulong)(i * sizeof(ClrDataAddress)));

Copy link
Contributor

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

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.

2 participants