[cDAC] Adding symbols and using heap dumps#126385
Merged
Merged
Conversation
rcj1
added
NO-MERGE
rcj1
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the cDAC DumpTests infrastructure to prefer heap dumps (instead of full dumps) and makes dump analysis more reliable by providing ClrMD with local symbol/module search paths (including a self-contained symbol layout for Helix runs).
Changes:
- Switch DumpTests to default to heap dumps (removing per-test
DumpType => "full"overrides) and update debuggee projects to generate heap dumps. - Add symbol/module path discovery in
DumpTestBaseand plumb these paths intoClrMdDumpHost.Open. - Update Helix dump-generation to copy
System.Private.CoreLib.dlland debuggee DLLs into asymbols/folder alongside dumps so analysis can resolve modules without the original testhost layout.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/DumpTests/SyncBlockDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/StackWalkDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/RCWDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/RCWCleanupListDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/LoaderDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/IXCLRDataValueDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/IXCLRDataFrameDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/IXCLRDataAppDomainDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/ISOSDacInterface13Tests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/ExceptionHandlingInfoDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/EcmaMetadataDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/DumpTestBase.cs | Passes additional symbol paths to ClrMD and adds logic to discover symbol/module directories (Helix symbols/ and local artifacts fallback). |
| src/native/managed/cdac/tests/DumpTests/ClrMdDumpHost.cs | Extends dump opening to accept additional symbol paths and configures ClrMD symbol search paths. |
| src/native/managed/cdac/tests/DumpTests/cdac-dump-helix.proj | Copies CoreLib + debuggee DLLs into a symbols/ tree next to dumps to enable symbol/module resolution on Helix machines. |
| src/native/managed/cdac/tests/DumpTests/ComWrappersDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/CCWDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/AsyncContinuationDumpTests.cs | Removes full-dump override so the test uses default heap dump behavior. |
| src/native/managed/cdac/tests/DumpTests/Debuggees/SyncBlock/SyncBlock.csproj | Switches debuggee dump generation to heap dumps. |
| src/native/managed/cdac/tests/DumpTests/Debuggees/StackWalk/StackWalk.csproj | Switches debuggee dump generation to heap dumps. |
| src/native/managed/cdac/tests/DumpTests/Debuggees/RCWCleanupList/RCWCleanupList.csproj | Switches debuggee dump generation to heap dumps. |
| src/native/managed/cdac/tests/DumpTests/Debuggees/RCW/RCW.csproj | Switches debuggee dump generation to heap dumps. |
| src/native/managed/cdac/tests/DumpTests/Debuggees/MultiModule/MultiModule.csproj | Switches debuggee dump generation to heap dumps. |
| src/native/managed/cdac/tests/DumpTests/Debuggees/LocalVariables/LocalVariables.csproj | Switches debuggee dump generation to heap dumps. |
| src/native/managed/cdac/tests/DumpTests/Debuggees/ExceptionHandlingInfo/ExceptionHandlingInfo.csproj | Switches debuggee dump generation to heap dumps. |
| src/native/managed/cdac/tests/DumpTests/Debuggees/ComWrappers/ComWrappers.csproj | Switches debuggee dump generation to heap dumps. |
| src/native/managed/cdac/tests/DumpTests/Debuggees/CCW/CCW.csproj | Switches debuggee dump generation to heap dumps. |
| src/native/managed/cdac/tests/DumpTests/Debuggees/AsyncContinuation/AsyncContinuation.csproj | Switches debuggee dump generation to heap dumps (and removes prior “full dump needed” comment). |
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
rcj1
removed
NO-MERGE
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/native/managed/cdac/tests/DumpTests/README.md:49
- The README still states that “Debuggees that need full memory content (e.g., metadata-heavy scenarios) override this to
Full”, but this PR changes multiple previously-Fulldebuggees toHeap. Please update the wording to reflect the new approach (heap dumps + local module files) so readers aren’t misled about whenFullis required.
The dump type is configured per-debuggee via the `DumpTypes` property in each debuggee's
`.csproj` (default: `Heap`, set in `Debuggees/Directory.Build.props`). Debuggees that
need full memory content (e.g., metadata-heavy scenarios) override this to `Full`.
hoyosjs
reviewed
Apr 27, 2026
hoyosjs
reviewed
Apr 27, 2026
hoyosjs
reviewed
Apr 27, 2026
hoyosjs
approved these changes
Apr 28, 2026
Member
hoyosjs
left a comment
hoyosjs
left a comment
There was a problem hiding this comment.
Sure - some stuff can get some cleanup. I also think this brings good wins for testing so we can clean up and improve as needed.
Contributor
Author
|
/ba-g android and iOS dead letter |
This was referenced Apr 28, 2026
max-charlamb
pushed a commit
to max-charlamb/runtime
that referenced
this pull request
May 19, 2026
The cdac and ClrMD read ECMA-335 metadata directly from the PE (`.dll`) — there is no managed source code / line-number assertion in these dump tests that would require `.pdb` files. The original code in PR dotnet#126385 (which set up this `symbols/` directory) intentionally only copied DLLs for the same reason. The actual bug this PR fixes is that `%(Identity)\%(Identity).dll` only picks up the primary DLL and silently drops any project-reference output that ends up in the same publish folder (e.g. `InterpreterStack/Trampoline.dll`). When the cdac later asked ClrMD to read metadata for `Trampoline.dll` from a Windows dump it couldn't find the file and threw `VirtualReadException`. Switching the glob to `*.dll` fixes that. The PDB-copy scaffolding I added in earlier commits has been the source of every CI failure on this PR (`|| (exit /b 0)` on Windows, `cp -t` portability, dash shell parsing) and is not needed. Drop it.
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.
For cDAC dump testing, we need metadata for our locally built System.Private.CoreLib for reading certain statics etc., as well as for debuggees for metadata tests. So that we can use heap dumps instead of full dumps, we add the symbol paths when we open the dump in ClrMD.