[cDAC] GetCodeHeaderData fork#120275

max-charlamb · 2025-09-30T21:05:25Z

fork of #119609 with GCInfo decoder

Adding GetCodeHeaderData cDAC API
Implementing unwind data size on platforms other than x86 in helper class
required additions to hot/cold lookup helpers
Implements IGCInfo contract to decode GCInfo
- Currently supports x64, arm64, and arm. I plan to move the existing x86 GCInfo decoding to this new contract, but want to make that a separate PR.

dotnet-policy-service · 2025-09-30T21:06:35Z

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

Copilot

Pull Request Overview

This PR implements the GetCodeHeaderData method and adds GC info decoder functionality to the contract DAC (cDAC) system. It introduces a new GCInfo contract with platform-specific implementations for decoding garbage collection metadata from native code.

Key Changes:

Implements GetCodeHeaderData method with proper type safety and error handling
Adds a comprehensive GCInfo contract with decoder capabilities for AMD64, ARM64, and ARM architectures
Extends ExecutionManager with new methods for method region info, JIT type detection, and method descriptor lookup

Reviewed Changes

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

Show a summary per file

File	Description
SOSDacImpl.cs	Implements GetCodeHeaderData with GCInfo decoder integration and debug assertions
ISOSDacInterface.cs	Updates interface signature and adds supporting data structures
CachingContractRegistry.cs	Registers the new GCInfo contract factory
GCInfo contract files	Complete GCInfo decoder implementation with platform-specific traits
ExecutionManager files	Extends with method region info, JIT type detection, and entrypoint resolution
Data structure files	Adds UnwindInfo, PortableEntryPoint, and related helper classes

Comments suppressed due to low confidence (1)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/PlatformTraits/IGCInfoTraits.cs:1

Corrected spelling of 'ENCBACE' to 'ENCBASE'.

// Licensed to the .NET Foundation under one or more agreements.

...ed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/GCInfoDecoder.cs

....Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/PlatformTraits/ARMGCInfoTraits.cs

...iagnostics.DataContractReader.Contracts/Contracts/GCInfo/PlatformTraits/ARM64GCInfoTraits.cs

...iagnostics.DataContractReader.Contracts/Contracts/GCInfo/PlatformTraits/AMD64GCInfoTraits.cs

Copilot

Pull Request Overview

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

....Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/FrameIterator.cs

...ractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.ReadyToRunJitManager.cs

davidwrighton · 2025-10-09T16:47:11Z

docs/design/datacontracts/GCInfo.md

+
+## Implementation
+
+The GCInfo contract has platform specific implementations as GCInfo differs per architecture. With the exception of x86, all platforms have a common encoding scheme with different encoding lengths and normalization functions for data. x86 uses an entirely different scheme which is not currently supported by this contract.


How do we handle the interpreter. It also uses the GC info.

Mirroring how the runtime handles this case, I believe it is acceptable to have a different entrypoint to decode the 'standard platform' code and the interpreter code.

IGCInfoHandle IGCInfo.DecodeGCInfo(TargetPointer gcInfoAddress, uint gcVersion)

IGCInfoHandle IGCInfo.DecodeInterpreterGCInfo(TargetPointer gcInfoAddress, uint gcVersion)

I don't believe this can be encapsulated in the current format as the ExecutionManager calls into the GCInfo contract to find the method region info. If the GCInfo contract differentiated between interpreted and jitted code, then it would need to call into ExecutionManager creating a cyclic dependency.

am11 · 2025-10-21T11:13:06Z

...iagnostics.DataContractReader.Contracts/Contracts/GCInfo/PlatformTraits/AMD64GCInfoTraits.cs

Shall we use similar arch-specific structure as: src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/{arch}/{arch}Unwinder.cs or bring that to this (single directory) plan?

I don't have a strong feeling either way. I didn't make /{arch}/ subdirectories here because each platform only has a single file. In the future we may have versions of traits and multiple files. I think it would make sense to have specific directories.

Copilot

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.

Copilot · 2026-02-17T22:47:12Z

docs/design/datacontracts/GCInfo.md

+| Operation | Normalization (Encode) | Denormalization (Decode) |
+| --- | --- | --- |
+| **Stack Base Register** | `reg ^ 0x29` | `reg ^ 0x29` |
+| **Code Length** | `length << 2` | `length >> 2` |
+| **Code Offset** | `offset << 2` | `offset >> 2` |
+| **Stack Slot** | `offset >> 3` | `offset << 3` |
+| **Stack Area Size** | `size >> 3` | `size << 3` |


The ARM64 Normalization/Denormalization table has swapped column values for Code Length, Code Offset, Stack Slot, and Stack Area Size.

For Code Length and Code Offset: Normalization should be >> 2 (shift right), Denormalization should be << 2 (shift left).
For Stack Slot and Stack Area Size: Normalization should be >> 3 (shift right), Denormalization should be << 3 (shift left).

The current table shows the operations reversed. The implementation in ARM64GCInfoTraits.cs is correct and matches the native code in src/coreclr/inc/gcinfotypes.h (lines 689-690, 686-687).

Copilot · 2026-02-17T22:47:13Z

...ed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/GCInfoDecoder.cs

+    #region Helper Methods
+
+    private static uint CeilOfLog2(ulong value)
+    {


The CeilOfLog2 method doesn't handle the edge case where value is 0. When value is 0, Math.Log2(0) returns negative infinity, which causes Math.Ceiling to also return negative infinity, and casting to uint is undefined behavior.

This could occur if _codeLength is 0 when DecodeSafePoints is called. Consider adding a check: if (value == 0) return 0; at the beginning of the CeilOfLog2 method, or handle the zero case in DecodeSafePoints before calling CeilOfLog2.

Suggested change

{

{

if (value == 0)

return 0;

Copilot · 2026-02-17T22:47:13Z

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs

+                data->JITType = JitTypes.TYPE_UNKNOWN;
+                data->GCInfo = 0;
+                data->MethodStart = 0;
+                data->MethodSize = 0;


In the non-code-block path, data->HotRegionSize and data->ColdRegionSize are not initialized. Only ColdRegionStart is set to 0. For consistency with the else branch where all three region-related fields are set, these should also be explicitly initialized to 0 in this path.

Suggested change

data->MethodSize = 0;

data->MethodSize = 0;

data->HotRegionSize = 0;

data->ColdRegionSize = 0;

Copilot · 2026-02-17T22:47:13Z

...ed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/GCInfoDecoder.cs

+            Debug.Assert(rangeStartOffset < rangeStopOffset);
+            Debug.Assert(rangeStartOffset >= prevEndOffset);
+
+            lastInterruptibleRangeStopOffsetNormalized = rangeStopOffsetNormalized;


The variable prevEndOffset is declared and used in a Debug.Assert (line 307) but is never updated in the loop. It remains 0 throughout. Based on the assertion logic checking that rangeStartOffset >= prevEndOffset, it appears this should be updated at the end of each iteration to track the previous range's end offset. Consider adding: prevEndOffset = rangeStopOffset; after line 309.

Suggested change

lastInterruptibleRangeStopOffsetNormalized = rangeStopOffsetNormalized;

lastInterruptibleRangeStopOffsetNormalized = rangeStopOffsetNormalized;

prevEndOffset = rangeStopOffset;

Copilot · 2026-02-17T22:47:14Z

...ative/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs

+    /// Gets an instance of the GCInfo contract for the target.
+    /// </summary>
+    public virtual IGCInfo GCInfo => GetContract<IGCInfo>();
+    /// </summary>


There's an extra closing XML doc comment tag /// </summary> on line 89 that should be removed. This creates a malformed XML documentation structure where the GCInfo property summary is followed by an orphaned closing tag.

jkotas · 2026-02-17T23:19:42Z

docs/design/datacontracts/ExecutionManager.md

+| `UnwindInfo` | `CountOfUnwindCodes` | Number of unwind codes in the unwind info. Only exists on some platforms |
+| `UnwindInfo` | `UnwindCodeOffset` | Offset of UnwindCodeOffset in the UnwindInfo data struct. Only exists on some platforms |


Suggested change

| `UnwindInfo` | `CountOfUnwindCodes` | Number of unwind codes in the unwind info. Only exists on some platforms |

| `UnwindInfo` | `UnwindCodeOffset` | Offset of UnwindCodeOffset in the UnwindInfo data struct. Only exists on some platforms |

This is UnwindInfo blob encoding. I do not think it makes sense to have abstracted offsets for parts of it.

I think the markdown just needs to be re-synced - I don't see any of these new UnwindInfo fields used in the actual implementation:
https://github.com/dotnet/runtime/pull/120275/changes#diff-1bb05a509581b4e0bb5620c1db064b28263ee278ba395861ecab4f96130de85bR622-R627

rcj1 and others added 5 commits September 11, 2025 17:11

adding GetCodeHeaderData cDAC API

2f904c3

fix

db6229a

code review

5e591f1

Merge branch 'main' into GetCodeHeaderData

41a9283

reduce diff

5183860

github-actions bot added the area-Diagnostics-coreclr label Sep 30, 2025

dotnet-policy-service bot assigned max-charlamb Sep 30, 2025

max-charlamb added 10 commits September 30, 2025 17:07

reduce diff

a005de0

reduce diff

3d951ad

add gcInfo

5e12360

Merge remote-tracking branch 'origin/main' into cdac-codeheaderdata

e099d99

add gcinfo contract

d5e403f

fix AMD64 traits

58944f4

fix decoder

4e1bbc3

put helpers under a namespace

6b0dea4

add docs

216586d

fix

4ffe7b0

max-charlamb marked this pull request as ready for review October 2, 2025 21:35

max-charlamb requested review from Copilot, davidwrighton, noahfalk, rcj1 and steveisok October 2, 2025 21:35

Copilot AI reviewed Oct 2, 2025

View reviewed changes

fixes

c4adcac

max-charlamb requested a review from Copilot October 2, 2025 21:42

Copilot AI reviewed Oct 2, 2025

View reviewed changes

....Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/FrameIterator.cs Show resolved Hide resolved

...ractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.ReadyToRunJitManager.cs Outdated Show resolved Hide resolved

remove unused function

1efdc76

build-analysis bot mentioned this pull request Oct 3, 2025

AppHost tests fail with "Failure extracting contents of the application bundle." #119249

Closed

max-charlamb added 2 commits October 3, 2025 12:21

update

e7117a8

allow lazily decoding GCInfo

9903674

davidwrighton reviewed Oct 9, 2025

View reviewed changes

am11 reviewed Oct 21, 2025

View reviewed changes

max-charlamb added 4 commits November 10, 2025 17:42

update

551e1eb

Merge remote-tracking branch 'origin/main' into cdac-codeheaderdata

cd5a14e

fix merge

e7a73f5

Merge remote-tracking branch 'origin/main' into cdac-codeheaderdata

5002aa9

Copilot AI review requested due to automatic review settings February 17, 2026 22:41

Copilot started reviewing on behalf of max-charlamb February 17, 2026 22:42 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

jkotas reviewed Feb 17, 2026

View reviewed changes

This was referenced Feb 18, 2026

[Android][CoreCLR] System.Security.Cryptography.Tests killed by lowmemorykiller #118603

Open

[android][clr] No peer certificates when executing System.Net.Http.Functional.Tests on Android emulator #124526

Open

steveisok mentioned this pull request Feb 21, 2026

[cDAC] Support APIs used by SOS #117782

Open

32 tasks


		## Implementation

		The GCInfo contract has platform specific implementations as GCInfo differs per architecture. With the exception of x86, all platforms have a common encoding scheme with different encoding lengths and normalization functions for data. x86 uses an entirely different scheme which is not currently supported by this contract.

-    {
+    {
+        if (value == 0)
+            return 0;

	lastInterruptibleRangeStopOffsetNormalized = rangeStopOffsetNormalized;
	lastInterruptibleRangeStopOffsetNormalized = rangeStopOffsetNormalized;
	prevEndOffset = rangeStopOffset;

		\| `UnwindInfo` \| `CountOfUnwindCodes` \| Number of unwind codes in the unwind info. Only exists on some platforms \|
		\| `UnwindInfo` \| `UnwindCodeOffset` \| Offset of UnwindCodeOffset in the UnwindInfo data struct. Only exists on some platforms \|

Comments

Conversation

max-charlamb commented Sep 30, 2025 • edited by steveisok Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotnet-policy-service bot commented Sep 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes:

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

davidwrighton Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

max-charlamb Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

am11 Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

max-charlamb Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

jkotas Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

noahfalk Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

max-charlamb commented Sep 30, 2025 •

edited by steveisok

Loading