[cDAC] Strip collectible tag bit from RangeSectionFragment.Next pointer#124929
[cDAC] Strip collectible tag bit from RangeSectionFragment.Next pointer#124929max-charlamb merged 2 commits intodotnet:mainfrom
Conversation
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>
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
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.Nextin the cDAC data contract. - Add a regression test that constructs a two-fragment chain where the head fragment’s
Nextpointer is tagged. - Document the tagged-pointer convention for
RangeSectionFragment.Nextin 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. |
src/native/managed/cdac/tests/ExecutionManager/ExecutionManagerTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.ExecutionManager.cs
Show resolved
Hide resolved
...managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/RangeSectionFragment.cs
Show resolved
Hide resolved
|
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 ? |
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. |
|
/ba-g cDAC only change should not impact runtime tests |
Summary
The native runtime's
RangeSectionFragmentPointer(codeman.h) uses bit 0 of theNextpointer 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 andVirtualReadExceptionduring stack walks.Root Cause
In the native runtime,
RangeSectionFragmentnodes are linked viaRangeSectionFragmentPointer, which stores a tagged pointer where bit 0 indicates whether the fragment belongs to a collectible ALC. The native code'sVolatileLoadWithoutBarrierstrips this bit before returning the fragment address. The cDAC'sData.RangeSectionFragmentclass was reading the rawNextfield 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.Addressand from the initial fragment pointer inFindFragment. Only theRangeSectionFragment.Nextlinked-list pointer was missed.Fix
Strip bit 0 from the
Nextpointer when reading it in theRangeSectionFragmentconstructor:Testing
GetMethodDesc_CollectibleFragmentNextregression test that creates a two-fragment chain where the head fragment'sNexthas the collectible tag bit set. The test fails without the fix (VirtualReadException) and passes with it.SOS.DualRuntimestest: all 15 threads now walk to completion.Documentation
docs/design/datacontracts/ExecutionManager.mdto document the tagged pointer convention onRangeSectionFragment.Nextand added a "Tagged pointers in the range section map" subsection.