Folded Ref Structs with Fixed Buffer causes System.AccessViolationException.

[VitaliiBachulis]

Version Used: C# 11

Steps to Reproduce:

1. Use source code in "Investigation" project.
2. https://github.com/VitaliiBachulis/Investigation
3. In the ref struct constructor of outer struct has to initialize inner struct instead of creating a temporary one and copy it in a field.
   Compiler has to generate:

  .method public hidebysig specialname rtspecialname 
          instance void  .ctor() cil managed
  {
    // Code size       7 (0x7)
    .maxstack  1
    IL_0000:  ldarg.0
    IL_0001:  call     instance void IntruderAlert.DebounceMeasurement::.ctor()
    IL_0006:  ret
  } // end of method Room::.ctor

instead of:

  .method public hidebysig specialname rtspecialname 
          instance void  .ctor() cil managed
  {
    // Code size       12 (0xc)
    .maxstack  8
    IL_0000:  ldarg.0
    IL_0001:  newobj     instance void IntruderAlert.DebounceMeasurement::.ctor()
    IL_0006:  stfld      valuetype IntruderAlert.DebounceMeasurement IntruderAlert.Room::debounce
    IL_000b:  ret
  } // end of method Room::.ctor

[jcouv]

Might you be able to extract a smaller repro for your situation (preferably on sharplab)? It sounds like you need two ref structs but I couldn't figure out what's needed beyond that. Thanks
Update: I copied a bunch of the code from your project and the program crashes. Here's the repro:

using System;
using static System.Console;

Start();

Room room = new();

room.TakeMeasurements(14);

WriteLine("You are lucky!");
ReadLine();


static void Start()
{
    WriteLine("Start simulation");
    unsafe { if (sizeof(SensorMeasurement) != SensorMeasurement.SizeInBytes) throw new(); }
}


public ref struct Room
{
    public Room() { }

    private DebounceMeasurement debounce = new();
    
    public void TakeMeasurements(int measureCount)
    {
        for (int counter = 0; counter < measureCount; counter++)
        {
            debounce.MakeMeasurement();
        } 
    }
}

public struct SensorMeasurement
{
    public const int SizeInBytes = 48;

    public long Field1;
    public long Field2;
    public long Field3;
    public long Field4;
    public long Field5;
    public long IsFilled;
    public void FillMeasurement()
    {
        Field1 = long.MaxValue;
        IsFilled = 1;
    }
}

public unsafe ref struct DebounceMeasurement
{
    private const int debounceSize = 12;
    private int totalMeasurements = 0;
    private fixed byte measurements[debounceSize * SensorMeasurement.SizeInBytes];
    private readonly Span<SensorMeasurement> recentMeasurements;
    public DebounceMeasurement()
    {
        fixed (byte* m = measurements)
        {
            recentMeasurements = new Span<SensorMeasurement>(m, debounceSize);
        }
    }
    public void MakeMeasurement()
    {
        int index = totalMeasurements % debounceSize;
        recentMeasurements[index].FillMeasurement();
        totalMeasurements++;
    }
}

[jcouv]

@VitaliiBachulis I can see the IL generated for the Room() constructor, but it looks like what I would have expected (constructs a new DebounceMeasurement then assigns it to the field). In terms of language rules, why should this not be the case?
Tagging @cston to advise (does the IL look wrong?).

[VitaliiBachulis]

If you set <TieredCompilation>false</TieredCompilation> in the project file - the project does not crashe and looks like it's all right. But the memory layout of the resulting struct is doubled. And generaded ASM code is strange. After apply IL fix by ILDasm/ILAsm - the program works as expected - and no struct doubles in memory (approved by WinDbg).

[VitaliiBachulis]

The semantic meaning for constructors of folded ref structs - not instantiation (allocation) of it's fields (ref struct) on the evaluation stack - but it's initialization.
Such as an allocation of the outer struct has already just happened on it's declaration so all included in it ref struct already allocated. (For me: folded ref structs look like discrimination union from the point of memory layout).

[VitaliiBachulis]

If you analyze IL code generated for instantiation of the Room struct. We can see that it generated - what is expected.

