Skip to content

Share ObjectiveCMarshal reference-tracking logic between CoreCLR and NativeAOT#128961

Merged
AaronRobinsonMSFT merged 9 commits into
mainfrom
copilot/share-objectivecmarshal-api
Jun 9, 2026
Merged

Share ObjectiveCMarshal reference-tracking logic between CoreCLR and NativeAOT#128961
AaronRobinsonMSFT merged 9 commits into
mainfrom
copilot/share-objectivecmarshal-api

Conversation

Copilot AI commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

ObjectiveCMarshal had divergent CoreCLR and NativeAOT implementations for tagged reference-tracking memory. This change moves CoreCLR to the same ConditionalWeakTable<object, ObjcTrackingInformation> model used by NativeAOT and updates diagnostics contracts accordingly.

  • Description
    • CoreCLR managed ObjectiveCMarshal now owns tagged memory
      • Added CoreCLR-side s_objects CWT and ObjcTrackingInformation lifecycle logic in ObjectiveCMarshal.CoreCLR.cs.
      • Unified CreateReferenceTrackingHandle / GetOrCreateReferenceTrackingMemory flow to use managed CWT-backed memory for both runtimes.
      • Initialize now passes the CWT instance to native for unmanaged lookup during GC callbacks.
    • CoreCLR native Objective-C interop path no longer uses syncblk tagged storage
      • Removed Objective-C-specific syncblk tagged-memory allocation/retrieval path from interoplibinterface_objc.cpp.
      • Replaced with unmanaged CWT lookup (ConditionalWeakTableObject::TryGetValue) to resolve tagged memory from managed state.
      • Split QCall responsibilities so native only allocates the ref-counted handle (ObjCMarshal_AllocateReferenceTrackingHandle).
    • Unmanaged CWT accessor usage enabled in runtime path
      • Exposed ConditionalWeakTable native lookup helpers for this suspended-runtime callback scenario and clarified constraints in comments.
    • cDAC ObjectiveCMarshal contract folded into version c1
      • Replaced ObjectiveCMarshal_1 implementation to read tagged memory via ObjectiveCMarshal.s_objects and CWT entries.
// CoreCLR native callback path now resolves tagged memory from managed CWT state.
CONDITIONAL_WEAK_TABLE_REF trackingTable = (CONDITIONAL_WEAK_TABLE_REF)ObjectFromHandle(g_ObjectiveCTrackingInfoTable);
OBJECTREF trackingInfoObj = NULL;
if (!trackingTable->TryGetValue(object, &trackingInfoObj))
    return false;

OBJC_TRACKING_INFO_REF trackingInfo = (OBJC_TRACKING_INFO_REF)trackingInfoObj;
void* taggedLocal = (void*)trackingInfo->_memory;

Copilot AI self-assigned this Jun 3, 2026
Copilot AI review requested due to automatic review settings June 3, 2026 20:13
Copilot AI removed the request for review from Copilot June 3, 2026 20:13
Copilot AI requested review from Copilot and removed request for Copilot June 3, 2026 20:38
Copilot AI changed the title [WIP] Share ObjectiveCMarshal API implementation between CoreCLR and NativeAOT Share ObjectiveCMarshal reference-tracking storage between CoreCLR and NativeAOT Jun 3, 2026
Copilot AI requested a review from AaronRobinsonMSFT June 3, 2026 20:45
@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.

@AaronRobinsonMSFT

Copy link
Copy Markdown
Member

@copilot Please fold the cDAC changes into version 1. There is no backwards compat concerns here. We can change version 1 without issue.

Copilot AI requested review from Copilot and removed request for Copilot June 4, 2026 20:50

Copilot AI commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Please fold the cDAC changes into version 1. There is no backwards compat concerns here. We can change version 1 without issue.

Addressed in 0edf747. I folded the ObjectiveCMarshal cDAC changes into version c1, removed the c2 registration/implementation, and updated the CoreCLR data descriptor contract entry back to c1.

Copilot AI changed the title Share ObjectiveCMarshal reference-tracking storage between CoreCLR and NativeAOT Share ObjectiveCMarshal reference-tracking storage between CoreCLR and NativeAOT (cDAC contract stays at v1) Jun 4, 2026
Copilot AI review requested due to automatic review settings June 8, 2026 18:13
Copilot AI and others added 3 commits June 8, 2026 11:16
Co-authored-by: AaronRobinsonMSFT <30635565+AaronRobinsonMSFT@users.noreply.github.com>
Co-authored-by: AaronRobinsonMSFT <30635565+AaronRobinsonMSFT@users.noreply.github.com>
- Use sizeof(TADDR) instead of sizeof(void*) in DAC for target size
- Mark s_objects as readonly; use local copy for ObjectHandleOnStack
- Guard gcinterface.dac.h include with DACCESS_COMPILE

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 8, 2026 18:52

Copilot AI 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's findings

  • Files reviewed: 26/26 changed files
  • Comments generated: 2

@jkotas jkotas marked this pull request as ready for review June 8, 2026 19:08
@jkotas jkotas requested a review from MichalStrehovsky as a code owner June 8, 2026 19:08
Comment thread src/coreclr/vm/object.h
AaronRobinsonMSFT and others added 2 commits June 8, 2026 18:43
Add tests exercising the ConditionalWeakTable-based GetTaggedMemory path:
- ObjectNotInTable: CWT exists but object not tracked
- NullMemory: object tracked but memory pointer is null
- HasMemory: object tracked with valid tagged memory

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…bjC stubs

- Use m_Array directly instead of GetGCSafeDataPtr() per review feedback
- Update Mono ObjectiveCMarshal stubs to match shared API changes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…al methods

Extract the unsafe MethodTable check into a partial method on each runtime
(CoreCLR, NativeAOT, Mono stub) so the shared ObjectiveCMarshal.cs no
longer requires an unsafe context for the validation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 9, 2026 02:50

Copilot AI 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's findings

  • Files reviewed: 27/27 changed files
  • Comments generated: 4

Comment thread src/coreclr/vm/object.h
Comment thread src/native/managed/cdac/tests/UnitTests/ObjectiveCMarshalTests.cs
Comment thread src/native/managed/cdac/tests/UnitTests/ObjectiveCMarshalTests.cs
Comment thread src/native/managed/cdac/tests/UnitTests/ObjectiveCMarshalTests.cs
@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 4b6b5d0 into main Jun 9, 2026
174 checks passed
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the copilot/share-objectivecmarshal-api branch June 9, 2026 14:29
@github-project-automation github-project-automation Bot moved this to Done in AppModel Jun 9, 2026
tommcdon added a commit that referenced this pull request Jun 12, 2026
In DAC mode, m_Array yields a host address, but PTR_KIND requires a
target address. GetDataPtr() computes the correct target address via
dac_cast<PTR_BYTE>(this) + offset.

This was broken by PR #128961 which changed from GetDataPtr() to
m_Array, breaking debugger inspection of exception stack traces,
ConditionalWeakTable lookups, and stack frame helper arrays in DAC mode.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
leculver added a commit that referenced this pull request Jun 16, 2026
Fixes failing OSX cDac test.  I think this is a regression from #128961.

This pull request updates the handling of pointer types in managed code
to better align with how .NET expresses raw pointers. The main change is
to allow fields typed as `IntPtr`/`UIntPtr` (reported as
`"nint"`/`"nuint"`) to be treated as pointers by the contract reader,
improving compatibility and correctness when reading pointer-sized
fields.

Improvements to pointer type handling:

* Updated `AssertPointerType` in `TargetFieldExtensions.cs` to accept
`"nint"` and `"nuint"` as valid pointer type names, in addition to
`"pointer"`, enabling proper handling of managed fields typed as
`IntPtr`/`UIntPtr`.

Test and contract descriptor alignment:

* Modified the test contract for the `_memory` field in
`ObjectiveCMarshalTests.cs` to explicitly set its type as `"nint"`,
ensuring that the pointer read path is validated against the real
managed contract descriptor.

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Share ObjectiveCMarshal API implementation between CoreCLR and NativeAOT

7 participants