[release/10.0] Ignore PhiArgs without matching actual preds

[dhartglassMSFT]

Backport of #125093 to release/10.0. fixes #130035 . Regression test in this PR is associated with the original PR 125093, not 130035

I also cherry-picked #125222 , which was a follow-up work item to #125093

Bug Description

A PHI node had a dead argument for a flow edge that no longer existed.
Assertion propagation code moved information along this edge. This caused the JIT to incorrectly assume a value cannot be null.

optVisitReachingAssertions is in different files in NET10 and NET11 so this had to be manually integrated.

Customer Impact

Customer-reported: #130035
Practical customer impact is code that looks like strVal ?? value.ToString(); may be improperly compiled, leading to hard to diagnose incorrect behavior at runtime.

Regression

• Yes
• No

Introduced in NET10 by #111985 to fold more nullchecks

Testing

Tested that the issue in #130035 reproduces on a fresh top-of-tree release/10.0, and that this PR fixes that issue.

Risk

Low, this is a port of a change that's been in the main runtime repo since March.

CC @EgorBo

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
12 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.

[Copilot]

Pull request overview

Backport to release/10.0 of the JIT fix that prevents assertion propagation from walking/using PHI arguments tied to non-existent CFG edges, which could otherwise lead to incorrect non-null assumptions and miscompilation.

Changes:

• Harden optVisitReachingAssertions to abort PHI-based assertion inference when PHI-arg predecessor blocks don’t match the block’s actual predecessor set.
• Make optGetEdgeAssertions return an empty assertion set for non-existent edges (including stale predecessors after edge redirection), instead of returning assertions from an unrelated edge.
• Add a JIT regression test (Runtime_124507) to cover the dead-PHI-arg assertion propagation scenario.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/coreclr/jit/assertionprop.cpp	Ensures assertion propagation only follows real predecessor edges and avoids using assertions from stale/non-existent edges.
src/tests/JIT/Regression/JitBlue/Runtime_124507/Runtime_124507.cs	Adds a regression test that would fail under the miscompilation caused by dead/stale PHI arg assertion propagation.
src/tests/JIT/Regression/JitBlue/Runtime_124507/Runtime_124507.csproj	Adds the minimal test project configuration (optimized build) for the new regression test.

[hez2010]

Maybe #125222 should be paired when backporting this fix otherwise it will fix an issue by introducing another issue?

[dhartglassMSFT]

Maybe #125222 should be paired when backporting this fix otherwise it will fix an issue by introducing another issue?

@EgorBo do we want to bundle 125222 along with this?

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[EgorBo]

Maybe #125222 should be paired when backporting this fix otherwise it will fix an issue by introducing another issue?

@EgorBo do we want to bundle 125222 along with this?

I think it won't hurt. It's not directly related, it's a real bug we had even before my fix, Andy just noticed it during review.

[azure-pipelines]

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

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "8fab6e400a02a6057b8b7661f8ffeeb70a4e3dcc",
  "last_dispatched_base_ref": "release/10.0",
  "last_dispatched_base_sha": "6d9e1b3058c87cb4ba51e2b5f160e098819a5ad2",
  "last_reviewed_commit": "8fab6e400a02a6057b8b7661f8ffeeb70a4e3dcc",
  "last_reviewed_base_ref": "release/10.0",
  "last_reviewed_base_sha": "6d9e1b3058c87cb4ba51e2b5f160e098819a5ad2",
  "last_recorded_worker_run_id": "29766949341",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "8fab6e400a02a6057b8b7661f8ffeeb70a4e3dcc",
      "review_id": 4737842440
    }
  ]
}

[github-actions]

Holistic Review

Motivation: This is a backport of #125093 (with follow-up #125222) to release/10.0, addressing customer-reported issue #130035. The JIT's assertion propagation could move information along a PHI-argument edge that no longer exists (a stale phi-pred after edge redirection), causing it to incorrectly conclude that a value cannot be null. This manifests in patterns like strVal ?? value.ToString(); being miscompiled, producing wrong runtime behavior.

Approach: The fix hardens optVisitReachingAssertions to conservatively abort the phi inference when a phi-pred is not an actual block pred; hardens optGetEdgeAssertions to return an empty assertion set for a BBJ_COND predecessor whose block is neither its true nor false target (stale edge); and, from #125222, fixes up bbAssertionOut/bbJtrueAssertionOut after fgMorphBlockStmt folds or alters a BBJ_COND block's edges. A Fuzzlyn-derived regression test (Runtime_124507) is added.

Summary: LGTM. I compared the changed assertionprop.cpp against the original merged commits (14c4360 / #125093 and a32dd4a / #125222) and the ported hunks are byte-for-byte equivalent. The one non-mechanical part — the optVisitReachingAssertions change, which lived in compiler.hpp on main but in assertionprop.cpp on release/10.0 — was relocated correctly and matches the original exactly. The optAssertionPropMain fix-up block and optGetEdgeAssertions rewrite are identical to upstream. The added regression test reproduces the pattern from the original PR and is placed under the standard JitBlue layout with a self-contained .csproj. The only failing CI check is check-labels (backport label gating), which is unrelated to code correctness. Risk is low: this ports a change that has been in main since March.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 112.7 AIC · ⌖ 10.1 AIC · ⊞ 10K

[JulieLeeMSFT]

/ba-g known issues on crypto.