Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers - #124674

Merged
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table
Mar 4, 2026
Merged

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers#124674
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table

Conversation

@noahfalk

@noahfalknoahfalk commented Feb 20, 2026

Copy link
Copy Markdown
Member

Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:

  • GetGenerationTable / GetGenerationTableSvr
  • GetFinalizationFillPointers / GetFinalizationFillPointersSvr

Add test infrastructure:

  • TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace, accumulates types/globals, and wires contracts via TestContractRegistry.
  • GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure GC mock data via Action and build directly into the target.
  • TestContractRegistry: Dictionary<Type, Lazy> replacement for Mock in the builder path.

Add tests:

  • 3 contract-level tests in GCTests.cs (x4 arch = 12)
  • 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)

Add documentation:

  • README.md files for cdac/, Legacy/, and tests/ directories
  • Copilot instruction to search for READMEs along the path hierarchy

Add a variety of fixes for pre-existing issues I discovered trying to run the SOS tests on Linux for both Release and Debug flavors. I'm guessing those testing configurations haven't been used much:

  • Fix cDAC library loading on Unix (PAL_GetPalHostModule)
  • Fix Thread data model docs (originally I had fixed the contract impl but a parallel change already took care of it so that part)
  • Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
    request.cpp where inner HRESULT declarations masked the return value
  • Fix cDAC legacy stack walk not advancing in Release builds
    (ClrDataStackWalk.Next must call legacy outside #if DEBUG)
  • Fix Debug assertion failures: catch VirtualReadException in
    GetAppDomainName and GetObjectData, fall back to legacy in
    EnumMethodInstanceByAddress

Fixes#124692
Fixes#124693
Fixes#124694
Fixes#124695

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 implements four ISOSDacInterface8 methods in the cDAC SOSDacImpl layer for querying GC generation tables and finalization fill pointers, along with comprehensive test infrastructure and documentation.

Changes:

  • Implements GetGenerationTable, GetGenerationTableSvr, GetFinalizationFillPointers, and GetFinalizationFillPointersSvr in SOSDacImpl.cs by delegating to the existing IGC contract
  • Introduces TestPlaceholderTarget.Builder pattern with fluent API and TestContractRegistry to replace Mock<ContractRegistry> for more maintainable test setup
  • Adds GCHeapBuilder with extension methods (AddGCHeapWks/AddGCHeapSvr) to configure GC mock data declaratively
  • Provides 36 test cases total (3 contract-level + 6 SOSDacImpl-level × 4 architectures each) with proper sign-extension handling
  • Documents the cDAC architecture, test patterns, and SOSDacImpl implementation conventions in new README files

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/TestPlaceholderTarget.csRemoves Moq dependency; adds fluent Builder class and TestContractRegistry for programmatic test target construction
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.csNew file: GCHeapBuilder config class and extension methods to set up workstation/server GC heap mock data
src/native/managed/cdac/tests/GCTests.csNew file: 3 contract-level tests verifying IGC.GetHeapData() correctly reads generation table and fill pointers
src/native/managed/cdac/tests/SOSDacInterface8Tests.csNew file: 6 SOSDacImpl-level tests validating buffer sizing protocol, HResult codes, and pointer sign-extension
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements 4 ISOSDacInterface8 methods with proper validation, error handling, and debug cross-validation
src/native/managed/cdac/tests/README.mdNew documentation: test architecture, builder patterns, sign-extension gotchas, and mock descriptor guidelines
src/native/managed/cdac/README.mdNew documentation: cDAC architecture overview, project structure, and integration testing workflow
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/README.mdNew documentation: SOSDacImpl implementation patterns, HResult conventions, and buffer sizing protocol
.github/copilot-instructions.mdAdds instruction to search for README files in directory hierarchy before making changes

Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs Outdated
Comment thread.github/copilot-instructions.md
Comment threadsrc/native/managed/cdac/tests/README.md
Comment threadsrc/native/managed/cdac/README.md
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from f9fb24b to 398b718CompareFebruary 22, 2026 08:08
@noahfalk
noahfalk marked this pull request as ready for review February 22, 2026 08:18
CopilotAI review requested due to automatic review settings February 22, 2026 08:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadeng/pipelines/runtime-diagnostics.yml
CopilotAI review requested due to automatic review settings February 26, 2026 09:15
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 33c4550 to 10ab4c2CompareFebruary 26, 2026 09:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

noahfalkand others added 4 commits March 1, 2026 03:55
…izationFillPointers
Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:
- GetGenerationTable / GetGenerationTableSvr
- GetFinalizationFillPointers / GetFinalizationFillPointersSvr
Add test infrastructure:
- TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace,
accumulates types/globals, and wires contracts via TestContractRegistry.
- GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure
GC mock data via Action<GCHeapBuilder> and build directly into the target.
- TestContractRegistry: Dictionary<Type, Lazy<IContract>> replacement for
Mock<ContractRegistry> in the builder path.
Add tests:
- 3 contract-level tests in GCTests.cs (x4 arch = 12)
- 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)
Add documentation:
- README.md files for cdac/, Legacy/, and tests/ directories
- Copilot instruction to search for READMEs along the path hierarchy
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes discovered while running SOS integration tests against the new
GetGenerationTable/GetFinalizationFillPointers implementation:
- Fix cDAC library loading on Unix (PAL_GetPalHostModule)
- Fix Thread data model crash on non-Windows (TryGetValue for
UEWatsonBucketTrackerBuckets)
- Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
request.cpp where inner HRESULT declarations masked the return value
- Fix cDAC legacy stack walk not advancing in Release builds
(ClrDataStackWalk.Next must call legacy outside #if DEBUG)
- Fix Debug assertion failures: catch VirtualReadException in
GetAppDomainName and GetObjectData, fall back to legacy in
EnumMethodInstanceByAddress
- Address PR review feedback: add insufficient-buffer tests, pNeeded
cross-validation assertions, and reduce test data duplication with
shared helpers
- Update README documentation with VirtualReadException guidance and
legacy delegation placement
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…atch, use heapData.GenerationTable.Count, and use local buffers for debug cross-validation
- Change void* to DacpGenerationData* in ISOSDacInterface8 methods
- Move null/validation checks inside try/catch blocks per PR dotnet#124814 pattern
- Replace ReadGlobal<uint>(TotalGenerationCount) with heapData.GenerationTable.Count
- Use local buffers for legacy DAC cross-validation to avoid overwriting cDAC data
- Use 'is null'/'is not null' instead of '== null'/'!= null'
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 1, 2026 11:55
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 77081c0 to 6ceae96CompareMarch 1, 2026 11:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

…EBUG loop bounds
- GetNumberGenerations: use gc.GetMaxGeneration()+1 via the GC contract
instead of gc.GetHeapData() which throws on Server GC targets.
- GetGenerationTable/GetFinalizationFillPointers DEBUG cross-validation:
limit comparison loops to pNeededLocal (the actual written count) instead
of the caller buffer size to avoid comparing uninitialized trailing entries.
- GetFinalizationFillPointersSvr: use Math.Min(cFillPointers, pNeededLocal)
for the same reason.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs

@max-charlambmax-charlamb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good modulo couple comments

…ally
The legacy enumeration is started and advanced unconditionally in
StartEnumMethodInstancesByAddress and EnumMethodInstanceByAddress,
but EndEnumMethodInstancesByAddress only cleaned up the legacy handle
under #if DEBUG. This would leak the legacy enumeration handle in
Release builds.
Remove the #if DEBUG guard so the cleanup runs in all builds.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 4, 2026 02:27

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

GetGenerationTable and GetFinalizationFillPointers are WKS-only APIs;
GetGenerationTableSvr and GetFinalizationFillPointersSvr are Server-only.
When called against the wrong GC mode, the native DAC returns E_FAIL,
but the cDAC was returning COR_E_INVALIDOPERATION (from the GC
contract's InvalidOperationException), which would cause a debug assert
mismatch during cross-validation.
Add explicit GC mode checks to all four methods and add 16 unit tests
validating the error behavior in both directions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@noahfalk
noahfalk merged commit 4895491 into dotnet:mainMar 4, 2026
110 of 114 checks passed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 4, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

4 participants

@noahfalk@max-charlamb@rcj1
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers - #124674

Merged
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table
Mar 4, 2026
Merged

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers#124674
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table

Conversation

@noahfalk

@noahfalknoahfalk commented Feb 20, 2026

Copy link
Copy Markdown
Member

Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:

  • GetGenerationTable / GetGenerationTableSvr
  • GetFinalizationFillPointers / GetFinalizationFillPointersSvr

Add test infrastructure:

  • TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace, accumulates types/globals, and wires contracts via TestContractRegistry.
  • GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure GC mock data via Action and build directly into the target.
  • TestContractRegistry: Dictionary<Type, Lazy> replacement for Mock in the builder path.

Add tests:

  • 3 contract-level tests in GCTests.cs (x4 arch = 12)
  • 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)

Add documentation:

  • README.md files for cdac/, Legacy/, and tests/ directories
  • Copilot instruction to search for READMEs along the path hierarchy

Add a variety of fixes for pre-existing issues I discovered trying to run the SOS tests on Linux for both Release and Debug flavors. I'm guessing those testing configurations haven't been used much:

  • Fix cDAC library loading on Unix (PAL_GetPalHostModule)
  • Fix Thread data model docs (originally I had fixed the contract impl but a parallel change already took care of it so that part)
  • Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
    request.cpp where inner HRESULT declarations masked the return value
  • Fix cDAC legacy stack walk not advancing in Release builds
    (ClrDataStackWalk.Next must call legacy outside #if DEBUG)
  • Fix Debug assertion failures: catch VirtualReadException in
    GetAppDomainName and GetObjectData, fall back to legacy in
    EnumMethodInstanceByAddress

Fixes#124692
Fixes#124693
Fixes#124694
Fixes#124695

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 implements four ISOSDacInterface8 methods in the cDAC SOSDacImpl layer for querying GC generation tables and finalization fill pointers, along with comprehensive test infrastructure and documentation.

Changes:

  • Implements GetGenerationTable, GetGenerationTableSvr, GetFinalizationFillPointers, and GetFinalizationFillPointersSvr in SOSDacImpl.cs by delegating to the existing IGC contract
  • Introduces TestPlaceholderTarget.Builder pattern with fluent API and TestContractRegistry to replace Mock<ContractRegistry> for more maintainable test setup
  • Adds GCHeapBuilder with extension methods (AddGCHeapWks/AddGCHeapSvr) to configure GC mock data declaratively
  • Provides 36 test cases total (3 contract-level + 6 SOSDacImpl-level × 4 architectures each) with proper sign-extension handling
  • Documents the cDAC architecture, test patterns, and SOSDacImpl implementation conventions in new README files

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/TestPlaceholderTarget.csRemoves Moq dependency; adds fluent Builder class and TestContractRegistry for programmatic test target construction
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.csNew file: GCHeapBuilder config class and extension methods to set up workstation/server GC heap mock data
src/native/managed/cdac/tests/GCTests.csNew file: 3 contract-level tests verifying IGC.GetHeapData() correctly reads generation table and fill pointers
src/native/managed/cdac/tests/SOSDacInterface8Tests.csNew file: 6 SOSDacImpl-level tests validating buffer sizing protocol, HResult codes, and pointer sign-extension
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements 4 ISOSDacInterface8 methods with proper validation, error handling, and debug cross-validation
src/native/managed/cdac/tests/README.mdNew documentation: test architecture, builder patterns, sign-extension gotchas, and mock descriptor guidelines
src/native/managed/cdac/README.mdNew documentation: cDAC architecture overview, project structure, and integration testing workflow
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/README.mdNew documentation: SOSDacImpl implementation patterns, HResult conventions, and buffer sizing protocol
.github/copilot-instructions.mdAdds instruction to search for README files in directory hierarchy before making changes

Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs Outdated
Comment thread.github/copilot-instructions.md
Comment threadsrc/native/managed/cdac/tests/README.md
Comment threadsrc/native/managed/cdac/README.md
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from f9fb24b to 398b718CompareFebruary 22, 2026 08:08
@noahfalk
noahfalk marked this pull request as ready for review February 22, 2026 08:18
CopilotAI review requested due to automatic review settings February 22, 2026 08:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadeng/pipelines/runtime-diagnostics.yml
CopilotAI review requested due to automatic review settings February 26, 2026 09:15
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 33c4550 to 10ab4c2CompareFebruary 26, 2026 09:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

noahfalkand others added 4 commits March 1, 2026 03:55
…izationFillPointers
Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:
- GetGenerationTable / GetGenerationTableSvr
- GetFinalizationFillPointers / GetFinalizationFillPointersSvr
Add test infrastructure:
- TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace,
accumulates types/globals, and wires contracts via TestContractRegistry.
- GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure
GC mock data via Action<GCHeapBuilder> and build directly into the target.
- TestContractRegistry: Dictionary<Type, Lazy<IContract>> replacement for
Mock<ContractRegistry> in the builder path.
Add tests:
- 3 contract-level tests in GCTests.cs (x4 arch = 12)
- 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)
Add documentation:
- README.md files for cdac/, Legacy/, and tests/ directories
- Copilot instruction to search for READMEs along the path hierarchy
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes discovered while running SOS integration tests against the new
GetGenerationTable/GetFinalizationFillPointers implementation:
- Fix cDAC library loading on Unix (PAL_GetPalHostModule)
- Fix Thread data model crash on non-Windows (TryGetValue for
UEWatsonBucketTrackerBuckets)
- Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
request.cpp where inner HRESULT declarations masked the return value
- Fix cDAC legacy stack walk not advancing in Release builds
(ClrDataStackWalk.Next must call legacy outside #if DEBUG)
- Fix Debug assertion failures: catch VirtualReadException in
GetAppDomainName and GetObjectData, fall back to legacy in
EnumMethodInstanceByAddress
- Address PR review feedback: add insufficient-buffer tests, pNeeded
cross-validation assertions, and reduce test data duplication with
shared helpers
- Update README documentation with VirtualReadException guidance and
legacy delegation placement
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…atch, use heapData.GenerationTable.Count, and use local buffers for debug cross-validation
- Change void* to DacpGenerationData* in ISOSDacInterface8 methods
- Move null/validation checks inside try/catch blocks per PR dotnet#124814 pattern
- Replace ReadGlobal<uint>(TotalGenerationCount) with heapData.GenerationTable.Count
- Use local buffers for legacy DAC cross-validation to avoid overwriting cDAC data
- Use 'is null'/'is not null' instead of '== null'/'!= null'
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 1, 2026 11:55
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 77081c0 to 6ceae96CompareMarch 1, 2026 11:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

…EBUG loop bounds
- GetNumberGenerations: use gc.GetMaxGeneration()+1 via the GC contract
instead of gc.GetHeapData() which throws on Server GC targets.
- GetGenerationTable/GetFinalizationFillPointers DEBUG cross-validation:
limit comparison loops to pNeededLocal (the actual written count) instead
of the caller buffer size to avoid comparing uninitialized trailing entries.
- GetFinalizationFillPointersSvr: use Math.Min(cFillPointers, pNeededLocal)
for the same reason.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs

@max-charlambmax-charlamb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good modulo couple comments

…ally
The legacy enumeration is started and advanced unconditionally in
StartEnumMethodInstancesByAddress and EnumMethodInstanceByAddress,
but EndEnumMethodInstancesByAddress only cleaned up the legacy handle
under #if DEBUG. This would leak the legacy enumeration handle in
Release builds.
Remove the #if DEBUG guard so the cleanup runs in all builds.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 4, 2026 02:27

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

GetGenerationTable and GetFinalizationFillPointers are WKS-only APIs;
GetGenerationTableSvr and GetFinalizationFillPointersSvr are Server-only.
When called against the wrong GC mode, the native DAC returns E_FAIL,
but the cDAC was returning COR_E_INVALIDOPERATION (from the GC
contract's InvalidOperationException), which would cause a debug assert
mismatch during cross-validation.
Add explicit GC mode checks to all four methods and add 16 unit tests
validating the error behavior in both directions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@noahfalk
noahfalk merged commit 4895491 into dotnet:mainMar 4, 2026
110 of 114 checks passed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 4, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

4 participants

@noahfalk@max-charlamb@rcj1
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers - #124674

Merged
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table
Mar 4, 2026
Merged

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers#124674
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table

Conversation

@noahfalk

@noahfalknoahfalk commented Feb 20, 2026

Copy link
Copy Markdown
Member

Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:

  • GetGenerationTable / GetGenerationTableSvr
  • GetFinalizationFillPointers / GetFinalizationFillPointersSvr

Add test infrastructure:

  • TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace, accumulates types/globals, and wires contracts via TestContractRegistry.
  • GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure GC mock data via Action and build directly into the target.
  • TestContractRegistry: Dictionary<Type, Lazy> replacement for Mock in the builder path.

Add tests:

  • 3 contract-level tests in GCTests.cs (x4 arch = 12)
  • 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)

Add documentation:

  • README.md files for cdac/, Legacy/, and tests/ directories
  • Copilot instruction to search for READMEs along the path hierarchy

Add a variety of fixes for pre-existing issues I discovered trying to run the SOS tests on Linux for both Release and Debug flavors. I'm guessing those testing configurations haven't been used much:

  • Fix cDAC library loading on Unix (PAL_GetPalHostModule)
  • Fix Thread data model docs (originally I had fixed the contract impl but a parallel change already took care of it so that part)
  • Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
    request.cpp where inner HRESULT declarations masked the return value
  • Fix cDAC legacy stack walk not advancing in Release builds
    (ClrDataStackWalk.Next must call legacy outside #if DEBUG)
  • Fix Debug assertion failures: catch VirtualReadException in
    GetAppDomainName and GetObjectData, fall back to legacy in
    EnumMethodInstanceByAddress

Fixes#124692
Fixes#124693
Fixes#124694
Fixes#124695

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 implements four ISOSDacInterface8 methods in the cDAC SOSDacImpl layer for querying GC generation tables and finalization fill pointers, along with comprehensive test infrastructure and documentation.

Changes:

  • Implements GetGenerationTable, GetGenerationTableSvr, GetFinalizationFillPointers, and GetFinalizationFillPointersSvr in SOSDacImpl.cs by delegating to the existing IGC contract
  • Introduces TestPlaceholderTarget.Builder pattern with fluent API and TestContractRegistry to replace Mock<ContractRegistry> for more maintainable test setup
  • Adds GCHeapBuilder with extension methods (AddGCHeapWks/AddGCHeapSvr) to configure GC mock data declaratively
  • Provides 36 test cases total (3 contract-level + 6 SOSDacImpl-level × 4 architectures each) with proper sign-extension handling
  • Documents the cDAC architecture, test patterns, and SOSDacImpl implementation conventions in new README files

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/TestPlaceholderTarget.csRemoves Moq dependency; adds fluent Builder class and TestContractRegistry for programmatic test target construction
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.csNew file: GCHeapBuilder config class and extension methods to set up workstation/server GC heap mock data
src/native/managed/cdac/tests/GCTests.csNew file: 3 contract-level tests verifying IGC.GetHeapData() correctly reads generation table and fill pointers
src/native/managed/cdac/tests/SOSDacInterface8Tests.csNew file: 6 SOSDacImpl-level tests validating buffer sizing protocol, HResult codes, and pointer sign-extension
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements 4 ISOSDacInterface8 methods with proper validation, error handling, and debug cross-validation
src/native/managed/cdac/tests/README.mdNew documentation: test architecture, builder patterns, sign-extension gotchas, and mock descriptor guidelines
src/native/managed/cdac/README.mdNew documentation: cDAC architecture overview, project structure, and integration testing workflow
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/README.mdNew documentation: SOSDacImpl implementation patterns, HResult conventions, and buffer sizing protocol
.github/copilot-instructions.mdAdds instruction to search for README files in directory hierarchy before making changes

Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs Outdated
Comment thread.github/copilot-instructions.md
Comment threadsrc/native/managed/cdac/tests/README.md
Comment threadsrc/native/managed/cdac/README.md
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from f9fb24b to 398b718CompareFebruary 22, 2026 08:08
@noahfalk
noahfalk marked this pull request as ready for review February 22, 2026 08:18
CopilotAI review requested due to automatic review settings February 22, 2026 08:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadeng/pipelines/runtime-diagnostics.yml
CopilotAI review requested due to automatic review settings February 26, 2026 09:15
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 33c4550 to 10ab4c2CompareFebruary 26, 2026 09:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

noahfalkand others added 4 commits March 1, 2026 03:55
…izationFillPointers
Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:
- GetGenerationTable / GetGenerationTableSvr
- GetFinalizationFillPointers / GetFinalizationFillPointersSvr
Add test infrastructure:
- TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace,
accumulates types/globals, and wires contracts via TestContractRegistry.
- GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure
GC mock data via Action<GCHeapBuilder> and build directly into the target.
- TestContractRegistry: Dictionary<Type, Lazy<IContract>> replacement for
Mock<ContractRegistry> in the builder path.
Add tests:
- 3 contract-level tests in GCTests.cs (x4 arch = 12)
- 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)
Add documentation:
- README.md files for cdac/, Legacy/, and tests/ directories
- Copilot instruction to search for READMEs along the path hierarchy
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes discovered while running SOS integration tests against the new
GetGenerationTable/GetFinalizationFillPointers implementation:
- Fix cDAC library loading on Unix (PAL_GetPalHostModule)
- Fix Thread data model crash on non-Windows (TryGetValue for
UEWatsonBucketTrackerBuckets)
- Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
request.cpp where inner HRESULT declarations masked the return value
- Fix cDAC legacy stack walk not advancing in Release builds
(ClrDataStackWalk.Next must call legacy outside #if DEBUG)
- Fix Debug assertion failures: catch VirtualReadException in
GetAppDomainName and GetObjectData, fall back to legacy in
EnumMethodInstanceByAddress
- Address PR review feedback: add insufficient-buffer tests, pNeeded
cross-validation assertions, and reduce test data duplication with
shared helpers
- Update README documentation with VirtualReadException guidance and
legacy delegation placement
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…atch, use heapData.GenerationTable.Count, and use local buffers for debug cross-validation
- Change void* to DacpGenerationData* in ISOSDacInterface8 methods
- Move null/validation checks inside try/catch blocks per PR dotnet#124814 pattern
- Replace ReadGlobal<uint>(TotalGenerationCount) with heapData.GenerationTable.Count
- Use local buffers for legacy DAC cross-validation to avoid overwriting cDAC data
- Use 'is null'/'is not null' instead of '== null'/'!= null'
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 1, 2026 11:55
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 77081c0 to 6ceae96CompareMarch 1, 2026 11:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

…EBUG loop bounds
- GetNumberGenerations: use gc.GetMaxGeneration()+1 via the GC contract
instead of gc.GetHeapData() which throws on Server GC targets.
- GetGenerationTable/GetFinalizationFillPointers DEBUG cross-validation:
limit comparison loops to pNeededLocal (the actual written count) instead
of the caller buffer size to avoid comparing uninitialized trailing entries.
- GetFinalizationFillPointersSvr: use Math.Min(cFillPointers, pNeededLocal)
for the same reason.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs

@max-charlambmax-charlamb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good modulo couple comments

…ally
The legacy enumeration is started and advanced unconditionally in
StartEnumMethodInstancesByAddress and EnumMethodInstanceByAddress,
but EndEnumMethodInstancesByAddress only cleaned up the legacy handle
under #if DEBUG. This would leak the legacy enumeration handle in
Release builds.
Remove the #if DEBUG guard so the cleanup runs in all builds.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 4, 2026 02:27

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

GetGenerationTable and GetFinalizationFillPointers are WKS-only APIs;
GetGenerationTableSvr and GetFinalizationFillPointersSvr are Server-only.
When called against the wrong GC mode, the native DAC returns E_FAIL,
but the cDAC was returning COR_E_INVALIDOPERATION (from the GC
contract's InvalidOperationException), which would cause a debug assert
mismatch during cross-validation.
Add explicit GC mode checks to all four methods and add 16 unit tests
validating the error behavior in both directions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@noahfalk
noahfalk merged commit 4895491 into dotnet:mainMar 4, 2026
110 of 114 checks passed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 4, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

4 participants

@noahfalk@max-charlamb@rcj1
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers - #124674

Merged
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table
Mar 4, 2026
Merged

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers#124674
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table

Conversation

@noahfalk

@noahfalknoahfalk commented Feb 20, 2026

Copy link
Copy Markdown
Member

Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:

  • GetGenerationTable / GetGenerationTableSvr
  • GetFinalizationFillPointers / GetFinalizationFillPointersSvr

Add test infrastructure:

  • TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace, accumulates types/globals, and wires contracts via TestContractRegistry.
  • GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure GC mock data via Action and build directly into the target.
  • TestContractRegistry: Dictionary<Type, Lazy> replacement for Mock in the builder path.

Add tests:

  • 3 contract-level tests in GCTests.cs (x4 arch = 12)
  • 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)

Add documentation:

  • README.md files for cdac/, Legacy/, and tests/ directories
  • Copilot instruction to search for READMEs along the path hierarchy

Add a variety of fixes for pre-existing issues I discovered trying to run the SOS tests on Linux for both Release and Debug flavors. I'm guessing those testing configurations haven't been used much:

  • Fix cDAC library loading on Unix (PAL_GetPalHostModule)
  • Fix Thread data model docs (originally I had fixed the contract impl but a parallel change already took care of it so that part)
  • Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
    request.cpp where inner HRESULT declarations masked the return value
  • Fix cDAC legacy stack walk not advancing in Release builds
    (ClrDataStackWalk.Next must call legacy outside #if DEBUG)
  • Fix Debug assertion failures: catch VirtualReadException in
    GetAppDomainName and GetObjectData, fall back to legacy in
    EnumMethodInstanceByAddress

Fixes#124692
Fixes#124693
Fixes#124694
Fixes#124695

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 implements four ISOSDacInterface8 methods in the cDAC SOSDacImpl layer for querying GC generation tables and finalization fill pointers, along with comprehensive test infrastructure and documentation.

Changes:

  • Implements GetGenerationTable, GetGenerationTableSvr, GetFinalizationFillPointers, and GetFinalizationFillPointersSvr in SOSDacImpl.cs by delegating to the existing IGC contract
  • Introduces TestPlaceholderTarget.Builder pattern with fluent API and TestContractRegistry to replace Mock<ContractRegistry> for more maintainable test setup
  • Adds GCHeapBuilder with extension methods (AddGCHeapWks/AddGCHeapSvr) to configure GC mock data declaratively
  • Provides 36 test cases total (3 contract-level + 6 SOSDacImpl-level × 4 architectures each) with proper sign-extension handling
  • Documents the cDAC architecture, test patterns, and SOSDacImpl implementation conventions in new README files

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/TestPlaceholderTarget.csRemoves Moq dependency; adds fluent Builder class and TestContractRegistry for programmatic test target construction
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.csNew file: GCHeapBuilder config class and extension methods to set up workstation/server GC heap mock data
src/native/managed/cdac/tests/GCTests.csNew file: 3 contract-level tests verifying IGC.GetHeapData() correctly reads generation table and fill pointers
src/native/managed/cdac/tests/SOSDacInterface8Tests.csNew file: 6 SOSDacImpl-level tests validating buffer sizing protocol, HResult codes, and pointer sign-extension
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements 4 ISOSDacInterface8 methods with proper validation, error handling, and debug cross-validation
src/native/managed/cdac/tests/README.mdNew documentation: test architecture, builder patterns, sign-extension gotchas, and mock descriptor guidelines
src/native/managed/cdac/README.mdNew documentation: cDAC architecture overview, project structure, and integration testing workflow
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/README.mdNew documentation: SOSDacImpl implementation patterns, HResult conventions, and buffer sizing protocol
.github/copilot-instructions.mdAdds instruction to search for README files in directory hierarchy before making changes

Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs Outdated
Comment thread.github/copilot-instructions.md
Comment threadsrc/native/managed/cdac/tests/README.md
Comment threadsrc/native/managed/cdac/README.md
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from f9fb24b to 398b718CompareFebruary 22, 2026 08:08
@noahfalk
noahfalk marked this pull request as ready for review February 22, 2026 08:18
CopilotAI review requested due to automatic review settings February 22, 2026 08:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadeng/pipelines/runtime-diagnostics.yml
CopilotAI review requested due to automatic review settings February 26, 2026 09:15
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 33c4550 to 10ab4c2CompareFebruary 26, 2026 09:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

noahfalkand others added 4 commits March 1, 2026 03:55
…izationFillPointers
Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:
- GetGenerationTable / GetGenerationTableSvr
- GetFinalizationFillPointers / GetFinalizationFillPointersSvr
Add test infrastructure:
- TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace,
accumulates types/globals, and wires contracts via TestContractRegistry.
- GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure
GC mock data via Action<GCHeapBuilder> and build directly into the target.
- TestContractRegistry: Dictionary<Type, Lazy<IContract>> replacement for
Mock<ContractRegistry> in the builder path.
Add tests:
- 3 contract-level tests in GCTests.cs (x4 arch = 12)
- 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)
Add documentation:
- README.md files for cdac/, Legacy/, and tests/ directories
- Copilot instruction to search for READMEs along the path hierarchy
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes discovered while running SOS integration tests against the new
GetGenerationTable/GetFinalizationFillPointers implementation:
- Fix cDAC library loading on Unix (PAL_GetPalHostModule)
- Fix Thread data model crash on non-Windows (TryGetValue for
UEWatsonBucketTrackerBuckets)
- Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
request.cpp where inner HRESULT declarations masked the return value
- Fix cDAC legacy stack walk not advancing in Release builds
(ClrDataStackWalk.Next must call legacy outside #if DEBUG)
- Fix Debug assertion failures: catch VirtualReadException in
GetAppDomainName and GetObjectData, fall back to legacy in
EnumMethodInstanceByAddress
- Address PR review feedback: add insufficient-buffer tests, pNeeded
cross-validation assertions, and reduce test data duplication with
shared helpers
- Update README documentation with VirtualReadException guidance and
legacy delegation placement
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…atch, use heapData.GenerationTable.Count, and use local buffers for debug cross-validation
- Change void* to DacpGenerationData* in ISOSDacInterface8 methods
- Move null/validation checks inside try/catch blocks per PR dotnet#124814 pattern
- Replace ReadGlobal<uint>(TotalGenerationCount) with heapData.GenerationTable.Count
- Use local buffers for legacy DAC cross-validation to avoid overwriting cDAC data
- Use 'is null'/'is not null' instead of '== null'/'!= null'
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 1, 2026 11:55
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 77081c0 to 6ceae96CompareMarch 1, 2026 11:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

…EBUG loop bounds
- GetNumberGenerations: use gc.GetMaxGeneration()+1 via the GC contract
instead of gc.GetHeapData() which throws on Server GC targets.
- GetGenerationTable/GetFinalizationFillPointers DEBUG cross-validation:
limit comparison loops to pNeededLocal (the actual written count) instead
of the caller buffer size to avoid comparing uninitialized trailing entries.
- GetFinalizationFillPointersSvr: use Math.Min(cFillPointers, pNeededLocal)
for the same reason.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs

@max-charlambmax-charlamb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good modulo couple comments

…ally
The legacy enumeration is started and advanced unconditionally in
StartEnumMethodInstancesByAddress and EnumMethodInstanceByAddress,
but EndEnumMethodInstancesByAddress only cleaned up the legacy handle
under #if DEBUG. This would leak the legacy enumeration handle in
Release builds.
Remove the #if DEBUG guard so the cleanup runs in all builds.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 4, 2026 02:27

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

GetGenerationTable and GetFinalizationFillPointers are WKS-only APIs;
GetGenerationTableSvr and GetFinalizationFillPointersSvr are Server-only.
When called against the wrong GC mode, the native DAC returns E_FAIL,
but the cDAC was returning COR_E_INVALIDOPERATION (from the GC
contract's InvalidOperationException), which would cause a debug assert
mismatch during cross-validation.
Add explicit GC mode checks to all four methods and add 16 unit tests
validating the error behavior in both directions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@noahfalk
noahfalk merged commit 4895491 into dotnet:mainMar 4, 2026
110 of 114 checks passed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 4, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

4 participants

@noahfalk@max-charlamb@rcj1
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers - #124674

Merged
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table
Mar 4, 2026
Merged

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers#124674
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table

Conversation

@noahfalk

@noahfalknoahfalk commented Feb 20, 2026

Copy link
Copy Markdown
Member

Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:

  • GetGenerationTable / GetGenerationTableSvr
  • GetFinalizationFillPointers / GetFinalizationFillPointersSvr

Add test infrastructure:

  • TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace, accumulates types/globals, and wires contracts via TestContractRegistry.
  • GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure GC mock data via Action and build directly into the target.
  • TestContractRegistry: Dictionary<Type, Lazy> replacement for Mock in the builder path.

Add tests:

  • 3 contract-level tests in GCTests.cs (x4 arch = 12)
  • 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)

Add documentation:

  • README.md files for cdac/, Legacy/, and tests/ directories
  • Copilot instruction to search for READMEs along the path hierarchy

Add a variety of fixes for pre-existing issues I discovered trying to run the SOS tests on Linux for both Release and Debug flavors. I'm guessing those testing configurations haven't been used much:

  • Fix cDAC library loading on Unix (PAL_GetPalHostModule)
  • Fix Thread data model docs (originally I had fixed the contract impl but a parallel change already took care of it so that part)
  • Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
    request.cpp where inner HRESULT declarations masked the return value
  • Fix cDAC legacy stack walk not advancing in Release builds
    (ClrDataStackWalk.Next must call legacy outside #if DEBUG)
  • Fix Debug assertion failures: catch VirtualReadException in
    GetAppDomainName and GetObjectData, fall back to legacy in
    EnumMethodInstanceByAddress

Fixes#124692
Fixes#124693
Fixes#124694
Fixes#124695

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 implements four ISOSDacInterface8 methods in the cDAC SOSDacImpl layer for querying GC generation tables and finalization fill pointers, along with comprehensive test infrastructure and documentation.

Changes:

  • Implements GetGenerationTable, GetGenerationTableSvr, GetFinalizationFillPointers, and GetFinalizationFillPointersSvr in SOSDacImpl.cs by delegating to the existing IGC contract
  • Introduces TestPlaceholderTarget.Builder pattern with fluent API and TestContractRegistry to replace Mock<ContractRegistry> for more maintainable test setup
  • Adds GCHeapBuilder with extension methods (AddGCHeapWks/AddGCHeapSvr) to configure GC mock data declaratively
  • Provides 36 test cases total (3 contract-level + 6 SOSDacImpl-level × 4 architectures each) with proper sign-extension handling
  • Documents the cDAC architecture, test patterns, and SOSDacImpl implementation conventions in new README files

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/TestPlaceholderTarget.csRemoves Moq dependency; adds fluent Builder class and TestContractRegistry for programmatic test target construction
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.csNew file: GCHeapBuilder config class and extension methods to set up workstation/server GC heap mock data
src/native/managed/cdac/tests/GCTests.csNew file: 3 contract-level tests verifying IGC.GetHeapData() correctly reads generation table and fill pointers
src/native/managed/cdac/tests/SOSDacInterface8Tests.csNew file: 6 SOSDacImpl-level tests validating buffer sizing protocol, HResult codes, and pointer sign-extension
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements 4 ISOSDacInterface8 methods with proper validation, error handling, and debug cross-validation
src/native/managed/cdac/tests/README.mdNew documentation: test architecture, builder patterns, sign-extension gotchas, and mock descriptor guidelines
src/native/managed/cdac/README.mdNew documentation: cDAC architecture overview, project structure, and integration testing workflow
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/README.mdNew documentation: SOSDacImpl implementation patterns, HResult conventions, and buffer sizing protocol
.github/copilot-instructions.mdAdds instruction to search for README files in directory hierarchy before making changes

Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs Outdated
Comment thread.github/copilot-instructions.md
Comment threadsrc/native/managed/cdac/tests/README.md
Comment threadsrc/native/managed/cdac/README.md
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from f9fb24b to 398b718CompareFebruary 22, 2026 08:08
@noahfalk
noahfalk marked this pull request as ready for review February 22, 2026 08:18
CopilotAI review requested due to automatic review settings February 22, 2026 08:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadeng/pipelines/runtime-diagnostics.yml
CopilotAI review requested due to automatic review settings February 26, 2026 09:15
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 33c4550 to 10ab4c2CompareFebruary 26, 2026 09:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

noahfalkand others added 4 commits March 1, 2026 03:55
…izationFillPointers
Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:
- GetGenerationTable / GetGenerationTableSvr
- GetFinalizationFillPointers / GetFinalizationFillPointersSvr
Add test infrastructure:
- TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace,
accumulates types/globals, and wires contracts via TestContractRegistry.
- GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure
GC mock data via Action<GCHeapBuilder> and build directly into the target.
- TestContractRegistry: Dictionary<Type, Lazy<IContract>> replacement for
Mock<ContractRegistry> in the builder path.
Add tests:
- 3 contract-level tests in GCTests.cs (x4 arch = 12)
- 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)
Add documentation:
- README.md files for cdac/, Legacy/, and tests/ directories
- Copilot instruction to search for READMEs along the path hierarchy
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes discovered while running SOS integration tests against the new
GetGenerationTable/GetFinalizationFillPointers implementation:
- Fix cDAC library loading on Unix (PAL_GetPalHostModule)
- Fix Thread data model crash on non-Windows (TryGetValue for
UEWatsonBucketTrackerBuckets)
- Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
request.cpp where inner HRESULT declarations masked the return value
- Fix cDAC legacy stack walk not advancing in Release builds
(ClrDataStackWalk.Next must call legacy outside #if DEBUG)
- Fix Debug assertion failures: catch VirtualReadException in
GetAppDomainName and GetObjectData, fall back to legacy in
EnumMethodInstanceByAddress
- Address PR review feedback: add insufficient-buffer tests, pNeeded
cross-validation assertions, and reduce test data duplication with
shared helpers
- Update README documentation with VirtualReadException guidance and
legacy delegation placement
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…atch, use heapData.GenerationTable.Count, and use local buffers for debug cross-validation
- Change void* to DacpGenerationData* in ISOSDacInterface8 methods
- Move null/validation checks inside try/catch blocks per PR dotnet#124814 pattern
- Replace ReadGlobal<uint>(TotalGenerationCount) with heapData.GenerationTable.Count
- Use local buffers for legacy DAC cross-validation to avoid overwriting cDAC data
- Use 'is null'/'is not null' instead of '== null'/'!= null'
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 1, 2026 11:55
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 77081c0 to 6ceae96CompareMarch 1, 2026 11:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

…EBUG loop bounds
- GetNumberGenerations: use gc.GetMaxGeneration()+1 via the GC contract
instead of gc.GetHeapData() which throws on Server GC targets.
- GetGenerationTable/GetFinalizationFillPointers DEBUG cross-validation:
limit comparison loops to pNeededLocal (the actual written count) instead
of the caller buffer size to avoid comparing uninitialized trailing entries.
- GetFinalizationFillPointersSvr: use Math.Min(cFillPointers, pNeededLocal)
for the same reason.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs

@max-charlambmax-charlamb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good modulo couple comments

…ally
The legacy enumeration is started and advanced unconditionally in
StartEnumMethodInstancesByAddress and EnumMethodInstanceByAddress,
but EndEnumMethodInstancesByAddress only cleaned up the legacy handle
under #if DEBUG. This would leak the legacy enumeration handle in
Release builds.
Remove the #if DEBUG guard so the cleanup runs in all builds.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 4, 2026 02:27

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

GetGenerationTable and GetFinalizationFillPointers are WKS-only APIs;
GetGenerationTableSvr and GetFinalizationFillPointersSvr are Server-only.
When called against the wrong GC mode, the native DAC returns E_FAIL,
but the cDAC was returning COR_E_INVALIDOPERATION (from the GC
contract's InvalidOperationException), which would cause a debug assert
mismatch during cross-validation.
Add explicit GC mode checks to all four methods and add 16 unit tests
validating the error behavior in both directions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@noahfalk
noahfalk merged commit 4895491 into dotnet:mainMar 4, 2026
110 of 114 checks passed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 4, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

4 participants

@noahfalk@max-charlamb@rcj1
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers - #124674

Merged
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table
Mar 4, 2026
Merged

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers#124674
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table

Conversation

@noahfalk

@noahfalknoahfalk commented Feb 20, 2026

Copy link
Copy Markdown
Member

Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:

  • GetGenerationTable / GetGenerationTableSvr
  • GetFinalizationFillPointers / GetFinalizationFillPointersSvr

Add test infrastructure:

  • TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace, accumulates types/globals, and wires contracts via TestContractRegistry.
  • GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure GC mock data via Action and build directly into the target.
  • TestContractRegistry: Dictionary<Type, Lazy> replacement for Mock in the builder path.

Add tests:

  • 3 contract-level tests in GCTests.cs (x4 arch = 12)
  • 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)

Add documentation:

  • README.md files for cdac/, Legacy/, and tests/ directories
  • Copilot instruction to search for READMEs along the path hierarchy

Add a variety of fixes for pre-existing issues I discovered trying to run the SOS tests on Linux for both Release and Debug flavors. I'm guessing those testing configurations haven't been used much:

  • Fix cDAC library loading on Unix (PAL_GetPalHostModule)
  • Fix Thread data model docs (originally I had fixed the contract impl but a parallel change already took care of it so that part)
  • Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
    request.cpp where inner HRESULT declarations masked the return value
  • Fix cDAC legacy stack walk not advancing in Release builds
    (ClrDataStackWalk.Next must call legacy outside #if DEBUG)
  • Fix Debug assertion failures: catch VirtualReadException in
    GetAppDomainName and GetObjectData, fall back to legacy in
    EnumMethodInstanceByAddress

Fixes#124692
Fixes#124693
Fixes#124694
Fixes#124695

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 implements four ISOSDacInterface8 methods in the cDAC SOSDacImpl layer for querying GC generation tables and finalization fill pointers, along with comprehensive test infrastructure and documentation.

Changes:

  • Implements GetGenerationTable, GetGenerationTableSvr, GetFinalizationFillPointers, and GetFinalizationFillPointersSvr in SOSDacImpl.cs by delegating to the existing IGC contract
  • Introduces TestPlaceholderTarget.Builder pattern with fluent API and TestContractRegistry to replace Mock<ContractRegistry> for more maintainable test setup
  • Adds GCHeapBuilder with extension methods (AddGCHeapWks/AddGCHeapSvr) to configure GC mock data declaratively
  • Provides 36 test cases total (3 contract-level + 6 SOSDacImpl-level × 4 architectures each) with proper sign-extension handling
  • Documents the cDAC architecture, test patterns, and SOSDacImpl implementation conventions in new README files

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/TestPlaceholderTarget.csRemoves Moq dependency; adds fluent Builder class and TestContractRegistry for programmatic test target construction
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.csNew file: GCHeapBuilder config class and extension methods to set up workstation/server GC heap mock data
src/native/managed/cdac/tests/GCTests.csNew file: 3 contract-level tests verifying IGC.GetHeapData() correctly reads generation table and fill pointers
src/native/managed/cdac/tests/SOSDacInterface8Tests.csNew file: 6 SOSDacImpl-level tests validating buffer sizing protocol, HResult codes, and pointer sign-extension
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements 4 ISOSDacInterface8 methods with proper validation, error handling, and debug cross-validation
src/native/managed/cdac/tests/README.mdNew documentation: test architecture, builder patterns, sign-extension gotchas, and mock descriptor guidelines
src/native/managed/cdac/README.mdNew documentation: cDAC architecture overview, project structure, and integration testing workflow
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/README.mdNew documentation: SOSDacImpl implementation patterns, HResult conventions, and buffer sizing protocol
.github/copilot-instructions.mdAdds instruction to search for README files in directory hierarchy before making changes

Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs Outdated
Comment thread.github/copilot-instructions.md
Comment threadsrc/native/managed/cdac/tests/README.md
Comment threadsrc/native/managed/cdac/README.md
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from f9fb24b to 398b718CompareFebruary 22, 2026 08:08
@noahfalk
noahfalk marked this pull request as ready for review February 22, 2026 08:18
CopilotAI review requested due to automatic review settings February 22, 2026 08:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadeng/pipelines/runtime-diagnostics.yml
CopilotAI review requested due to automatic review settings February 26, 2026 09:15
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 33c4550 to 10ab4c2CompareFebruary 26, 2026 09:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

noahfalkand others added 4 commits March 1, 2026 03:55
…izationFillPointers
Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:
- GetGenerationTable / GetGenerationTableSvr
- GetFinalizationFillPointers / GetFinalizationFillPointersSvr
Add test infrastructure:
- TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace,
accumulates types/globals, and wires contracts via TestContractRegistry.
- GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure
GC mock data via Action<GCHeapBuilder> and build directly into the target.
- TestContractRegistry: Dictionary<Type, Lazy<IContract>> replacement for
Mock<ContractRegistry> in the builder path.
Add tests:
- 3 contract-level tests in GCTests.cs (x4 arch = 12)
- 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)
Add documentation:
- README.md files for cdac/, Legacy/, and tests/ directories
- Copilot instruction to search for READMEs along the path hierarchy
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes discovered while running SOS integration tests against the new
GetGenerationTable/GetFinalizationFillPointers implementation:
- Fix cDAC library loading on Unix (PAL_GetPalHostModule)
- Fix Thread data model crash on non-Windows (TryGetValue for
UEWatsonBucketTrackerBuckets)
- Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
request.cpp where inner HRESULT declarations masked the return value
- Fix cDAC legacy stack walk not advancing in Release builds
(ClrDataStackWalk.Next must call legacy outside #if DEBUG)
- Fix Debug assertion failures: catch VirtualReadException in
GetAppDomainName and GetObjectData, fall back to legacy in
EnumMethodInstanceByAddress
- Address PR review feedback: add insufficient-buffer tests, pNeeded
cross-validation assertions, and reduce test data duplication with
shared helpers
- Update README documentation with VirtualReadException guidance and
legacy delegation placement
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…atch, use heapData.GenerationTable.Count, and use local buffers for debug cross-validation
- Change void* to DacpGenerationData* in ISOSDacInterface8 methods
- Move null/validation checks inside try/catch blocks per PR dotnet#124814 pattern
- Replace ReadGlobal<uint>(TotalGenerationCount) with heapData.GenerationTable.Count
- Use local buffers for legacy DAC cross-validation to avoid overwriting cDAC data
- Use 'is null'/'is not null' instead of '== null'/'!= null'
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 1, 2026 11:55
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 77081c0 to 6ceae96CompareMarch 1, 2026 11:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

…EBUG loop bounds
- GetNumberGenerations: use gc.GetMaxGeneration()+1 via the GC contract
instead of gc.GetHeapData() which throws on Server GC targets.
- GetGenerationTable/GetFinalizationFillPointers DEBUG cross-validation:
limit comparison loops to pNeededLocal (the actual written count) instead
of the caller buffer size to avoid comparing uninitialized trailing entries.
- GetFinalizationFillPointersSvr: use Math.Min(cFillPointers, pNeededLocal)
for the same reason.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs

@max-charlambmax-charlamb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good modulo couple comments

…ally
The legacy enumeration is started and advanced unconditionally in
StartEnumMethodInstancesByAddress and EnumMethodInstanceByAddress,
but EndEnumMethodInstancesByAddress only cleaned up the legacy handle
under #if DEBUG. This would leak the legacy enumeration handle in
Release builds.
Remove the #if DEBUG guard so the cleanup runs in all builds.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 4, 2026 02:27

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

GetGenerationTable and GetFinalizationFillPointers are WKS-only APIs;
GetGenerationTableSvr and GetFinalizationFillPointersSvr are Server-only.
When called against the wrong GC mode, the native DAC returns E_FAIL,
but the cDAC was returning COR_E_INVALIDOPERATION (from the GC
contract's InvalidOperationException), which would cause a debug assert
mismatch during cross-validation.
Add explicit GC mode checks to all four methods and add 16 unit tests
validating the error behavior in both directions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@noahfalk
noahfalk merged commit 4895491 into dotnet:mainMar 4, 2026
110 of 114 checks passed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 4, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

4 participants

@noahfalk@max-charlamb@rcj1
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers - #124674

Merged
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table
Mar 4, 2026
Merged

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers#124674
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table

Conversation

@noahfalk

@noahfalknoahfalk commented Feb 20, 2026

Copy link
Copy Markdown
Member

Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:

  • GetGenerationTable / GetGenerationTableSvr
  • GetFinalizationFillPointers / GetFinalizationFillPointersSvr

Add test infrastructure:

  • TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace, accumulates types/globals, and wires contracts via TestContractRegistry.
  • GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure GC mock data via Action and build directly into the target.
  • TestContractRegistry: Dictionary<Type, Lazy> replacement for Mock in the builder path.

Add tests:

  • 3 contract-level tests in GCTests.cs (x4 arch = 12)
  • 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)

Add documentation:

  • README.md files for cdac/, Legacy/, and tests/ directories
  • Copilot instruction to search for READMEs along the path hierarchy

Add a variety of fixes for pre-existing issues I discovered trying to run the SOS tests on Linux for both Release and Debug flavors. I'm guessing those testing configurations haven't been used much:

  • Fix cDAC library loading on Unix (PAL_GetPalHostModule)
  • Fix Thread data model docs (originally I had fixed the contract impl but a parallel change already took care of it so that part)
  • Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
    request.cpp where inner HRESULT declarations masked the return value
  • Fix cDAC legacy stack walk not advancing in Release builds
    (ClrDataStackWalk.Next must call legacy outside #if DEBUG)
  • Fix Debug assertion failures: catch VirtualReadException in
    GetAppDomainName and GetObjectData, fall back to legacy in
    EnumMethodInstanceByAddress

Fixes#124692
Fixes#124693
Fixes#124694
Fixes#124695

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 implements four ISOSDacInterface8 methods in the cDAC SOSDacImpl layer for querying GC generation tables and finalization fill pointers, along with comprehensive test infrastructure and documentation.

Changes:

  • Implements GetGenerationTable, GetGenerationTableSvr, GetFinalizationFillPointers, and GetFinalizationFillPointersSvr in SOSDacImpl.cs by delegating to the existing IGC contract
  • Introduces TestPlaceholderTarget.Builder pattern with fluent API and TestContractRegistry to replace Mock<ContractRegistry> for more maintainable test setup
  • Adds GCHeapBuilder with extension methods (AddGCHeapWks/AddGCHeapSvr) to configure GC mock data declaratively
  • Provides 36 test cases total (3 contract-level + 6 SOSDacImpl-level × 4 architectures each) with proper sign-extension handling
  • Documents the cDAC architecture, test patterns, and SOSDacImpl implementation conventions in new README files

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/TestPlaceholderTarget.csRemoves Moq dependency; adds fluent Builder class and TestContractRegistry for programmatic test target construction
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.csNew file: GCHeapBuilder config class and extension methods to set up workstation/server GC heap mock data
src/native/managed/cdac/tests/GCTests.csNew file: 3 contract-level tests verifying IGC.GetHeapData() correctly reads generation table and fill pointers
src/native/managed/cdac/tests/SOSDacInterface8Tests.csNew file: 6 SOSDacImpl-level tests validating buffer sizing protocol, HResult codes, and pointer sign-extension
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements 4 ISOSDacInterface8 methods with proper validation, error handling, and debug cross-validation
src/native/managed/cdac/tests/README.mdNew documentation: test architecture, builder patterns, sign-extension gotchas, and mock descriptor guidelines
src/native/managed/cdac/README.mdNew documentation: cDAC architecture overview, project structure, and integration testing workflow
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/README.mdNew documentation: SOSDacImpl implementation patterns, HResult conventions, and buffer sizing protocol
.github/copilot-instructions.mdAdds instruction to search for README files in directory hierarchy before making changes

Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs Outdated
Comment thread.github/copilot-instructions.md
Comment threadsrc/native/managed/cdac/tests/README.md
Comment threadsrc/native/managed/cdac/README.md
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from f9fb24b to 398b718CompareFebruary 22, 2026 08:08
@noahfalk
noahfalk marked this pull request as ready for review February 22, 2026 08:18
CopilotAI review requested due to automatic review settings February 22, 2026 08:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadeng/pipelines/runtime-diagnostics.yml
CopilotAI review requested due to automatic review settings February 26, 2026 09:15
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 33c4550 to 10ab4c2CompareFebruary 26, 2026 09:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

noahfalkand others added 4 commits March 1, 2026 03:55
…izationFillPointers
Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:
- GetGenerationTable / GetGenerationTableSvr
- GetFinalizationFillPointers / GetFinalizationFillPointersSvr
Add test infrastructure:
- TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace,
accumulates types/globals, and wires contracts via TestContractRegistry.
- GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure
GC mock data via Action<GCHeapBuilder> and build directly into the target.
- TestContractRegistry: Dictionary<Type, Lazy<IContract>> replacement for
Mock<ContractRegistry> in the builder path.
Add tests:
- 3 contract-level tests in GCTests.cs (x4 arch = 12)
- 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)
Add documentation:
- README.md files for cdac/, Legacy/, and tests/ directories
- Copilot instruction to search for READMEs along the path hierarchy
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes discovered while running SOS integration tests against the new
GetGenerationTable/GetFinalizationFillPointers implementation:
- Fix cDAC library loading on Unix (PAL_GetPalHostModule)
- Fix Thread data model crash on non-Windows (TryGetValue for
UEWatsonBucketTrackerBuckets)
- Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
request.cpp where inner HRESULT declarations masked the return value
- Fix cDAC legacy stack walk not advancing in Release builds
(ClrDataStackWalk.Next must call legacy outside #if DEBUG)
- Fix Debug assertion failures: catch VirtualReadException in
GetAppDomainName and GetObjectData, fall back to legacy in
EnumMethodInstanceByAddress
- Address PR review feedback: add insufficient-buffer tests, pNeeded
cross-validation assertions, and reduce test data duplication with
shared helpers
- Update README documentation with VirtualReadException guidance and
legacy delegation placement
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…atch, use heapData.GenerationTable.Count, and use local buffers for debug cross-validation
- Change void* to DacpGenerationData* in ISOSDacInterface8 methods
- Move null/validation checks inside try/catch blocks per PR dotnet#124814 pattern
- Replace ReadGlobal<uint>(TotalGenerationCount) with heapData.GenerationTable.Count
- Use local buffers for legacy DAC cross-validation to avoid overwriting cDAC data
- Use 'is null'/'is not null' instead of '== null'/'!= null'
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 1, 2026 11:55
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 77081c0 to 6ceae96CompareMarch 1, 2026 11:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

…EBUG loop bounds
- GetNumberGenerations: use gc.GetMaxGeneration()+1 via the GC contract
instead of gc.GetHeapData() which throws on Server GC targets.
- GetGenerationTable/GetFinalizationFillPointers DEBUG cross-validation:
limit comparison loops to pNeededLocal (the actual written count) instead
of the caller buffer size to avoid comparing uninitialized trailing entries.
- GetFinalizationFillPointersSvr: use Math.Min(cFillPointers, pNeededLocal)
for the same reason.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs

@max-charlambmax-charlamb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good modulo couple comments

…ally
The legacy enumeration is started and advanced unconditionally in
StartEnumMethodInstancesByAddress and EnumMethodInstanceByAddress,
but EndEnumMethodInstancesByAddress only cleaned up the legacy handle
under #if DEBUG. This would leak the legacy enumeration handle in
Release builds.
Remove the #if DEBUG guard so the cleanup runs in all builds.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 4, 2026 02:27

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

GetGenerationTable and GetFinalizationFillPointers are WKS-only APIs;
GetGenerationTableSvr and GetFinalizationFillPointersSvr are Server-only.
When called against the wrong GC mode, the native DAC returns E_FAIL,
but the cDAC was returning COR_E_INVALIDOPERATION (from the GC
contract's InvalidOperationException), which would cause a debug assert
mismatch during cross-validation.
Add explicit GC mode checks to all four methods and add 16 unit tests
validating the error behavior in both directions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@noahfalk
noahfalk merged commit 4895491 into dotnet:mainMar 4, 2026
110 of 114 checks passed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 4, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

4 participants

@noahfalk@max-charlamb@rcj1
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers - #124674

Merged
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table
Mar 4, 2026
Merged

Add cDAC tests and implementation for GetGenerationTable and GetFinalizationFillPointers#124674
noahfalk merged 7 commits into
dotnet:mainfrom
noahfalk:124583_cdac_gc_generation_table

Conversation

@noahfalk

@noahfalknoahfalk commented Feb 20, 2026

Copy link
Copy Markdown
Member

Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:

  • GetGenerationTable / GetGenerationTableSvr
  • GetFinalizationFillPointers / GetFinalizationFillPointersSvr

Add test infrastructure:

  • TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace, accumulates types/globals, and wires contracts via TestContractRegistry.
  • GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure GC mock data via Action and build directly into the target.
  • TestContractRegistry: Dictionary<Type, Lazy> replacement for Mock in the builder path.

Add tests:

  • 3 contract-level tests in GCTests.cs (x4 arch = 12)
  • 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)

Add documentation:

  • README.md files for cdac/, Legacy/, and tests/ directories
  • Copilot instruction to search for READMEs along the path hierarchy

Add a variety of fixes for pre-existing issues I discovered trying to run the SOS tests on Linux for both Release and Debug flavors. I'm guessing those testing configurations haven't been used much:

  • Fix cDAC library loading on Unix (PAL_GetPalHostModule)
  • Fix Thread data model docs (originally I had fixed the contract impl but a parallel change already took care of it so that part)
  • Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
    request.cpp where inner HRESULT declarations masked the return value
  • Fix cDAC legacy stack walk not advancing in Release builds
    (ClrDataStackWalk.Next must call legacy outside #if DEBUG)
  • Fix Debug assertion failures: catch VirtualReadException in
    GetAppDomainName and GetObjectData, fall back to legacy in
    EnumMethodInstanceByAddress

Fixes#124692
Fixes#124693
Fixes#124694
Fixes#124695

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 implements four ISOSDacInterface8 methods in the cDAC SOSDacImpl layer for querying GC generation tables and finalization fill pointers, along with comprehensive test infrastructure and documentation.

Changes:

  • Implements GetGenerationTable, GetGenerationTableSvr, GetFinalizationFillPointers, and GetFinalizationFillPointersSvr in SOSDacImpl.cs by delegating to the existing IGC contract
  • Introduces TestPlaceholderTarget.Builder pattern with fluent API and TestContractRegistry to replace Mock<ContractRegistry> for more maintainable test setup
  • Adds GCHeapBuilder with extension methods (AddGCHeapWks/AddGCHeapSvr) to configure GC mock data declaratively
  • Provides 36 test cases total (3 contract-level + 6 SOSDacImpl-level × 4 architectures each) with proper sign-extension handling
  • Documents the cDAC architecture, test patterns, and SOSDacImpl implementation conventions in new README files

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/TestPlaceholderTarget.csRemoves Moq dependency; adds fluent Builder class and TestContractRegistry for programmatic test target construction
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.csNew file: GCHeapBuilder config class and extension methods to set up workstation/server GC heap mock data
src/native/managed/cdac/tests/GCTests.csNew file: 3 contract-level tests verifying IGC.GetHeapData() correctly reads generation table and fill pointers
src/native/managed/cdac/tests/SOSDacInterface8Tests.csNew file: 6 SOSDacImpl-level tests validating buffer sizing protocol, HResult codes, and pointer sign-extension
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements 4 ISOSDacInterface8 methods with proper validation, error handling, and debug cross-validation
src/native/managed/cdac/tests/README.mdNew documentation: test architecture, builder patterns, sign-extension gotchas, and mock descriptor guidelines
src/native/managed/cdac/README.mdNew documentation: cDAC architecture overview, project structure, and integration testing workflow
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/README.mdNew documentation: SOSDacImpl implementation patterns, HResult conventions, and buffer sizing protocol
.github/copilot-instructions.mdAdds instruction to search for README files in directory hierarchy before making changes

Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs Outdated
Comment thread.github/copilot-instructions.md
Comment threadsrc/native/managed/cdac/tests/README.md
Comment threadsrc/native/managed/cdac/README.md
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from f9fb24b to 398b718CompareFebruary 22, 2026 08:08
@noahfalk
noahfalk marked this pull request as ready for review February 22, 2026 08:18
CopilotAI review requested due to automatic review settings February 22, 2026 08:18

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadsrc/native/managed/cdac/tests/MockDescriptors/MockDescriptors.GC.cs Outdated
Comment threadeng/pipelines/runtime-diagnostics.yml
CopilotAI review requested due to automatic review settings February 26, 2026 09:15
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 33c4550 to 10ab4c2CompareFebruary 26, 2026 09:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

noahfalkand others added 4 commits March 1, 2026 03:55
…izationFillPointers
Implement the four ISOSDacInterface8 methods in cDAC SOSDacImpl:
- GetGenerationTable / GetGenerationTableSvr
- GetFinalizationFillPointers / GetFinalizationFillPointersSvr
Add test infrastructure:
- TestPlaceholderTarget.Builder: fluent builder that owns MockMemorySpace,
accumulates types/globals, and wires contracts via TestContractRegistry.
- GCHeapBuilder + extension methods (AddGCHeapWks/AddGCHeapSvr): configure
GC mock data via Action<GCHeapBuilder> and build directly into the target.
- TestContractRegistry: Dictionary<Type, Lazy<IContract>> replacement for
Mock<ContractRegistry> in the builder path.
Add tests:
- 3 contract-level tests in GCTests.cs (x4 arch = 12)
- 6 SOSDacImpl-level tests in SOSDacInterface8Tests.cs (x4 arch = 24)
Add documentation:
- README.md files for cdac/, Legacy/, and tests/ directories
- Copilot instruction to search for READMEs along the path hierarchy
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes discovered while running SOS integration tests against the new
GetGenerationTable/GetFinalizationFillPointers implementation:
- Fix cDAC library loading on Unix (PAL_GetPalHostModule)
- Fix Thread data model crash on non-Windows (TryGetValue for
UEWatsonBucketTrackerBuckets)
- Fix HRESULT variable shadowing in 8 SOSDacInterface APIs in
request.cpp where inner HRESULT declarations masked the return value
- Fix cDAC legacy stack walk not advancing in Release builds
(ClrDataStackWalk.Next must call legacy outside #if DEBUG)
- Fix Debug assertion failures: catch VirtualReadException in
GetAppDomainName and GetObjectData, fall back to legacy in
EnumMethodInstanceByAddress
- Address PR review feedback: add insufficient-buffer tests, pNeeded
cross-validation assertions, and reduce test data duplication with
shared helpers
- Update README documentation with VirtualReadException guidance and
legacy delegation placement
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…atch, use heapData.GenerationTable.Count, and use local buffers for debug cross-validation
- Change void* to DacpGenerationData* in ISOSDacInterface8 methods
- Move null/validation checks inside try/catch blocks per PR dotnet#124814 pattern
- Replace ReadGlobal<uint>(TotalGenerationCount) with heapData.GenerationTable.Count
- Use local buffers for legacy DAC cross-validation to avoid overwriting cDAC data
- Use 'is null'/'is not null' instead of '== null'/'!= null'
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 1, 2026 11:55
@noahfalk
noahfalkforce-pushed the 124583_cdac_gc_generation_table branch from 77081c0 to 6ceae96CompareMarch 1, 2026 11:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

…EBUG loop bounds
- GetNumberGenerations: use gc.GetMaxGeneration()+1 via the GC contract
instead of gc.GetHeapData() which throws on Server GC targets.
- GetGenerationTable/GetFinalizationFillPointers DEBUG cross-validation:
limit comparison loops to pNeededLocal (the actual written count) instead
of the caller buffer size to avoid comparing uninitialized trailing entries.
- GetFinalizationFillPointersSvr: use Math.Min(cFillPointers, pNeededLocal)
for the same reason.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment threadsrc/native/managed/cdac/tests/SOSDacInterface8Tests.cs

@max-charlambmax-charlamb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good modulo couple comments

…ally
The legacy enumeration is started and advanced unconditionally in
StartEnumMethodInstancesByAddress and EnumMethodInstanceByAddress,
but EndEnumMethodInstancesByAddress only cleaned up the legacy handle
under #if DEBUG. This would leak the legacy enumeration handle in
Release builds.
Remove the #if DEBUG guard so the cleanup runs in all builds.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings March 4, 2026 02:27

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

GetGenerationTable and GetFinalizationFillPointers are WKS-only APIs;
GetGenerationTableSvr and GetFinalizationFillPointersSvr are Server-only.
When called against the wrong GC mode, the native DAC returns E_FAIL,
but the cDAC was returning COR_E_INVALIDOPERATION (from the GC
contract's InvalidOperationException), which would cause a debug assert
mismatch during cross-validation.
Add explicit GC mode checks to all four methods and add 16 unit tests
validating the error behavior in both directions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@noahfalk
noahfalk merged commit 4895491 into dotnet:mainMar 4, 2026
110 of 114 checks passed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 4, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

4 participants

@noahfalk@max-charlamb@rcj1