Skip to content

Implement ISOSDacInterface::GetObjectClassName in cDAC SOSDacImpl#124816

Merged
max-charlamb merged 7 commits intomainfrom
copilot/implement-get-object-class-name
Feb 26, 2026
Merged

Implement ISOSDacInterface::GetObjectClassName in cDAC SOSDacImpl#124816
max-charlamb merged 7 commits intomainfrom
copilot/implement-get-object-class-name

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 24, 2026

Description

Implements the cDAC version of ISOSDacInterface::GetObjectClassName in SOSDacImpl.cs, replacing the legacy-only stub. Based on ClrDataAccess::GetObjectClassName in request.cpp, following the established pattern from GetMethodTableName.

Implementation (SOSDacImpl.cs)

  • Gets the object's MethodTable via IObject.GetMethodTableAddress, then resolves the TypeHandle via IRuntimeTypeSystem.GetTypeHandle
  • Checks for null modulePointer from GetModule before calling GetModuleHandleFromModulePtr to prevent ArgumentNullException
  • Handles unloaded modules and null module pointers ("<Unloaded Type>") and normal types via TypeNameBuilder.AppendType with FormatNamespace | FormatFullInst
  • Falls back to DacStreams.StringFromEEAddress when TypeNameBuilder throws
  • Includes #if DEBUG validation against legacy DAC

Unit tests (ObjectTests.cs)

  • GetObjectClassName_ZeroAddress — zero address returns error HResult
  • GetObjectClassName_UnloadedModule — object with unloaded module returns "<Unloaded Type>" (uses Moq to mock IObject, IRuntimeTypeSystem, and ILoader contracts)
  • GetObjectClassName_NullBufferReturnsNeededSize — null buffer still populates needed size

All 3 tests run across 4 architectures (32/64-bit × little/big-endian).

Dump tests (RuntimeTypeSystemDumpTests.cs)

  • RuntimeTypeSystem_ObjectMethodTableHasLoadedModule — verifies System.Object MT → module → loaded image flow
  • RuntimeTypeSystem_StringMethodTableHasLoadedModule — same for System.String

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…unit and dump tests

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement ISOSDacInterface::GetObjectClassName in SOSDacImpl.cs Implement ISOSDacInterface::GetObjectClassName in cDAC SOSDacImpl Feb 24, 2026
Copy link
Copy Markdown
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.

@copilot , address feedback

… case, update tests

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 24, 2026

@copilot , address feedback

All feedback addressed in 0239f9f.

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

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

Implements the cDAC version of ISOSDacInterface::GetObjectClassName in SOSDacImpl, replacing the legacy-only stub, and adds unit + dump-based tests to validate method-table → type/module resolution and string output behavior.

Changes:

  • Add cDAC implementation of GetObjectClassName using IObject, IRuntimeTypeSystem, ILoader, and TypeNameBuilder with a fallback to DacStreams.
  • Add unit tests covering zero address, unloaded-type naming, and “null buffer returns needed size” behavior.
  • Add dump-based integration tests validating that System.Object and System.String resolve to modules with loaded image contents.

Reviewed changes

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

File Description
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs Implements GetObjectClassName in cDAC, including unloaded-module handling, TypeNameBuilder formatting, and DEBUG cross-check vs legacy DAC.
src/native/managed/cdac/tests/ObjectTests.cs Adds unit tests for GetObjectClassName argument handling and unloaded-type naming behavior.
src/native/managed/cdac/tests/DumpTests/RuntimeTypeSystemDumpTests.cs Adds dump tests ensuring Object/String MTs map to modules with loaded image contents via Loader contract.

Copy link
Copy Markdown
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.

changes lgtm

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 25, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh /usr/bin/gh pr checks 124816 --repo dotnet/runtime (http block)
    • Triggering command: /usr/bin/gh gh pr checks 124816 (http block)

If you need me to access, download, or install something from one of these locations, you can either:

…check

Co-authored-by: rcj1 <77995559+rcj1@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@rcj1 rcj1 left a comment

Choose a reason for hiding this comment

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

lgtm

@max-charlamb max-charlamb merged commit a1df4fb into main Feb 26, 2026
52 of 54 checks passed
@max-charlamb max-charlamb deleted the copilot/implement-get-object-class-name branch February 26, 2026 23:21
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 29, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants