Implement IXCLRDataProcess3::GetFunctionTable in cDAC#131284
Conversation
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Implements dynamic function table enumeration in cDAC by adding an ExecutionManager contract API to walk code heaps and collect inline RUNTIME_FUNCTION entries, then wiring that into the IXCLRDataProcess3::GetFunctionTable COM shim so out-of-process unwinders can query function tables without relying on the DAC export.
Changes:
- Add
IExecutionManager.GetDynamicFunctionTableEntriesand implement it by matching the targetDYNAMIC_FUNCTION_TABLEto a code heap and reverse-walking the nibble map to collect unwind entries. - Extend cDAC data descriptors/types to represent
DynamicFunctionTableand (64-bit)CodeHeapListNode.CLRPersonalityRoutine, and update the ExecutionManager contract documentation. - Add unit tests covering ordering, stub filtering, module-base matching, and COM buffer semantics for
GetFunctionTable.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/UnitTests/MockDescriptors/MockDescriptors.ExecutionManager.cs | Extend mock layouts/builders to model dynamic function tables and (64-bit) personality routine, and to author inline unwind info entries for tests. |
| src/native/managed/cdac/tests/UnitTests/FunctionTableAccessTests.cs | Add end-to-end tests for IXCLRDataProcess3::GetFunctionTable HRESULT + buffer semantics and byte-for-byte copying behavior. |
| src/native/managed/cdac/tests/UnitTests/ExecutionManager/ExecutionManagerTests.cs | Add contract-level tests for dynamic function table enumeration (ordering, stub filtering, module-base matching, context flag stripping). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs | Implement IXCLRDataProcess3.GetFunctionTable by calling the ExecutionManager contract and marshaling contiguous RUNTIME_FUNCTION bytes. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs | Add DynamicFunctionTable to the well-known DataType enum. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/DynamicFunctionTable.cs | Introduce an IData view for the target DYNAMIC_FUNCTION_TABLE fields needed by the contract. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/CodeHeapListNode.cs | Add optional (64-bit) CLRPersonalityRoutine field for module-base matching semantics. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.EEJitManager.cs | Add helper to enumerate function table entries by walking the nibble map backwards and collecting per-method unwind entries. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.cs | Implement GetDynamicFunctionTableEntries by reading DynamicFunctionTable, stripping context flags, matching heap module base, and enumerating entries. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_1.cs | Forward the new API through the v1 contract implementation. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManager_2.cs | Forward the new API through the v2 contract implementation. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IExecutionManager.cs | Add the new contract method to the public abstractions interface. |
| src/coreclr/vm/datadescriptor/datadescriptor.inc | Add descriptors for DynamicFunctionTable (platform-gated) and CodeHeapListNode.CLRPersonalityRoutine (64-bit). |
| docs/design/datacontracts/ExecutionManager.md | Document the new contract API, required descriptors, and the enumeration/marshaling algorithm. |
a43c609 to
9641844
Compare
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs:47
- GetFunctionTable now unconditionally relies on DynamicFunctionTable/RuntimeFunction type info. Those descriptors are only emitted for !TARGET_UNIX && !TARGET_X86 (see src/coreclr/vm/datadescriptor/datadescriptor.inc), so on unsupported targets this will throw and return an unrelated exception HRESULT instead of a stable E_NOTIMPL.
IExecutionManager executionManager = _target.Contracts.ExecutionManager;
IReadOnlyList<TargetPointer> functionEntries =
executionManager.GetDynamicFunctionTableEntries(tableAddress.ToTargetPointer(_target));
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs:67
- Span lengths are int. If totalBytes is between (int.MaxValue, uint.MaxValue], the current code will accept a sufficiently large bufferSize but then throw when creating the destination Span, returning an exception HRESULT. It should detect this and fail gracefully.
if (buffer is null || bufferSize < totalBytes)
return HResults.S_FALSE;
Span<byte> destination = new(buffer, checked((int)totalBytes));
int entrySize = checked((int)runtimeFunctionSize);
noahfalk
left a comment
There was a problem hiding this comment.
LGTM - a few doc suggestions inline
Fills in the stub added in dotnet#130762 so out-of-process unwinders (windbg) can enumerate dynamic function tables via cDAC instead of the DAC's exported OutOfProcessFunctionTableCallbackEx. - Add ExecutionManager.GetDynamicFunctionTableEntries, a faithful port of OutOfProcessFunctionTableCallbackEx: strip the DYNAMIC_FUNCTION_TABLE Context flag bits to find the owning EEJitManager, match a code heap by module base (HeapList::GetModuleBase - CLRPersonalityRoutine when set, else MapBase), then reverse-walk the nibble map collecting each real code header's inline RUNTIME_FUNCTION entries (skipping stub blocks). - Add data descriptors: DynamicFunctionTable (Context, MinimumAddress) and CodeHeapListNode.CLRPersonalityRoutine (64-bit only). - Implement SOSDacImpl.IXCLRDataProcess3.GetFunctionTable marshaling per the IDL contract (E_POINTER, size query, S_FALSE, S_OK). - Update ExecutionManager data contract doc and add unit tests covering enumeration order, stub filtering, module-base matching (including the zero-personality-routine fallback), and the COM buffer semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b09fa3f-0aee-4846-84e2-0930640d03cc
DYNAMIC_FUNCTION_TABLE registration is available only on Windows non-x86 targets. Return an empty result on other targets before accessing its platform-specific descriptor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b09fa3f-0aee-4846-84e2-0930640d03cc
- Document dynamic function table entries at the contract level - Regenerate descriptor usage documentation - Avoid aggregate Int32-sized COM output spans - Preserve platform-gate test setup after rebase Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b09fa3f-0aee-4846-84e2-0930640d03cc
9641844 to
ec69759
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.IXCLRDataProcess.cs:60
- GetFunctionTable reads DataType.RuntimeFunction size before checking whether any entries were found. On unsupported platforms (where GetDynamicFunctionTableEntries returns an empty list) or for unmatched tables, this can still require RuntimeFunction type info and turn an otherwise-successful "0 entries" result into an exception/HRESULT failure if the RuntimeFunction descriptor isn't available on that target.
IExecutionManager executionManager = _target.Contracts.ExecutionManager;
IReadOnlyList<TargetPointer> functionEntries =
executionManager.GetDynamicFunctionTableEntries(tableAddress.ToTargetPointer(_target));
uint runtimeFunctionSize = _target.GetTypeInfo(DataType.RuntimeFunction).Size!.Value;
uint count = (uint)functionEntries.Count;
ulong totalBytes = (ulong)count * runtimeFunctionSize;
if (totalBytes > uint.MaxValue)
return HResults.E_FAIL;
*entries = count;
*bytesNeeded = (uint)totalBytes;
// An empty or unmatched table reports zero entries and succeeds.
if (count == 0)
return HResults.S_OK;
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.EEJitManager.cs:189
- The offset computation for each unwind entry does the multiplication in 32-bit (
i * runtimeFunctionSize) before casting to ulong. If NumUnwindInfos is ever large, this can wrap and produce incorrect target addresses. The DAC code computes offsets in pointer-sized arithmetic.
Data.RealCodeHeader realCodeHeader = Target.ProcessedData.GetOrAdd<Data.RealCodeHeader>(codeHeaderAddress);
for (uint i = 0; i < realCodeHeader.NumUnwindInfos; i++)
entries.Add(realCodeHeader.UnwindInfos + (ulong)(i * runtimeFunctionSize));
}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b09fa3f-0aee-4846-84e2-0930640d03cc
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/DataType.cs:140
DataTypeis a public enum with implicit values. InsertingDynamicFunctionTablehere will renumber all subsequent members, which can be a breaking change for any consumers that persist or interop using the numeric values. Prefer appending new enum members at the end (or assigning explicit values to all members) to keep existing numeric values stable.
RealCodeHeader,
InterpreterRealCodeHeader,
CodeHeapListNode,
CodeHeap,
LoaderCodeHeap,
HostCodeHeap,
DynamicFunctionTable,
MethodDescVersioningState,
ILCodeVersioningState,
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IExecutionManager.cs:118
IExecutionManageris a public contract interface (in a packable Abstractions assembly). AddingGetDynamicFunctionTableEntriesintroduces new public API surface; this typically requires an approved API proposal/issue to be linked, or alternatively keeping the APIinternaluntil approval. As-is, there’s no evidence of API approval being tracked in this PR.
bool IsGcSafe(TargetCodePointer instructionPointer) => throw new NotImplementedException();
List<ExceptionClauseInfo> GetExceptionClauses(CodeBlockHandle codeInfoHandle) => throw new NotImplementedException();
uint GetStackParameterSize(CodeBlockHandle codeInfoHandle) => throw new NotImplementedException();
JitManagerInfo GetEEJitManagerInfo() => throw new NotImplementedException();
IEnumerable<ICodeHeapInfo> GetCodeHeapInfos() => throw new NotImplementedException();
IReadOnlyList<TargetPointer> GetDynamicFunctionTableEntries(TargetPointer tableAddress) => throw new NotImplementedException();
// Classify a code address as a known stub kind (precode, jump stub, VSD stub, etc.)
|
/ba-g analysis is stuck - all legs are green |
Fills in the `IXCLRDataProcess3::GetFunctionTable` stub added in dotnet#130762 so out-of-process unwinders (such as windbg) can enumerate dynamic function tables via cDAC instead of the DAC''s exported `OutOfProcessFunctionTableCallbackEx`. ## What - **Contract**: Adds `IExecutionManager.GetDynamicFunctionTableEntries(tableAddress)`, a faithful port of `OutOfProcessFunctionTableCallbackEx` (`src/coreclr/debug/daccess/fntableaccess.cpp`): - Reads the target `DYNAMIC_FUNCTION_TABLE`, strips the `Context` flag bits to find the owning `EEJitManager`, and walks its code-heap list. - Matches the heap by module base using `HeapList::GetModuleBase` semantics (the personality routine when set, otherwise the map base) against `MinimumAddress`. - Reverse-walks the nibble map from the end of the used region, resolves each real code header (skipping stub code blocks), and collects the inline `RUNTIME_FUNCTION` entries. - **Data descriptors**: Adds `DynamicFunctionTable` (`Context`, `MinimumAddress`) and `CodeHeapListNode.CLRPersonalityRoutine` (64-bit only). - **SOS shim**: Implements `SOSDacImpl.IXCLRDataProcess3.GetFunctionTable` marshaling per the IDL contract — `E_POINTER` for null out-params, always reports required byte/entry counts, `S_OK` with zero for an empty/unmatched table, `S_FALSE` for a size query or undersized buffer, `S_OK` after copying the contiguous `RUNTIME_FUNCTION` bytes. - **Docs**: Updates `docs/design/datacontracts/ExecutionManager.md` with the new API, descriptor fields, and algorithm. ## Testing - New unit tests (all 4 architectures x c1/c2 nibble-map versions) covering enumeration order, stub filtering, module-base matching (including the zero-personality-routine fallback to map base), and the COM buffer semantics (size query, too-small buffer, successful copy, unmatched table). - Full cDAC unit suite passes; `build.cmd clr` succeeds (validates the descriptor compiles). > [!NOTE] > This pull request was authored with the assistance of GitHub Copilot (AI). --------- Co-authored-by: Max Charlamb <maxcharlamb@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b09fa3f-0aee-4846-84e2-0930640d03cc
Fills in the
IXCLRDataProcess3::GetFunctionTablestub added in #130762 so out-of-process unwinders (such as windbg) can enumerate dynamic function tables via cDAC instead of the DAC''s exportedOutOfProcessFunctionTableCallbackEx.What
IExecutionManager.GetDynamicFunctionTableEntries(tableAddress), a faithful port ofOutOfProcessFunctionTableCallbackEx(src/coreclr/debug/daccess/fntableaccess.cpp):DYNAMIC_FUNCTION_TABLE, strips theContextflag bits to find the owningEEJitManager, and walks its code-heap list.HeapList::GetModuleBasesemantics (the personality routine when set, otherwise the map base) againstMinimumAddress.RUNTIME_FUNCTIONentries.DynamicFunctionTable(Context,MinimumAddress) andCodeHeapListNode.CLRPersonalityRoutine(64-bit only).SOSDacImpl.IXCLRDataProcess3.GetFunctionTablemarshaling per the IDL contract —E_POINTERfor null out-params, always reports required byte/entry counts,S_OKwith zero for an empty/unmatched table,S_FALSEfor a size query or undersized buffer,S_OKafter copying the contiguousRUNTIME_FUNCTIONbytes.docs/design/datacontracts/ExecutionManager.mdwith the new API, descriptor fields, and algorithm.Testing
build.cmd clrsucceeds (validates the descriptor compiles).Note
This pull request was authored with the assistance of GitHub Copilot (AI).