Forward ICLRSymbolProvider calls to a host-supplied symbol resolver#5877
Open
max-charlamb wants to merge 1 commit into
Open
Forward ICLRSymbolProvider calls to a host-supplied symbol resolver#5877max-charlamb wants to merge 1 commit into
max-charlamb wants to merge 1 commit into
Conversation
Plumbs ClrMD's IClrSymbolProvider hook through the diagnostics host so the (c)DAC can map runtime addresses back to module-qualified symbols via the host's IModuleService / IModuleSymbols. Also unifies SOS's existing legacy-DAC symbol-provider thunks onto the same shared adapter. * HostSymbolProvider (new): IClrSymbolProvider adapter over IModuleService; registered as a per-target service via [ServiceExport]. * RuntimeProvider: sets DataTargetOptions.SymbolProvider from the registered service so ClrMD's COM data target wrapper forwards ICLRSymbolProvider calls to the host. * SOS.Hosting/DataTargetWrapper: exposes ICLRSymbolProvider on the legacy-DAC CCW (was missing), delegating to the same IClrSymbolProvider service for a single resolver code path. * Runtime: extracts the ClrFlavor -> RuntimeType mapping into a single GetRuntimeType helper; adds a generic GetDacFilePath fallback so runtimes that ship only a cDAC (no legacy DAC binary) load via the cDAC without the user having to set ForceUseContractReader. * ServiceManager: invokes RuntimeHelpers.RunModuleConstructor on each loaded extension so module initializers fire at load time, allowing extensions to register process-wide state (e.g. ClrMD IClrInfoProvider) before any provider factory is invoked. Depends on a ClrMD package that contains IClrSymbolProvider and DataTargetOptions.SymbolProvider (microsoft/clrmd PR pending; the corresponding package-version bump in eng/Version.Details.xml will land via darc once the upstream PR merges). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR plumbs ClrMD’s symbol-resolution hook through the diagnostics host so both ClrMD’s data target and SOS’s legacy-DAC COM data target can resolve module-qualified symbols via a host-provided implementation. It also adjusts extension loading to proactively run module initializers and refactors some runtime-type/DAC selection logic.
Changes:
- Add a per-target
IClrSymbolProvideradapter over the host’s module/symbol services and pass it into ClrMD viaDataTargetOptions.SymbolProvider. - Expose
ICLRSymbolProvideron SOS’s legacy-DAC data-target CCW and forward calls to the sharedIClrSymbolProviderservice. - Run extension module initializers at load time; refactor runtime flavor→type mapping and add a CDAC fallback in
GetDacFilePath.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/SOS/SOS.Hosting/DataTargetWrapper.cs | Implements ICLRSymbolProvider on the legacy-DAC data target CCW and forwards symbol queries to a managed provider. |
| src/Microsoft.Diagnostics.DebugServices.Implementation/ServiceManager.cs | Forces module initializers to run when registering an extension assembly. |
| src/Microsoft.Diagnostics.DebugServices.Implementation/RuntimeProvider.cs | Supplies DataTargetOptions.SymbolProvider from the host service container when creating ClrMD DataTarget. |
| src/Microsoft.Diagnostics.DebugServices.Implementation/Runtime.cs | Centralizes flavor→RuntimeType mapping and adds a CDAC fallback path in GetDacFilePath. |
| src/Microsoft.Diagnostics.DebugServices.Implementation/HostSymbolProvider.cs | New adapter implementing ClrMD’s IClrSymbolProvider on top of IModuleService/IModuleSymbols. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+399
to
+403
| if (!_symbolProvider.TryGetSymbolName(address, out string symbolName, out ulong displacement) | ||
| || string.IsNullOrEmpty(symbolName)) | ||
| { | ||
| return HResult.S_FALSE; | ||
| } |
Comment on lines
+405
to
+424
| if (pcchNameActual != null) | ||
| { | ||
| *pcchNameActual = (uint)symbolName.Length; | ||
| } | ||
| if (pDisplacement != null) | ||
| { | ||
| *pDisplacement = displacement; | ||
| } | ||
|
|
||
| if (cchName == 0 || pName == null) | ||
| { | ||
| return HResult.S_OK; | ||
| } | ||
|
|
||
| int copy = Math.Min(symbolName.Length, (int)cchName); | ||
| for (int i = 0; i < copy; i++) | ||
| { | ||
| pName[i] = symbolName[i]; | ||
| } | ||
| return HResult.S_OK; |
| return HResult.S_FALSE; | ||
| } | ||
|
|
||
| if (_symbolProvider.TryGetSymbolAddress(name, out ulong address) && address != 0) |
Comment on lines
+59
to
+63
| string moduleName = !string.IsNullOrEmpty(module.FileName) | ||
| ? Path.GetFileName(module.FileName) | ||
| : bareName; | ||
| symbolName = bareName.IndexOf('!') >= 0 ? bareName : $"{moduleName}!{bareName}"; | ||
| return true; |
Comment on lines
+173
to
+179
| // Force-run the assembly's module initializer ([ModuleInitializer]-marked | ||
| // methods) so extensions can register process-wide state before any | ||
| // provider factory is invoked. | ||
| foreach (System.Reflection.Module module in assembly.GetModules()) | ||
| { | ||
| System.Runtime.CompilerServices.RuntimeHelpers.RunModuleConstructor(module.ModuleHandle); | ||
| } |
Comment on lines
+124
to
+132
| if (_dacFilePath is null) | ||
| { | ||
| _cdacFilePath ??= GetLibraryPath(DebugLibraryKind.CDac); | ||
| if (_cdacFilePath is not null) | ||
| { | ||
| verifySignature = false; | ||
| return _cdacFilePath; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requires: microsoft/clrmd#1482
Plumbs ClrMD's IClrSymbolProvider hook through the diagnostics host so the (c)DAC can map runtime addresses back to module-qualified symbols via the host's IModuleService / IModuleSymbols. Also unifies SOS's existing legacy-DAC symbol-provider thunks onto the same shared adapter.
HostSymbolProvider (new): IClrSymbolProvider adapter over IModuleService; registered as a per-target service via [ServiceExport].
RuntimeProvider: sets DataTargetOptions.SymbolProvider from the registered service so ClrMD's COM data target wrapper forwards ICLRSymbolProvider calls to the host.
SOS.Hosting/DataTargetWrapper: exposes ICLRSymbolProvider on the legacy-DAC CCW (was missing), delegating to the same IClrSymbolProvider service for a single resolver code path.
Runtime: extracts the ClrFlavor -> RuntimeType mapping into a single GetRuntimeType helper; adds a generic GetDacFilePath fallback so runtimes that ship only a cDAC (no legacy DAC binary) load via the cDAC without the user having to set ForceUseContractReader.
ServiceManager: invokes RuntimeHelpers.RunModuleConstructor on each loaded extension so module initializers fire at load time, allowing extensions to register process-wide state (e.g. ClrMD IClrInfoProvider) before any provider factory is invoked.
Depends on a ClrMD package that contains IClrSymbolProvider and DataTargetOptions.SymbolProvider (microsoft/clrmd PR pending; the corresponding package-version bump in eng/Version.Details.xml will land via darc once the upstream PR merges).