Skip to content

Comments

Implement ISOSDacInterface::GetJitManagerList in cDAC SOSDacImpl#124815

Open
Copilot wants to merge 6 commits intomainfrom
copilot/implement-get-jit-manager-list
Open

Implement ISOSDacInterface::GetJitManagerList in cDAC SOSDacImpl#124815
Copilot wants to merge 6 commits intomainfrom
copilot/implement-get-jit-manager-list

Conversation

Copy link
Contributor

Copilot AI commented Feb 24, 2026

  • Implement ISOSDacInterface::GetJitManagerList in cDAC SOSDacImpl
  • Rename GetJitManagerInfo to GetEEJitManagerInfo per review feedback
  • Fix cdac_data EEJitManagerPtr to use typed pointer (matching SystemDomain pattern)
  • Investigate and fix CI failure: data descriptor does not evaluate to a constant
  • Run code review
  • Run CodeQL security check

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@dotnet-policy-service
Copy link
Contributor

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

Copilot AI and others added 2 commits February 24, 2026 20:52
- Add AllCodeHeaps field to cdac_data<EEJitManager> and EEJitManagerAddress global pointer
- Update datadescriptor.inc with new type field and global pointer
- Add JitManagerInfo struct and GetJitManagerInfo() to IExecutionManager contract
- Implement GetJitManagerInfo in ExecutionManagerCore, ExecutionManager_1, ExecutionManager_2
- Define DacpJitManagerInfo struct in ISOSDacInterface.cs
- Implement GetJitManagerList in SOSDacImpl.cs with DEBUG comparison
- Add unit tests for GetJitManagerInfo (16 new tests, all pass)
- Update MockDescriptors.ExecutionManager with EEJitManager type and global

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
… descriptors

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copilot AI changed the title [WIP] Add GetJitManagerList implementation to ISOSDacInterface Implement ISOSDacInterface::GetJitManagerList in cDAC SOSDacImpl Feb 24, 2026
Copy link
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.

Pretty close, @copilot address comments.

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copy link
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

Implements the cDAC-side ISOSDacInterface::GetJitManagerList by plumbing the EEJitManager address and heap list through CoreCLR data descriptors into the managed ExecutionManager contract, then consuming it from SOSDacImpl with DEBUG legacy-DAC comparison.

Changes:

  • Add CoreCLR data descriptor support for EEJitManager::m_pAllCodeHeaps and expose a global pointer to ExecutionManager::m_pEEJitManager.
  • Add JitManagerInfo + GetEEJitManagerInfo() to the managed IExecutionManager contract and implement it in ExecutionManagerCore.
  • Update SOS COM interface/implementation to use a typed DacpJitManagerInfo and add contract-level unit tests + documentation updates.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.ExecutionManager.cs Extends mock target descriptors/globals to model EEJitManager and its heap list pointer.
src/native/managed/cdac/tests/ExecutionManager/ExecutionManagerTests.cs Adds unit tests validating GetEEJitManagerInfo() across arch/version permutations.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs Replaces GetJitManagerList stub with a contract-backed implementation + DEBUG legacy comparison.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ISOSDacInterface.cs Introduces DacpJitManagerInfo and updates COM signature to use a typed pointer.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/EEJitManager.cs Adds AllCodeHeaps data-model field read from the target.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_1.cs Forwards GetEEJitManagerInfo() to the core implementation.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_2.cs Forwards GetEEJitManagerInfo() to the core implementation.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.cs Implements GetEEJitManagerInfo() by reading the new global + EEJitManager data.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs Adds Globals.EEJitManagerAddress constant name.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IExecutionManager.cs Adds JitManagerInfo struct and the new contract method.
src/coreclr/vm/datadescriptor/datadescriptor.inc Exposes EEJitManager.AllCodeHeaps and the EEJitManagerAddress global pointer.
src/coreclr/vm/codeman.h Adds cdac_data access for m_pAllCodeHeaps and exposes m_pEEJitManager pointer location.
docs/design/datacontracts/ExecutionManager.md Documents the new descriptor/global and contract API.

@max-charlamb
Copy link
Member

@copilot , use the CI-Analysis tool to look at the runtime-diagnostics pipeline failures. There is a problem that one of the datadescriptors added does not evaluate to a constant.

Copilot AI and others added 2 commits February 24, 2026 22:47
…data<ExecutionManager>

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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