.method private hidebysig static 
	void '<Main>$' (
		string[] args
	) cil managed 
{
	// {
	IL_0000: ldloca.s 0
	// new Room().TakeMeasurements(14);
	IL_0002: call instance void IntruderAlert.Room::.ctor()
	IL_0007: ldloca.s 0
	IL_0009: ldc.i4.s 14
	IL_000b: call instance void IntruderAlert.Room::TakeMeasurements(int32)

[VitaliiBachulis]

but the constructor of the Room struct does not follow the same initialization logic in it's implementation.

.method public hidebysig specialname rtspecialname 
	instance void .ctor () cil managed 
{
	// Method begins at RVA 0x2126
	// Header size: 1
	// Code size: 12 (0xc)
	.maxstack 8

	// debounce = new DebounceMeasurement();
	IL_0000: ldarg.0
	IL_0001: newobj instance void IntruderAlert.DebounceMeasurement::.ctor()
	IL_0006: stfld valuetype IntruderAlert.DebounceMeasurement IntruderAlert.Room::debounce

[VitaliiBachulis]

sharplab does not allow to understand the problem. I have spent a lot of time using ILDasm and WinDbg to clarify what happens.

[VitaliiBachulis]

@jcouv

Value types are not usually created using newobj. They are usually allocated either as arguments or local variables, using newarr (for zero-based, one-dimensional arrays), or as fields of objects. Once allocated, they are initialized using Initobj. However, the newobj instruction can be used to create a new instance of a value type on the stack, that can then be passed as an argument, stored in a local, and so on.

newobj

I think that for the Value types the newobj is acceptable in case of creating and than boxing a value type. Are I wrong?

[VitaliiBachulis]

At first time I was fooled that problem is in a JIT compiler (because program crashed only in QuickJit and looked as worked fine in Optimized compilation). But after apply an IL fix - I checked all variants and in all cases JIT generated ASM correctly and program works fine.

Also in the current state the program does not always crashes even in QuickJit - depending on the size of fixed-sized buffer and iteration counts - if it was small - I saw a false positive behavior.

Current IL codegen makes JIT compiler madness :(. It produces 2 different semantic logic (ASMx64 result) for the same MSIL depending on the level of JIT compilation and all have incorrect beheviour.

[VitaliiBachulis]

For really see the picture the best way is to look at process virtual addresses values on an evaluation stack. I did step by step ASM's commands running and watched the result of commands on the stack.

[VitaliiBachulis]

I think this case could be included in a test plan for: ##67826

[cston]

I believe the current behavior is by design. The instance field initializer corresponds to an assignment to the field (see language specification), and since the field is a struct, the initializer value is copied. As a result, the Span<> in the copied value refers to the fixed buffer in the original instance.

[VitaliiBachulis]

@cston let me discuss with you:

1. If the current behavior is by design - how do you explain the next: 1 and 2
2. The C# spec says For instance fields, variable initializers correspond to assignment statements that are executed when an instance of the class is created.. It tell nothing about a way of assignment!
3. As I understand C# spec describes mostly C# syntax - but not IL codegen which explained that.

You are right that "As a result, the Span<> in the copied value refers to the fixed buffer in the original instance." But it also has side effects:

• If a user does not apply "Tired Compilation" or (I'm not tested - AOT compilation) than we don't have any visible side effects. But under cover we have a doubled struct ("DebounceMeasurement") with a "fixed buffer" on the evaluation stack (it can be too big ).
  Such as the JIT allocates 2 copies of the "DebounceMeasurement" and than read Span<> value and read/write counter value from a second copy but save "fixed buffer" values into a first one.
• Even if a user does apply "Tired Compilation" - we also may not have a visible side effect: depending on the size of fixed-sized buffer and iteration counts - if it was small. Else we have a corrupted stack and an AccessViolation.

P.S. They are rhetorical questions.

[jaredpar]

If the current behavior is by design - how do you explain the next: #67799 (comment) and #67799 (comment)

The compiler will in some cases initialize struct values in place. This is an optimization though, it is not a semantic guarantee. The compiler can, will and has changed when this optimization does and does occur through the lifetime of the compiler.

I think this case could be included in a test plan for: #67826

I agree this is a case we need to consider when thinking about inline arrays. The inline array case though is very different because it's entirely safe code. It is up to the compiler to ensure that the code doesn't end up referring to temporary artifacts. I'm pretty sure the existing lifetime rules account for this but it's a good case to make sure we test.

The case you present here is very different. It's using unsafe code to create a Span<T>. That pattern is only safe if the struct is never copied. That is a very hard guarantee to make with struct values in .NET because copying is fairly inherent to their nature.

This particular behavior is "By Design" in that we do not guarantee, nor do we wish to guarantee, that copies do not happen in the x = new() case.