Skip to content

Conversation

rcj1
Copy link
Contributor

@rcj1 rcj1 commented Jul 13, 2025

No description provided.

@Copilot Copilot AI review requested due to automatic review settings July 13, 2025 17:59
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

Implements cDAC support for retrieving the TLS index on a target runtime.

  • Added the GetTLSIndex method in SOSDacImpl.cs with low-level reads, error handling, and debug assertions against the legacy DAC.
  • Introduced new constants TlsIndex and TlsOutOfIndexes in the data contract for TLS globals.
  • Exposed TLS_OUT_OF_INDEXES and g_TlsIndex in datadescriptor.h for cDAC consumption.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/native/managed/cdac/mscordaccore_universal/Legacy/SOSDacImpl.cs Implements GetTLSIndex logic, argument validation, and debug checks
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs Adds TlsIndex and TlsOutOfIndexes to the Globals contract
src/coreclr/debug/runtimeinfo/datadescriptor.h Declares CDAC globals for TlsOutOfIndexes and TlsIndex
Comments suppressed due to low confidence (1)

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

  • Add unit tests for GetTLSIndex covering successful index retrieval, the S_FALSE case when TlsIndex == TlsOutOfIndexes, and error paths to ensure the implementation behaves as expected.
    int ISOSDacInterface.GetTLSIndex(uint* pIndex)

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
Copy link
Contributor Author

rcj1 commented Jul 17, 2025

/ba-g android-x64 Release AllSubsets_CoreCLR timeout

@jkotas
Copy link
Member

jkotas commented Jul 17, 2025

LGTM otherwise

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.

Thanks

@rcj1
Copy link
Contributor Author

rcj1 commented Jul 17, 2025

/ba-g baseservices-exceptions Work Item failure

@rcj1 rcj1 merged commit 33bc25d into dotnet:main Jul 17, 2025
95 of 99 checks passed
@rcj1 rcj1 deleted the GetTLSIndex branch July 17, 2025 15:59
@github-actions github-actions bot locked and limited conversation to collaborators Aug 17, 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.

2 participants