Skip to content

adding GetOtherNotificationFlags cDAC API #117616

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

Merged
merged 6 commits into from
Jul 22, 2025

Conversation

rcj1
Copy link
Contributor

@rcj1 rcj1 commented Jul 14, 2025

No description provided.

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

This PR introduces a new cDAC API GetOtherNotificationFlags to read notification flags from the target process, updates the global constants to include DacNotificationFlags, and registers this global in the native descriptor.

  • Implements IXCLRDataProcess.GetOtherNotificationFlags with exception handling and debug cross-checks.
  • Adds DacNotificationFlags to managed contract constants.
  • Registers DacNotificationFlags in datadescriptor.h.

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.IXCLRDataProcess.cs Added new GetOtherNotificationFlags implementation
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs Added DacNotificationFlags constant
src/coreclr/debug/runtimeinfo/datadescriptor.h Registered DacNotificationFlags with CDAC_GLOBAL_POINTER
Comments suppressed due to low confidence (2)

src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.IXCLRDataProcess.cs:213

  • The new GetOtherNotificationFlags API lacks unit tests. Consider adding tests to cover successful reads, exception paths, and the debug-mode comparison against the legacy implementation.
    int IXCLRDataProcess.GetOtherNotificationFlags(uint* flags)

src/coreclr/debug/runtimeinfo/datadescriptor.h:971

  • [nitpick] This line is missing the leading space for alignment with the surrounding CDAC_GLOBAL_POINTER entries; adding it will keep the code formatting consistent.
CDAC_GLOBAL_POINTER(DacNotificationFlags, &::g_dacNotificationFlags)

Copy link
Contributor

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

@rcj1
Copy link
Contributor Author

rcj1 commented Jul 22, 2025

/ba-g infrastructure timeouts

@rcj1 rcj1 merged commit b131b6d into dotnet:main Jul 22, 2025
90 of 98 checks passed
@rcj1 rcj1 deleted the GetOtherNotificationFlags branch July 22, 2025 15:37
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