dotnet / dotnet/runtime

JIT: (bug) Physical promotion: struct copy between overlapping slices of the same local is not done from a snapshot

Open
#133,858 1 comment 1 reaction 2 assignees Claimed by @jakobbotsch View on GitHub
area-CodeGen-coreclr
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

With `LayoutKind.Explicit`, two struct fields of the same local can overlap. Physical promotion decomposes `s.P1 = s.P0` into sequential per-field stores without noticing the overlap, so a later store reads a slot an earlier store already overwrote. Affects .NET 10 as well.

### Minimal Repro

```csharp
using System;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

struct Pair { public int A; public int B; }

[StructLayout(LayoutKind.Explicit)]
struct S
{
[FieldOffset(0)] public Pair P0; // [0, 8)
[FieldOffset(4)] public Pair P1; // [4, 12)
}

class Program
{
[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
static string Test(int a, int b, int c)
{
S s = default;
s.P0.A = a; // offset 0
s.P0.B = b; // offset 4
s.P1.B = c; // offset 8
s.P1 = s.P0; // [4,12) <- snapshot of [0,8)
return $"{s.P0.A},{s.P0.B},{s.P1.B}";
}

static void Main() => Console.WriteLine(Test(1, 2, 3));
}
```

### Expected

```
1,1,2
```

`s.P1 = s.P0` copies the old value of `s.P0` (`{1,2}`). MinOpts and `DOTNET_JitEnablePhysicalPromotion=0` print this.

### Actual

```
1,1,1
```

The decomposed copy is a straight-line sequence, so the second store reads the value the first store just wrote:

```
V78 (V03.[004..008)) <- V77 (V03.[000..004))
V79 (V03.[008..012)) <- V78 (V03.[004..008)) (last use)
```

### Notes

- Only overlapping copies where the destination starts above the source are wrong; `s.P0 = s.P1` and non-overlapping explicit layouts are fine.
- Root cause: in `promotiondecomposition.cpp`, `ReplaceVisitor::HandleStructStore` dispatches to `CopyBetweenFields`, which walks the destination/source replacement lists in parallel and never checks whether both sides are the *same* local with overlapping ranges.
- Fix direction: copy via temps, order the per-field stores high-to-low when dst starts above src, or bail out of decomposition for such stores.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.