Skip to content

[cDAC] Change pointer data to be compile-time constant#118543

Merged
max-charlamb merged 3 commits intodotnet:mainfrom
max-charlamb:cdac-pointer-data
Aug 8, 2025
Merged

[cDAC] Change pointer data to be compile-time constant#118543
max-charlamb merged 3 commits intodotnet:mainfrom
max-charlamb:cdac-pointer-data

Conversation

@max-charlamb
Copy link
Copy Markdown
Member

  • Updates the pointer data array to be a constexpr. On MSVC, this requires enabling a flag to allow external linkage on a constexpr variable.
  • StressLog used a global value which was not a compile-time constant (&g_pStressLog->modules). I updated the contract to reference this field as an offset when the StressLog is available. When it is not available it uses the old pointer (StressLogAnalyzer.cs scenario).

@max-charlamb max-charlamb added the enhancement Product code improvement that does NOT require public API changes/additions label Aug 8, 2025
Copilot AI review requested due to automatic review settings August 8, 2025 15:59
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

This PR changes pointer data arrays in the cDAC (Contract Data Access) system from runtime values to compile-time constants by making them constexpr. The key motivation is to enable compile-time evaluation of these data structures, which requires special handling for MSVC compiler compatibility.

Key changes:

  • Convert pointer data arrays to constexpr with MSVC-specific compiler flag
  • Refactor StressLog module table access to use field offsets instead of global pointers
  • Update data contracts to support both legacy and new access patterns

Reviewed Changes

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

Show a summary per file
File Description
src/coreclr/clrdatadescriptors.cmake Adds MSVC compiler flag /Zc:externConstexpr to enable external linkage on constexpr variables
src/coreclr/debug/datadescriptor-shared/contractpointerdata.cpp Changes pointer data array from uintptr_t to constexpr void* array
src/coreclr/debug/datadescriptor-shared/contractdescriptorstub.c Updates pointer data types from uintptr_t* to void**
src/coreclr/debug/datadescriptor-shared/contract-descriptor.c.in Updates pointer data types from uintptr_t* to void**
src/coreclr/inc/stresslog.h Changes cdac_offsets template members from const to constexpr
src/coreclr/vm/datadescriptor/datadescriptor.inc Removes global StressLogModuleTable pointer, adds Modules field to StressLog type
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/StressLog.cs Adds optional Modules property to support new field-based access
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StressLog.cs Implements fallback logic to access module table via StressLog field when global pointer unavailable
src/tools/StressLogAnalyzer/src/Program.cs Adds Modules field definition for StressLogAnalyzer compatibility
docs/design/datacontracts/StressLog.md Updates documentation to reflect removal of global StressLogModuleTable and addition of Modules field

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

@max-charlamb max-charlamb requested a review from jkotas August 8, 2025 16:53
Comment thread docs/design/datacontracts/StressLog.md
Copy link
Copy Markdown
Member

@jkoritzinsky jkoritzinsky left a comment

Choose a reason for hiding this comment

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

Stresslog changes LGTM

@max-charlamb max-charlamb merged commit e22ee70 into dotnet:main Aug 8, 2025
98 checks passed
@max-charlamb max-charlamb deleted the cdac-pointer-data branch August 8, 2025 18:38
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Diagnostics-coreclr enhancement Product code improvement that does NOT require public API changes/additions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants