Skip to content

[cDAC] Strip collectible tag bit from RangeSectionFragment.Next pointer#124929

Merged
max-charlamb merged 2 commits intodotnet:mainfrom
max-charlamb:cdac-rangesectionmap-fix
Feb 27, 2026
Merged

[cDAC] Strip collectible tag bit from RangeSectionFragment.Next pointer#124929
max-charlamb merged 2 commits intodotnet:mainfrom
max-charlamb:cdac-rangesectionmap-fix

Conversation

@max-charlamb
Copy link
Member

Summary

The native runtime's RangeSectionFragmentPointer (codeman.h) uses bit 0 of the Next pointer as a collectible flag for fragments belonging to collectible assembly load contexts. The cDAC was reading the raw tagged pointer without stripping this bit, causing reads at a misaligned address that produced garbage field values and VirtualReadException during stack walks.

Root Cause

In the native runtime, RangeSectionFragment nodes are linked via RangeSectionFragmentPointer, which stores a tagged pointer where bit 0 indicates whether the fragment belongs to a collectible ALC. The native code's VolatileLoadWithoutBarrier strips this bit before returning the fragment address. The cDAC's Data.RangeSectionFragment class was reading the raw Next field without stripping the bit. When following a linked list of fragments, the cDAC used the tagged address (off by 1 byte), causing all subsequent field reads to return garbage data.

Note: The cDAC already correctly strips the tag from interior map pointers via InteriorMapValue.Address and from the initial fragment pointer in FindFragment. Only the RangeSectionFragment.Next linked-list pointer was missed.

Fix

Strip bit 0 from the Next pointer when reading it in the RangeSectionFragment constructor:

Next = target.ReadPointer(address + (ulong)type.Fields[nameof(Next)].Offset) & ~1ul;

Testing

  • Added GetMethodDesc_CollectibleFragmentNext regression test that creates a two-fragment chain where the head fragment's Next has the collectible tag bit set. The test fails without the fix (VirtualReadException) and passes with it.
  • All 876 existing cDAC unit tests pass.
  • Verified against a CI dump from a failing SOS.DualRuntimes test: all 15 threads now walk to completion.

Documentation

  • Updated docs/design/datacontracts/ExecutionManager.md to document the tagged pointer convention on RangeSectionFragment.Next and added a "Tagged pointers in the range section map" subsection.

The native runtime's RangeSectionFragmentPointer (codeman.h) uses bit 0 of
the Next pointer as a collectible flag for fragments belonging to collectible
assembly load contexts. The cDAC was reading the raw tagged pointer without
stripping this bit, causing reads at a misaligned address that produced
garbage field values and VirtualReadException during stack walks.

The fix masks off bit 0 when reading the Next field, matching the behavior
of InteriorMapValue.Address which already strips the tag for interior map
pointers.

Added a regression test that chains two fragments where the head's Next has
the collectible bit set. Updated ExecutionManager.md to document the tagged
pointer convention.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings February 26, 2026 23:05
@dotnet-policy-service
Copy link
Contributor

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes cDAC stack-walk failures caused by following a tagged RangeSectionFragment.Next pointer without clearing bit 0 (collectible ALC flag), aligning cDAC behavior with the runtime’s RangeSectionFragmentPointer semantics.

Changes:

  • Mask off bit 0 when reading RangeSectionFragment.Next in the cDAC data contract.
  • Add a regression test that constructs a two-fragment chain where the head fragment’s Next pointer is tagged.
  • Document the tagged-pointer convention for RangeSectionFragment.Next in the ExecutionManager design doc.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/RangeSectionFragment.cs Strips the collectible tag bit from Next when materializing RangeSectionFragment.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.ExecutionManager.cs Adds helpers to create an unmapped tail fragment and a head fragment with tagged Next for testing.
src/native/managed/cdac/tests/ExecutionManager/ExecutionManagerTests.cs Adds GetMethodDesc_CollectibleFragmentNext regression test to validate traversal with tagged Next.
docs/design/datacontracts/ExecutionManager.md Updates contract documentation to describe the tagged-pointer convention for fragment linkage.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings February 26, 2026 23:16
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

@hoyosjs
Copy link
Member

hoyosjs commented Feb 26, 2026

Only question is if Next exposes the untagged ptr but our contract md says it's tagged, Do we expect Microsoft.Diagnostics.DataContractReader.Contracts to expose the data exactly ?

@noahfalk
Copy link
Member

our contract md says it's tagged, Do we expect Microsoft.Diagnostics.DataContractReader.Contracts to expose the data exactly ?

If the contract says the runtime field is tagged I'd consider that an accurate description of the data. If the contract hypothetically has an API that returns the field then I'd expect the doc on that API to clarify whether it is returning the tagged or untagged value.

@max-charlamb
Copy link
Member Author

/ba-g cDAC only change should not impact runtime tests

@max-charlamb max-charlamb merged commit 3176283 into dotnet:main Feb 27, 2026
54 of 58 checks passed
@max-charlamb max-charlamb deleted the cdac-rangesectionmap-fix branch February 27, 2026 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants