Skip to content

JIT: Preserve evaluation order in GC bulk copies - #133805

Open
EgorBo with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-gc-bulk-copy-helper
Open

JIT: Preserve evaluation order in GC bulk copies#133805
EgorBo with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-gc-bulk-copy-helper

Conversation

Copilot AI commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

GC bulk-copy lowering can fault on the destination before evaluating the source, skipping side effects or masking source exceptions. In Unsafe.Add(ref dst, Index()) = GetSrc(), GetSrc() must run before the copy faults.

  • Lowering: Defer both null checks until both addresses are evaluated. Move GT_PUTARG_* nodes afterward so argument placement remains adjacent to the helper call.
  • Regression coverage: Add six cases covering valid/null destinations and valid/null/throwing sources, asserting evaluation order, exception types, and copied references.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
16 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

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix struct copy fault in GC bulk-copy helper JIT: Preserve evaluation order in GC bulk copies Sep 12, 2026
Copilot AI requested a review from EgorBo September 12, 2026 22:15
@EgorBo
EgorBo marked this pull request as ready for review September 12, 2026 23:45
Copilot AI lite review requested due to automatic review settings September 12, 2026 23:45
@azure-pipelines

Copy link
Copy Markdown
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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Automated review does not establish sufficient confidence for approval.

Pull request overview

This pull request preserves evaluation order in JIT GC bulk-copy lowering and adds regression coverage.

Changes:

  • Defers null checks until both addresses are evaluated.
  • Keeps argument placement adjacent to the helper call.
  • Adds six evaluation-order and exception regression cases.
File summaries
File Summary
src/tests/JIT/Regression_ro_2/Runtime_133587.cs Adds regression tests for evaluation order and copy behavior.
src/coreclr/jit/lower.cpp Adjusts GC bulk-copy lowering order.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

@jkotas jkotas added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed area-Infrastructure labels Sep 13, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

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

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

JIT: (bug) Struct copy through the GC bulk-copy helper faults on the destination before the source expression is evaluated

4 participants