-
Notifications
You must be signed in to change notification settings - Fork 253
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
Changes for ClrMD 2.1 #999
Conversation
ClrMD is getting out of the business of file format reading. :) While it's useful to have a bunch of file reading abilities hanging around, there are simply better libraries for reading PE, Elf, and Mach-O images. The intent here is still read images enough to perform ClrMD's operations, but we will: Drop our dependency on System.Reflection.Runtime (unfortunately it doesn't react well when there's missing chunks of PE files and it's causing problems with Single File executables). Mark all file readers as internal and slim them down to just what ClrMD needs. Provide a new way to open Stream objects for files loaded into memory. These Streams will be formatted as if they were files on disk (not mapped into memory), meaning you can pass these files off to libraries like System.Reflection.Metadata or Marklio.Metadata and get fully featured file format readers without relying on ClrMD's buggy implementation. In effect, you may have to pick up a new dependency, but the end result will be better than where we are today. This is only a small piece of this overhaul.
- Make ModuleInfo a lot less strange. Now it's an abstract base class defining a lot of properties, but defers most of its operations to specific implementations. - Removed VersionInfo. - Removed IExportReader, this is now defined on ModuleInfo.
- Handle single-file exe. - Now consolidate all information about dacs and dbis into ClrInfo.cs and expose them via ClrInfo.DebuggingLibraries.
Marking ElfFile internal is really going to break SOS! I use it to lookup exports (ElfFile.TryGetExportSymbol). The symstore ELF reader doesn't have the symbol lookup support or otherwise, I'd use that. |
Don't pick up the change just yet. I was also going to send you an email first thing monday. =) Let me dig through the SOS code on Monday and see what needs to be there. I didn't realize you took a direct dependency on it, so if this disrupts things too much I'm happy to either add it back or modify SOS to work better with the new system. I'm not trying to put more work on you here with these changes. =) If you are only using this for export symbols, note that we actually now expose symbol reading directly through |
Also note I also plan to make creating ModuleInfo from a stream or base address possible through ClrMD, so even if you don't have the right ModuleInfo laying around it will be possible to construct one. This is not currently in the main codebase yet, mostly because the changeset was getting too big and I like to check in with smaller, more manageable PRs. SOS is unique in that you are connected to my main branch builds, so I would expect the firehose of changes to be broken for SOS for a couple of days, but the end goal here is to get to a place where these commonly needed functions are exposed to you through more sensible APIs (like Fundamentally, I plan to rewrite a lot of the elf reading code in ClrMD next to slim it down and optimize it for when we are missing data in a dump. Also, PEImage and Elf* will go request their underlying files from the symbol store when it fails a read. These are very difficult to do with the current implementation. So even if we end up exposing Elf* as public again, I'll still need to make some SOS tweaks when I change the surface area of those implementations. Let's sync up on Monday to make sure I'm headed in the right direction. |
The direct DARC dependency allows for more immediate fixes in the diagnostics repo from clrmd, but this is the downside. Technically there should be a 2.0 branch/build/DARC subscription that we could subscribe, but that is some work. I did find GetSymbolAddress() The new ModuleInfo is better. Using System.Version is better. I haven't looked at the ClrInfo changes yet, but it shouldn't take me that long to do them. I'd rather be the one to make them. |
Sounds good, let me know if you run into trouble or have questions. |
Also, the new |
ClrMD 2.1 initial changes
Please read docs/Migrating21.md.
Mark PEImage and Elf* as internal
ClrMD is getting out of the business of file format reading. :)
While it's useful to have a bunch of file reading abilities hanging around, there are simply better libraries for reading PE, Elf, and Mach-O images.
The intent here is still read images enough to perform ClrMD's operations, but we will:
In effect, you may have to pick up a new dependency, but the end result will be better than where we are today.
Rework Symbol Server API implementation
ClrMD's symbol server code had several issues which are now fixed:
symsrv*
symbol paths, as well as fully implementing the sympath syntax.Rework ClrInfo creation
While this is mostly an internal change, greatly clean up how we locate CLR runtimes in the process.
Rework ModuleInfo
ModuleInfo is now an abstract base class with implementations for Elf, Mach-O and PE images implementing various functionality. This really cleaned up an ugly wart in creating ModuleInfo objects while eliminating some unneeded interfaces. Additionally we provide a way to get resources and exports from ModuleInfo now.
Rework DAC library info
We no longer provide just a single
ClrInfo.DacLibrary
but instead enumerate all known debugging libraries for the given runtime withClrInfo.DebuggingLibraries
. This will provide all dac and dbi libraries and index properties.