Skip to content

[cDAC] Fix GetLocalVariableCount to return E_FAIL for missing local signatures#126115

Merged
max-charlamb merged 1 commit intodotnet:mainfrom
max-charlamb:cdac-getlocalvariablecount-efail
Mar 25, 2026
Merged

[cDAC] Fix GetLocalVariableCount to return E_FAIL for missing local signatures#126115
max-charlamb merged 1 commit intodotnet:mainfrom
max-charlamb:cdac-getlocalvariablecount-efail

Conversation

@max-charlamb
Copy link
Copy Markdown
Member

@max-charlamb max-charlamb commented Mar 25, 2026

Changes

GetLocalVariableCount — return E_FAIL for missing local signatures

The native DAC's GetLocalSig returns E_FAIL when a method has no local signature (dynamic methods, IL stubs, methods with no declared locals). The cDAC was returning S_OK with count 0 instead. This mismatch caused validation asserts to fire.

Now GetLocalVariableCount throws Marshal.GetExceptionForHR(E_FAIL) when GetLocalSignatureReader returns null, matching the native DAC behavior exactly.

Testing

  • 117/117 local dump tests pass ✅
  • Verified GetNumLocalVariables returns E_FAIL for methods without locals (DAC parity)

Copilot AI review requested due to automatic review settings March 25, 2026 18:59
@max-charlamb max-charlamb changed the title cDAC: Fix GetLocalVariableCount DAC parity and GetNumArguments validation [cDAC] Fix GetLocalVariableCount DAC parity and GetNumArguments validation Mar 25, 2026
@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.

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

Aligns cDAC ClrDataFrame behavior with the native DAC to avoid Debug.Assert parity failures when analyzing older runtime dumps (e.g., 10.0.5), particularly around locals/argument signature handling.

Changes:

  • GetNumArguments: relax DEBUG parity validation to allow cDAC success when native DAC can fail due to MetaSig pointer-traversal differences.
  • GetLocalVariableCount: return E_FAIL (via exception->HRESULT) when no local signature is available, matching native DAC behavior.

…gnatures

The native DAC's GetLocalSig returns E_FAIL when a method has no local
signature (dynamic methods, IL stubs, methods with no declared locals).
The cDAC was returning S_OK with count 0 instead. This mismatch caused
Debug.Assert validation failures when running against older runtimes
where the cDAC succeeds but the legacy DAC returns E_FAIL.

Now GetLocalVariableCount throws Marshal.GetExceptionForHR(E_FAIL) when
GetLocalSignatureReader returns null, matching the native DAC behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@max-charlamb max-charlamb force-pushed the cdac-getlocalvariablecount-efail branch from 109eed5 to f4a66e4 Compare March 25, 2026 19:02
@max-charlamb max-charlamb changed the title [cDAC] Fix GetLocalVariableCount DAC parity and GetNumArguments validation cDAC: Fix GetLocalVariableCount to return E_FAIL for missing local signatures Mar 25, 2026
@max-charlamb max-charlamb changed the title cDAC: Fix GetLocalVariableCount to return E_FAIL for missing local signatures [cDAC] Fix GetLocalVariableCount to return E_FAIL for missing local signatures Mar 25, 2026
@max-charlamb max-charlamb requested a review from rcj1 March 25, 2026 21:19
@max-charlamb
Copy link
Copy Markdown
Member Author

/ba-g cDAC only change

@max-charlamb max-charlamb merged commit 91557f5 into dotnet:main Mar 25, 2026
54 of 61 checks passed
@max-charlamb max-charlamb deleted the cdac-getlocalvariablecount-efail branch March 25, 2026 21:20
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 25, 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.

3 participants