JIT: Fix null-check ordering in EH regions

[AndyAyersMS]

Ensure this pointer null checks happen before inlinee side effects observable
by catch handlers and exception filters. Prevent early propagation from folding
these checks across such side effects, and add regression coverage for both
cases.

Fixes #133714

Note

This pull request description was generated by GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 5 pipeline(s).
11 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[AndyAyersMS]

@EgorBo PTAL
fyi @dotnet/jit-contrib

No diffs expected

[Copilot]

🟡 Changes recommended

Nested inlining can still miss the enclosing protected-region state, leaving the null-check ordering bug unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

JIT: Fix null-check ordering in EH regions

This PR preserves receiver null-check ordering across inlinee side effects observable by exception handlers and filters.

Changes:

• Treat assignments as visible side effects in protected regions.
• Prevent unsafe null-check folding.
• Add catch and filter regression coverage.

File summaries

File	Summary
src/tests/JIT/Regression_ro_2/Runtime_133714.cs	Adds regression coverage.
src/coreclr/jit/importer.cpp	Preserves null checks across inlinee assignments.
src/coreclr/jit/earlyprop.cpp	Restricts null-check folding in EH regions.
src/coreclr/jit/compiler.h	Updates the helper parameter name.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[AndyAyersMS]

diffs

There were a couple of minor diffs, looks like just one method.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

[AndyAyersMS]

The new test is failing on mono and coreclr Wasm. Going to exclude mono and see if #133932 fixes things for Wasm.

[AndyAyersMS]

The new test is failing on mono and coreclr Wasm. Going to exclude mono and see if #133932 fixes things for Wasm.

Yes, that fixes the Wasm failure. So will wait on that PR.

[Copilot]

🔵 Needs a closer look

A moderate unresolved concern remains about broad assignment visibility in EH regions.

Review details

Suppressed comments (1)

src/coreclr/jit/importer.cpp:14846

• This broadens the side-effect test from GTF_GLOBALLY_VISIBLE_SIDE_EFFECTS to every GTF_ASG whenever the call site has EH successors. That makes assignments to inlinee-private temporaries EH-visible too, so an inlined method that initializes an unobservable local before its first this dereference will retain a separate null check inside every protected region, even though no handler can observe that local. Please restrict this to stores that can actually reach an EH successor (rather than treating all assignments as visible) to avoid a broad null-check-elimination regression.

        return GTF_GLOBALLY_VISIBLE_SIDE_EFFECTS(flags) || (localStoresAreVisible && ((flags & GTF_ASG) != 0));

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[AndyAyersMS]

@jakobbotsch will need re-approval, the new test causes mono failures and a (since fixed) Wasm failure.