[cDAC] Fix DebugInfo error codes for methods without debug metadata and add P/Invoke dump tests#124783
Merged
max-charlamb merged 4 commits intodotnet:mainfrom Feb 25, 2026
Merged
Conversation
github-actions
Bot
added
the
area-CodeGen-coreclr
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug in the cDAC's DebugInfo contract where GetILAddressMap and GetILOffsetsByAddress returned incorrect error codes for methods without debug metadata (e.g., ILStubs). The fix changes GetMethodNativeMap to return null instead of an empty sequence when no debug info exists, matching the legacy DAC's E_FAIL behavior. Additionally, the PR adds comprehensive dump-based integration tests for P/Invoke scenarios and introduces test infrastructure for OS-aware test skipping.
Changes:
- Modified
IDebugInfo.GetMethodNativeMapto return nullableIEnumerable<OffsetMapping>?to distinguish "no debug info" (null → E_FAIL) from "debug info exists but empty" (empty sequence → E_NOINTERFACE) - Added OS-aware test infrastructure (
DumpInfo,SkipOnOSAttribute) enabling platform-specific test skipping - Added two new dump-based test suites for P/Invoke scenarios (VarargPInvoke and PInvokeStub) with Windows-only debuggees
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IDebugInfo.cs | Changed return type to nullable and added documentation for null semantics |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/DebugInfo/DebugInfo_1.cs | Added null check for debugInfo pointer, returns null when debug info doesn't exist |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/DebugInfo/DebugInfo_2.cs | Added null check for debugInfo pointer, returns null when debug info doesn't exist |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataMethodInstance.cs | Updated to handle null return from GetMethodNativeMap and throw E_FAIL |
| docs/design/datacontracts/DebugInfo.md | Updated contract documentation to explain null vs empty sequence semantics |
| src/native/managed/cdac/tests/DumpTests/DumpInfo.cs | New metadata class for OS/arch information stored in dump-info.json |
| src/native/managed/cdac/tests/DumpTests/SkipOnOSAttribute.cs | New attribute for OS-based test skipping with include-only and exclude modes |
| src/native/managed/cdac/tests/DumpTests/DumpTestBase.cs | Updated to load dump metadata and evaluate SkipOnOS attributes |
| src/native/managed/cdac/tests/DumpTests/DumpTestHelpers.cs | Generalized FindFailFastThread to FindThreadWithMethod for reusability |
| src/native/managed/cdac/tests/DumpTests/DumpTests.targets | Added MSBuild target to generate dump-info.json during dump creation |
| src/native/managed/cdac/tests/DumpTests/RuntimeInfoDumpTests.cs | Enhanced tests to validate against dump metadata instead of just enum validity |
| src/native/managed/cdac/tests/DumpTests/VarargPInvokeDumpTests.cs | New test suite for vararg P/Invoke stack walking and ILStub handling |
| src/native/managed/cdac/tests/DumpTests/PInvokeStubDumpTests.cs | New test suite for SetLastError P/Invoke with ILStub frames |
| src/native/managed/cdac/tests/DumpTests/Debuggees/VarargPInvoke/Program.cs | Debuggee that crashes in sprintf vararg call to test VarargPInvokeStub |
| src/native/managed/cdac/tests/DumpTests/Debuggees/VarargPInvoke/VarargPInvoke.csproj | Project configuration for VarargPInvoke debuggee |
| src/native/managed/cdac/tests/DumpTests/Debuggees/PInvokeStub/Program.cs | Debuggee that crashes in memcpy to test ILStub with SetLastError |
| src/native/managed/cdac/tests/DumpTests/Debuggees/PInvokeStub/PInvokeStub.csproj | Project configuration for PInvokeStub debuggee |
| src/native/managed/cdac/tests/DumpTests/Microsoft.Diagnostics.DataContractReader.DumpTests.csproj | Added HResults.cs link and Legacy project reference |
| src/native/managed/cdac/tests/DumpTests/README.md | Updated documentation with OS-based skipping and new debuggees |
max-charlamb
added
area-Diagnostics-coreclr
and removed
area-CodeGen-coreclr
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
noahfalk
approved these changes
Feb 24, 2026
Member
noahfalk
left a comment
noahfalk
left a comment
There was a problem hiding this comment.
LGTM modulo one API suggestion there
iremyux
pushed a commit
to iremyux/dotnet-runtime
that referenced
this pull request
Mar 2, 2026
…nd add P/Invoke dump tests (dotnet#124783) ## Summary Fix `GetILAddressMap` and `GetILOffsetsByAddress` in the cDAC to return `E_FAIL` (matching the legacy DAC) when a method has no debug info, and add dump-based integration tests exercising ILStub and VarargPInvoke stack frames. ## Problem ILStubs are JIT-compiled but the JIT does not generate debug metadata for them — their `RealCodeHeader.DebugInfo` pointer is null. When SOS commands like `!clru -il` call `GetILAddressMap` on an ILStub: - **Legacy DAC**: `ClrDataAccess::GetMethodNativeMap` calls `DebugInfoManager::GetBoundariesAndVars`, which returns `FALSE` -> returns `E_FAIL` (0x80004005). This propagates through `GetILAddressMap` without ever reaching the `E_NOINTERFACE` path. - **cDAC (before this fix)**: `DebugInfo_1/2.GetMethodNativeMap` attempted to read from the null debug info pointer -> `VirtualReadException` (0x80131c49) escaped through `catch (Exception ex) { hr = ex.HResult; }`, producing an HResult that doesn't match the DAC. This caused `Debug.Assert(hrLocal == hr)` failures in CI when running SOS tests against the cDAC. ## Fix ### Contract change: `HasDebugInfo` API and non-nullable `GetMethodNativeMap` The DAC has two distinct failure paths for empty/missing debug info: | Scenario | DAC behavior | HResult | |----------|-------------|---------| | No debug info exists (ILStubs) | `GetBoundariesAndVars` returns `FALSE` | `E_FAIL` (0x80004005) | | Debug info exists, zero entries | `numMap == 0` after successful retrieval | `E_NOINTERFACE` (0x80004002) | To avoid having a nullable return on `GetMethodNativeMap` (where `null` vs empty is a subtle distinction), the fix adds a new `bool HasDebugInfo(TargetCodePointer pCode)` API to the `IDebugInfo` contract. This lets callers explicitly check for the existence of debug info before calling `GetMethodNativeMap`, which always returns a non-nullable `IEnumerable<OffsetMapping>` (empty when no debug info exists). In `ClrDataMethodInstance`, both `GetILOffsetsByAddress` and `GetILAddressMap` call `HasDebugInfo` first and throw `E_FAIL` when it returns `false`, matching the legacy DAC behavior. ## New dump tests ### Test infrastructure - **`DumpInfo`** — Metadata class that records OS/arch in `dump-info.json` alongside dumps, enabling platform-aware test skipping. - **`SkipOnOSAttribute`** — `[SkipOnOS(IncludeOnly = "windows")]` skips tests when the dump wasn't generated on the specified OS. Supports both exclude and include-only modes. - **`DumpTests.targets`** — MSBuild target that auto-generates `dump-info.json` during dump creation. ### Debuggees - **`VarargPInvoke`** — Calls `sprintf` via `__arglist`, triggering the `VarargPInvokeStub` assembly thunk path. Crashes inside native code so the stub frame is on the stack. - **`PInvokeStub`** — Uses `SetLastError=true` on a P/Invoke to force an ILStub `DynamicMethodDesc`. Crashes inside `memcpy` with null args.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
Fix
GetILAddressMapandGetILOffsetsByAddressin the cDAC to returnE_FAIL(matching the legacy DAC) when a method has no debug info, and add dump-based integration tests exercising ILStub and VarargPInvoke stack frames.Problem
ILStubs are JIT-compiled but the JIT does not generate debug metadata for them — their
RealCodeHeader.DebugInfopointer is null. When SOS commands like!clru -ilcallGetILAddressMapon an ILStub:ClrDataAccess::GetMethodNativeMapcallsDebugInfoManager::GetBoundariesAndVars, which returnsFALSE-> returnsE_FAIL(0x80004005). This propagates throughGetILAddressMapwithout ever reaching theE_NOINTERFACEpath.DebugInfo_1/2.GetMethodNativeMapattempted to read from the null debug info pointer ->VirtualReadException(0x80131c49) escaped throughcatch (Exception ex) { hr = ex.HResult; }, producing an HResult that doesn't match the DAC.This caused
Debug.Assert(hrLocal == hr)failures in CI when running SOS tests against the cDAC.Fix
Contract change:
HasDebugInfoAPI and non-nullableGetMethodNativeMapThe DAC has two distinct failure paths for empty/missing debug info:
GetBoundariesAndVarsreturnsFALSEE_FAIL(0x80004005)numMap == 0after successful retrievalE_NOINTERFACE(0x80004002)To avoid having a nullable return on
GetMethodNativeMap(wherenullvs empty is a subtle distinction), the fix adds a newbool HasDebugInfo(TargetCodePointer pCode)API to theIDebugInfocontract. This lets callers explicitly check for the existence of debug info before callingGetMethodNativeMap, which always returns a non-nullableIEnumerable<OffsetMapping>(empty when no debug info exists).In
ClrDataMethodInstance, bothGetILOffsetsByAddressandGetILAddressMapcallHasDebugInfofirst and throwE_FAILwhen it returnsfalse, matching the legacy DAC behavior.New dump tests
Test infrastructure
DumpInfo— Metadata class that records OS/arch indump-info.jsonalongside dumps, enabling platform-aware test skipping.SkipOnOSAttribute—[SkipOnOS(IncludeOnly = "windows")]skips tests when the dump wasn't generated on the specified OS. Supports both exclude and include-only modes.DumpTests.targets— MSBuild target that auto-generatesdump-info.jsonduring dump creation.Debuggees
VarargPInvoke— Callssprintfvia__arglist, triggering theVarargPInvokeStubassembly thunk path. Crashes inside native code so the stub frame is on the stack.PInvokeStub— UsesSetLastError=trueon a P/Invoke to force an ILStubDynamicMethodDesc. Crashes insidememcpywith null args.CI Failure