Skip to content

Conversation

rcj1
Copy link
Contributor

@rcj1 rcj1 commented Jul 22, 2025

No description provided.

@rcj1 rcj1 requested review from Copilot and max-charlamb and removed request for Copilot July 22, 2025 17:17
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for retrieving Assembly Load Context (ALC) information through the cDAC API by implementing the GetAssemblyLoadContext method. The implementation provides a way to traverse from a method table to its associated Assembly Load Context by following the chain: MethodTable → Module → PEAssembly → HostAssembly/FallbackBinder → AssemblyBinder → ManagedAssemblyLoadContext.

Key changes include:

  • Implementation of GetAssemblyLoadContext method in SOSDacImpl.cs with proper error handling and debug validation
  • Addition of new data structures (BinderSpaceAssembly, AssemblyBinder) to support ALC traversal
  • Extension of existing data structures (PEAssembly) with additional fields for binder information
  • Addition of GetBinderAssemblyLoadContext method to the ILoader contract

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
SOSDacImpl.cs Implements GetAssemblyLoadContext method with cDAC contract calls and debug validation
PEAssembly.cs Adds HostAssembly and FallbackBinder fields to support ALC traversal
BinderSpaceAssembly.cs New data structure representing binder space assembly with Binder field
AssemblyBinder.cs New data structure representing assembly binder with ManagedAssemblyLoadContext field
Loader_1.cs Implements GetBinderALC method to traverse ALC hierarchy
DataType.cs Adds enum entries for new data types
ILoader.cs Adds GetBinderAssemblyLoadContext method to interface
peassembly.h Adds cdac_data template specialization with new field offsets
assemblybinder.h Adds cdac_data template specialization and friend declaration
datadescriptor.h Adds CDAC type definitions for new data structures
assembly.hpp Adds cdac_data template specialization for BINDER_SPACE::Assembly
Loader.md Updates documentation with new API and data structure descriptions
Comments suppressed due to low confidence (2)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Loader_1.cs:298

  • The method name 'GetBinderALC' is inconsistent with the interface method name 'GetBinderAssemblyLoadContext'. The implementation should match the interface method name for clarity.
    TargetPointer ILoader.GetBinderALC(ModuleHandle handle)

src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs:946

  • The code calls 'GetBinderAssemblyLoadContext' but the actual implementation method is named 'GetBinderALC', which will cause a runtime error due to the method name mismatch.
            // which is apparently not expected by SOS.

Copy link
Contributor

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

@rcj1 rcj1 marked this pull request as draft July 22, 2025 17:42
@rcj1 rcj1 marked this pull request as ready for review July 22, 2025 17:44
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@max-charlamb Could you please review this as well?

  • There is a merge conflict to resolve
  • It would be nice to rename ManagedAssemblyLoadContext -> AssemblyLoadContext everywhere as mentioned above (in a separate PR)

Copy link
Member

@max-charlamb max-charlamb left a comment

Choose a reason for hiding this comment

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

lgtm mod one small nit 👍

@jkotas
Copy link
Member

jkotas commented Jul 29, 2025

/ba-g System.Collections.Tests failure is knonw issue #117939

@jkotas jkotas merged commit 2db9fdf into dotnet:main Jul 29, 2025
98 of 100 checks passed
@rcj1 rcj1 deleted the GetAssemblyLoadContext branch July 29, 2025 01:40
@github-actions github-actions bot locked and limited conversation to collaborators Aug 28, 2025
Sign up for free to 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.

3 participants