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] Deploy cdac with SOS in .NET 10 #108720

Open
7 tasks
Tracked by #99298
lambdageek opened this issue Oct 9, 2024 · 10 comments
Open
7 tasks
Tracked by #99298

[cdac] Deploy cdac with SOS in .NET 10 #108720

lambdageek opened this issue Oct 9, 2024 · 10 comments
Assignees
Labels
area-Diagnostics-coreclr enhancement Product code improvement that does NOT require public API changes/additions tracking This issue is tracking the completion of other related issues.
Milestone

Comments

@lambdageek
Copy link
Member

lambdageek commented Oct 9, 2024

Part of #99298

We would like to ship the cdac together with SOS and to allow SOS to load and use the cdac instead of the brittle DAC in .NET 10. This issue tracks the runtime changes necessary, and summarizes the SOS changes.

  • Rename cdacreader.dll to mscorcdac.dll (see comments new name is still up in flux)
  • Remove brittle DAC ability to load & delegate to cDAC
  • Remove the cdacreader_ entrypoints from mscorcdac
  • Define a ICLRRuntmeSymbol IDL interface with a TryGetSymbol API (for finding the cDAC machine descriptor from a target image)
  • Add a CLRDataCreateInstance entrypoint to cdac and use it to instantiate the ContractDescriptorTarget and the SOSDacImpl
  • Publish mscorcdac.dll in a transport packages.
    We will need one per supported SOS platform (that is, platforms where SOS can run, not platforms that it can debug)
  • (nice to have) [cdacreader] Simplify build order and dependencies #104158

Summary SOS changes:

  • Add a command to select cdac vs brittle DAC
  • Pull the apropriate platform-specific mscorcdac.dll from the transport nuget feed and consume it in the SOS build and publish mscorcdac.dll together with SOS
  • Implement the ICLRRuntimeSymbol::TryGetSymbol API on the ICLRDataTarget passed to CLRDataCreateInstance
  • Load and instantiate cdac instead of the brittle DAC, if enabled
  • Work with WinDbg to change how SOS is inserted and consumed

A related issue #108553 is tracking the work to implement in cdac the DAC APIs needed by SOS and CLRMA

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Oct 9, 2024
@lambdageek lambdageek added this to the 10.0.0 milestone Oct 9, 2024
Copy link
Contributor

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

@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Oct 9, 2024
@lambdageek lambdageek added the tracking This issue is tracking the completion of other related issues. label Oct 9, 2024
@lambdageek
Copy link
Member Author

lambdageek commented Oct 9, 2024

The entrypoint would be

STDAPI CLRDataCreateInstance(REFIID iid, ICLRDataTarget* target, void** iface);

Where iid will be one of the ISOSDacInterfaceNN interfaces

The cdac as currently imagined needs two things to get going:

  1. A way to read from the target address space
  2. The address of the contract descriptor which is a symbol with the name DotNetRuntimeContractDescriptor

We can get the former using ICLRDataTarget::ReadVirtual.

For the latter we could QI target for an interface like this:

interface ICLRRuntimeSymbol : IUnknown
{
   HRESULT TryGetSymbol([in] LPCSTR symbolName, [out] CLRDATA_ADDRESS* address);
}

similar to what the brittle DAC does with ICLRRuntimeLocator

@tommcdon tommcdon added the enhancement Product code improvement that does NOT require public API changes/additions label Oct 9, 2024
@noahfalk
Copy link
Member

noahfalk commented Oct 9, 2024

I assume when SOS is targetting downlevel runtimes it would still load an older brittle DAC?

@lambdageek
Copy link
Member Author

I assume when SOS is targetting downlevel runtimes it would still load an older brittle DAC?

I think there's two separate answers:

  1. In the long term, SOS should have some kind of a policy. For example: try the cdac and if it returns E_NOTIMPL or some other error code, try a brittle DAC. (I'm not sure what a reasonable policy might be)
  2. For the purposes of trying things out, @mikem8361 and I were talking about having some gesture (ie: you have to run some SOS command) to opt into the cdac.

@mikem8361
Copy link
Member

We can get the former using ICLRDataTarget::ReadVirtual and the latter using something like this:

interface ICLRDataTarget4 : ICLRDataTarget3
{
HRESULT TryGetSymbol([in] LPCSTR symbolName, [out] CLRDATA_ADDRESS* address);
}

Now I'm thinking this should be more like ICLRRuntimeLocator and not inherit with only TryGetSymbol on it. It would allow a more minimal data target impl for cases that are only going to use the cDAC (like in testing, etc.). SOS will have to implement all the interfaces it does now and it would be easy to add this one more. We can call it something like ICLRRuntimeSymbol.

@jkoritzinsky
Copy link
Member

What about for diagnostic experiences that currently depend on contracts not defined by ISOSDacInterfaceX interfaces? In particular, there are a few that depend on the "SOS Breaking Change Version".

Some of these APIs are relatively easily representable in cDAC contracts with a managed API but projecting them through an unmanaged interface could be expensive. Is there some way we could come up with a solution that wouldn't require going through an unmanaged API?

@lambdageek
Copy link
Member Author

What about for diagnostic experiences that currently depend on contracts not defined by ISOSDacInterfaceX interfaces? In particular, there are a few that depend on the "SOS Breaking Change Version".

Some of these APIs are relatively easily representable in cDAC contracts with a managed API but projecting them through an unmanaged interface could be expensive. Is there some way we could come up with a solution that wouldn't require going through an unmanaged API?

I think mscorcdac and Microsoft.Diagnostics.DataContractReader are separate products (that happen to be built from substantially similar sources). We could have a separate plan for how we ship the managed library.

@AaronRobinsonMSFT
Copy link
Member

mscorcdac.dll

This name is a bit suspect. We've removed "ms" from many places - mscorlib being the most obvious example. Another issue with this name is that it is far too similar to existing names. That "c" in the middle is doing a lot to lifting to handle the disambiguation when quickly reading names. I would suggest something like spelling out the "dac" or even just dropping the "ms" - cordac or perhaps diagnostic.access.control?

@mikem8361
Copy link
Member

I'm not hung up on the name. I suggested something similar to the existing DAC name, but not for any good reason. I guess I assumed that "ms" was supposed to be in the name.

@noahfalk
Copy link
Member

noahfalk commented Oct 11, 2024

diagnostic.access.control

The original acronym DAC stands for "Data Access Component". Its a pretty generic name :) I'd avoid "access control" as that term has strong connotations of security authentication and authorization. Any of these names sounded fine to me:
cdac.dll, cordac.dll, contractdac.dll

@lambdageek lambdageek removed their assignment Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Diagnostics-coreclr enhancement Product code improvement that does NOT require public API changes/additions tracking This issue is tracking the completion of other related issues.
Projects
None yet
Development

No branches or pull requests

8 participants