Add cDAC ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API - #125895

Merged
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis
May 6, 2026
Merged

Add cDAC ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API#125895
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis

Conversation

CopilotAI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements cDAC APIs for Objective-C interop diagnostics as a new ObjectiveCMarshal contract, plus adds IsTrackedReferenceWithFinalizer to the RuntimeTypeSystem contract. Based on draft PR #118250, adjusted per review feedback (contract renamed, TargetNUInt instead of nuint, APIs in dedicated contract).

Note

This PR was generated with GitHub Copilot.

Changes

New ObjectiveCMarshal cDAC Contract

  • IObjectiveCMarshal (Abstractions/Contracts/IObjectiveCMarshal.cs): Interface with a single API:

    TargetPointerGetTaggedMemory(TargetPointeraddress,outTargetNUIntsize);

    Returns the tagged memory pointer for an Objective-C tracked reference object (or TargetPointer.Null if none). Sets size to 2 * pointerSize bytes only on success.

  • ObjectiveCMarshal_1 (Contracts/ObjectiveCMarshal_1.cs): Version 1 implementation — reads the sync block from the object header, then extracts InteropSyncBlockInfo.TaggedMemory.

  • ObjectiveCMarshalFactory (Contracts/ObjectiveCMarshalFactory.cs): Contract factory.

  • Registered in ContractRegistry and CachingContractRegistry.

  • datadescriptor.inc: Added TaggedMemory field to InteropSyncBlockInfo type (under #ifdef FEATURE_OBJCMARSHAL) and CDAC_GLOBAL_CONTRACT(ObjectiveCMarshal, 1) (also guarded).

  • syncblk.h: Added TaggedMemory offset to cdac_data<InteropSyncBlockInfo> under #ifdef FEATURE_OBJCMARSHAL.

  • InteropSyncBlockInfo.cs (Data class): Added TaggedMemory property using TryGetValue to handle the optional FEATURE_OBJCMARSHAL field.

IsTrackedReferenceWithFinalizer on RuntimeTypeSystem

  • Added IsTrackedReferenceWithFinalizer = 0x04000000 to WFLAGS_HIGH enum in MethodTableFlags_1.cs, plus a convenience property bool IsTrackedReferenceWithFinalizer.

  • Added IsTrackedReferenceWithFinalizer(TypeHandle) to IRuntimeTypeSystem.cs and implemented it in RuntimeTypeSystem_1.cs.

SOSDacImpl.csISOSDacInterface11

Replaced the stub implementations of IsTrackedType and GetTaggedMemory with full cDAC implementations:

  • IsTrackedType: uses IRuntimeTypeSystem.IsTrackedReferenceWithFinalizer and IObjectiveCMarshal.GetTaggedMemory; returns S_OK if tracked, S_FALSE if not, E_INVALIDARG for null inputs.
  • GetTaggedMemory: uses IObjectiveCMarshal.GetTaggedMemory; returns S_OK with address and size if tagged memory exists, S_FALSE otherwise.

Both methods include #if DEBUG assertions against the legacy DAC implementation for validation.

Documentation

  • docs/design/datacontracts/ObjectiveCMarshal.md: New contract documentation.
  • docs/design/datacontracts/RuntimeTypeSystem.md: Added IsTrackedReferenceWithFinalizer to API listing and pseudocode.

Testing

All 1322 existing cDAC unit tests pass. The new contract will be exercised when running on an Apple platform with FEATURE_OBJCMARSHAL enabled.

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

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

LGTM - one refactor suggestion inline

Comment threaddocs/design/datacontracts/ObjectiveCMarshal.md Outdated
max-charlamb added a commit that referenced this pull request Apr 17, 2026
> [!NOTE]
> This PR description was generated with the assistance of GitHub
Copilot.
## Summary
Add a granular, per-method allowlist (`LegacyFallbackHelper`) that
controls which delegation-only APIs may fall back to the legacy DAC when
`CDAC_NO_FALLBACK=1` is set. This enables selective no-fallback testing
— blocking fallback for most APIs while allowing specific APIs that are
known to not yet be implemented in the cDAC.
Wire this into the runtime-diagnostics CI pipeline using the
`-noFallback` flag from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806).
All fallback attempts (both allowed and blocked) are logged to stderr
with method name, file, and line number for capture by the diagnostics
test infrastructure.
## Changes
### LegacyFallbackHelper.cs — Granular fallback control
New static helper that every delegation-only call site invokes via
`CanFallback()`. Uses `[CallerMemberName]`, `[CallerFilePath]`, and
`[CallerLineNumber]` to identify the call site.
- **Normal mode** (`CDAC_NO_FALLBACK` unset): Always returns `true`
(single `bool` check, `[AggressiveInlining]`)
- **No-fallback mode** (`CDAC_NO_FALLBACK=1`): Checks method name
against a `HashSet<string>` allowlist and file name against a file-level
allowlist
**Per-method allowlist:**
| Method | Reason |
|--------|--------|
| `EnumMemoryRegions` | Dump creation — cDAC has no memory enumeration
implementation |
| `GetInterface` | IMetaDataImport QI ([PR
#127028](#127028)) |
| `GetMethodDefinitionByToken` | IXCLRDataModule — not yet implemented
in cDAC |
| `IsTrackedType` | GC heap analysis ([PR
#125895](#125895)) |
| `TraverseLoaderHeap` | Loader heap traversal ([PR
#125129](#125129)) |
**File-level allowlist:**
| File | Reason |
|------|--------|
| `DacDbiImpl.cs` | Entire DBI/ICorDebug interface (122 methods) —
deferred |
### Entrypoints.cs — Simplified creation
Both `CreateSosInterface` and `CreateDacDbiInterface` now follow the
same pattern: the legacy implementation is always passed through, and
`LegacyFallbackHelper.CanFallback()` at each call site decides whether
to delegate. Removed `prevent_release`, `noFallback` env var check, and
null-legacy-ref logic.
### 13 Legacy wrapper files — Instrumented delegation sites
All 296 delegation-only methods across all legacy wrapper files now call
`LegacyFallbackHelper.CanFallback()`:
- `SOSDacImpl.cs` (12 methods)
- `SOSDacImpl.IXCLRDataProcess.cs` (38 methods, `Flush()` intentionally
excluded — cache management)
- `ClrDataModule.cs` (29 methods + IMetaDataImport QI)
- `DacDbiImpl.cs` (122 methods)
- Other wrappers: `ClrDataTask.cs`, `ClrDataExceptionState.cs`,
`ClrDataFrame.cs`, `ClrDataValue.cs`, `ClrDataTypeInstance.cs`,
`ClrDataMethodInstance.cs`, `ClrDataStackWalk.cs`, `ClrDataProcess.cs`
### CI Pipeline — `-noFallback` flag
Updated `runtime-diag-job.yml` to accept a `noFallback` parameter that
passes `-noFallback` to the diagnostics build script. The
`cDAC_no_fallback` leg in `runtime-diagnostics.yml` now uses
`noFallback: true` instead of setting `CDAC_NO_FALLBACK` as a
pipeline-level environment variable. The `-noFallback` flag (from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806))
properly:
- Sets `DOTNET_ENABLE_CDAC=1` and `CDAC_NO_FALLBACK=1` on the debugger
process
- Defines `CDAC_NO_FALLBACK_TESTING` to skip `ClrStack -i` tests
(ICorDebug not implemented in cDAC)
### Stderr logging
Every fallback attempt is logged to stderr in the format:
```
[cDAC] Allowed fallback: CreateStackWalk at DacDbiImpl.cs:590
[cDAC] Blocked fallback: SomeMethod at SOSDacImpl.cs:123
```
The diagnostics test infrastructure (`ProcessRunner`) captures stderr
and routes it to xunit test output with `STDERROR:` prefix, making
fallback usage visible in test results.
## Test Results
With `CDAC_NO_FALLBACK=1` and the current allowlist, running the full
SOS test suite against a private runtime build:
- **24 passed**, **2 failed** (flaky/pre-existing), **2 skipped**
(Linux-only)
- **0 blocked fallbacks**
## Motivation
The existing cDAC test leg always has the legacy DAC as a fallback, so
unimplemented APIs are silently handled. The granular no-fallback mode
makes gaps visible per-method, helping track progress toward full cDAC
coverage while keeping tests green for known-deferred APIs.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@max-charlamb

Copy link
Copy Markdown
Member

We should make sure to remove IsTrackedType from the LegacyFallbackHelper in this PR.

@max-charlamb

Copy link
Copy Markdown
Member

@copilot, please merge from main, fix any issues, then address feedback on this PR.

@rcj1

rcj1 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

@copilot@max-charlamb there is a reason this is not merged, and that is that we cannot have proper testing because the Mac tests are not set up. No action is needed at the moment.

Copilot stopped work on behalf of max-charlamb due to an error April 21, 2026 16:03
CopilotAI requested a review from max-charlambApril 21, 2026 16:03
Copilot stopped work on behalf of rcj1 due to an error April 21, 2026 16:03
…dReferenceWithFinalizer APIs
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fa8e5180-e07a-43c6-b618-58c6b60fd64e
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 5, 2026 17:52
@rcj1
rcj1force-pushed the copilot/add-cdac-apis branch from 3f77676 to 502054fCompareMay 5, 2026 17:52

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

Note

Copilot was unable to run its full agentic suite in this review.

Adds Objective-C interop support to the cDAC reader and legacy SOS DAC bridge by introducing a new ObjectiveCMarshal contract, exposing tracked-reference metadata, and adding unit/dump coverage around tagged memory.

Changes:

  • Add a new IObjectiveCMarshal contract and wire it through cDAC registration, runtime data descriptors, and legacy ISOSDacInterface11 implementations.
  • Extend object/sync-block data readers and test mocks to surface Objective-C tagged memory and tracked-reference-with-finalizer metadata.
  • Add macOS-specific dump-test infrastructure, a new ObjectiveCMarshal debuggee, and unit/integration tests for tagged-memory scenarios.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/ObjectiveCMarshalTests.csAdds unit tests for GetTaggedMemory contract behavior.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.SyncBlock.csExtends mock sync-block/interop info with tagged-memory support.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Object.csThreads tagged-memory data through mock object creation helpers.
src/native/managed/cdac/tests/DumpTests/ObjectiveCMarshalDumpTests.csAdds dump-based tests for tracked objects and tagged memory.
src/native/managed/cdac/tests/DumpTests/DumpTests.targetsFilters macOS-only debuggees on non-macOS hosts.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/Program.csAdds a macOS-only debuggee that creates a tracked object and crashes.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/ObjectiveCMarshal.csprojDeclares the new debuggee as macOS-only and full-dump capable.
src/native/managed/cdac/tests/DumpTests/Debuggees/Directory.Build.targetsPropagates MacOnly metadata from debuggee projects.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements ISOSDacInterface11 tagged-memory/tracked-type APIs via cDAC.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodTableFlags_1.csAdds the tracked-reference-with-finalizer method-table flag.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/InteropSyncBlockInfo.csReads tagged-memory pointer from interop sync-block info.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.csRegisters the new IObjectiveCMarshal contract implementation.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.csExposes IsTrackedReferenceWithFinalizer from method-table flags.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ObjectiveCMarshal_1.csImplements tagged-memory lookup from an object's sync block.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.csFactors sync-block address lookup into IObject.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.csAdds public contract API for tracked-reference-with-finalizer checks.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObjectiveCMarshal.csIntroduces the public Objective-C marshal contract API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObject.csAdds public sync-block address lookup API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.csExposes ObjectiveCMarshal from the contract registry.
src/coreclr/vm/syncblk.hPublishes tagged-memory field offset for cDAC data descriptors.
src/coreclr/vm/methodtable.hMarks tracked-reference-with-finalizer flag as cDAC-dependent.
src/coreclr/vm/datadescriptor/datadescriptor.incExposes tagged-memory field and ObjectiveCMarshal global contract.
docs/design/datacontracts/RuntimeTypeSystem.mdDocuments the new runtime-type-system API.
docs/design/datacontracts/ObjectiveCMarshal.mdAdds design documentation for the new contract.
docs/design/datacontracts/Object.mdDocuments the new sync-block lookup helper in Object contract.

@github-actions

This comment has been minimized.

CopilotAI review requested due to automatic review settings May 6, 2026 17:36
CopilotAI reviewed May 6, 2026

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.

Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #125895

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: This PR adds a new ObjectiveCMarshal cDAC contract to expose Objective-C interop tagged memory information through the diagnostic data contract reader, replacing the legacy DAC fallback for ISOSDacInterface11.IsTrackedType and GetTaggedMemory. It also refactors the Object contract to extract GetSyncBlockAddress as a reusable API. The motivation is clear and well-justified — enabling the cDAC path for macOS Objective-C diagnostic scenarios.

Approach: The approach is sound and follows established cDAC patterns exactly. A new IObjectiveCMarshal contract with a single GetTaggedMemory API, a helper IsTrackedReferenceWithFinalizer on IRuntimeTypeSystem, and good code deduplication by extracting sync block lookup into GetSyncBlockAddress. The conditional contract registration via #ifdef FEATURE_OBJCMARSHAL and the TryGetContract pattern for graceful fallback on non-macOS targets is the correct approach.

Summary: ✅ LGTM. The implementation correctly mirrors the legacy C++ DAC behavior (ClrDataAccess::IsTrackedType / ClrDataAccess::GetTaggedMemory), the flag value matches the runtime definition, field reads use TryGetValue for conditional feature availability, and the test coverage is comprehensive with both unit and dump-based integration tests.


Detailed Findings

✅ Correctness — SOSDacImpl matches legacy DAC semantics

The new IsTrackedType implementation correctly:

  • Returns S_OK when tracked, S_FALSE when not (matching request.cpp:5445-5447)
  • Returns E_INVALIDARG for null pointers (via ArgumentException.HResult)
  • Checks hasTaggedMemory independently using TryGetContract<IObjectiveCMarshal> — gracefully no-ops when the contract isn't available (non-macOS)
  • The GetTaggedMemory method returns S_FALSE when no tagged memory exists and S_OK on success

The #if DEBUG validation blocks correctly compare against the legacy implementation.

✅ Flag Value — IsTrackedReferenceWithFinalizer = 0x04000000

The flag value in MethodTableFlags_1.cs matches exactly with enum_flag_IsTrackedReferenceWithFinalizer in src/coreclr/vm/methodtable.h:3821. The [cDAC] annotation comment was correctly added to the runtime header to signal this dependency.

✅ Data Descriptor — Conditional field exposure

datadescriptor.inc correctly wraps the TaggedMemory field under #ifdef FEATURE_OBJCMARSHAL, and InteropSyncBlockInfo.cs uses TryGetValue for graceful fallback when the field isn't present. This is consistent with how RCW/CCW/CCF handle conditional FEATURE_COMINTEROP.

✅ Refactoring — GetSyncBlockAddress extraction

The extracted GetSyncBlockAddress method correctly centralizes sync block index lookup logic. The condition (syncBlockValue & (_syncBlockIsHashCode | _syncBlockIsHashOrSyncBlockIndex)) != _syncBlockIsHashOrSyncBlockIndex correctly identifies sync block indices (not inline hash codes). The removal of _syncTableEntries field is correct — the lookup now delegates to _target.Contracts.SyncBlock.GetSyncBlock(index).

✅ Tagged Memory Size — Hardcoded 2 * PointerSize

new TargetNUInt(2 * (ulong)_target.PointerSize) matches the runtime's PTR_VOID m_taggedAlloc[NUM_TAGGED_MEMORY_CALLBACKS] where NUM_TAGGED_MEMORY_CALLBACKS = 2.

✅ Test Coverage — Comprehensive

  • Unit tests (ObjectiveCMarshalTests.cs): Three key scenarios tested — no sync block, null tagged memory, valid tagged memory with size assertion.
  • Dump integration tests (ObjectiveCMarshalDumpTests.cs): Gated to macOS with [SkipOnOS(IncludeOnly = "osx")].
  • Build infrastructure: MacOnly filtering mirrors the existing WindowsOnly pattern.
  • Mock infrastructure: Extended correctly with taggedMemory parameter.

💡 Minor — Redundant object header read in TryGetHashCode (non-blocking, follow-up)

In Object_1.cs, TryGetHashCode reads the object header (lines 110-112), then if hash isn't inline, calls GetSyncBlockAddress which reads the same header again (lines 136-138). Since ProcessedData.GetOrAdd caches by address, this is functionally a no-op. Acceptable for diagnostic tooling.

✅ Contract Registration — Properly ordered

Registrations maintain alphabetical ordering in CoreCLRContracts.cs, datadescriptor.inc, and ContractRegistry.cs.

Generated by Code Review for issue #125895 ·

@rcj1
rcj1 marked this pull request as ready for review May 6, 2026 20:24
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

1 similar comment
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@max-charlamb@rcj1@noahfalk
, '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 ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API - #125895

Merged
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis
May 6, 2026
Merged

Add cDAC ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API#125895
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis

Conversation

CopilotAI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements cDAC APIs for Objective-C interop diagnostics as a new ObjectiveCMarshal contract, plus adds IsTrackedReferenceWithFinalizer to the RuntimeTypeSystem contract. Based on draft PR #118250, adjusted per review feedback (contract renamed, TargetNUInt instead of nuint, APIs in dedicated contract).

Note

This PR was generated with GitHub Copilot.

Changes

New ObjectiveCMarshal cDAC Contract

  • IObjectiveCMarshal (Abstractions/Contracts/IObjectiveCMarshal.cs): Interface with a single API:

    TargetPointerGetTaggedMemory(TargetPointeraddress,outTargetNUIntsize);

    Returns the tagged memory pointer for an Objective-C tracked reference object (or TargetPointer.Null if none). Sets size to 2 * pointerSize bytes only on success.

  • ObjectiveCMarshal_1 (Contracts/ObjectiveCMarshal_1.cs): Version 1 implementation — reads the sync block from the object header, then extracts InteropSyncBlockInfo.TaggedMemory.

  • ObjectiveCMarshalFactory (Contracts/ObjectiveCMarshalFactory.cs): Contract factory.

  • Registered in ContractRegistry and CachingContractRegistry.

  • datadescriptor.inc: Added TaggedMemory field to InteropSyncBlockInfo type (under #ifdef FEATURE_OBJCMARSHAL) and CDAC_GLOBAL_CONTRACT(ObjectiveCMarshal, 1) (also guarded).

  • syncblk.h: Added TaggedMemory offset to cdac_data<InteropSyncBlockInfo> under #ifdef FEATURE_OBJCMARSHAL.

  • InteropSyncBlockInfo.cs (Data class): Added TaggedMemory property using TryGetValue to handle the optional FEATURE_OBJCMARSHAL field.

IsTrackedReferenceWithFinalizer on RuntimeTypeSystem

  • Added IsTrackedReferenceWithFinalizer = 0x04000000 to WFLAGS_HIGH enum in MethodTableFlags_1.cs, plus a convenience property bool IsTrackedReferenceWithFinalizer.

  • Added IsTrackedReferenceWithFinalizer(TypeHandle) to IRuntimeTypeSystem.cs and implemented it in RuntimeTypeSystem_1.cs.

SOSDacImpl.csISOSDacInterface11

Replaced the stub implementations of IsTrackedType and GetTaggedMemory with full cDAC implementations:

  • IsTrackedType: uses IRuntimeTypeSystem.IsTrackedReferenceWithFinalizer and IObjectiveCMarshal.GetTaggedMemory; returns S_OK if tracked, S_FALSE if not, E_INVALIDARG for null inputs.
  • GetTaggedMemory: uses IObjectiveCMarshal.GetTaggedMemory; returns S_OK with address and size if tagged memory exists, S_FALSE otherwise.

Both methods include #if DEBUG assertions against the legacy DAC implementation for validation.

Documentation

  • docs/design/datacontracts/ObjectiveCMarshal.md: New contract documentation.
  • docs/design/datacontracts/RuntimeTypeSystem.md: Added IsTrackedReferenceWithFinalizer to API listing and pseudocode.

Testing

All 1322 existing cDAC unit tests pass. The new contract will be exercised when running on an Apple platform with FEATURE_OBJCMARSHAL enabled.

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

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

LGTM - one refactor suggestion inline

Comment threaddocs/design/datacontracts/ObjectiveCMarshal.md Outdated
max-charlamb added a commit that referenced this pull request Apr 17, 2026
> [!NOTE]
> This PR description was generated with the assistance of GitHub
Copilot.
## Summary
Add a granular, per-method allowlist (`LegacyFallbackHelper`) that
controls which delegation-only APIs may fall back to the legacy DAC when
`CDAC_NO_FALLBACK=1` is set. This enables selective no-fallback testing
— blocking fallback for most APIs while allowing specific APIs that are
known to not yet be implemented in the cDAC.
Wire this into the runtime-diagnostics CI pipeline using the
`-noFallback` flag from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806).
All fallback attempts (both allowed and blocked) are logged to stderr
with method name, file, and line number for capture by the diagnostics
test infrastructure.
## Changes
### LegacyFallbackHelper.cs — Granular fallback control
New static helper that every delegation-only call site invokes via
`CanFallback()`. Uses `[CallerMemberName]`, `[CallerFilePath]`, and
`[CallerLineNumber]` to identify the call site.
- **Normal mode** (`CDAC_NO_FALLBACK` unset): Always returns `true`
(single `bool` check, `[AggressiveInlining]`)
- **No-fallback mode** (`CDAC_NO_FALLBACK=1`): Checks method name
against a `HashSet<string>` allowlist and file name against a file-level
allowlist
**Per-method allowlist:**
| Method | Reason |
|--------|--------|
| `EnumMemoryRegions` | Dump creation — cDAC has no memory enumeration
implementation |
| `GetInterface` | IMetaDataImport QI ([PR
#127028](#127028)) |
| `GetMethodDefinitionByToken` | IXCLRDataModule — not yet implemented
in cDAC |
| `IsTrackedType` | GC heap analysis ([PR
#125895](#125895)) |
| `TraverseLoaderHeap` | Loader heap traversal ([PR
#125129](#125129)) |
**File-level allowlist:**
| File | Reason |
|------|--------|
| `DacDbiImpl.cs` | Entire DBI/ICorDebug interface (122 methods) —
deferred |
### Entrypoints.cs — Simplified creation
Both `CreateSosInterface` and `CreateDacDbiInterface` now follow the
same pattern: the legacy implementation is always passed through, and
`LegacyFallbackHelper.CanFallback()` at each call site decides whether
to delegate. Removed `prevent_release`, `noFallback` env var check, and
null-legacy-ref logic.
### 13 Legacy wrapper files — Instrumented delegation sites
All 296 delegation-only methods across all legacy wrapper files now call
`LegacyFallbackHelper.CanFallback()`:
- `SOSDacImpl.cs` (12 methods)
- `SOSDacImpl.IXCLRDataProcess.cs` (38 methods, `Flush()` intentionally
excluded — cache management)
- `ClrDataModule.cs` (29 methods + IMetaDataImport QI)
- `DacDbiImpl.cs` (122 methods)
- Other wrappers: `ClrDataTask.cs`, `ClrDataExceptionState.cs`,
`ClrDataFrame.cs`, `ClrDataValue.cs`, `ClrDataTypeInstance.cs`,
`ClrDataMethodInstance.cs`, `ClrDataStackWalk.cs`, `ClrDataProcess.cs`
### CI Pipeline — `-noFallback` flag
Updated `runtime-diag-job.yml` to accept a `noFallback` parameter that
passes `-noFallback` to the diagnostics build script. The
`cDAC_no_fallback` leg in `runtime-diagnostics.yml` now uses
`noFallback: true` instead of setting `CDAC_NO_FALLBACK` as a
pipeline-level environment variable. The `-noFallback` flag (from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806))
properly:
- Sets `DOTNET_ENABLE_CDAC=1` and `CDAC_NO_FALLBACK=1` on the debugger
process
- Defines `CDAC_NO_FALLBACK_TESTING` to skip `ClrStack -i` tests
(ICorDebug not implemented in cDAC)
### Stderr logging
Every fallback attempt is logged to stderr in the format:
```
[cDAC] Allowed fallback: CreateStackWalk at DacDbiImpl.cs:590
[cDAC] Blocked fallback: SomeMethod at SOSDacImpl.cs:123
```
The diagnostics test infrastructure (`ProcessRunner`) captures stderr
and routes it to xunit test output with `STDERROR:` prefix, making
fallback usage visible in test results.
## Test Results
With `CDAC_NO_FALLBACK=1` and the current allowlist, running the full
SOS test suite against a private runtime build:
- **24 passed**, **2 failed** (flaky/pre-existing), **2 skipped**
(Linux-only)
- **0 blocked fallbacks**
## Motivation
The existing cDAC test leg always has the legacy DAC as a fallback, so
unimplemented APIs are silently handled. The granular no-fallback mode
makes gaps visible per-method, helping track progress toward full cDAC
coverage while keeping tests green for known-deferred APIs.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@max-charlamb

Copy link
Copy Markdown
Member

We should make sure to remove IsTrackedType from the LegacyFallbackHelper in this PR.

@max-charlamb

Copy link
Copy Markdown
Member

@copilot, please merge from main, fix any issues, then address feedback on this PR.

@rcj1

rcj1 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

@copilot@max-charlamb there is a reason this is not merged, and that is that we cannot have proper testing because the Mac tests are not set up. No action is needed at the moment.

Copilot stopped work on behalf of max-charlamb due to an error April 21, 2026 16:03
CopilotAI requested a review from max-charlambApril 21, 2026 16:03
Copilot stopped work on behalf of rcj1 due to an error April 21, 2026 16:03
…dReferenceWithFinalizer APIs
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fa8e5180-e07a-43c6-b618-58c6b60fd64e
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 5, 2026 17:52
@rcj1
rcj1force-pushed the copilot/add-cdac-apis branch from 3f77676 to 502054fCompareMay 5, 2026 17:52

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

Note

Copilot was unable to run its full agentic suite in this review.

Adds Objective-C interop support to the cDAC reader and legacy SOS DAC bridge by introducing a new ObjectiveCMarshal contract, exposing tracked-reference metadata, and adding unit/dump coverage around tagged memory.

Changes:

  • Add a new IObjectiveCMarshal contract and wire it through cDAC registration, runtime data descriptors, and legacy ISOSDacInterface11 implementations.
  • Extend object/sync-block data readers and test mocks to surface Objective-C tagged memory and tracked-reference-with-finalizer metadata.
  • Add macOS-specific dump-test infrastructure, a new ObjectiveCMarshal debuggee, and unit/integration tests for tagged-memory scenarios.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/ObjectiveCMarshalTests.csAdds unit tests for GetTaggedMemory contract behavior.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.SyncBlock.csExtends mock sync-block/interop info with tagged-memory support.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Object.csThreads tagged-memory data through mock object creation helpers.
src/native/managed/cdac/tests/DumpTests/ObjectiveCMarshalDumpTests.csAdds dump-based tests for tracked objects and tagged memory.
src/native/managed/cdac/tests/DumpTests/DumpTests.targetsFilters macOS-only debuggees on non-macOS hosts.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/Program.csAdds a macOS-only debuggee that creates a tracked object and crashes.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/ObjectiveCMarshal.csprojDeclares the new debuggee as macOS-only and full-dump capable.
src/native/managed/cdac/tests/DumpTests/Debuggees/Directory.Build.targetsPropagates MacOnly metadata from debuggee projects.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements ISOSDacInterface11 tagged-memory/tracked-type APIs via cDAC.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodTableFlags_1.csAdds the tracked-reference-with-finalizer method-table flag.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/InteropSyncBlockInfo.csReads tagged-memory pointer from interop sync-block info.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.csRegisters the new IObjectiveCMarshal contract implementation.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.csExposes IsTrackedReferenceWithFinalizer from method-table flags.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ObjectiveCMarshal_1.csImplements tagged-memory lookup from an object's sync block.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.csFactors sync-block address lookup into IObject.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.csAdds public contract API for tracked-reference-with-finalizer checks.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObjectiveCMarshal.csIntroduces the public Objective-C marshal contract API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObject.csAdds public sync-block address lookup API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.csExposes ObjectiveCMarshal from the contract registry.
src/coreclr/vm/syncblk.hPublishes tagged-memory field offset for cDAC data descriptors.
src/coreclr/vm/methodtable.hMarks tracked-reference-with-finalizer flag as cDAC-dependent.
src/coreclr/vm/datadescriptor/datadescriptor.incExposes tagged-memory field and ObjectiveCMarshal global contract.
docs/design/datacontracts/RuntimeTypeSystem.mdDocuments the new runtime-type-system API.
docs/design/datacontracts/ObjectiveCMarshal.mdAdds design documentation for the new contract.
docs/design/datacontracts/Object.mdDocuments the new sync-block lookup helper in Object contract.

@github-actions

This comment has been minimized.

CopilotAI review requested due to automatic review settings May 6, 2026 17:36
CopilotAI reviewed May 6, 2026

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.

Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #125895

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: This PR adds a new ObjectiveCMarshal cDAC contract to expose Objective-C interop tagged memory information through the diagnostic data contract reader, replacing the legacy DAC fallback for ISOSDacInterface11.IsTrackedType and GetTaggedMemory. It also refactors the Object contract to extract GetSyncBlockAddress as a reusable API. The motivation is clear and well-justified — enabling the cDAC path for macOS Objective-C diagnostic scenarios.

Approach: The approach is sound and follows established cDAC patterns exactly. A new IObjectiveCMarshal contract with a single GetTaggedMemory API, a helper IsTrackedReferenceWithFinalizer on IRuntimeTypeSystem, and good code deduplication by extracting sync block lookup into GetSyncBlockAddress. The conditional contract registration via #ifdef FEATURE_OBJCMARSHAL and the TryGetContract pattern for graceful fallback on non-macOS targets is the correct approach.

Summary: ✅ LGTM. The implementation correctly mirrors the legacy C++ DAC behavior (ClrDataAccess::IsTrackedType / ClrDataAccess::GetTaggedMemory), the flag value matches the runtime definition, field reads use TryGetValue for conditional feature availability, and the test coverage is comprehensive with both unit and dump-based integration tests.


Detailed Findings

✅ Correctness — SOSDacImpl matches legacy DAC semantics

The new IsTrackedType implementation correctly:

  • Returns S_OK when tracked, S_FALSE when not (matching request.cpp:5445-5447)
  • Returns E_INVALIDARG for null pointers (via ArgumentException.HResult)
  • Checks hasTaggedMemory independently using TryGetContract<IObjectiveCMarshal> — gracefully no-ops when the contract isn't available (non-macOS)
  • The GetTaggedMemory method returns S_FALSE when no tagged memory exists and S_OK on success

The #if DEBUG validation blocks correctly compare against the legacy implementation.

✅ Flag Value — IsTrackedReferenceWithFinalizer = 0x04000000

The flag value in MethodTableFlags_1.cs matches exactly with enum_flag_IsTrackedReferenceWithFinalizer in src/coreclr/vm/methodtable.h:3821. The [cDAC] annotation comment was correctly added to the runtime header to signal this dependency.

✅ Data Descriptor — Conditional field exposure

datadescriptor.inc correctly wraps the TaggedMemory field under #ifdef FEATURE_OBJCMARSHAL, and InteropSyncBlockInfo.cs uses TryGetValue for graceful fallback when the field isn't present. This is consistent with how RCW/CCW/CCF handle conditional FEATURE_COMINTEROP.

✅ Refactoring — GetSyncBlockAddress extraction

The extracted GetSyncBlockAddress method correctly centralizes sync block index lookup logic. The condition (syncBlockValue & (_syncBlockIsHashCode | _syncBlockIsHashOrSyncBlockIndex)) != _syncBlockIsHashOrSyncBlockIndex correctly identifies sync block indices (not inline hash codes). The removal of _syncTableEntries field is correct — the lookup now delegates to _target.Contracts.SyncBlock.GetSyncBlock(index).

✅ Tagged Memory Size — Hardcoded 2 * PointerSize

new TargetNUInt(2 * (ulong)_target.PointerSize) matches the runtime's PTR_VOID m_taggedAlloc[NUM_TAGGED_MEMORY_CALLBACKS] where NUM_TAGGED_MEMORY_CALLBACKS = 2.

✅ Test Coverage — Comprehensive

  • Unit tests (ObjectiveCMarshalTests.cs): Three key scenarios tested — no sync block, null tagged memory, valid tagged memory with size assertion.
  • Dump integration tests (ObjectiveCMarshalDumpTests.cs): Gated to macOS with [SkipOnOS(IncludeOnly = "osx")].
  • Build infrastructure: MacOnly filtering mirrors the existing WindowsOnly pattern.
  • Mock infrastructure: Extended correctly with taggedMemory parameter.

💡 Minor — Redundant object header read in TryGetHashCode (non-blocking, follow-up)

In Object_1.cs, TryGetHashCode reads the object header (lines 110-112), then if hash isn't inline, calls GetSyncBlockAddress which reads the same header again (lines 136-138). Since ProcessedData.GetOrAdd caches by address, this is functionally a no-op. Acceptable for diagnostic tooling.

✅ Contract Registration — Properly ordered

Registrations maintain alphabetical ordering in CoreCLRContracts.cs, datadescriptor.inc, and ContractRegistry.cs.

Generated by Code Review for issue #125895 ·

@rcj1
rcj1 marked this pull request as ready for review May 6, 2026 20:24
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

1 similar comment
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@max-charlamb@rcj1@noahfalk
, '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 ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API - #125895

Merged
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis
May 6, 2026
Merged

Add cDAC ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API#125895
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis

Conversation

CopilotAI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements cDAC APIs for Objective-C interop diagnostics as a new ObjectiveCMarshal contract, plus adds IsTrackedReferenceWithFinalizer to the RuntimeTypeSystem contract. Based on draft PR #118250, adjusted per review feedback (contract renamed, TargetNUInt instead of nuint, APIs in dedicated contract).

Note

This PR was generated with GitHub Copilot.

Changes

New ObjectiveCMarshal cDAC Contract

  • IObjectiveCMarshal (Abstractions/Contracts/IObjectiveCMarshal.cs): Interface with a single API:

    TargetPointerGetTaggedMemory(TargetPointeraddress,outTargetNUIntsize);

    Returns the tagged memory pointer for an Objective-C tracked reference object (or TargetPointer.Null if none). Sets size to 2 * pointerSize bytes only on success.

  • ObjectiveCMarshal_1 (Contracts/ObjectiveCMarshal_1.cs): Version 1 implementation — reads the sync block from the object header, then extracts InteropSyncBlockInfo.TaggedMemory.

  • ObjectiveCMarshalFactory (Contracts/ObjectiveCMarshalFactory.cs): Contract factory.

  • Registered in ContractRegistry and CachingContractRegistry.

  • datadescriptor.inc: Added TaggedMemory field to InteropSyncBlockInfo type (under #ifdef FEATURE_OBJCMARSHAL) and CDAC_GLOBAL_CONTRACT(ObjectiveCMarshal, 1) (also guarded).

  • syncblk.h: Added TaggedMemory offset to cdac_data<InteropSyncBlockInfo> under #ifdef FEATURE_OBJCMARSHAL.

  • InteropSyncBlockInfo.cs (Data class): Added TaggedMemory property using TryGetValue to handle the optional FEATURE_OBJCMARSHAL field.

IsTrackedReferenceWithFinalizer on RuntimeTypeSystem

  • Added IsTrackedReferenceWithFinalizer = 0x04000000 to WFLAGS_HIGH enum in MethodTableFlags_1.cs, plus a convenience property bool IsTrackedReferenceWithFinalizer.

  • Added IsTrackedReferenceWithFinalizer(TypeHandle) to IRuntimeTypeSystem.cs and implemented it in RuntimeTypeSystem_1.cs.

SOSDacImpl.csISOSDacInterface11

Replaced the stub implementations of IsTrackedType and GetTaggedMemory with full cDAC implementations:

  • IsTrackedType: uses IRuntimeTypeSystem.IsTrackedReferenceWithFinalizer and IObjectiveCMarshal.GetTaggedMemory; returns S_OK if tracked, S_FALSE if not, E_INVALIDARG for null inputs.
  • GetTaggedMemory: uses IObjectiveCMarshal.GetTaggedMemory; returns S_OK with address and size if tagged memory exists, S_FALSE otherwise.

Both methods include #if DEBUG assertions against the legacy DAC implementation for validation.

Documentation

  • docs/design/datacontracts/ObjectiveCMarshal.md: New contract documentation.
  • docs/design/datacontracts/RuntimeTypeSystem.md: Added IsTrackedReferenceWithFinalizer to API listing and pseudocode.

Testing

All 1322 existing cDAC unit tests pass. The new contract will be exercised when running on an Apple platform with FEATURE_OBJCMARSHAL enabled.

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

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

LGTM - one refactor suggestion inline

Comment threaddocs/design/datacontracts/ObjectiveCMarshal.md Outdated
max-charlamb added a commit that referenced this pull request Apr 17, 2026
> [!NOTE]
> This PR description was generated with the assistance of GitHub
Copilot.
## Summary
Add a granular, per-method allowlist (`LegacyFallbackHelper`) that
controls which delegation-only APIs may fall back to the legacy DAC when
`CDAC_NO_FALLBACK=1` is set. This enables selective no-fallback testing
— blocking fallback for most APIs while allowing specific APIs that are
known to not yet be implemented in the cDAC.
Wire this into the runtime-diagnostics CI pipeline using the
`-noFallback` flag from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806).
All fallback attempts (both allowed and blocked) are logged to stderr
with method name, file, and line number for capture by the diagnostics
test infrastructure.
## Changes
### LegacyFallbackHelper.cs — Granular fallback control
New static helper that every delegation-only call site invokes via
`CanFallback()`. Uses `[CallerMemberName]`, `[CallerFilePath]`, and
`[CallerLineNumber]` to identify the call site.
- **Normal mode** (`CDAC_NO_FALLBACK` unset): Always returns `true`
(single `bool` check, `[AggressiveInlining]`)
- **No-fallback mode** (`CDAC_NO_FALLBACK=1`): Checks method name
against a `HashSet<string>` allowlist and file name against a file-level
allowlist
**Per-method allowlist:**
| Method | Reason |
|--------|--------|
| `EnumMemoryRegions` | Dump creation — cDAC has no memory enumeration
implementation |
| `GetInterface` | IMetaDataImport QI ([PR
#127028](#127028)) |
| `GetMethodDefinitionByToken` | IXCLRDataModule — not yet implemented
in cDAC |
| `IsTrackedType` | GC heap analysis ([PR
#125895](#125895)) |
| `TraverseLoaderHeap` | Loader heap traversal ([PR
#125129](#125129)) |
**File-level allowlist:**
| File | Reason |
|------|--------|
| `DacDbiImpl.cs` | Entire DBI/ICorDebug interface (122 methods) —
deferred |
### Entrypoints.cs — Simplified creation
Both `CreateSosInterface` and `CreateDacDbiInterface` now follow the
same pattern: the legacy implementation is always passed through, and
`LegacyFallbackHelper.CanFallback()` at each call site decides whether
to delegate. Removed `prevent_release`, `noFallback` env var check, and
null-legacy-ref logic.
### 13 Legacy wrapper files — Instrumented delegation sites
All 296 delegation-only methods across all legacy wrapper files now call
`LegacyFallbackHelper.CanFallback()`:
- `SOSDacImpl.cs` (12 methods)
- `SOSDacImpl.IXCLRDataProcess.cs` (38 methods, `Flush()` intentionally
excluded — cache management)
- `ClrDataModule.cs` (29 methods + IMetaDataImport QI)
- `DacDbiImpl.cs` (122 methods)
- Other wrappers: `ClrDataTask.cs`, `ClrDataExceptionState.cs`,
`ClrDataFrame.cs`, `ClrDataValue.cs`, `ClrDataTypeInstance.cs`,
`ClrDataMethodInstance.cs`, `ClrDataStackWalk.cs`, `ClrDataProcess.cs`
### CI Pipeline — `-noFallback` flag
Updated `runtime-diag-job.yml` to accept a `noFallback` parameter that
passes `-noFallback` to the diagnostics build script. The
`cDAC_no_fallback` leg in `runtime-diagnostics.yml` now uses
`noFallback: true` instead of setting `CDAC_NO_FALLBACK` as a
pipeline-level environment variable. The `-noFallback` flag (from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806))
properly:
- Sets `DOTNET_ENABLE_CDAC=1` and `CDAC_NO_FALLBACK=1` on the debugger
process
- Defines `CDAC_NO_FALLBACK_TESTING` to skip `ClrStack -i` tests
(ICorDebug not implemented in cDAC)
### Stderr logging
Every fallback attempt is logged to stderr in the format:
```
[cDAC] Allowed fallback: CreateStackWalk at DacDbiImpl.cs:590
[cDAC] Blocked fallback: SomeMethod at SOSDacImpl.cs:123
```
The diagnostics test infrastructure (`ProcessRunner`) captures stderr
and routes it to xunit test output with `STDERROR:` prefix, making
fallback usage visible in test results.
## Test Results
With `CDAC_NO_FALLBACK=1` and the current allowlist, running the full
SOS test suite against a private runtime build:
- **24 passed**, **2 failed** (flaky/pre-existing), **2 skipped**
(Linux-only)
- **0 blocked fallbacks**
## Motivation
The existing cDAC test leg always has the legacy DAC as a fallback, so
unimplemented APIs are silently handled. The granular no-fallback mode
makes gaps visible per-method, helping track progress toward full cDAC
coverage while keeping tests green for known-deferred APIs.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@max-charlamb

Copy link
Copy Markdown
Member

We should make sure to remove IsTrackedType from the LegacyFallbackHelper in this PR.

@max-charlamb

Copy link
Copy Markdown
Member

@copilot, please merge from main, fix any issues, then address feedback on this PR.

@rcj1

rcj1 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

@copilot@max-charlamb there is a reason this is not merged, and that is that we cannot have proper testing because the Mac tests are not set up. No action is needed at the moment.

Copilot stopped work on behalf of max-charlamb due to an error April 21, 2026 16:03
CopilotAI requested a review from max-charlambApril 21, 2026 16:03
Copilot stopped work on behalf of rcj1 due to an error April 21, 2026 16:03
…dReferenceWithFinalizer APIs
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fa8e5180-e07a-43c6-b618-58c6b60fd64e
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 5, 2026 17:52
@rcj1
rcj1force-pushed the copilot/add-cdac-apis branch from 3f77676 to 502054fCompareMay 5, 2026 17:52

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

Note

Copilot was unable to run its full agentic suite in this review.

Adds Objective-C interop support to the cDAC reader and legacy SOS DAC bridge by introducing a new ObjectiveCMarshal contract, exposing tracked-reference metadata, and adding unit/dump coverage around tagged memory.

Changes:

  • Add a new IObjectiveCMarshal contract and wire it through cDAC registration, runtime data descriptors, and legacy ISOSDacInterface11 implementations.
  • Extend object/sync-block data readers and test mocks to surface Objective-C tagged memory and tracked-reference-with-finalizer metadata.
  • Add macOS-specific dump-test infrastructure, a new ObjectiveCMarshal debuggee, and unit/integration tests for tagged-memory scenarios.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/ObjectiveCMarshalTests.csAdds unit tests for GetTaggedMemory contract behavior.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.SyncBlock.csExtends mock sync-block/interop info with tagged-memory support.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Object.csThreads tagged-memory data through mock object creation helpers.
src/native/managed/cdac/tests/DumpTests/ObjectiveCMarshalDumpTests.csAdds dump-based tests for tracked objects and tagged memory.
src/native/managed/cdac/tests/DumpTests/DumpTests.targetsFilters macOS-only debuggees on non-macOS hosts.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/Program.csAdds a macOS-only debuggee that creates a tracked object and crashes.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/ObjectiveCMarshal.csprojDeclares the new debuggee as macOS-only and full-dump capable.
src/native/managed/cdac/tests/DumpTests/Debuggees/Directory.Build.targetsPropagates MacOnly metadata from debuggee projects.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements ISOSDacInterface11 tagged-memory/tracked-type APIs via cDAC.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodTableFlags_1.csAdds the tracked-reference-with-finalizer method-table flag.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/InteropSyncBlockInfo.csReads tagged-memory pointer from interop sync-block info.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.csRegisters the new IObjectiveCMarshal contract implementation.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.csExposes IsTrackedReferenceWithFinalizer from method-table flags.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ObjectiveCMarshal_1.csImplements tagged-memory lookup from an object's sync block.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.csFactors sync-block address lookup into IObject.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.csAdds public contract API for tracked-reference-with-finalizer checks.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObjectiveCMarshal.csIntroduces the public Objective-C marshal contract API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObject.csAdds public sync-block address lookup API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.csExposes ObjectiveCMarshal from the contract registry.
src/coreclr/vm/syncblk.hPublishes tagged-memory field offset for cDAC data descriptors.
src/coreclr/vm/methodtable.hMarks tracked-reference-with-finalizer flag as cDAC-dependent.
src/coreclr/vm/datadescriptor/datadescriptor.incExposes tagged-memory field and ObjectiveCMarshal global contract.
docs/design/datacontracts/RuntimeTypeSystem.mdDocuments the new runtime-type-system API.
docs/design/datacontracts/ObjectiveCMarshal.mdAdds design documentation for the new contract.
docs/design/datacontracts/Object.mdDocuments the new sync-block lookup helper in Object contract.

@github-actions

This comment has been minimized.

CopilotAI review requested due to automatic review settings May 6, 2026 17:36
CopilotAI reviewed May 6, 2026

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.

Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #125895

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: This PR adds a new ObjectiveCMarshal cDAC contract to expose Objective-C interop tagged memory information through the diagnostic data contract reader, replacing the legacy DAC fallback for ISOSDacInterface11.IsTrackedType and GetTaggedMemory. It also refactors the Object contract to extract GetSyncBlockAddress as a reusable API. The motivation is clear and well-justified — enabling the cDAC path for macOS Objective-C diagnostic scenarios.

Approach: The approach is sound and follows established cDAC patterns exactly. A new IObjectiveCMarshal contract with a single GetTaggedMemory API, a helper IsTrackedReferenceWithFinalizer on IRuntimeTypeSystem, and good code deduplication by extracting sync block lookup into GetSyncBlockAddress. The conditional contract registration via #ifdef FEATURE_OBJCMARSHAL and the TryGetContract pattern for graceful fallback on non-macOS targets is the correct approach.

Summary: ✅ LGTM. The implementation correctly mirrors the legacy C++ DAC behavior (ClrDataAccess::IsTrackedType / ClrDataAccess::GetTaggedMemory), the flag value matches the runtime definition, field reads use TryGetValue for conditional feature availability, and the test coverage is comprehensive with both unit and dump-based integration tests.


Detailed Findings

✅ Correctness — SOSDacImpl matches legacy DAC semantics

The new IsTrackedType implementation correctly:

  • Returns S_OK when tracked, S_FALSE when not (matching request.cpp:5445-5447)
  • Returns E_INVALIDARG for null pointers (via ArgumentException.HResult)
  • Checks hasTaggedMemory independently using TryGetContract<IObjectiveCMarshal> — gracefully no-ops when the contract isn't available (non-macOS)
  • The GetTaggedMemory method returns S_FALSE when no tagged memory exists and S_OK on success

The #if DEBUG validation blocks correctly compare against the legacy implementation.

✅ Flag Value — IsTrackedReferenceWithFinalizer = 0x04000000

The flag value in MethodTableFlags_1.cs matches exactly with enum_flag_IsTrackedReferenceWithFinalizer in src/coreclr/vm/methodtable.h:3821. The [cDAC] annotation comment was correctly added to the runtime header to signal this dependency.

✅ Data Descriptor — Conditional field exposure

datadescriptor.inc correctly wraps the TaggedMemory field under #ifdef FEATURE_OBJCMARSHAL, and InteropSyncBlockInfo.cs uses TryGetValue for graceful fallback when the field isn't present. This is consistent with how RCW/CCW/CCF handle conditional FEATURE_COMINTEROP.

✅ Refactoring — GetSyncBlockAddress extraction

The extracted GetSyncBlockAddress method correctly centralizes sync block index lookup logic. The condition (syncBlockValue & (_syncBlockIsHashCode | _syncBlockIsHashOrSyncBlockIndex)) != _syncBlockIsHashOrSyncBlockIndex correctly identifies sync block indices (not inline hash codes). The removal of _syncTableEntries field is correct — the lookup now delegates to _target.Contracts.SyncBlock.GetSyncBlock(index).

✅ Tagged Memory Size — Hardcoded 2 * PointerSize

new TargetNUInt(2 * (ulong)_target.PointerSize) matches the runtime's PTR_VOID m_taggedAlloc[NUM_TAGGED_MEMORY_CALLBACKS] where NUM_TAGGED_MEMORY_CALLBACKS = 2.

✅ Test Coverage — Comprehensive

  • Unit tests (ObjectiveCMarshalTests.cs): Three key scenarios tested — no sync block, null tagged memory, valid tagged memory with size assertion.
  • Dump integration tests (ObjectiveCMarshalDumpTests.cs): Gated to macOS with [SkipOnOS(IncludeOnly = "osx")].
  • Build infrastructure: MacOnly filtering mirrors the existing WindowsOnly pattern.
  • Mock infrastructure: Extended correctly with taggedMemory parameter.

💡 Minor — Redundant object header read in TryGetHashCode (non-blocking, follow-up)

In Object_1.cs, TryGetHashCode reads the object header (lines 110-112), then if hash isn't inline, calls GetSyncBlockAddress which reads the same header again (lines 136-138). Since ProcessedData.GetOrAdd caches by address, this is functionally a no-op. Acceptable for diagnostic tooling.

✅ Contract Registration — Properly ordered

Registrations maintain alphabetical ordering in CoreCLRContracts.cs, datadescriptor.inc, and ContractRegistry.cs.

Generated by Code Review for issue #125895 ·

@rcj1
rcj1 marked this pull request as ready for review May 6, 2026 20:24
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

1 similar comment
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@max-charlamb@rcj1@noahfalk
, '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 ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API - #125895

Merged
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis
May 6, 2026
Merged

Add cDAC ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API#125895
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis

Conversation

CopilotAI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements cDAC APIs for Objective-C interop diagnostics as a new ObjectiveCMarshal contract, plus adds IsTrackedReferenceWithFinalizer to the RuntimeTypeSystem contract. Based on draft PR #118250, adjusted per review feedback (contract renamed, TargetNUInt instead of nuint, APIs in dedicated contract).

Note

This PR was generated with GitHub Copilot.

Changes

New ObjectiveCMarshal cDAC Contract

  • IObjectiveCMarshal (Abstractions/Contracts/IObjectiveCMarshal.cs): Interface with a single API:

    TargetPointerGetTaggedMemory(TargetPointeraddress,outTargetNUIntsize);

    Returns the tagged memory pointer for an Objective-C tracked reference object (or TargetPointer.Null if none). Sets size to 2 * pointerSize bytes only on success.

  • ObjectiveCMarshal_1 (Contracts/ObjectiveCMarshal_1.cs): Version 1 implementation — reads the sync block from the object header, then extracts InteropSyncBlockInfo.TaggedMemory.

  • ObjectiveCMarshalFactory (Contracts/ObjectiveCMarshalFactory.cs): Contract factory.

  • Registered in ContractRegistry and CachingContractRegistry.

  • datadescriptor.inc: Added TaggedMemory field to InteropSyncBlockInfo type (under #ifdef FEATURE_OBJCMARSHAL) and CDAC_GLOBAL_CONTRACT(ObjectiveCMarshal, 1) (also guarded).

  • syncblk.h: Added TaggedMemory offset to cdac_data<InteropSyncBlockInfo> under #ifdef FEATURE_OBJCMARSHAL.

  • InteropSyncBlockInfo.cs (Data class): Added TaggedMemory property using TryGetValue to handle the optional FEATURE_OBJCMARSHAL field.

IsTrackedReferenceWithFinalizer on RuntimeTypeSystem

  • Added IsTrackedReferenceWithFinalizer = 0x04000000 to WFLAGS_HIGH enum in MethodTableFlags_1.cs, plus a convenience property bool IsTrackedReferenceWithFinalizer.

  • Added IsTrackedReferenceWithFinalizer(TypeHandle) to IRuntimeTypeSystem.cs and implemented it in RuntimeTypeSystem_1.cs.

SOSDacImpl.csISOSDacInterface11

Replaced the stub implementations of IsTrackedType and GetTaggedMemory with full cDAC implementations:

  • IsTrackedType: uses IRuntimeTypeSystem.IsTrackedReferenceWithFinalizer and IObjectiveCMarshal.GetTaggedMemory; returns S_OK if tracked, S_FALSE if not, E_INVALIDARG for null inputs.
  • GetTaggedMemory: uses IObjectiveCMarshal.GetTaggedMemory; returns S_OK with address and size if tagged memory exists, S_FALSE otherwise.

Both methods include #if DEBUG assertions against the legacy DAC implementation for validation.

Documentation

  • docs/design/datacontracts/ObjectiveCMarshal.md: New contract documentation.
  • docs/design/datacontracts/RuntimeTypeSystem.md: Added IsTrackedReferenceWithFinalizer to API listing and pseudocode.

Testing

All 1322 existing cDAC unit tests pass. The new contract will be exercised when running on an Apple platform with FEATURE_OBJCMARSHAL enabled.

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

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

LGTM - one refactor suggestion inline

Comment threaddocs/design/datacontracts/ObjectiveCMarshal.md Outdated
max-charlamb added a commit that referenced this pull request Apr 17, 2026
> [!NOTE]
> This PR description was generated with the assistance of GitHub
Copilot.
## Summary
Add a granular, per-method allowlist (`LegacyFallbackHelper`) that
controls which delegation-only APIs may fall back to the legacy DAC when
`CDAC_NO_FALLBACK=1` is set. This enables selective no-fallback testing
— blocking fallback for most APIs while allowing specific APIs that are
known to not yet be implemented in the cDAC.
Wire this into the runtime-diagnostics CI pipeline using the
`-noFallback` flag from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806).
All fallback attempts (both allowed and blocked) are logged to stderr
with method name, file, and line number for capture by the diagnostics
test infrastructure.
## Changes
### LegacyFallbackHelper.cs — Granular fallback control
New static helper that every delegation-only call site invokes via
`CanFallback()`. Uses `[CallerMemberName]`, `[CallerFilePath]`, and
`[CallerLineNumber]` to identify the call site.
- **Normal mode** (`CDAC_NO_FALLBACK` unset): Always returns `true`
(single `bool` check, `[AggressiveInlining]`)
- **No-fallback mode** (`CDAC_NO_FALLBACK=1`): Checks method name
against a `HashSet<string>` allowlist and file name against a file-level
allowlist
**Per-method allowlist:**
| Method | Reason |
|--------|--------|
| `EnumMemoryRegions` | Dump creation — cDAC has no memory enumeration
implementation |
| `GetInterface` | IMetaDataImport QI ([PR
#127028](#127028)) |
| `GetMethodDefinitionByToken` | IXCLRDataModule — not yet implemented
in cDAC |
| `IsTrackedType` | GC heap analysis ([PR
#125895](#125895)) |
| `TraverseLoaderHeap` | Loader heap traversal ([PR
#125129](#125129)) |
**File-level allowlist:**
| File | Reason |
|------|--------|
| `DacDbiImpl.cs` | Entire DBI/ICorDebug interface (122 methods) —
deferred |
### Entrypoints.cs — Simplified creation
Both `CreateSosInterface` and `CreateDacDbiInterface` now follow the
same pattern: the legacy implementation is always passed through, and
`LegacyFallbackHelper.CanFallback()` at each call site decides whether
to delegate. Removed `prevent_release`, `noFallback` env var check, and
null-legacy-ref logic.
### 13 Legacy wrapper files — Instrumented delegation sites
All 296 delegation-only methods across all legacy wrapper files now call
`LegacyFallbackHelper.CanFallback()`:
- `SOSDacImpl.cs` (12 methods)
- `SOSDacImpl.IXCLRDataProcess.cs` (38 methods, `Flush()` intentionally
excluded — cache management)
- `ClrDataModule.cs` (29 methods + IMetaDataImport QI)
- `DacDbiImpl.cs` (122 methods)
- Other wrappers: `ClrDataTask.cs`, `ClrDataExceptionState.cs`,
`ClrDataFrame.cs`, `ClrDataValue.cs`, `ClrDataTypeInstance.cs`,
`ClrDataMethodInstance.cs`, `ClrDataStackWalk.cs`, `ClrDataProcess.cs`
### CI Pipeline — `-noFallback` flag
Updated `runtime-diag-job.yml` to accept a `noFallback` parameter that
passes `-noFallback` to the diagnostics build script. The
`cDAC_no_fallback` leg in `runtime-diagnostics.yml` now uses
`noFallback: true` instead of setting `CDAC_NO_FALLBACK` as a
pipeline-level environment variable. The `-noFallback` flag (from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806))
properly:
- Sets `DOTNET_ENABLE_CDAC=1` and `CDAC_NO_FALLBACK=1` on the debugger
process
- Defines `CDAC_NO_FALLBACK_TESTING` to skip `ClrStack -i` tests
(ICorDebug not implemented in cDAC)
### Stderr logging
Every fallback attempt is logged to stderr in the format:
```
[cDAC] Allowed fallback: CreateStackWalk at DacDbiImpl.cs:590
[cDAC] Blocked fallback: SomeMethod at SOSDacImpl.cs:123
```
The diagnostics test infrastructure (`ProcessRunner`) captures stderr
and routes it to xunit test output with `STDERROR:` prefix, making
fallback usage visible in test results.
## Test Results
With `CDAC_NO_FALLBACK=1` and the current allowlist, running the full
SOS test suite against a private runtime build:
- **24 passed**, **2 failed** (flaky/pre-existing), **2 skipped**
(Linux-only)
- **0 blocked fallbacks**
## Motivation
The existing cDAC test leg always has the legacy DAC as a fallback, so
unimplemented APIs are silently handled. The granular no-fallback mode
makes gaps visible per-method, helping track progress toward full cDAC
coverage while keeping tests green for known-deferred APIs.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@max-charlamb

Copy link
Copy Markdown
Member

We should make sure to remove IsTrackedType from the LegacyFallbackHelper in this PR.

@max-charlamb

Copy link
Copy Markdown
Member

@copilot, please merge from main, fix any issues, then address feedback on this PR.

@rcj1

rcj1 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

@copilot@max-charlamb there is a reason this is not merged, and that is that we cannot have proper testing because the Mac tests are not set up. No action is needed at the moment.

Copilot stopped work on behalf of max-charlamb due to an error April 21, 2026 16:03
CopilotAI requested a review from max-charlambApril 21, 2026 16:03
Copilot stopped work on behalf of rcj1 due to an error April 21, 2026 16:03
…dReferenceWithFinalizer APIs
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fa8e5180-e07a-43c6-b618-58c6b60fd64e
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 5, 2026 17:52
@rcj1
rcj1force-pushed the copilot/add-cdac-apis branch from 3f77676 to 502054fCompareMay 5, 2026 17:52

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

Note

Copilot was unable to run its full agentic suite in this review.

Adds Objective-C interop support to the cDAC reader and legacy SOS DAC bridge by introducing a new ObjectiveCMarshal contract, exposing tracked-reference metadata, and adding unit/dump coverage around tagged memory.

Changes:

  • Add a new IObjectiveCMarshal contract and wire it through cDAC registration, runtime data descriptors, and legacy ISOSDacInterface11 implementations.
  • Extend object/sync-block data readers and test mocks to surface Objective-C tagged memory and tracked-reference-with-finalizer metadata.
  • Add macOS-specific dump-test infrastructure, a new ObjectiveCMarshal debuggee, and unit/integration tests for tagged-memory scenarios.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/ObjectiveCMarshalTests.csAdds unit tests for GetTaggedMemory contract behavior.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.SyncBlock.csExtends mock sync-block/interop info with tagged-memory support.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Object.csThreads tagged-memory data through mock object creation helpers.
src/native/managed/cdac/tests/DumpTests/ObjectiveCMarshalDumpTests.csAdds dump-based tests for tracked objects and tagged memory.
src/native/managed/cdac/tests/DumpTests/DumpTests.targetsFilters macOS-only debuggees on non-macOS hosts.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/Program.csAdds a macOS-only debuggee that creates a tracked object and crashes.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/ObjectiveCMarshal.csprojDeclares the new debuggee as macOS-only and full-dump capable.
src/native/managed/cdac/tests/DumpTests/Debuggees/Directory.Build.targetsPropagates MacOnly metadata from debuggee projects.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements ISOSDacInterface11 tagged-memory/tracked-type APIs via cDAC.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodTableFlags_1.csAdds the tracked-reference-with-finalizer method-table flag.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/InteropSyncBlockInfo.csReads tagged-memory pointer from interop sync-block info.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.csRegisters the new IObjectiveCMarshal contract implementation.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.csExposes IsTrackedReferenceWithFinalizer from method-table flags.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ObjectiveCMarshal_1.csImplements tagged-memory lookup from an object's sync block.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.csFactors sync-block address lookup into IObject.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.csAdds public contract API for tracked-reference-with-finalizer checks.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObjectiveCMarshal.csIntroduces the public Objective-C marshal contract API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObject.csAdds public sync-block address lookup API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.csExposes ObjectiveCMarshal from the contract registry.
src/coreclr/vm/syncblk.hPublishes tagged-memory field offset for cDAC data descriptors.
src/coreclr/vm/methodtable.hMarks tracked-reference-with-finalizer flag as cDAC-dependent.
src/coreclr/vm/datadescriptor/datadescriptor.incExposes tagged-memory field and ObjectiveCMarshal global contract.
docs/design/datacontracts/RuntimeTypeSystem.mdDocuments the new runtime-type-system API.
docs/design/datacontracts/ObjectiveCMarshal.mdAdds design documentation for the new contract.
docs/design/datacontracts/Object.mdDocuments the new sync-block lookup helper in Object contract.

@github-actions

This comment has been minimized.

CopilotAI review requested due to automatic review settings May 6, 2026 17:36
CopilotAI reviewed May 6, 2026

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.

Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #125895

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: This PR adds a new ObjectiveCMarshal cDAC contract to expose Objective-C interop tagged memory information through the diagnostic data contract reader, replacing the legacy DAC fallback for ISOSDacInterface11.IsTrackedType and GetTaggedMemory. It also refactors the Object contract to extract GetSyncBlockAddress as a reusable API. The motivation is clear and well-justified — enabling the cDAC path for macOS Objective-C diagnostic scenarios.

Approach: The approach is sound and follows established cDAC patterns exactly. A new IObjectiveCMarshal contract with a single GetTaggedMemory API, a helper IsTrackedReferenceWithFinalizer on IRuntimeTypeSystem, and good code deduplication by extracting sync block lookup into GetSyncBlockAddress. The conditional contract registration via #ifdef FEATURE_OBJCMARSHAL and the TryGetContract pattern for graceful fallback on non-macOS targets is the correct approach.

Summary: ✅ LGTM. The implementation correctly mirrors the legacy C++ DAC behavior (ClrDataAccess::IsTrackedType / ClrDataAccess::GetTaggedMemory), the flag value matches the runtime definition, field reads use TryGetValue for conditional feature availability, and the test coverage is comprehensive with both unit and dump-based integration tests.


Detailed Findings

✅ Correctness — SOSDacImpl matches legacy DAC semantics

The new IsTrackedType implementation correctly:

  • Returns S_OK when tracked, S_FALSE when not (matching request.cpp:5445-5447)
  • Returns E_INVALIDARG for null pointers (via ArgumentException.HResult)
  • Checks hasTaggedMemory independently using TryGetContract<IObjectiveCMarshal> — gracefully no-ops when the contract isn't available (non-macOS)
  • The GetTaggedMemory method returns S_FALSE when no tagged memory exists and S_OK on success

The #if DEBUG validation blocks correctly compare against the legacy implementation.

✅ Flag Value — IsTrackedReferenceWithFinalizer = 0x04000000

The flag value in MethodTableFlags_1.cs matches exactly with enum_flag_IsTrackedReferenceWithFinalizer in src/coreclr/vm/methodtable.h:3821. The [cDAC] annotation comment was correctly added to the runtime header to signal this dependency.

✅ Data Descriptor — Conditional field exposure

datadescriptor.inc correctly wraps the TaggedMemory field under #ifdef FEATURE_OBJCMARSHAL, and InteropSyncBlockInfo.cs uses TryGetValue for graceful fallback when the field isn't present. This is consistent with how RCW/CCW/CCF handle conditional FEATURE_COMINTEROP.

✅ Refactoring — GetSyncBlockAddress extraction

The extracted GetSyncBlockAddress method correctly centralizes sync block index lookup logic. The condition (syncBlockValue & (_syncBlockIsHashCode | _syncBlockIsHashOrSyncBlockIndex)) != _syncBlockIsHashOrSyncBlockIndex correctly identifies sync block indices (not inline hash codes). The removal of _syncTableEntries field is correct — the lookup now delegates to _target.Contracts.SyncBlock.GetSyncBlock(index).

✅ Tagged Memory Size — Hardcoded 2 * PointerSize

new TargetNUInt(2 * (ulong)_target.PointerSize) matches the runtime's PTR_VOID m_taggedAlloc[NUM_TAGGED_MEMORY_CALLBACKS] where NUM_TAGGED_MEMORY_CALLBACKS = 2.

✅ Test Coverage — Comprehensive

  • Unit tests (ObjectiveCMarshalTests.cs): Three key scenarios tested — no sync block, null tagged memory, valid tagged memory with size assertion.
  • Dump integration tests (ObjectiveCMarshalDumpTests.cs): Gated to macOS with [SkipOnOS(IncludeOnly = "osx")].
  • Build infrastructure: MacOnly filtering mirrors the existing WindowsOnly pattern.
  • Mock infrastructure: Extended correctly with taggedMemory parameter.

💡 Minor — Redundant object header read in TryGetHashCode (non-blocking, follow-up)

In Object_1.cs, TryGetHashCode reads the object header (lines 110-112), then if hash isn't inline, calls GetSyncBlockAddress which reads the same header again (lines 136-138). Since ProcessedData.GetOrAdd caches by address, this is functionally a no-op. Acceptable for diagnostic tooling.

✅ Contract Registration — Properly ordered

Registrations maintain alphabetical ordering in CoreCLRContracts.cs, datadescriptor.inc, and ContractRegistry.cs.

Generated by Code Review for issue #125895 ·

@rcj1
rcj1 marked this pull request as ready for review May 6, 2026 20:24
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

1 similar comment
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@max-charlamb@rcj1@noahfalk
, '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 ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API - #125895

Merged
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis
May 6, 2026
Merged

Add cDAC ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API#125895
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis

Conversation

CopilotAI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements cDAC APIs for Objective-C interop diagnostics as a new ObjectiveCMarshal contract, plus adds IsTrackedReferenceWithFinalizer to the RuntimeTypeSystem contract. Based on draft PR #118250, adjusted per review feedback (contract renamed, TargetNUInt instead of nuint, APIs in dedicated contract).

Note

This PR was generated with GitHub Copilot.

Changes

New ObjectiveCMarshal cDAC Contract

  • IObjectiveCMarshal (Abstractions/Contracts/IObjectiveCMarshal.cs): Interface with a single API:

    TargetPointerGetTaggedMemory(TargetPointeraddress,outTargetNUIntsize);

    Returns the tagged memory pointer for an Objective-C tracked reference object (or TargetPointer.Null if none). Sets size to 2 * pointerSize bytes only on success.

  • ObjectiveCMarshal_1 (Contracts/ObjectiveCMarshal_1.cs): Version 1 implementation — reads the sync block from the object header, then extracts InteropSyncBlockInfo.TaggedMemory.

  • ObjectiveCMarshalFactory (Contracts/ObjectiveCMarshalFactory.cs): Contract factory.

  • Registered in ContractRegistry and CachingContractRegistry.

  • datadescriptor.inc: Added TaggedMemory field to InteropSyncBlockInfo type (under #ifdef FEATURE_OBJCMARSHAL) and CDAC_GLOBAL_CONTRACT(ObjectiveCMarshal, 1) (also guarded).

  • syncblk.h: Added TaggedMemory offset to cdac_data<InteropSyncBlockInfo> under #ifdef FEATURE_OBJCMARSHAL.

  • InteropSyncBlockInfo.cs (Data class): Added TaggedMemory property using TryGetValue to handle the optional FEATURE_OBJCMARSHAL field.

IsTrackedReferenceWithFinalizer on RuntimeTypeSystem

  • Added IsTrackedReferenceWithFinalizer = 0x04000000 to WFLAGS_HIGH enum in MethodTableFlags_1.cs, plus a convenience property bool IsTrackedReferenceWithFinalizer.

  • Added IsTrackedReferenceWithFinalizer(TypeHandle) to IRuntimeTypeSystem.cs and implemented it in RuntimeTypeSystem_1.cs.

SOSDacImpl.csISOSDacInterface11

Replaced the stub implementations of IsTrackedType and GetTaggedMemory with full cDAC implementations:

  • IsTrackedType: uses IRuntimeTypeSystem.IsTrackedReferenceWithFinalizer and IObjectiveCMarshal.GetTaggedMemory; returns S_OK if tracked, S_FALSE if not, E_INVALIDARG for null inputs.
  • GetTaggedMemory: uses IObjectiveCMarshal.GetTaggedMemory; returns S_OK with address and size if tagged memory exists, S_FALSE otherwise.

Both methods include #if DEBUG assertions against the legacy DAC implementation for validation.

Documentation

  • docs/design/datacontracts/ObjectiveCMarshal.md: New contract documentation.
  • docs/design/datacontracts/RuntimeTypeSystem.md: Added IsTrackedReferenceWithFinalizer to API listing and pseudocode.

Testing

All 1322 existing cDAC unit tests pass. The new contract will be exercised when running on an Apple platform with FEATURE_OBJCMARSHAL enabled.

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

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

LGTM - one refactor suggestion inline

Comment threaddocs/design/datacontracts/ObjectiveCMarshal.md Outdated
max-charlamb added a commit that referenced this pull request Apr 17, 2026
> [!NOTE]
> This PR description was generated with the assistance of GitHub
Copilot.
## Summary
Add a granular, per-method allowlist (`LegacyFallbackHelper`) that
controls which delegation-only APIs may fall back to the legacy DAC when
`CDAC_NO_FALLBACK=1` is set. This enables selective no-fallback testing
— blocking fallback for most APIs while allowing specific APIs that are
known to not yet be implemented in the cDAC.
Wire this into the runtime-diagnostics CI pipeline using the
`-noFallback` flag from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806).
All fallback attempts (both allowed and blocked) are logged to stderr
with method name, file, and line number for capture by the diagnostics
test infrastructure.
## Changes
### LegacyFallbackHelper.cs — Granular fallback control
New static helper that every delegation-only call site invokes via
`CanFallback()`. Uses `[CallerMemberName]`, `[CallerFilePath]`, and
`[CallerLineNumber]` to identify the call site.
- **Normal mode** (`CDAC_NO_FALLBACK` unset): Always returns `true`
(single `bool` check, `[AggressiveInlining]`)
- **No-fallback mode** (`CDAC_NO_FALLBACK=1`): Checks method name
against a `HashSet<string>` allowlist and file name against a file-level
allowlist
**Per-method allowlist:**
| Method | Reason |
|--------|--------|
| `EnumMemoryRegions` | Dump creation — cDAC has no memory enumeration
implementation |
| `GetInterface` | IMetaDataImport QI ([PR
#127028](#127028)) |
| `GetMethodDefinitionByToken` | IXCLRDataModule — not yet implemented
in cDAC |
| `IsTrackedType` | GC heap analysis ([PR
#125895](#125895)) |
| `TraverseLoaderHeap` | Loader heap traversal ([PR
#125129](#125129)) |
**File-level allowlist:**
| File | Reason |
|------|--------|
| `DacDbiImpl.cs` | Entire DBI/ICorDebug interface (122 methods) —
deferred |
### Entrypoints.cs — Simplified creation
Both `CreateSosInterface` and `CreateDacDbiInterface` now follow the
same pattern: the legacy implementation is always passed through, and
`LegacyFallbackHelper.CanFallback()` at each call site decides whether
to delegate. Removed `prevent_release`, `noFallback` env var check, and
null-legacy-ref logic.
### 13 Legacy wrapper files — Instrumented delegation sites
All 296 delegation-only methods across all legacy wrapper files now call
`LegacyFallbackHelper.CanFallback()`:
- `SOSDacImpl.cs` (12 methods)
- `SOSDacImpl.IXCLRDataProcess.cs` (38 methods, `Flush()` intentionally
excluded — cache management)
- `ClrDataModule.cs` (29 methods + IMetaDataImport QI)
- `DacDbiImpl.cs` (122 methods)
- Other wrappers: `ClrDataTask.cs`, `ClrDataExceptionState.cs`,
`ClrDataFrame.cs`, `ClrDataValue.cs`, `ClrDataTypeInstance.cs`,
`ClrDataMethodInstance.cs`, `ClrDataStackWalk.cs`, `ClrDataProcess.cs`
### CI Pipeline — `-noFallback` flag
Updated `runtime-diag-job.yml` to accept a `noFallback` parameter that
passes `-noFallback` to the diagnostics build script. The
`cDAC_no_fallback` leg in `runtime-diagnostics.yml` now uses
`noFallback: true` instead of setting `CDAC_NO_FALLBACK` as a
pipeline-level environment variable. The `-noFallback` flag (from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806))
properly:
- Sets `DOTNET_ENABLE_CDAC=1` and `CDAC_NO_FALLBACK=1` on the debugger
process
- Defines `CDAC_NO_FALLBACK_TESTING` to skip `ClrStack -i` tests
(ICorDebug not implemented in cDAC)
### Stderr logging
Every fallback attempt is logged to stderr in the format:
```
[cDAC] Allowed fallback: CreateStackWalk at DacDbiImpl.cs:590
[cDAC] Blocked fallback: SomeMethod at SOSDacImpl.cs:123
```
The diagnostics test infrastructure (`ProcessRunner`) captures stderr
and routes it to xunit test output with `STDERROR:` prefix, making
fallback usage visible in test results.
## Test Results
With `CDAC_NO_FALLBACK=1` and the current allowlist, running the full
SOS test suite against a private runtime build:
- **24 passed**, **2 failed** (flaky/pre-existing), **2 skipped**
(Linux-only)
- **0 blocked fallbacks**
## Motivation
The existing cDAC test leg always has the legacy DAC as a fallback, so
unimplemented APIs are silently handled. The granular no-fallback mode
makes gaps visible per-method, helping track progress toward full cDAC
coverage while keeping tests green for known-deferred APIs.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@max-charlamb

Copy link
Copy Markdown
Member

We should make sure to remove IsTrackedType from the LegacyFallbackHelper in this PR.

@max-charlamb

Copy link
Copy Markdown
Member

@copilot, please merge from main, fix any issues, then address feedback on this PR.

@rcj1

rcj1 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

@copilot@max-charlamb there is a reason this is not merged, and that is that we cannot have proper testing because the Mac tests are not set up. No action is needed at the moment.

Copilot stopped work on behalf of max-charlamb due to an error April 21, 2026 16:03
CopilotAI requested a review from max-charlambApril 21, 2026 16:03
Copilot stopped work on behalf of rcj1 due to an error April 21, 2026 16:03
…dReferenceWithFinalizer APIs
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fa8e5180-e07a-43c6-b618-58c6b60fd64e
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 5, 2026 17:52
@rcj1
rcj1force-pushed the copilot/add-cdac-apis branch from 3f77676 to 502054fCompareMay 5, 2026 17:52

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

Note

Copilot was unable to run its full agentic suite in this review.

Adds Objective-C interop support to the cDAC reader and legacy SOS DAC bridge by introducing a new ObjectiveCMarshal contract, exposing tracked-reference metadata, and adding unit/dump coverage around tagged memory.

Changes:

  • Add a new IObjectiveCMarshal contract and wire it through cDAC registration, runtime data descriptors, and legacy ISOSDacInterface11 implementations.
  • Extend object/sync-block data readers and test mocks to surface Objective-C tagged memory and tracked-reference-with-finalizer metadata.
  • Add macOS-specific dump-test infrastructure, a new ObjectiveCMarshal debuggee, and unit/integration tests for tagged-memory scenarios.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/ObjectiveCMarshalTests.csAdds unit tests for GetTaggedMemory contract behavior.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.SyncBlock.csExtends mock sync-block/interop info with tagged-memory support.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Object.csThreads tagged-memory data through mock object creation helpers.
src/native/managed/cdac/tests/DumpTests/ObjectiveCMarshalDumpTests.csAdds dump-based tests for tracked objects and tagged memory.
src/native/managed/cdac/tests/DumpTests/DumpTests.targetsFilters macOS-only debuggees on non-macOS hosts.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/Program.csAdds a macOS-only debuggee that creates a tracked object and crashes.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/ObjectiveCMarshal.csprojDeclares the new debuggee as macOS-only and full-dump capable.
src/native/managed/cdac/tests/DumpTests/Debuggees/Directory.Build.targetsPropagates MacOnly metadata from debuggee projects.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements ISOSDacInterface11 tagged-memory/tracked-type APIs via cDAC.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodTableFlags_1.csAdds the tracked-reference-with-finalizer method-table flag.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/InteropSyncBlockInfo.csReads tagged-memory pointer from interop sync-block info.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.csRegisters the new IObjectiveCMarshal contract implementation.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.csExposes IsTrackedReferenceWithFinalizer from method-table flags.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ObjectiveCMarshal_1.csImplements tagged-memory lookup from an object's sync block.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.csFactors sync-block address lookup into IObject.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.csAdds public contract API for tracked-reference-with-finalizer checks.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObjectiveCMarshal.csIntroduces the public Objective-C marshal contract API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObject.csAdds public sync-block address lookup API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.csExposes ObjectiveCMarshal from the contract registry.
src/coreclr/vm/syncblk.hPublishes tagged-memory field offset for cDAC data descriptors.
src/coreclr/vm/methodtable.hMarks tracked-reference-with-finalizer flag as cDAC-dependent.
src/coreclr/vm/datadescriptor/datadescriptor.incExposes tagged-memory field and ObjectiveCMarshal global contract.
docs/design/datacontracts/RuntimeTypeSystem.mdDocuments the new runtime-type-system API.
docs/design/datacontracts/ObjectiveCMarshal.mdAdds design documentation for the new contract.
docs/design/datacontracts/Object.mdDocuments the new sync-block lookup helper in Object contract.

@github-actions

This comment has been minimized.

CopilotAI review requested due to automatic review settings May 6, 2026 17:36
CopilotAI reviewed May 6, 2026

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.

Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #125895

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: This PR adds a new ObjectiveCMarshal cDAC contract to expose Objective-C interop tagged memory information through the diagnostic data contract reader, replacing the legacy DAC fallback for ISOSDacInterface11.IsTrackedType and GetTaggedMemory. It also refactors the Object contract to extract GetSyncBlockAddress as a reusable API. The motivation is clear and well-justified — enabling the cDAC path for macOS Objective-C diagnostic scenarios.

Approach: The approach is sound and follows established cDAC patterns exactly. A new IObjectiveCMarshal contract with a single GetTaggedMemory API, a helper IsTrackedReferenceWithFinalizer on IRuntimeTypeSystem, and good code deduplication by extracting sync block lookup into GetSyncBlockAddress. The conditional contract registration via #ifdef FEATURE_OBJCMARSHAL and the TryGetContract pattern for graceful fallback on non-macOS targets is the correct approach.

Summary: ✅ LGTM. The implementation correctly mirrors the legacy C++ DAC behavior (ClrDataAccess::IsTrackedType / ClrDataAccess::GetTaggedMemory), the flag value matches the runtime definition, field reads use TryGetValue for conditional feature availability, and the test coverage is comprehensive with both unit and dump-based integration tests.


Detailed Findings

✅ Correctness — SOSDacImpl matches legacy DAC semantics

The new IsTrackedType implementation correctly:

  • Returns S_OK when tracked, S_FALSE when not (matching request.cpp:5445-5447)
  • Returns E_INVALIDARG for null pointers (via ArgumentException.HResult)
  • Checks hasTaggedMemory independently using TryGetContract<IObjectiveCMarshal> — gracefully no-ops when the contract isn't available (non-macOS)
  • The GetTaggedMemory method returns S_FALSE when no tagged memory exists and S_OK on success

The #if DEBUG validation blocks correctly compare against the legacy implementation.

✅ Flag Value — IsTrackedReferenceWithFinalizer = 0x04000000

The flag value in MethodTableFlags_1.cs matches exactly with enum_flag_IsTrackedReferenceWithFinalizer in src/coreclr/vm/methodtable.h:3821. The [cDAC] annotation comment was correctly added to the runtime header to signal this dependency.

✅ Data Descriptor — Conditional field exposure

datadescriptor.inc correctly wraps the TaggedMemory field under #ifdef FEATURE_OBJCMARSHAL, and InteropSyncBlockInfo.cs uses TryGetValue for graceful fallback when the field isn't present. This is consistent with how RCW/CCW/CCF handle conditional FEATURE_COMINTEROP.

✅ Refactoring — GetSyncBlockAddress extraction

The extracted GetSyncBlockAddress method correctly centralizes sync block index lookup logic. The condition (syncBlockValue & (_syncBlockIsHashCode | _syncBlockIsHashOrSyncBlockIndex)) != _syncBlockIsHashOrSyncBlockIndex correctly identifies sync block indices (not inline hash codes). The removal of _syncTableEntries field is correct — the lookup now delegates to _target.Contracts.SyncBlock.GetSyncBlock(index).

✅ Tagged Memory Size — Hardcoded 2 * PointerSize

new TargetNUInt(2 * (ulong)_target.PointerSize) matches the runtime's PTR_VOID m_taggedAlloc[NUM_TAGGED_MEMORY_CALLBACKS] where NUM_TAGGED_MEMORY_CALLBACKS = 2.

✅ Test Coverage — Comprehensive

  • Unit tests (ObjectiveCMarshalTests.cs): Three key scenarios tested — no sync block, null tagged memory, valid tagged memory with size assertion.
  • Dump integration tests (ObjectiveCMarshalDumpTests.cs): Gated to macOS with [SkipOnOS(IncludeOnly = "osx")].
  • Build infrastructure: MacOnly filtering mirrors the existing WindowsOnly pattern.
  • Mock infrastructure: Extended correctly with taggedMemory parameter.

💡 Minor — Redundant object header read in TryGetHashCode (non-blocking, follow-up)

In Object_1.cs, TryGetHashCode reads the object header (lines 110-112), then if hash isn't inline, calls GetSyncBlockAddress which reads the same header again (lines 136-138). Since ProcessedData.GetOrAdd caches by address, this is functionally a no-op. Acceptable for diagnostic tooling.

✅ Contract Registration — Properly ordered

Registrations maintain alphabetical ordering in CoreCLRContracts.cs, datadescriptor.inc, and ContractRegistry.cs.

Generated by Code Review for issue #125895 ·

@rcj1
rcj1 marked this pull request as ready for review May 6, 2026 20:24
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

1 similar comment
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@max-charlamb@rcj1@noahfalk
, '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 ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API - #125895

Merged
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis
May 6, 2026
Merged

Add cDAC ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API#125895
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis

Conversation

CopilotAI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements cDAC APIs for Objective-C interop diagnostics as a new ObjectiveCMarshal contract, plus adds IsTrackedReferenceWithFinalizer to the RuntimeTypeSystem contract. Based on draft PR #118250, adjusted per review feedback (contract renamed, TargetNUInt instead of nuint, APIs in dedicated contract).

Note

This PR was generated with GitHub Copilot.

Changes

New ObjectiveCMarshal cDAC Contract

  • IObjectiveCMarshal (Abstractions/Contracts/IObjectiveCMarshal.cs): Interface with a single API:

    TargetPointerGetTaggedMemory(TargetPointeraddress,outTargetNUIntsize);

    Returns the tagged memory pointer for an Objective-C tracked reference object (or TargetPointer.Null if none). Sets size to 2 * pointerSize bytes only on success.

  • ObjectiveCMarshal_1 (Contracts/ObjectiveCMarshal_1.cs): Version 1 implementation — reads the sync block from the object header, then extracts InteropSyncBlockInfo.TaggedMemory.

  • ObjectiveCMarshalFactory (Contracts/ObjectiveCMarshalFactory.cs): Contract factory.

  • Registered in ContractRegistry and CachingContractRegistry.

  • datadescriptor.inc: Added TaggedMemory field to InteropSyncBlockInfo type (under #ifdef FEATURE_OBJCMARSHAL) and CDAC_GLOBAL_CONTRACT(ObjectiveCMarshal, 1) (also guarded).

  • syncblk.h: Added TaggedMemory offset to cdac_data<InteropSyncBlockInfo> under #ifdef FEATURE_OBJCMARSHAL.

  • InteropSyncBlockInfo.cs (Data class): Added TaggedMemory property using TryGetValue to handle the optional FEATURE_OBJCMARSHAL field.

IsTrackedReferenceWithFinalizer on RuntimeTypeSystem

  • Added IsTrackedReferenceWithFinalizer = 0x04000000 to WFLAGS_HIGH enum in MethodTableFlags_1.cs, plus a convenience property bool IsTrackedReferenceWithFinalizer.

  • Added IsTrackedReferenceWithFinalizer(TypeHandle) to IRuntimeTypeSystem.cs and implemented it in RuntimeTypeSystem_1.cs.

SOSDacImpl.csISOSDacInterface11

Replaced the stub implementations of IsTrackedType and GetTaggedMemory with full cDAC implementations:

  • IsTrackedType: uses IRuntimeTypeSystem.IsTrackedReferenceWithFinalizer and IObjectiveCMarshal.GetTaggedMemory; returns S_OK if tracked, S_FALSE if not, E_INVALIDARG for null inputs.
  • GetTaggedMemory: uses IObjectiveCMarshal.GetTaggedMemory; returns S_OK with address and size if tagged memory exists, S_FALSE otherwise.

Both methods include #if DEBUG assertions against the legacy DAC implementation for validation.

Documentation

  • docs/design/datacontracts/ObjectiveCMarshal.md: New contract documentation.
  • docs/design/datacontracts/RuntimeTypeSystem.md: Added IsTrackedReferenceWithFinalizer to API listing and pseudocode.

Testing

All 1322 existing cDAC unit tests pass. The new contract will be exercised when running on an Apple platform with FEATURE_OBJCMARSHAL enabled.

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

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

LGTM - one refactor suggestion inline

Comment threaddocs/design/datacontracts/ObjectiveCMarshal.md Outdated
max-charlamb added a commit that referenced this pull request Apr 17, 2026
> [!NOTE]
> This PR description was generated with the assistance of GitHub
Copilot.
## Summary
Add a granular, per-method allowlist (`LegacyFallbackHelper`) that
controls which delegation-only APIs may fall back to the legacy DAC when
`CDAC_NO_FALLBACK=1` is set. This enables selective no-fallback testing
— blocking fallback for most APIs while allowing specific APIs that are
known to not yet be implemented in the cDAC.
Wire this into the runtime-diagnostics CI pipeline using the
`-noFallback` flag from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806).
All fallback attempts (both allowed and blocked) are logged to stderr
with method name, file, and line number for capture by the diagnostics
test infrastructure.
## Changes
### LegacyFallbackHelper.cs — Granular fallback control
New static helper that every delegation-only call site invokes via
`CanFallback()`. Uses `[CallerMemberName]`, `[CallerFilePath]`, and
`[CallerLineNumber]` to identify the call site.
- **Normal mode** (`CDAC_NO_FALLBACK` unset): Always returns `true`
(single `bool` check, `[AggressiveInlining]`)
- **No-fallback mode** (`CDAC_NO_FALLBACK=1`): Checks method name
against a `HashSet<string>` allowlist and file name against a file-level
allowlist
**Per-method allowlist:**
| Method | Reason |
|--------|--------|
| `EnumMemoryRegions` | Dump creation — cDAC has no memory enumeration
implementation |
| `GetInterface` | IMetaDataImport QI ([PR
#127028](#127028)) |
| `GetMethodDefinitionByToken` | IXCLRDataModule — not yet implemented
in cDAC |
| `IsTrackedType` | GC heap analysis ([PR
#125895](#125895)) |
| `TraverseLoaderHeap` | Loader heap traversal ([PR
#125129](#125129)) |
**File-level allowlist:**
| File | Reason |
|------|--------|
| `DacDbiImpl.cs` | Entire DBI/ICorDebug interface (122 methods) —
deferred |
### Entrypoints.cs — Simplified creation
Both `CreateSosInterface` and `CreateDacDbiInterface` now follow the
same pattern: the legacy implementation is always passed through, and
`LegacyFallbackHelper.CanFallback()` at each call site decides whether
to delegate. Removed `prevent_release`, `noFallback` env var check, and
null-legacy-ref logic.
### 13 Legacy wrapper files — Instrumented delegation sites
All 296 delegation-only methods across all legacy wrapper files now call
`LegacyFallbackHelper.CanFallback()`:
- `SOSDacImpl.cs` (12 methods)
- `SOSDacImpl.IXCLRDataProcess.cs` (38 methods, `Flush()` intentionally
excluded — cache management)
- `ClrDataModule.cs` (29 methods + IMetaDataImport QI)
- `DacDbiImpl.cs` (122 methods)
- Other wrappers: `ClrDataTask.cs`, `ClrDataExceptionState.cs`,
`ClrDataFrame.cs`, `ClrDataValue.cs`, `ClrDataTypeInstance.cs`,
`ClrDataMethodInstance.cs`, `ClrDataStackWalk.cs`, `ClrDataProcess.cs`
### CI Pipeline — `-noFallback` flag
Updated `runtime-diag-job.yml` to accept a `noFallback` parameter that
passes `-noFallback` to the diagnostics build script. The
`cDAC_no_fallback` leg in `runtime-diagnostics.yml` now uses
`noFallback: true` instead of setting `CDAC_NO_FALLBACK` as a
pipeline-level environment variable. The `-noFallback` flag (from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806))
properly:
- Sets `DOTNET_ENABLE_CDAC=1` and `CDAC_NO_FALLBACK=1` on the debugger
process
- Defines `CDAC_NO_FALLBACK_TESTING` to skip `ClrStack -i` tests
(ICorDebug not implemented in cDAC)
### Stderr logging
Every fallback attempt is logged to stderr in the format:
```
[cDAC] Allowed fallback: CreateStackWalk at DacDbiImpl.cs:590
[cDAC] Blocked fallback: SomeMethod at SOSDacImpl.cs:123
```
The diagnostics test infrastructure (`ProcessRunner`) captures stderr
and routes it to xunit test output with `STDERROR:` prefix, making
fallback usage visible in test results.
## Test Results
With `CDAC_NO_FALLBACK=1` and the current allowlist, running the full
SOS test suite against a private runtime build:
- **24 passed**, **2 failed** (flaky/pre-existing), **2 skipped**
(Linux-only)
- **0 blocked fallbacks**
## Motivation
The existing cDAC test leg always has the legacy DAC as a fallback, so
unimplemented APIs are silently handled. The granular no-fallback mode
makes gaps visible per-method, helping track progress toward full cDAC
coverage while keeping tests green for known-deferred APIs.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@max-charlamb

Copy link
Copy Markdown
Member

We should make sure to remove IsTrackedType from the LegacyFallbackHelper in this PR.

@max-charlamb

Copy link
Copy Markdown
Member

@copilot, please merge from main, fix any issues, then address feedback on this PR.

@rcj1

rcj1 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

@copilot@max-charlamb there is a reason this is not merged, and that is that we cannot have proper testing because the Mac tests are not set up. No action is needed at the moment.

Copilot stopped work on behalf of max-charlamb due to an error April 21, 2026 16:03
CopilotAI requested a review from max-charlambApril 21, 2026 16:03
Copilot stopped work on behalf of rcj1 due to an error April 21, 2026 16:03
…dReferenceWithFinalizer APIs
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fa8e5180-e07a-43c6-b618-58c6b60fd64e
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 5, 2026 17:52
@rcj1
rcj1force-pushed the copilot/add-cdac-apis branch from 3f77676 to 502054fCompareMay 5, 2026 17:52

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

Note

Copilot was unable to run its full agentic suite in this review.

Adds Objective-C interop support to the cDAC reader and legacy SOS DAC bridge by introducing a new ObjectiveCMarshal contract, exposing tracked-reference metadata, and adding unit/dump coverage around tagged memory.

Changes:

  • Add a new IObjectiveCMarshal contract and wire it through cDAC registration, runtime data descriptors, and legacy ISOSDacInterface11 implementations.
  • Extend object/sync-block data readers and test mocks to surface Objective-C tagged memory and tracked-reference-with-finalizer metadata.
  • Add macOS-specific dump-test infrastructure, a new ObjectiveCMarshal debuggee, and unit/integration tests for tagged-memory scenarios.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/ObjectiveCMarshalTests.csAdds unit tests for GetTaggedMemory contract behavior.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.SyncBlock.csExtends mock sync-block/interop info with tagged-memory support.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Object.csThreads tagged-memory data through mock object creation helpers.
src/native/managed/cdac/tests/DumpTests/ObjectiveCMarshalDumpTests.csAdds dump-based tests for tracked objects and tagged memory.
src/native/managed/cdac/tests/DumpTests/DumpTests.targetsFilters macOS-only debuggees on non-macOS hosts.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/Program.csAdds a macOS-only debuggee that creates a tracked object and crashes.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/ObjectiveCMarshal.csprojDeclares the new debuggee as macOS-only and full-dump capable.
src/native/managed/cdac/tests/DumpTests/Debuggees/Directory.Build.targetsPropagates MacOnly metadata from debuggee projects.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements ISOSDacInterface11 tagged-memory/tracked-type APIs via cDAC.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodTableFlags_1.csAdds the tracked-reference-with-finalizer method-table flag.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/InteropSyncBlockInfo.csReads tagged-memory pointer from interop sync-block info.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.csRegisters the new IObjectiveCMarshal contract implementation.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.csExposes IsTrackedReferenceWithFinalizer from method-table flags.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ObjectiveCMarshal_1.csImplements tagged-memory lookup from an object's sync block.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.csFactors sync-block address lookup into IObject.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.csAdds public contract API for tracked-reference-with-finalizer checks.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObjectiveCMarshal.csIntroduces the public Objective-C marshal contract API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObject.csAdds public sync-block address lookup API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.csExposes ObjectiveCMarshal from the contract registry.
src/coreclr/vm/syncblk.hPublishes tagged-memory field offset for cDAC data descriptors.
src/coreclr/vm/methodtable.hMarks tracked-reference-with-finalizer flag as cDAC-dependent.
src/coreclr/vm/datadescriptor/datadescriptor.incExposes tagged-memory field and ObjectiveCMarshal global contract.
docs/design/datacontracts/RuntimeTypeSystem.mdDocuments the new runtime-type-system API.
docs/design/datacontracts/ObjectiveCMarshal.mdAdds design documentation for the new contract.
docs/design/datacontracts/Object.mdDocuments the new sync-block lookup helper in Object contract.

@github-actions

This comment has been minimized.

CopilotAI review requested due to automatic review settings May 6, 2026 17:36
CopilotAI reviewed May 6, 2026

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.

Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #125895

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: This PR adds a new ObjectiveCMarshal cDAC contract to expose Objective-C interop tagged memory information through the diagnostic data contract reader, replacing the legacy DAC fallback for ISOSDacInterface11.IsTrackedType and GetTaggedMemory. It also refactors the Object contract to extract GetSyncBlockAddress as a reusable API. The motivation is clear and well-justified — enabling the cDAC path for macOS Objective-C diagnostic scenarios.

Approach: The approach is sound and follows established cDAC patterns exactly. A new IObjectiveCMarshal contract with a single GetTaggedMemory API, a helper IsTrackedReferenceWithFinalizer on IRuntimeTypeSystem, and good code deduplication by extracting sync block lookup into GetSyncBlockAddress. The conditional contract registration via #ifdef FEATURE_OBJCMARSHAL and the TryGetContract pattern for graceful fallback on non-macOS targets is the correct approach.

Summary: ✅ LGTM. The implementation correctly mirrors the legacy C++ DAC behavior (ClrDataAccess::IsTrackedType / ClrDataAccess::GetTaggedMemory), the flag value matches the runtime definition, field reads use TryGetValue for conditional feature availability, and the test coverage is comprehensive with both unit and dump-based integration tests.


Detailed Findings

✅ Correctness — SOSDacImpl matches legacy DAC semantics

The new IsTrackedType implementation correctly:

  • Returns S_OK when tracked, S_FALSE when not (matching request.cpp:5445-5447)
  • Returns E_INVALIDARG for null pointers (via ArgumentException.HResult)
  • Checks hasTaggedMemory independently using TryGetContract<IObjectiveCMarshal> — gracefully no-ops when the contract isn't available (non-macOS)
  • The GetTaggedMemory method returns S_FALSE when no tagged memory exists and S_OK on success

The #if DEBUG validation blocks correctly compare against the legacy implementation.

✅ Flag Value — IsTrackedReferenceWithFinalizer = 0x04000000

The flag value in MethodTableFlags_1.cs matches exactly with enum_flag_IsTrackedReferenceWithFinalizer in src/coreclr/vm/methodtable.h:3821. The [cDAC] annotation comment was correctly added to the runtime header to signal this dependency.

✅ Data Descriptor — Conditional field exposure

datadescriptor.inc correctly wraps the TaggedMemory field under #ifdef FEATURE_OBJCMARSHAL, and InteropSyncBlockInfo.cs uses TryGetValue for graceful fallback when the field isn't present. This is consistent with how RCW/CCW/CCF handle conditional FEATURE_COMINTEROP.

✅ Refactoring — GetSyncBlockAddress extraction

The extracted GetSyncBlockAddress method correctly centralizes sync block index lookup logic. The condition (syncBlockValue & (_syncBlockIsHashCode | _syncBlockIsHashOrSyncBlockIndex)) != _syncBlockIsHashOrSyncBlockIndex correctly identifies sync block indices (not inline hash codes). The removal of _syncTableEntries field is correct — the lookup now delegates to _target.Contracts.SyncBlock.GetSyncBlock(index).

✅ Tagged Memory Size — Hardcoded 2 * PointerSize

new TargetNUInt(2 * (ulong)_target.PointerSize) matches the runtime's PTR_VOID m_taggedAlloc[NUM_TAGGED_MEMORY_CALLBACKS] where NUM_TAGGED_MEMORY_CALLBACKS = 2.

✅ Test Coverage — Comprehensive

  • Unit tests (ObjectiveCMarshalTests.cs): Three key scenarios tested — no sync block, null tagged memory, valid tagged memory with size assertion.
  • Dump integration tests (ObjectiveCMarshalDumpTests.cs): Gated to macOS with [SkipOnOS(IncludeOnly = "osx")].
  • Build infrastructure: MacOnly filtering mirrors the existing WindowsOnly pattern.
  • Mock infrastructure: Extended correctly with taggedMemory parameter.

💡 Minor — Redundant object header read in TryGetHashCode (non-blocking, follow-up)

In Object_1.cs, TryGetHashCode reads the object header (lines 110-112), then if hash isn't inline, calls GetSyncBlockAddress which reads the same header again (lines 136-138). Since ProcessedData.GetOrAdd caches by address, this is functionally a no-op. Acceptable for diagnostic tooling.

✅ Contract Registration — Properly ordered

Registrations maintain alphabetical ordering in CoreCLRContracts.cs, datadescriptor.inc, and ContractRegistry.cs.

Generated by Code Review for issue #125895 ·

@rcj1
rcj1 marked this pull request as ready for review May 6, 2026 20:24
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

1 similar comment
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@max-charlamb@rcj1@noahfalk
, '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 ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API - #125895

Merged
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis
May 6, 2026
Merged

Add cDAC ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API#125895
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis

Conversation

CopilotAI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements cDAC APIs for Objective-C interop diagnostics as a new ObjectiveCMarshal contract, plus adds IsTrackedReferenceWithFinalizer to the RuntimeTypeSystem contract. Based on draft PR #118250, adjusted per review feedback (contract renamed, TargetNUInt instead of nuint, APIs in dedicated contract).

Note

This PR was generated with GitHub Copilot.

Changes

New ObjectiveCMarshal cDAC Contract

  • IObjectiveCMarshal (Abstractions/Contracts/IObjectiveCMarshal.cs): Interface with a single API:

    TargetPointerGetTaggedMemory(TargetPointeraddress,outTargetNUIntsize);

    Returns the tagged memory pointer for an Objective-C tracked reference object (or TargetPointer.Null if none). Sets size to 2 * pointerSize bytes only on success.

  • ObjectiveCMarshal_1 (Contracts/ObjectiveCMarshal_1.cs): Version 1 implementation — reads the sync block from the object header, then extracts InteropSyncBlockInfo.TaggedMemory.

  • ObjectiveCMarshalFactory (Contracts/ObjectiveCMarshalFactory.cs): Contract factory.

  • Registered in ContractRegistry and CachingContractRegistry.

  • datadescriptor.inc: Added TaggedMemory field to InteropSyncBlockInfo type (under #ifdef FEATURE_OBJCMARSHAL) and CDAC_GLOBAL_CONTRACT(ObjectiveCMarshal, 1) (also guarded).

  • syncblk.h: Added TaggedMemory offset to cdac_data<InteropSyncBlockInfo> under #ifdef FEATURE_OBJCMARSHAL.

  • InteropSyncBlockInfo.cs (Data class): Added TaggedMemory property using TryGetValue to handle the optional FEATURE_OBJCMARSHAL field.

IsTrackedReferenceWithFinalizer on RuntimeTypeSystem

  • Added IsTrackedReferenceWithFinalizer = 0x04000000 to WFLAGS_HIGH enum in MethodTableFlags_1.cs, plus a convenience property bool IsTrackedReferenceWithFinalizer.

  • Added IsTrackedReferenceWithFinalizer(TypeHandle) to IRuntimeTypeSystem.cs and implemented it in RuntimeTypeSystem_1.cs.

SOSDacImpl.csISOSDacInterface11

Replaced the stub implementations of IsTrackedType and GetTaggedMemory with full cDAC implementations:

  • IsTrackedType: uses IRuntimeTypeSystem.IsTrackedReferenceWithFinalizer and IObjectiveCMarshal.GetTaggedMemory; returns S_OK if tracked, S_FALSE if not, E_INVALIDARG for null inputs.
  • GetTaggedMemory: uses IObjectiveCMarshal.GetTaggedMemory; returns S_OK with address and size if tagged memory exists, S_FALSE otherwise.

Both methods include #if DEBUG assertions against the legacy DAC implementation for validation.

Documentation

  • docs/design/datacontracts/ObjectiveCMarshal.md: New contract documentation.
  • docs/design/datacontracts/RuntimeTypeSystem.md: Added IsTrackedReferenceWithFinalizer to API listing and pseudocode.

Testing

All 1322 existing cDAC unit tests pass. The new contract will be exercised when running on an Apple platform with FEATURE_OBJCMARSHAL enabled.

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

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

LGTM - one refactor suggestion inline

Comment threaddocs/design/datacontracts/ObjectiveCMarshal.md Outdated
max-charlamb added a commit that referenced this pull request Apr 17, 2026
> [!NOTE]
> This PR description was generated with the assistance of GitHub
Copilot.
## Summary
Add a granular, per-method allowlist (`LegacyFallbackHelper`) that
controls which delegation-only APIs may fall back to the legacy DAC when
`CDAC_NO_FALLBACK=1` is set. This enables selective no-fallback testing
— blocking fallback for most APIs while allowing specific APIs that are
known to not yet be implemented in the cDAC.
Wire this into the runtime-diagnostics CI pipeline using the
`-noFallback` flag from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806).
All fallback attempts (both allowed and blocked) are logged to stderr
with method name, file, and line number for capture by the diagnostics
test infrastructure.
## Changes
### LegacyFallbackHelper.cs — Granular fallback control
New static helper that every delegation-only call site invokes via
`CanFallback()`. Uses `[CallerMemberName]`, `[CallerFilePath]`, and
`[CallerLineNumber]` to identify the call site.
- **Normal mode** (`CDAC_NO_FALLBACK` unset): Always returns `true`
(single `bool` check, `[AggressiveInlining]`)
- **No-fallback mode** (`CDAC_NO_FALLBACK=1`): Checks method name
against a `HashSet<string>` allowlist and file name against a file-level
allowlist
**Per-method allowlist:**
| Method | Reason |
|--------|--------|
| `EnumMemoryRegions` | Dump creation — cDAC has no memory enumeration
implementation |
| `GetInterface` | IMetaDataImport QI ([PR
#127028](#127028)) |
| `GetMethodDefinitionByToken` | IXCLRDataModule — not yet implemented
in cDAC |
| `IsTrackedType` | GC heap analysis ([PR
#125895](#125895)) |
| `TraverseLoaderHeap` | Loader heap traversal ([PR
#125129](#125129)) |
**File-level allowlist:**
| File | Reason |
|------|--------|
| `DacDbiImpl.cs` | Entire DBI/ICorDebug interface (122 methods) —
deferred |
### Entrypoints.cs — Simplified creation
Both `CreateSosInterface` and `CreateDacDbiInterface` now follow the
same pattern: the legacy implementation is always passed through, and
`LegacyFallbackHelper.CanFallback()` at each call site decides whether
to delegate. Removed `prevent_release`, `noFallback` env var check, and
null-legacy-ref logic.
### 13 Legacy wrapper files — Instrumented delegation sites
All 296 delegation-only methods across all legacy wrapper files now call
`LegacyFallbackHelper.CanFallback()`:
- `SOSDacImpl.cs` (12 methods)
- `SOSDacImpl.IXCLRDataProcess.cs` (38 methods, `Flush()` intentionally
excluded — cache management)
- `ClrDataModule.cs` (29 methods + IMetaDataImport QI)
- `DacDbiImpl.cs` (122 methods)
- Other wrappers: `ClrDataTask.cs`, `ClrDataExceptionState.cs`,
`ClrDataFrame.cs`, `ClrDataValue.cs`, `ClrDataTypeInstance.cs`,
`ClrDataMethodInstance.cs`, `ClrDataStackWalk.cs`, `ClrDataProcess.cs`
### CI Pipeline — `-noFallback` flag
Updated `runtime-diag-job.yml` to accept a `noFallback` parameter that
passes `-noFallback` to the diagnostics build script. The
`cDAC_no_fallback` leg in `runtime-diagnostics.yml` now uses
`noFallback: true` instead of setting `CDAC_NO_FALLBACK` as a
pipeline-level environment variable. The `-noFallback` flag (from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806))
properly:
- Sets `DOTNET_ENABLE_CDAC=1` and `CDAC_NO_FALLBACK=1` on the debugger
process
- Defines `CDAC_NO_FALLBACK_TESTING` to skip `ClrStack -i` tests
(ICorDebug not implemented in cDAC)
### Stderr logging
Every fallback attempt is logged to stderr in the format:
```
[cDAC] Allowed fallback: CreateStackWalk at DacDbiImpl.cs:590
[cDAC] Blocked fallback: SomeMethod at SOSDacImpl.cs:123
```
The diagnostics test infrastructure (`ProcessRunner`) captures stderr
and routes it to xunit test output with `STDERROR:` prefix, making
fallback usage visible in test results.
## Test Results
With `CDAC_NO_FALLBACK=1` and the current allowlist, running the full
SOS test suite against a private runtime build:
- **24 passed**, **2 failed** (flaky/pre-existing), **2 skipped**
(Linux-only)
- **0 blocked fallbacks**
## Motivation
The existing cDAC test leg always has the legacy DAC as a fallback, so
unimplemented APIs are silently handled. The granular no-fallback mode
makes gaps visible per-method, helping track progress toward full cDAC
coverage while keeping tests green for known-deferred APIs.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@max-charlamb

Copy link
Copy Markdown
Member

We should make sure to remove IsTrackedType from the LegacyFallbackHelper in this PR.

@max-charlamb

Copy link
Copy Markdown
Member

@copilot, please merge from main, fix any issues, then address feedback on this PR.

@rcj1

rcj1 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

@copilot@max-charlamb there is a reason this is not merged, and that is that we cannot have proper testing because the Mac tests are not set up. No action is needed at the moment.

Copilot stopped work on behalf of max-charlamb due to an error April 21, 2026 16:03
CopilotAI requested a review from max-charlambApril 21, 2026 16:03
Copilot stopped work on behalf of rcj1 due to an error April 21, 2026 16:03
…dReferenceWithFinalizer APIs
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fa8e5180-e07a-43c6-b618-58c6b60fd64e
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 5, 2026 17:52
@rcj1
rcj1force-pushed the copilot/add-cdac-apis branch from 3f77676 to 502054fCompareMay 5, 2026 17:52

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

Note

Copilot was unable to run its full agentic suite in this review.

Adds Objective-C interop support to the cDAC reader and legacy SOS DAC bridge by introducing a new ObjectiveCMarshal contract, exposing tracked-reference metadata, and adding unit/dump coverage around tagged memory.

Changes:

  • Add a new IObjectiveCMarshal contract and wire it through cDAC registration, runtime data descriptors, and legacy ISOSDacInterface11 implementations.
  • Extend object/sync-block data readers and test mocks to surface Objective-C tagged memory and tracked-reference-with-finalizer metadata.
  • Add macOS-specific dump-test infrastructure, a new ObjectiveCMarshal debuggee, and unit/integration tests for tagged-memory scenarios.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/ObjectiveCMarshalTests.csAdds unit tests for GetTaggedMemory contract behavior.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.SyncBlock.csExtends mock sync-block/interop info with tagged-memory support.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Object.csThreads tagged-memory data through mock object creation helpers.
src/native/managed/cdac/tests/DumpTests/ObjectiveCMarshalDumpTests.csAdds dump-based tests for tracked objects and tagged memory.
src/native/managed/cdac/tests/DumpTests/DumpTests.targetsFilters macOS-only debuggees on non-macOS hosts.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/Program.csAdds a macOS-only debuggee that creates a tracked object and crashes.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/ObjectiveCMarshal.csprojDeclares the new debuggee as macOS-only and full-dump capable.
src/native/managed/cdac/tests/DumpTests/Debuggees/Directory.Build.targetsPropagates MacOnly metadata from debuggee projects.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements ISOSDacInterface11 tagged-memory/tracked-type APIs via cDAC.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodTableFlags_1.csAdds the tracked-reference-with-finalizer method-table flag.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/InteropSyncBlockInfo.csReads tagged-memory pointer from interop sync-block info.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.csRegisters the new IObjectiveCMarshal contract implementation.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.csExposes IsTrackedReferenceWithFinalizer from method-table flags.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ObjectiveCMarshal_1.csImplements tagged-memory lookup from an object's sync block.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.csFactors sync-block address lookup into IObject.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.csAdds public contract API for tracked-reference-with-finalizer checks.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObjectiveCMarshal.csIntroduces the public Objective-C marshal contract API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObject.csAdds public sync-block address lookup API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.csExposes ObjectiveCMarshal from the contract registry.
src/coreclr/vm/syncblk.hPublishes tagged-memory field offset for cDAC data descriptors.
src/coreclr/vm/methodtable.hMarks tracked-reference-with-finalizer flag as cDAC-dependent.
src/coreclr/vm/datadescriptor/datadescriptor.incExposes tagged-memory field and ObjectiveCMarshal global contract.
docs/design/datacontracts/RuntimeTypeSystem.mdDocuments the new runtime-type-system API.
docs/design/datacontracts/ObjectiveCMarshal.mdAdds design documentation for the new contract.
docs/design/datacontracts/Object.mdDocuments the new sync-block lookup helper in Object contract.

@github-actions

This comment has been minimized.

CopilotAI review requested due to automatic review settings May 6, 2026 17:36
CopilotAI reviewed May 6, 2026

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.

Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #125895

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: This PR adds a new ObjectiveCMarshal cDAC contract to expose Objective-C interop tagged memory information through the diagnostic data contract reader, replacing the legacy DAC fallback for ISOSDacInterface11.IsTrackedType and GetTaggedMemory. It also refactors the Object contract to extract GetSyncBlockAddress as a reusable API. The motivation is clear and well-justified — enabling the cDAC path for macOS Objective-C diagnostic scenarios.

Approach: The approach is sound and follows established cDAC patterns exactly. A new IObjectiveCMarshal contract with a single GetTaggedMemory API, a helper IsTrackedReferenceWithFinalizer on IRuntimeTypeSystem, and good code deduplication by extracting sync block lookup into GetSyncBlockAddress. The conditional contract registration via #ifdef FEATURE_OBJCMARSHAL and the TryGetContract pattern for graceful fallback on non-macOS targets is the correct approach.

Summary: ✅ LGTM. The implementation correctly mirrors the legacy C++ DAC behavior (ClrDataAccess::IsTrackedType / ClrDataAccess::GetTaggedMemory), the flag value matches the runtime definition, field reads use TryGetValue for conditional feature availability, and the test coverage is comprehensive with both unit and dump-based integration tests.


Detailed Findings

✅ Correctness — SOSDacImpl matches legacy DAC semantics

The new IsTrackedType implementation correctly:

  • Returns S_OK when tracked, S_FALSE when not (matching request.cpp:5445-5447)
  • Returns E_INVALIDARG for null pointers (via ArgumentException.HResult)
  • Checks hasTaggedMemory independently using TryGetContract<IObjectiveCMarshal> — gracefully no-ops when the contract isn't available (non-macOS)
  • The GetTaggedMemory method returns S_FALSE when no tagged memory exists and S_OK on success

The #if DEBUG validation blocks correctly compare against the legacy implementation.

✅ Flag Value — IsTrackedReferenceWithFinalizer = 0x04000000

The flag value in MethodTableFlags_1.cs matches exactly with enum_flag_IsTrackedReferenceWithFinalizer in src/coreclr/vm/methodtable.h:3821. The [cDAC] annotation comment was correctly added to the runtime header to signal this dependency.

✅ Data Descriptor — Conditional field exposure

datadescriptor.inc correctly wraps the TaggedMemory field under #ifdef FEATURE_OBJCMARSHAL, and InteropSyncBlockInfo.cs uses TryGetValue for graceful fallback when the field isn't present. This is consistent with how RCW/CCW/CCF handle conditional FEATURE_COMINTEROP.

✅ Refactoring — GetSyncBlockAddress extraction

The extracted GetSyncBlockAddress method correctly centralizes sync block index lookup logic. The condition (syncBlockValue & (_syncBlockIsHashCode | _syncBlockIsHashOrSyncBlockIndex)) != _syncBlockIsHashOrSyncBlockIndex correctly identifies sync block indices (not inline hash codes). The removal of _syncTableEntries field is correct — the lookup now delegates to _target.Contracts.SyncBlock.GetSyncBlock(index).

✅ Tagged Memory Size — Hardcoded 2 * PointerSize

new TargetNUInt(2 * (ulong)_target.PointerSize) matches the runtime's PTR_VOID m_taggedAlloc[NUM_TAGGED_MEMORY_CALLBACKS] where NUM_TAGGED_MEMORY_CALLBACKS = 2.

✅ Test Coverage — Comprehensive

  • Unit tests (ObjectiveCMarshalTests.cs): Three key scenarios tested — no sync block, null tagged memory, valid tagged memory with size assertion.
  • Dump integration tests (ObjectiveCMarshalDumpTests.cs): Gated to macOS with [SkipOnOS(IncludeOnly = "osx")].
  • Build infrastructure: MacOnly filtering mirrors the existing WindowsOnly pattern.
  • Mock infrastructure: Extended correctly with taggedMemory parameter.

💡 Minor — Redundant object header read in TryGetHashCode (non-blocking, follow-up)

In Object_1.cs, TryGetHashCode reads the object header (lines 110-112), then if hash isn't inline, calls GetSyncBlockAddress which reads the same header again (lines 136-138). Since ProcessedData.GetOrAdd caches by address, this is functionally a no-op. Acceptable for diagnostic tooling.

✅ Contract Registration — Properly ordered

Registrations maintain alphabetical ordering in CoreCLRContracts.cs, datadescriptor.inc, and ContractRegistry.cs.

Generated by Code Review for issue #125895 ·

@rcj1
rcj1 marked this pull request as ready for review May 6, 2026 20:24
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

1 similar comment
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@max-charlamb@rcj1@noahfalk
, '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 ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API - #125895

Merged
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis
May 6, 2026
Merged

Add cDAC ObjectiveCMarshal contract and IsTrackedReferenceWithFinalizer API#125895
rcj1 merged 3 commits into
mainfrom
copilot/add-cdac-apis

Conversation

CopilotAI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements cDAC APIs for Objective-C interop diagnostics as a new ObjectiveCMarshal contract, plus adds IsTrackedReferenceWithFinalizer to the RuntimeTypeSystem contract. Based on draft PR #118250, adjusted per review feedback (contract renamed, TargetNUInt instead of nuint, APIs in dedicated contract).

Note

This PR was generated with GitHub Copilot.

Changes

New ObjectiveCMarshal cDAC Contract

  • IObjectiveCMarshal (Abstractions/Contracts/IObjectiveCMarshal.cs): Interface with a single API:

    TargetPointerGetTaggedMemory(TargetPointeraddress,outTargetNUIntsize);

    Returns the tagged memory pointer for an Objective-C tracked reference object (or TargetPointer.Null if none). Sets size to 2 * pointerSize bytes only on success.

  • ObjectiveCMarshal_1 (Contracts/ObjectiveCMarshal_1.cs): Version 1 implementation — reads the sync block from the object header, then extracts InteropSyncBlockInfo.TaggedMemory.

  • ObjectiveCMarshalFactory (Contracts/ObjectiveCMarshalFactory.cs): Contract factory.

  • Registered in ContractRegistry and CachingContractRegistry.

  • datadescriptor.inc: Added TaggedMemory field to InteropSyncBlockInfo type (under #ifdef FEATURE_OBJCMARSHAL) and CDAC_GLOBAL_CONTRACT(ObjectiveCMarshal, 1) (also guarded).

  • syncblk.h: Added TaggedMemory offset to cdac_data<InteropSyncBlockInfo> under #ifdef FEATURE_OBJCMARSHAL.

  • InteropSyncBlockInfo.cs (Data class): Added TaggedMemory property using TryGetValue to handle the optional FEATURE_OBJCMARSHAL field.

IsTrackedReferenceWithFinalizer on RuntimeTypeSystem

  • Added IsTrackedReferenceWithFinalizer = 0x04000000 to WFLAGS_HIGH enum in MethodTableFlags_1.cs, plus a convenience property bool IsTrackedReferenceWithFinalizer.

  • Added IsTrackedReferenceWithFinalizer(TypeHandle) to IRuntimeTypeSystem.cs and implemented it in RuntimeTypeSystem_1.cs.

SOSDacImpl.csISOSDacInterface11

Replaced the stub implementations of IsTrackedType and GetTaggedMemory with full cDAC implementations:

  • IsTrackedType: uses IRuntimeTypeSystem.IsTrackedReferenceWithFinalizer and IObjectiveCMarshal.GetTaggedMemory; returns S_OK if tracked, S_FALSE if not, E_INVALIDARG for null inputs.
  • GetTaggedMemory: uses IObjectiveCMarshal.GetTaggedMemory; returns S_OK with address and size if tagged memory exists, S_FALSE otherwise.

Both methods include #if DEBUG assertions against the legacy DAC implementation for validation.

Documentation

  • docs/design/datacontracts/ObjectiveCMarshal.md: New contract documentation.
  • docs/design/datacontracts/RuntimeTypeSystem.md: Added IsTrackedReferenceWithFinalizer to API listing and pseudocode.

Testing

All 1322 existing cDAC unit tests pass. The new contract will be exercised when running on an Apple platform with FEATURE_OBJCMARSHAL enabled.

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

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

LGTM - one refactor suggestion inline

Comment threaddocs/design/datacontracts/ObjectiveCMarshal.md Outdated
max-charlamb added a commit that referenced this pull request Apr 17, 2026
> [!NOTE]
> This PR description was generated with the assistance of GitHub
Copilot.
## Summary
Add a granular, per-method allowlist (`LegacyFallbackHelper`) that
controls which delegation-only APIs may fall back to the legacy DAC when
`CDAC_NO_FALLBACK=1` is set. This enables selective no-fallback testing
— blocking fallback for most APIs while allowing specific APIs that are
known to not yet be implemented in the cDAC.
Wire this into the runtime-diagnostics CI pipeline using the
`-noFallback` flag from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806).
All fallback attempts (both allowed and blocked) are logged to stderr
with method name, file, and line number for capture by the diagnostics
test infrastructure.
## Changes
### LegacyFallbackHelper.cs — Granular fallback control
New static helper that every delegation-only call site invokes via
`CanFallback()`. Uses `[CallerMemberName]`, `[CallerFilePath]`, and
`[CallerLineNumber]` to identify the call site.
- **Normal mode** (`CDAC_NO_FALLBACK` unset): Always returns `true`
(single `bool` check, `[AggressiveInlining]`)
- **No-fallback mode** (`CDAC_NO_FALLBACK=1`): Checks method name
against a `HashSet<string>` allowlist and file name against a file-level
allowlist
**Per-method allowlist:**
| Method | Reason |
|--------|--------|
| `EnumMemoryRegions` | Dump creation — cDAC has no memory enumeration
implementation |
| `GetInterface` | IMetaDataImport QI ([PR
#127028](#127028)) |
| `GetMethodDefinitionByToken` | IXCLRDataModule — not yet implemented
in cDAC |
| `IsTrackedType` | GC heap analysis ([PR
#125895](#125895)) |
| `TraverseLoaderHeap` | Loader heap traversal ([PR
#125129](#125129)) |
**File-level allowlist:**
| File | Reason |
|------|--------|
| `DacDbiImpl.cs` | Entire DBI/ICorDebug interface (122 methods) —
deferred |
### Entrypoints.cs — Simplified creation
Both `CreateSosInterface` and `CreateDacDbiInterface` now follow the
same pattern: the legacy implementation is always passed through, and
`LegacyFallbackHelper.CanFallback()` at each call site decides whether
to delegate. Removed `prevent_release`, `noFallback` env var check, and
null-legacy-ref logic.
### 13 Legacy wrapper files — Instrumented delegation sites
All 296 delegation-only methods across all legacy wrapper files now call
`LegacyFallbackHelper.CanFallback()`:
- `SOSDacImpl.cs` (12 methods)
- `SOSDacImpl.IXCLRDataProcess.cs` (38 methods, `Flush()` intentionally
excluded — cache management)
- `ClrDataModule.cs` (29 methods + IMetaDataImport QI)
- `DacDbiImpl.cs` (122 methods)
- Other wrappers: `ClrDataTask.cs`, `ClrDataExceptionState.cs`,
`ClrDataFrame.cs`, `ClrDataValue.cs`, `ClrDataTypeInstance.cs`,
`ClrDataMethodInstance.cs`, `ClrDataStackWalk.cs`, `ClrDataProcess.cs`
### CI Pipeline — `-noFallback` flag
Updated `runtime-diag-job.yml` to accept a `noFallback` parameter that
passes `-noFallback` to the diagnostics build script. The
`cDAC_no_fallback` leg in `runtime-diagnostics.yml` now uses
`noFallback: true` instead of setting `CDAC_NO_FALLBACK` as a
pipeline-level environment variable. The `-noFallback` flag (from
[dotnet/diagnostics#5806](dotnet/diagnostics#5806))
properly:
- Sets `DOTNET_ENABLE_CDAC=1` and `CDAC_NO_FALLBACK=1` on the debugger
process
- Defines `CDAC_NO_FALLBACK_TESTING` to skip `ClrStack -i` tests
(ICorDebug not implemented in cDAC)
### Stderr logging
Every fallback attempt is logged to stderr in the format:
```
[cDAC] Allowed fallback: CreateStackWalk at DacDbiImpl.cs:590
[cDAC] Blocked fallback: SomeMethod at SOSDacImpl.cs:123
```
The diagnostics test infrastructure (`ProcessRunner`) captures stderr
and routes it to xunit test output with `STDERROR:` prefix, making
fallback usage visible in test results.
## Test Results
With `CDAC_NO_FALLBACK=1` and the current allowlist, running the full
SOS test suite against a private runtime build:
- **24 passed**, **2 failed** (flaky/pre-existing), **2 skipped**
(Linux-only)
- **0 blocked fallbacks**
## Motivation
The existing cDAC test leg always has the legacy DAC as a fallback, so
unimplemented APIs are silently handled. The granular no-fallback mode
makes gaps visible per-method, helping track progress toward full cDAC
coverage while keeping tests green for known-deferred APIs.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@max-charlamb

Copy link
Copy Markdown
Member

We should make sure to remove IsTrackedType from the LegacyFallbackHelper in this PR.

@max-charlamb

Copy link
Copy Markdown
Member

@copilot, please merge from main, fix any issues, then address feedback on this PR.

@rcj1

rcj1 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

@copilot@max-charlamb there is a reason this is not merged, and that is that we cannot have proper testing because the Mac tests are not set up. No action is needed at the moment.

Copilot stopped work on behalf of max-charlamb due to an error April 21, 2026 16:03
CopilotAI requested a review from max-charlambApril 21, 2026 16:03
Copilot stopped work on behalf of rcj1 due to an error April 21, 2026 16:03
…dReferenceWithFinalizer APIs
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fa8e5180-e07a-43c6-b618-58c6b60fd64e
Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 5, 2026 17:52
@rcj1
rcj1force-pushed the copilot/add-cdac-apis branch from 3f77676 to 502054fCompareMay 5, 2026 17:52

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

Note

Copilot was unable to run its full agentic suite in this review.

Adds Objective-C interop support to the cDAC reader and legacy SOS DAC bridge by introducing a new ObjectiveCMarshal contract, exposing tracked-reference metadata, and adding unit/dump coverage around tagged memory.

Changes:

  • Add a new IObjectiveCMarshal contract and wire it through cDAC registration, runtime data descriptors, and legacy ISOSDacInterface11 implementations.
  • Extend object/sync-block data readers and test mocks to surface Objective-C tagged memory and tracked-reference-with-finalizer metadata.
  • Add macOS-specific dump-test infrastructure, a new ObjectiveCMarshal debuggee, and unit/integration tests for tagged-memory scenarios.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
src/native/managed/cdac/tests/ObjectiveCMarshalTests.csAdds unit tests for GetTaggedMemory contract behavior.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.SyncBlock.csExtends mock sync-block/interop info with tagged-memory support.
src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Object.csThreads tagged-memory data through mock object creation helpers.
src/native/managed/cdac/tests/DumpTests/ObjectiveCMarshalDumpTests.csAdds dump-based tests for tracked objects and tagged memory.
src/native/managed/cdac/tests/DumpTests/DumpTests.targetsFilters macOS-only debuggees on non-macOS hosts.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/Program.csAdds a macOS-only debuggee that creates a tracked object and crashes.
src/native/managed/cdac/tests/DumpTests/Debuggees/ObjectiveCMarshal/ObjectiveCMarshal.csprojDeclares the new debuggee as macOS-only and full-dump capable.
src/native/managed/cdac/tests/DumpTests/Debuggees/Directory.Build.targetsPropagates MacOnly metadata from debuggee projects.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.csImplements ISOSDacInterface11 tagged-memory/tracked-type APIs via cDAC.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodTableFlags_1.csAdds the tracked-reference-with-finalizer method-table flag.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/InteropSyncBlockInfo.csReads tagged-memory pointer from interop sync-block info.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/CoreCLRContracts.csRegisters the new IObjectiveCMarshal contract implementation.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.csExposes IsTrackedReferenceWithFinalizer from method-table flags.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ObjectiveCMarshal_1.csImplements tagged-memory lookup from an object's sync block.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Object_1.csFactors sync-block address lookup into IObject.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.csAdds public contract API for tracked-reference-with-finalizer checks.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObjectiveCMarshal.csIntroduces the public Objective-C marshal contract API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IObject.csAdds public sync-block address lookup API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.csExposes ObjectiveCMarshal from the contract registry.
src/coreclr/vm/syncblk.hPublishes tagged-memory field offset for cDAC data descriptors.
src/coreclr/vm/methodtable.hMarks tracked-reference-with-finalizer flag as cDAC-dependent.
src/coreclr/vm/datadescriptor/datadescriptor.incExposes tagged-memory field and ObjectiveCMarshal global contract.
docs/design/datacontracts/RuntimeTypeSystem.mdDocuments the new runtime-type-system API.
docs/design/datacontracts/ObjectiveCMarshal.mdAdds design documentation for the new contract.
docs/design/datacontracts/Object.mdDocuments the new sync-block lookup helper in Object contract.

@github-actions

This comment has been minimized.

CopilotAI review requested due to automatic review settings May 6, 2026 17:36
CopilotAI reviewed May 6, 2026

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.

Copilot encountered an error: Your billing is not configured or you have Copilot licenses from multiple standalone organizations or enterprises. To use premium requests, select a billing entity via the GitHub site, under Settings > Copilot > Features.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #125895

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: This PR adds a new ObjectiveCMarshal cDAC contract to expose Objective-C interop tagged memory information through the diagnostic data contract reader, replacing the legacy DAC fallback for ISOSDacInterface11.IsTrackedType and GetTaggedMemory. It also refactors the Object contract to extract GetSyncBlockAddress as a reusable API. The motivation is clear and well-justified — enabling the cDAC path for macOS Objective-C diagnostic scenarios.

Approach: The approach is sound and follows established cDAC patterns exactly. A new IObjectiveCMarshal contract with a single GetTaggedMemory API, a helper IsTrackedReferenceWithFinalizer on IRuntimeTypeSystem, and good code deduplication by extracting sync block lookup into GetSyncBlockAddress. The conditional contract registration via #ifdef FEATURE_OBJCMARSHAL and the TryGetContract pattern for graceful fallback on non-macOS targets is the correct approach.

Summary: ✅ LGTM. The implementation correctly mirrors the legacy C++ DAC behavior (ClrDataAccess::IsTrackedType / ClrDataAccess::GetTaggedMemory), the flag value matches the runtime definition, field reads use TryGetValue for conditional feature availability, and the test coverage is comprehensive with both unit and dump-based integration tests.


Detailed Findings

✅ Correctness — SOSDacImpl matches legacy DAC semantics

The new IsTrackedType implementation correctly:

  • Returns S_OK when tracked, S_FALSE when not (matching request.cpp:5445-5447)
  • Returns E_INVALIDARG for null pointers (via ArgumentException.HResult)
  • Checks hasTaggedMemory independently using TryGetContract<IObjectiveCMarshal> — gracefully no-ops when the contract isn't available (non-macOS)
  • The GetTaggedMemory method returns S_FALSE when no tagged memory exists and S_OK on success

The #if DEBUG validation blocks correctly compare against the legacy implementation.

✅ Flag Value — IsTrackedReferenceWithFinalizer = 0x04000000

The flag value in MethodTableFlags_1.cs matches exactly with enum_flag_IsTrackedReferenceWithFinalizer in src/coreclr/vm/methodtable.h:3821. The [cDAC] annotation comment was correctly added to the runtime header to signal this dependency.

✅ Data Descriptor — Conditional field exposure

datadescriptor.inc correctly wraps the TaggedMemory field under #ifdef FEATURE_OBJCMARSHAL, and InteropSyncBlockInfo.cs uses TryGetValue for graceful fallback when the field isn't present. This is consistent with how RCW/CCW/CCF handle conditional FEATURE_COMINTEROP.

✅ Refactoring — GetSyncBlockAddress extraction

The extracted GetSyncBlockAddress method correctly centralizes sync block index lookup logic. The condition (syncBlockValue & (_syncBlockIsHashCode | _syncBlockIsHashOrSyncBlockIndex)) != _syncBlockIsHashOrSyncBlockIndex correctly identifies sync block indices (not inline hash codes). The removal of _syncTableEntries field is correct — the lookup now delegates to _target.Contracts.SyncBlock.GetSyncBlock(index).

✅ Tagged Memory Size — Hardcoded 2 * PointerSize

new TargetNUInt(2 * (ulong)_target.PointerSize) matches the runtime's PTR_VOID m_taggedAlloc[NUM_TAGGED_MEMORY_CALLBACKS] where NUM_TAGGED_MEMORY_CALLBACKS = 2.

✅ Test Coverage — Comprehensive

  • Unit tests (ObjectiveCMarshalTests.cs): Three key scenarios tested — no sync block, null tagged memory, valid tagged memory with size assertion.
  • Dump integration tests (ObjectiveCMarshalDumpTests.cs): Gated to macOS with [SkipOnOS(IncludeOnly = "osx")].
  • Build infrastructure: MacOnly filtering mirrors the existing WindowsOnly pattern.
  • Mock infrastructure: Extended correctly with taggedMemory parameter.

💡 Minor — Redundant object header read in TryGetHashCode (non-blocking, follow-up)

In Object_1.cs, TryGetHashCode reads the object header (lines 110-112), then if hash isn't inline, calls GetSyncBlockAddress which reads the same header again (lines 136-138). Since ProcessedData.GetOrAdd caches by address, this is functionally a no-op. Acceptable for diagnostic tooling.

✅ Contract Registration — Properly ordered

Registrations maintain alphabetical ordering in CoreCLRContracts.cs, datadescriptor.inc, and ContractRegistry.cs.

Generated by Code Review for issue #125895 ·

@rcj1
rcj1 marked this pull request as ready for review May 6, 2026 20:24
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

1 similar comment
@rcj1

rcj1 commented May 6, 2026

Copy link
Copy Markdown
Contributor

/ba-g already ran CI

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@max-charlamb@rcj1@noahfalk