Skip to content

[cDAC] Loosen EHInfo test to account for possible optimization of finally clause into fault clause#125892

Merged
rcj1 merged 2 commits intodotnet:mainfrom
rcj1:ehinfo-test-fix
Mar 23, 2026
Merged

[cDAC] Loosen EHInfo test to account for possible optimization of finally clause into fault clause#125892
rcj1 merged 2 commits intodotnet:mainfrom
rcj1:ehinfo-test-fix

Conversation

@rcj1
Copy link
Copy Markdown
Contributor

@rcj1 rcj1 commented Mar 21, 2026

No description provided.

@rcj1 rcj1 changed the title Loosen EHInfo test to account for possible optimization of finally clause into fault clause [cDAC] Loosen EHInfo test to account for possible optimization of finally clause into fault clause Mar 21, 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

This PR loosens a cDAC dump-based integration test for ExecutionManager EH clause enumeration to accommodate a JIT optimization that can transform a try/finally into a try/fault (notably via Compiler::fgCloneFinally()), which can change the reported clause type.

Changes:

  • Rename the test to reflect the updated expectation (Finally or Fault).
  • Update the assertion to accept either ExceptionClauseFlags.Finally or ExceptionClauseFlags.Fault.
  • Add an explanatory comment documenting why the test allows both outcomes.

Copy link
Copy Markdown
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

I spent a while trying to figure out whether the spec or usage in SOS stated whether we expected the flag to represent the original clause or how the JIT lowered the clause. Nothing is specified but given its role in SOS disassembly viewer it seemed like the JIT lowering was a reasonable thing to report.

I'd suggest adding a comment in the data contract spec clarifying that these flags are set based on how the JIT lowered it and may not match the corresponding IL flag.

@rcj1
Copy link
Copy Markdown
Contributor Author

rcj1 commented Mar 23, 2026

/ba-g comment only change

1 similar comment
@rcj1
Copy link
Copy Markdown
Contributor Author

rcj1 commented Mar 23, 2026

/ba-g comment only change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants