Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behavior when using ref structs in foreach loop #73741

Closed
dotlogix opened this issue May 28, 2024 · 0 comments · Fixed by #74255
Closed

Unexpected behavior when using ref structs in foreach loop #73741

dotlogix opened this issue May 28, 2024 · 0 comments · Fixed by #74255
Assignees
Milestone

Comments

@dotlogix
Copy link

dotlogix commented May 28, 2024

Description

Copied from the dotnet repo as suggested
dotnet/runtime#102665

There seems to be an undesired behavior when using struct refs inside of a foreach loop.
When modifying byref fields of a struct within a foreach loop increment and similar operations do not work as expected.

I am not sure what exactly is causing this issue, but I also tested this with local variables and there it works as expected. I assume there is sth strange going on during the foreach.

Reproduction Steps

Consider this example project:
RefTest.zip

Code:

using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

namespace RefTest;

public unsafe class Program
{
    public static void Main(string[] args)
    {
        // Setup
        const int count = 10;
        var buffer = (ValueComponent*)NativeMemory.AlignedAlloc((nuint)(count * sizeof(ValueComponent)), 64);
        for (var i = 0; i < count; i++)
        {
            buffer[i] = new ValueComponent { Value = i };
        }
        
        var query = new QueryEnumerable(buffer, count);
        Console.WriteLine("Initial: ");
        foreach (var result in query)
        {
            Console.Write(result.Value.Value);
            Console.Write(", ");
        }
        Console.WriteLine();
        
        // PostIncrement
        Console.WriteLine("PostIncrement: ");
        foreach (var result in query)
        {
            result.Value.Value++;
        }
        foreach (var result in query)
        {
            Console.Write(result.Value.Value);
            Console.Write(", ");
        }
        Console.WriteLine();

        // Expanded Increment
        Console.WriteLine("ExpandedIncrement: ");
        foreach (var result in query)
        {
            result.Value.Value = result.Value.Value + 1;
        }
        foreach (var result in query)
        {
            Console.Write(result.Value.Value);
            Console.Write(", ");
        }
    }
}

public readonly unsafe struct QueryEnumerable(ValueComponent* buffer, int count)
{
    public QueryEnumerator GetEnumerator() => new(buffer, count);
}

public unsafe ref struct QueryEnumerator(ValueComponent* buffer, int count)
{
    private int _index = 0;
    public QueryResult Current => new(ref Unsafe.Add(ref Unsafe.AsRef<ValueComponent>(buffer), _index));

    public bool MoveNext()
    {
        var index = _index + 1;
        if (index >= count) return false;
        _index = index;
        return true;
    }
}

public ref struct QueryResult
{
    public readonly ref ValueComponent Value;

    public QueryResult(ref ValueComponent value)
    {
        Value = ref value;
    }
}

public struct ValueComponent
{
    public int Value;
}

Output:

Initial: 
1, 2, 3, 4, 5, 6, 7, 8, 9, 
PostIncrement: 
1, 2, 3, 4, 5, 6, 7, 8, 9, 
ExpandedIncrement:
2, 3, 4, 5, 6, 7, 8, 9, 10,

Expected behavior

The PostIncrement loop and the ExpandedIncrement loop should do the same.

Actual behavior

PostIncrement does not work as expected, but the ExpandedIncrement does

Regression?

Can't test this right now.

Known Workarounds

The ExpandedIncrement version works as expected but this could definetly cause errors down the line when someone misuses this slightly

Configuration

Version: .Net 9.0, .Net 8.0
OS: Windows
Architecture: AnyCpu

I don't have other devices to test this code unfortunately

Other information

I think it might be related to the foreach loop. Also the IL code of both operations is different. The PostIncrement version uses:

      IL_00c3: ldloca.s     result_V_8
      IL_00c5: ldfld        valuetype RefTest.ValueComponent& RefTest.QueryResult::Value
      IL_00ca: ldfld        int32 RefTest.ValueComponent::Value
      IL_00cf: stloc.s      V_9
      IL_00d1: ldloca.s     V_9
      IL_00d3: dup
      IL_00d4: ldind.i4
      IL_00d5: ldc.i4.1
      IL_00d6: add
      IL_00d7: stind.i4

While the ExpandedIncrement version uses this:

    IL_0146: ldloca.s     result_V_13
    IL_0148: ldfld        valuetype RefTest.ValueComponent& RefTest.QueryResult::Value
    IL_014d: ldloca.s     result_V_13
    IL_014f: ldfld        valuetype RefTest.ValueComponent& RefTest.QueryResult::Value
    IL_0154: ldfld        int32 RefTest.ValueComponent::Value
    IL_0159: ldc.i4.1
    IL_015a: add
    IL_015b: stfld        int32 RefTest.ValueComponent::Value

The lowered version of this code also works correctly btw:

var enumerator = query.GetEnumerator();
while (enumerator.MoveNext())
{
    enumerator.Current.Value.Value++;
}
      IL_00b9: ldloca.s     enumerator
      IL_00bb: call         instance valuetype RefTest.QueryResult RefTest.QueryEnumerator::get_Current()
      IL_00c0: stloc.s      V_8
      IL_00c2: ldloca.s     V_8
      IL_00c4: ldfld        valuetype RefTest.ValueComponent& RefTest.QueryResult::Value
      IL_00c9: ldflda       int32 RefTest.ValueComponent::Value
      IL_00ce: dup
      IL_00cf: ldind.i4
      IL_00d0: ldc.i4.1
      IL_00d1: add
      IL_00d2: stind.i4

And this version works as well

foreach (var result in query)
{
    ref var valueComponent = ref result.Value;
    valueComponent.Value++;
}
IL_0146: ldloca.s     result_V_13
IL_0148: ldfld        valuetype RefTest.ValueComponent& RefTest.QueryResult::Value
IL_014d: stloc.s      valueComponent

// [45 13 - 45 36]
IL_014f: ldloc.s      valueComponent
IL_0151: ldflda       int32 RefTest.ValueComponent::Value
IL_0156: dup
IL_0157: ldind.i4
IL_0158: ldc.i4.1
IL_0159: add
IL_015a: stind.i4
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels May 28, 2024
@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 4, 2024
@jaredpar jaredpar added this to the 17.12 milestone Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants