Skip to content

Restrict GetILAddressMap HResult debug assertion to specific error codes in cDAC#124615

Closed
Copilot wants to merge 4 commits intomainfrom
copilot/fix-runtime-diagnostics-pipeline
Closed

Restrict GetILAddressMap HResult debug assertion to specific error codes in cDAC#124615
Copilot wants to merge 4 commits intomainfrom
copilot/fix-runtime-diagnostics-pipeline

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 19, 2026

Description

The cDAC runtime-diagnostics pipeline fails because the legacy DAC returns E_FAIL (0x80004005) while the cDAC returns CORDBG_E_READVIRTUAL_FAILURE (0x80131c49) for GetILAddressMap failures. The strict Debug.Assert on HResult equality trips on this mismatch.

Changed the assertion in ClrDataMethodInstance.cs to use a local IsReadFailure helper that specifically accepts only HResults.E_FAIL or CorDbgHResults.CORDBG_E_READVIRTUAL_FAILURE, rather than accepting any negative (failing) HResult. This ensures unexpected error codes are still caught while allowing the known DAC/cDAC mismatch.

This fixes the VarargPInvokeInteropMD in runtime-diagnostics.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

The DAC returns E_FAIL while the cDAC returns CORDBG_E_READVIRTUAL_FAILURE
for GetILAddressMap failures. Use the same permissive pattern already used
for GetILOffsetsByAddress: treat any two failure HResults (both negative)
as equivalent.

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix assertion in cDAC runtime-diagnostics pipeline Relax GetILAddressMap HResult debug assertion in cDAC Feb 19, 2026
Copilot AI requested a review from max-charlamb February 19, 2026 19:45
@max-charlamb
Copy link
Copy Markdown
Member

@copilot, the change was not made

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

…fsetsByAddress

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 19, 2026

@copilot, the change was not made

The change was in the branch but wasn't showing in the PR due to a push sync issue. Re-pushed in 20d21023 — the PR diff now correctly shows the assertion change from Debug.Assert(hrLocal == hr, ...) to Debug.Assert(hrLocal == hr || (hrLocal < 0 && hr < 0), ...).

@max-charlamb
Copy link
Copy Markdown
Member

@copilot we should verify that the error codes are CORDBG_E_READVIRTUAL_FAILURE or HResults.E_FAIL. We shouldn't accept any failing HResult.

… E_FAIL

Instead of accepting any failing HResult, verify that both error codes
are specifically CORDBG_E_READVIRTUAL_FAILURE or E_FAIL.

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 19, 2026

@copilot we should verify that the error codes are CORDBG_E_READVIRTUAL_FAILURE or HResults.E_FAIL. We shouldn't accept any failing HResult.

Updated in 52cd33ec — the assertion now uses a local IsReadFailure helper that only accepts HResults.E_FAIL or CorDbgHResults.CORDBG_E_READVIRTUAL_FAILURE instead of any negative HResult.

Copilot AI changed the title Relax GetILAddressMap HResult debug assertion in cDAC Restrict GetILAddressMap HResult debug assertion to specific error codes in cDAC Feb 19, 2026
@max-charlamb max-charlamb marked this pull request as ready for review February 19, 2026 20:08
@max-charlamb max-charlamb requested review from Copilot and rcj1 February 19, 2026 20:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a cDAC runtime-diagnostics pipeline failure caused by a debug assertion that was too strict when comparing HResults between the legacy DAC and cDAC implementations. The legacy DAC returns E_FAIL while cDAC returns CORDBG_E_READVIRTUAL_FAILURE for read failures in GetILAddressMap, causing the strict equality check to fail.

Changes:

  • Updated GetILAddressMap debug assertion to accept specific known error codes (E_FAIL or CORDBG_E_READVIRTUAL_FAILURE) instead of requiring exact HResult equality
  • Added an IsReadFailure helper function to encapsulate the specific error code checking logic
  • Removed an extraneous blank line for consistency

@rcj1
Copy link
Copy Markdown
Contributor

rcj1 commented Feb 19, 2026

Any reason not to modify the cDAC to match the DAC by wrapping the boundaries in a throw E_FAIL?

@max-charlamb
Copy link
Copy Markdown
Member

Any reason not to modify the cDAC to match the DAC by wrapping the boundaries in a throw E_FAIL?

I was potentially going to do this. We would want to E_FAIL if there is no DebugInfo for the particular method. It's a more invasive change but possible. I'd like to get my dump testing PR merged and then I can have a test for this particular situation.

@max-charlamb
Copy link
Copy Markdown
Member

Closing in favor of #124783

@max-charlamb max-charlamb deleted the copilot/fix-runtime-diagnostics-pipeline branch February 24, 2026 18:53
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants