Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cdac] Start Loader contract and implement ISOSDacInterface::GetModuleData in cDAC #104257

Merged
merged 16 commits into from
Jul 9, 2024

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Jul 1, 2024

  • Start a Loader contract - currently contains what is needed for GetModuleData
  • Implement ISOSDacInterface::GetModuleData in cDAC
  • Store base address and is reflection emit bit on Module for easier diagnostics access

Contributes to #99302

Copy link
Contributor

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

[Flags]
enum ModuleFlags
{
EditAndContinue = 0x00000008, // Edit and Continue is enabled for this module
Copy link
Member Author

Choose a reason for hiding this comment

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

This was the only value I found in dotnet/diagnostics and microsoft/clrmd that was actually used. And it was just to print an explicit IS_EDIT_AND_CONTINUE as part of dumpmodule (also prints the raw flag values).

src/coreclr/debug/runtimeinfo/contracts.jsonc Show resolved Hide resolved
Comment on lines 16 to 18
if (peImagePointer != TargetPointer.Null)
PEImage = target.ProcessedData.GetOrAdd<PEImage>(peImagePointer);
}
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we're going to have a problem in practice with the cDAC being more eager to read data from the target process up front. I think the brittle DAC is lazier about following pointers in target memory. It might be able to emit more info to the debugger before failing.

Copy link
Member

Choose a reason for hiding this comment

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

This is a concern for mini and triage dumps only. What we need to do for those is make sure that our routines for ensuring that enough data structures are live are updated so that they make sure to cover the important scenarios needed for examining dumps. Also we will likely need to have the ability for some of the apis to produce incomplete results. The only reason this works in the existing scheme is that there are a selected set of places in the DAC where failures are just ignored. We will eventually need to add the same things to the cDAC, but I don't think we should do that yet.

docs/design/datacontracts/Loader.md Outdated Show resolved Hide resolved
docs/design/datacontracts/Loader.md Outdated Show resolved Hide resolved
docs/design/datacontracts/Loader.md Outdated Show resolved Hide resolved
src/native/managed/cdacreader/src/Contracts/Loader_1.cs Outdated Show resolved Hide resolved
@elinor-fung elinor-fung merged commit 04a40c1 into dotnet:main Jul 9, 2024
151 checks passed
@elinor-fung elinor-fung deleted the cdac-module-data branch July 9, 2024 15:54
matouskozak added a commit to matouskozak/runtime that referenced this pull request Jul 11, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 9, 2024
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.

5 participants