Adding SetGcNotification API and related runtime simplification

[rcj1]

• Previously, SetGcNotification was implemented through a complex lookup table that we allocated at first usage. This is the same structure used in the JitNotifications, and presumably copied from there. However, we don't need this complex structure; we just need to store the generations for which we need a notification. This can be stored in a single short bitmask; the 0th bit is set if we need gen 0 notifications, etc. I have implemented the necessary changes in the runtime to do this.

• I have implemented SetGcNotification. GetGcNotification is not used anywhere, so I made it E_NOTIMPL in both the DAC and cDAC.

• Tested by ensuring that the debugger breaks on the generation of GC specified by SetGcNotification (by manually calling GC.Collect())

[Copilot]

Pull Request Overview

This PR implements the SetGcNotification API and significantly simplifies the GC notification system in the runtime. Instead of using a complex lookup table structure copied from JIT notifications, it replaces it with a simple 16-bit bitmask where each bit represents whether notifications are needed for a specific generation.

Key changes:

• Replaces complex GcNotifications class with simple namespace containing two functions
• Implements SetGcNotification API in both DAC and cDAC with proper error handling
• Removes unused GetGcNotification functionality (returns E_NOTIMPL)

Reviewed Changes

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

Show a summary per file

File	Description
SOSDacImpl.IXCLRDataProcess.cs	Implements SetGcNotification in cDAC with validation and debug assertions
IXCLRData.cs	Adds GcEvt_t enum to replace uint type for better type safety
Notifications_1.cs	Core implementation using bitmask to set/clear generation flags
NotificationsFactory.cs	Factory class for creating Notifications contract instances
Constants.cs	Adds GcNotificationFlags global constant
INotifications.cs	Defines the Notifications contract interface
ContractRegistry.cs	Registers Notifications contract in the system
vars.hpp/vars.cpp	Declares and defines g_gcNotificationFlags global variable
util.hpp/util.cpp	Replaces complex GcNotifications class with simple namespace functions
gcenv.ee.cpp	Simplifies GC notification checking logic
datadescriptor.inc	Adds global pointer and contract registration
dacvars.h	Updates DAC variable definitions
dacimpl.h/daccess.cpp	Removes complex notification table management
Notifications.md	Documents the new Notifications contract API

[dotnet-policy-service]

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

[max-charlamb]

It looks like GetGcNotification(...) would be trivial to implement. All we do is convert the flag back to the current notification status. Would this be worth implementing for the cDAC/DAC?

Looks pretty good otherwise

[rcj1]

It looks like GetGcNotification(...) would be trivial to implement. All we do is convert the flag back to the current notification status. Would this be worth implementing for the cDAC/DAC?

Looks pretty good otherwise

It is trivial to implement, but I wanted to make it not implemented because we use TranslateExceptionRecordToNotification to parse the GC notification exception, in all the relevant code bases there is no use of GetGcNotification, and we can always add it if we need to.

[max-charlamb]

It looks like GetGcNotification(...) would be trivial to implement. All we do is convert the flag back to the current notification status. Would this be worth implementing for the cDAC/DAC?
Looks pretty good otherwise

It is trivial to implement, but I wanted to make it not implemented because we use TranslateExceptionRecordToNotification to parse the GC notification exception, in all the relevant code bases there is no use of GetGcNotification, and we can always add it if we need to.

GetGcNotification has a different purpose than parsing the notifications, it gets the currently set flags. I agree that it doesn't seem to be used anywhere so I think it is fine to remove.

I also started a pipeline run with the tests in dotnet/diagnostics#5562, this should verify that findroots still works correctly.

• runtime-diagnostics runtime: main diagnostics: [SOS/DAC] Add tests for DAC GC bugs diagnostics#5562
  • https://dev.azure.com/dnceng-public/public/_build/results?buildId=1158187&view=results
• runtime-diagnostics runtime: this PR diagnostics [SOS/DAC] Add tests for DAC GC bugs diagnostics#5562
  • https://dev.azure.com/dnceng-public/public/_build/results?buildId=1158154&view=results

[max-charlamb]

Looks good to merge if you fix the few comments.