CompoundAssignment.Bug1021941 fail in .net7p6

[dibarbet]

Found in #63062 - disabling per infraswat to unblock upgrade to .net7p6, integration tests, and merge of -vs-deps back to main

These tests consistently fail (in all legs on multiple retries) on .net7p6 with an error similar to

Roslyn.Test.Utilities.ExecutionException :
Execution failed for assembly '01aff525-fc82-4b43-ac86-50de849ffa07, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
Expected:
Position get for item 'Goo'
Position set for item 'Goo'

Actual: Position get for item 'Goo'
Position set for item 'Bar'

https://dev.azure.com/dnceng/public/_build/results?buildId=1921926&view=ms.vss-test-web.build-test-results-tab&runId=49765824&resultId=145381&paneView=debug

[jaredpar]

This appears to be a regression in the runtime execution here. Can confirm the same code behaves according to the tests in net6.0 and fail in the most recent net7.0.

[AlekseyTs]

I think compilers code gen is wrong. The address of the receiver is getting popped from the stack as part of the method invocation and that is when the underlying value is supposed to be obtained and dereferenced. We should be able to adjust compiler's code gen to enforce correct behavior by using a reference to a compiler generated temp for the case when receiver is a reference type, thus preventing unexpected receiver change. I think we are doing something like that for conditional access code gen. Hopefully that shouldn't bloat IL too much.

[AlekseyTs]

On desktop framework I can observe an incorrect behavior for the following code in C#:

using System;

interface IMoveable
{
    int Position {get;set;}
    void GetName(int x);
}

class Item : IMoveable
{
    public string Name {get; set;}

    public int Position
    {
        get
        {
            Console.WriteLine("Position get for item '{0}'", Name);
            return 0;
        }
        set
        {
            Console.WriteLine("Position set for item '{0}'", Name);
        }
    }

    public void GetName(int x)
    {
        Console.WriteLine("Position GetName for item '{0}'", Name);
    }
}

class Program
{
    static void Main()
    {
        var item1 = new Item {Name = "1"};
        Shift1(item1);

        var item2 = new Item {Name = "2"};
        Shift2(item2);

        var item3 = new Item {Name = "3"};
        Conditional1(item3);

        var item4 = new Item {Name = "4"};
        Conditional2(item4);
    }

    static void Shift1<T>(T item) where T : class, IMoveable
    {
        item.Position += GetOffset(ref item);
    }

    static void Shift2<T>(T item) where T : IMoveable
    {
        item.Position += GetOffset(ref item);
    }

    static void Conditional1<T>(T item) where T : class, IMoveable
    {
        item?.GetName(GetOffset(ref item));
    }

    static void Conditional2<T>(T item) where T : IMoveable
    {
        item?.GetName(GetOffset(ref item));
    }
    
    static int value = 0;
    static int GetOffset<T>(ref T item)
    {
        item = (T)(IMoveable)new Item {Name = (--value).ToString()};
        return 0;
    }
}

Observed:

Position get for item '1'
Position set for item '-1'
Position get for item '2'
Position set for item '-2'
Position GetName for item '3'
Position GetName for item '-4'

Expected:

Position get for item '1'
Position set for item '1'
Position get for item '2'
Position set for item '2'
Position GetName for item '3'
Position GetName for item '4'

Note that conditional access without a class constraint also behaves incorrectly.

Equivalent VB code produces the same output as C#:

Imports System

Interface IMoveable
    Property Position As Integer
    Sub GetName(x As Integer)
End Interface

Class Item
    Implements IMoveable

    Public Property Name As String

    Public Property Position As Integer Implements IMoveable.Position
        Get
            Console.WriteLine("Position get for item '{0}'", Me.Name)
            Return 0
        End Get
        Set
            Console.WriteLine("Position set for item '{0}'", Me.Name)
        End Set
    End Property

    Public Sub GetName(x As Integer) Implements IMoveable.GetName
        Console.WriteLine("Position GetName for item '{0}'", Me.Name)
    End Sub
End Class

Class Program
    Shared Sub Main()
        Dim item1 = New Item With {.Name = "1"}
        Shift1(item1)

        Dim item2 = New Item With {.Name = "2"}
        Shift2(item2)

        Dim item3 = New Item With {.Name = "3"}
        Conditional1(item3)

        Dim item4 = New Item With {.Name = "4"}
        Conditional2(item4)
    End Sub

    Shared Sub Shift1(Of T As {Class, IMoveable})(item As T)
        item.Position += GetOffset(item)
    End Sub

    Shared Sub Shift2(Of T As {IMoveable})(item As T)
        item.Position += GetOffset(item)
    End Sub

    Shared Sub Conditional1(Of T As {Class, IMoveable})(item As T)
        item?.GetName(GetOffset(item))
    End Sub

    Shared Sub Conditional2(Of T As {IMoveable})(item As T)
        item?.GetName(GetOffset(item))
    End Sub

    Shared value as Integer

    Shared Function GetOffset(Of T)(ByRef item As T) As Integer
        value -= 1
        item = DirectCast(DirectCast(New Item With {.Name = value.ToString()}, IMoveable), T)
        Return 0
    End Function
End Class

Looking at IL for Shift1.
C#:

.method private hidebysig static void  Shift1<class (IMoveable) T>(!!T item) cil managed
{
  // Code size       38 (0x26)
  .maxstack  3
  .locals init (!!T& V_0)
  IL_0000:  nop
  IL_0001:  ldarga.s   item
  IL_0003:  stloc.0
  IL_0004:  ldloc.0
  IL_0005:  ldloc.0
  IL_0006:  constrained. !!T
  IL_000c:  callvirt   instance int32 IMoveable::get_Position()
  IL_0011:  ldarga.s   item
  IL_0013:  call       int32 Program::GetOffset<!!0>(!!0&)
  IL_0018:  add
  IL_0019:  constrained. !!T
  IL_001f:  callvirt   instance void IMoveable::set_Position(int32)
  IL_0024:  nop
  IL_0025:  ret
} // end of method Program::Shift1

VB:

.method public static void  Shift1<class (IMoveable) T>(!!T item) cil managed
{
  // Code size       37 (0x25)
  .maxstack  8
  IL_0000:  nop
  IL_0001:  ldarga.s   item
  IL_0003:  ldarga.s   item
  IL_0005:  constrained. !!T
  IL_000b:  callvirt   instance int32 IMoveable::get_Position()
  IL_0010:  ldarga.s   item
  IL_0012:  call       int32 Program::GetOffset<!!0>(!!0&)
  IL_0017:  add.ovf
  IL_0018:  constrained. !!T
  IL_001e:  callvirt   instance void IMoveable::set_Position(int32)
  IL_0023:  nop
  IL_0024:  ret
} // end of method Program::Shift1

Both functions push the same reference to the stack.

I expect both languages to generate IL that captures argument in a temp and then uses address of that temp for constrained calls. Or some other form of IL that doesn't use references as receivers for the calls.

For Shift2, C# version of IL uses

 IL_0001:  ldarga.s   item
 IL_0003:  dup

vs. VB version

  IL_0001:  ldarga.s   item
  IL_0003:  ldarga.s   item

Expected IL for this case should avoid copying a value type and should copy a reference type.

IL for Conditional1 is equivalent for both languages:

.method private hidebysig static void  Conditional1<class (IMoveable) T>(!!T item) cil managed
{
  // Code size       27 (0x1b)
  .maxstack  8
  IL_0000:  nop
  IL_0001:  ldarg.0
  IL_0002:  box        !!T
  IL_0007:  dup
  IL_0008:  brtrue.s   IL_000d
  IL_000a:  pop
  IL_000b:  br.s       IL_001a
  IL_000d:  ldarga.s   item
  IL_000f:  call       int32 Program::GetOffset<!!0>(!!0&)
  IL_0014:  callvirt   instance void IMoveable::GetName(int32)
  IL_0019:  nop
  IL_001a:  ret
} // end of method Program::Conditional1

Note, that it doesn't use constrained call and doesn't use argument reference as a receiver. The IL looks good to me and it behaves as expected.

For Conditional2, IL looks equivalent and problematic for both languages:

{
  // Code size       33 (0x21)
  .maxstack  8
  IL_0000:  nop
  IL_0001:  ldarg.0
  IL_0002:  box        !!T
  IL_0007:  brtrue.s   IL_000b
  IL_0009:  br.s       IL_0020
  IL_000b:  ldarga.s   item
  IL_000d:  ldarga.s   item
  IL_000f:  call       int32 Program::GetOffset<!!0>(!!0&)
  IL_0014:  constrained. !!T
  IL_001a:  callvirt   instance void IMoveable::GetName(int32)
  IL_001f:  nop
  IL_0020:  ret
} // end of method Program::Conditional2

[AlekseyTs]

It looks like there is a problem with just a call:

using System;

interface IMoveable
{
    void GetName(int x);
}

class Item : IMoveable
{
    public string Name {get; set;}

    public void GetName(int x)
    {
        Console.WriteLine("Position GetName for item '{0}'", Name);
    }
}

class Program
{
    static void Main()
    {
        var item1 = new Item {Name = "1"};
        Call1(item1);

        var item2 = new Item {Name = "2"};
        Call2(item2);
    }

    static void Call1<T>(T item) where T : class, IMoveable
    {
        item.GetName(GetOffset(ref item));
    }

    static void Call2<T>(T item) where T : IMoveable
    {
        item.GetName(GetOffset(ref item));
    }
    
    static int value = 0;
    static int GetOffset<T>(ref T item)
    {
        item = (T)(IMoveable)new Item {Name = (--value).ToString()};
        return 0;
    }
}

Observed:

Position GetName for item '1'
Position GetName for item '-2'

Expected:

Position GetName for item '1'
Position GetName for item '2'

Equivalent VB code:

Imports System

Interface IMoveable
    Sub GetName(x As Integer)
End Interface

Class Item
    Implements IMoveable

    Public Property Name As String

    Public Sub GetName(x As Integer) Implements IMoveable.GetName
        Console.WriteLine("Position GetName for item '{0}'", Me.Name)
    End Sub
End Class

Class Program
    Shared Sub Main()
        Dim item1 = New Item With {.Name = "1"}
        Call1(item1)

        Dim item2 = New Item With {.Name = "2"}
        Call2(item2)
    End Sub

    Private Shared Sub Call1(Of T As {Class, IMoveable})(item As T)
        item.GetName(GetOffset(item))
    End Sub

    Private Shared Sub Call2(Of T As {IMoveable})(item As T)
        item.GetName(GetOffset(item))
    End Sub

    Shared value as Integer

    Shared Function GetOffset(Of T)(ByRef item As T) As Integer
        value -= 1
        item = DirectCast(DirectCast(New Item With {.Name = value.ToString()}, IMoveable), T)
        Return 0
    End Function
End Class

Observed:

Position GetName for item '-1'
Position GetName for item '-2'

IL for C#:

.method private hidebysig static void  Call1<class (IMoveable) T>(!!T item) cil managed
{
  // Code size       21 (0x15)
  .maxstack  8
  IL_0000:  nop
  IL_0001:  ldarg.0
  IL_0002:  box        !!T
  IL_0007:  ldarga.s   item
  IL_0009:  call       int32 Program::GetOffset<!!0>(!!0&)
  IL_000e:  callvirt   instance void IMoveable::GetName(int32)
  IL_0013:  nop
  IL_0014:  ret
} // end of method Program::Call1

.method private hidebysig static void  Call2<(IMoveable) T>(!!T item) cil managed
{
  // Code size       23 (0x17)
  .maxstack  8
  IL_0000:  nop
  IL_0001:  ldarga.s   item
  IL_0003:  ldarga.s   item
  IL_0005:  call       int32 Program::GetOffset<!!0>(!!0&)
  IL_000a:  constrained. !!T
  IL_0010:  callvirt   instance void IMoveable::GetName(int32)
  IL_0015:  nop
  IL_0016:  ret
} // end of method Program::Call2

IL for VB:

.method private static void  Call1<class (IMoveable) T>(!!T item) cil managed
{
  // Code size       23 (0x17)
  .maxstack  8
  IL_0000:  nop
  IL_0001:  ldarga.s   item
  IL_0003:  ldarga.s   item
  IL_0005:  call       int32 Program::GetOffset<!!0>(!!0&)
  IL_000a:  constrained. !!T
  IL_0010:  callvirt   instance void IMoveable::GetName(int32)
  IL_0015:  nop
  IL_0016:  ret
} // end of method Program::Call1

.method private static void  Call2<(IMoveable) T>(!!T item) cil managed
{
  // Code size       23 (0x17)
  .maxstack  8
  IL_0000:  nop
  IL_0001:  ldarga.s   item
  IL_0003:  ldarga.s   item
  IL_0005:  call       int32 Program::GetOffset<!!0>(!!0&)
  IL_000a:  constrained. !!T
  IL_0010:  callvirt   instance void IMoveable::GetName(int32)
  IL_0015:  nop
  IL_0016:  ret
} // end of method Program::Call2

C# version of Call1 doesn't use constrained call, that is why it behaves as expected.

[AlekseyTs]

For some reason unit-tests when executed in VS produce expected output.