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] Make DAC load and use cDAC when available #100946

Merged
merged 17 commits into from
Apr 17, 2024

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Apr 12, 2024

  • If DOTNET_ENABLE_CDAC environment variable is 1, Look for cdacreader next to DAC and load it if found
  • Implement ISOSDacInterface in cDAC - currently returns E_NOTIMPL for everything
    • This was kind of gross, but also the simplest way I could see of being able to slowly replace parts of the existing DAC while validating new functionality against existing.
  • Make DAC delegate to cDAC (if available) for GetThreadStoreData and GetBreakingChangeVersion
  • Initialize cDAC with function for reading from the target
    • Just tested reading a pointer manually by setting an address to read in the debugger

Contributes to #99298
Resolves #99301

Copy link
Contributor

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

@elinor-fung elinor-fung changed the title Make DAC load and use cDAC when available [cdac] Make DAC load and use cDAC when available Apr 12, 2024
@elinor-fung elinor-fung marked this pull request as ready for review April 12, 2024 17:25
src/coreclr/debug/daccess/cdac.h Outdated Show resolved Hide resolved
src/coreclr/debug/daccess/cdac.cpp Outdated Show resolved Hide resolved
src/coreclr/debug/daccess/cdac.cpp Outdated Show resolved Hide resolved
src/coreclr/debug/daccess/cdac.cpp Show resolved Hide resolved
src/coreclr/debug/daccess/daccess.cpp Outdated Show resolved Hide resolved
Copy link
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.

This current model would require each implementation in the existing DAC of the ISOSDacInterfaceX interfaces to be edited each time a new method is implemented in the cDAC. Could we implement this using a COM Aggregation object that implements all the interface methods by calling the cDAC first and then calling the DAC implementation instead of updating each method implementation in the DAC?

@elinor-fung
Copy link
Member Author

object that implements all the interface methods by calling the cDAC first and then calling the DAC implementation instead of updating each method implementation in the DAC?

That was actually my original plan. After getting in there and seeing just how many methods there were and how many of them don't seem relevant any more, I wasn't sure it was worth it. But maybe we start to implement more and decide it is. I could go either way.

Co-authored-by: Aaron Robinson <arobins@microsoft.com>
Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>
@elinor-fung elinor-fung merged commit e126f8f into dotnet:main Apr 17, 2024
149 of 152 checks passed
@elinor-fung elinor-fung deleted the dac-use-cdac branch April 17, 2024 04:37
matouskozak pushed a commit to matouskozak/runtime that referenced this pull request Apr 30, 2024
- If `DOTNET_ENABLE_CDAC` environment variable is `1`, Look for `cdacreader` next to DAC and load it if found
- Implement `ISOSDacInterface` in cDAC - currently returns `E_NOTIMPL` for everything
- Make DAC delegate to cDAC (if available) for GetThreadStoreData and GetBreakingChangeVersion
- Initialize cDAC with function for reading from the target
@github-actions github-actions bot locked and limited conversation to collaborators May 17, 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.

[cdac] Enable the DAC to delegate operations to the cDAC
5 participants